fix(secrets): stay silent when Bitwarden enabled but unconfigured - #32784
Closed
konsisumer wants to merge 1 commit into
Closed
fix(secrets): stay silent when Bitwarden enabled but unconfigured#32784konsisumer wants to merge 1 commit into
konsisumer wants to merge 1 commit into
Conversation
When secrets.bitwarden.enabled is true but neither an access token nor a project_id is configured, the integration was never actually set up, so suppress the startup warning instead of nagging users on every launch. The token/project_id warnings still fire once setup is partway done. Refs NousResearch#32715
19 tasks
Contributor
Author
|
Closing — superseded by the consolidated #46641, which folds this PR's change in alongside the related work on the same issue. Reopen if that's not right. |
This file contains hidden or 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
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.
What does this PR do?
Stops the repeated
Bitwarden Secrets Manager: ... BWS_ACCESS_TOKEN is not setwarning from printing on every Hermes startup for users who never actually set up Bitwarden Secrets Manager.When
secrets.bitwarden.enabledistruebut neither an access token nor aproject_idis configured, the integration was never set up — the warning is pure noise. We now return a clean (no-error) result in that case so nothing is printed. The token /project_idwarnings still fire once setup is partway done (one of the two present), where they're genuinely actionable.Note on the two other options the reporter suggested: both are already in
main. The default config shipssecrets.bitwarden.enabled: false(hermes_cli/config.py), and the status line is already deduplicated to print once per process via the applied-home guard added in #32271. This PR closes the remaining gap for configs that haveenabled: truewith nothing else filled in.Related Issue
Refs #32715
Type of Change
Changes Made
agent/secret_sources/bitwarden.py: inapply_bitwarden_secrets, return early with a cleanFetchResult(noerror) when enabled but both the access token andproject_idare empty, so no warning is emitted.tests/test_bitwarden_secrets.py: addtest_apply_unconfigured_is_silentcovering the no-token + no-project_id case; existingtest_apply_missing_token(has project_id) andtest_apply_missing_project_id(has token) still assert the warning fires when setup is partway done.How to Test
secrets.bitwarden.enabled: true, noBWS_ACCESS_TOKEN, and an emptyproject_id, runhermes— the Bitwarden warning no longer prints.project_id(but still no token) and runhermes— theBWS_ACCESS_TOKEN is not setwarning prints (actionable, partway-configured case).pytest tests/test_bitwarden_secrets.py tests/test_env_loader_secret_sources.py -q— all pass (45 tests).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — ran the affected suites (45 passed). Note: thescripts/run_tests.shwrapper aborts in this environment becausepytest-timeoutisn't installed there (unrecognized arguments: --timeout); directpytest --timeout=60passes.Documentation & Housekeeping
docs/, docstrings) — N/A (behavior-only change; existing docs describe the partway-configured warnings, which still fire)cli-config.yaml.exampleif I added/changed config keys — N/A (no config keys changed)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Ascripts/check-windows-footguns.pyclean)