Update Bitwarden code to use env instead of secret manager if env is present #1592
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Update Bitwarden integration to prioritize environment variables over AWS Secrets for credentials in `bitwarden_helper.py` and `context_manager.py`. > > - **Behavior**: > - Update `generate_session_key()` in `bitwarden_helper.py` to use environment variables for Bitwarden credentials if available, otherwise fallback to AWS Secrets. > - Modify `register_parameter_value()` in `context_manager.py` to prioritize environment variables for Bitwarden credentials over AWS Secrets. > - **Configuration**: > - Add `BITWARDEN_CLIENT_ID`, `BITWARDEN_CLIENT_SECRET`, and `BITWARDEN_MASTER_PASSWORD` to `Settings` in `config.py`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for dc4c3c63a68c08dcac8fef3b01b6f0cbe7c6c2c1. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4f9c64a in 11 seconds
More details
- Looked at
93lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/context_manager.py:159
- Draft comment:
The code for fetching Bitwarden credentials is repeated multiple times. Consider refactoring this into a helper function to improve maintainability and reduce redundancy. This applies to lines 234 and 292 as well. - Reason this comment was not posted:
Confidence changes required:50%
The current implementation of fetching Bitwarden credentials prioritizes environment variables over AWS secrets, which is correct as per the PR description. However, the repeated code for fetching credentials can be refactored to improve maintainability and reduce redundancy.
Workflow ID: wflow_U7dsFUqpmeO5t1OJ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 4f9c64a in 22 seconds
More details
- Looked at
93lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/context_manager.py:162
- Draft comment:
The logic to fetch Bitwarden credentials is repeated multiple times. Consider refactoring this into a helper function to adhere to the DRY principle. This applies to lines 162-170, 237-245, and 295-303. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_KzJnSfUUnXdt9ec9
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
suchintan
approved these changes
Jan 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Update Bitwarden credential retrieval to prioritize environment variables over AWS secrets in
context_manager.py.register_parameter_valueincontext_manager.pyto use Bitwarden credentials from environment variables if available, otherwise fallback to AWS secrets.BITWARDEN_CLIENT_ID,BITWARDEN_CLIENT_SECRET, andBITWARDEN_MASTER_PASSWORDtoSettingsinconfig.py.This description was created by
for 4f9c64a. It will automatically update as commits are pushed.