Skip to content

fix(cli): dedupe Bitwarden BWS_ACCESS_TOKEN warning across subprocesses - #34314

Closed
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/bitwarden-warning-cross-process-dedup
Closed

fix(cli): dedupe Bitwarden BWS_ACCESS_TOKEN warning across subprocesses#34314
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/bitwarden-warning-cross-process-dedup

Conversation

@konsisumer

Copy link
Copy Markdown
Contributor

What does this PR do?

Dedupes the Bitwarden Secrets Manager status line across the child Python processes spawned during hermes startup, so a user with secrets.bitwarden.enabled: true but no BWS_ACCESS_TOKEN no longer sees the same warning 2–3 times before the interactive session starts.

The in-process _APPLIED_HOMES guard (added in #32271 the day before this issue was filed) already collapses within-process repeats. That guard is module state, so it doesn't survive a subprocess boundary — and the spawned helpers (gateway, TUI server, ACP adapter, ...) each import their entrypoint module which calls load_hermes_dotenv() at import time, so each one re-emitted the warning. This PR adds a cross-process marker via os.environ so siblings inherit it and stay quiet.

Functional behavior in each subprocess is unchanged: the config read, the (cached) fetch attempt, the credential injection into os.environ, the ASCII sanitization pass, and the source-tracking dict update all still run. Only the duplicated stderr noise is suppressed.

Related Issue

Fixes #32715

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/env_loader.py: introduced _BWS_STATUS_PRINTED_ENV = "_HERMES_BWS_STATUS_PRINTED"; gated the three print(" Bitwarden Secrets Manager: ...") call sites on its absence; set it before printing (so a sibling race only lets the first one through); extended reset_secret_source_cache() to pop the marker so tests and explicit refresh callers can re-emit.
  • tests/test_env_loader_secret_sources.py: added test_apply_external_secret_sources_dedupes_across_subprocesses (pre-set marker + enabled: true config → no stderr) and test_apply_external_secret_sources_prints_warning_once_then_sets_marker (marker absent → warning printed and marker set to "1").

How to Test

  1. Reproduce the bug on main: in a fresh HERMES_HOME, write ~/.hermes/config.yaml with secrets:\n bitwarden:\n enabled: true\n project_id: foo\n, ensure BWS_ACCESS_TOKEN is unset, run hermes, and confirm the secrets.bitwarden.enabled is true but BWS_ACCESS_TOKEN is not set... line appears 2–3 times on stderr before the prompt.
  2. Apply this branch and repeat step 1 — the line should appear exactly once.
  3. Run the targeted suites locally: $VIRTUAL_ENV/bin/pytest tests/test_env_loader_secret_sources.py tests/test_bitwarden_secrets.py tests/hermes_cli/test_env_loader.py -q --timeout=60. The new tests in test_env_loader_secret_sources.py directly exercise the cross-process and first-emit behaviors via monkeypatch.setenv(env_loader._BWS_STATUS_PRINTED_ENV, "1").
  4. Confirm credential injection still works: run test_apply_external_secret_sources_records_bitwarden_origin (unchanged); it stubs a successful BSM fetch and asserts get_secret_source("ANTHROPIC_API_KEY") == "bitwarden" still holds, verifying that the gated print sites did not also gate the sanitization / source-tracking work.
  5. Sanity: $VIRTUAL_ENV/bin/ruff check hermes_cli/env_loader.py tests/test_env_loader_secret_sources.py and ruff format --check both clean; scripts/check-windows-footguns.py clean for the two files.

What platforms tested on

  • macOS on darwin-arm64 (local) — full targeted pytest run, ruff lint/format, Windows footgun checker all clean. The change is plain os.environ reads/writes with no platform-specific surface, so behavior is identical on Linux, macOS, native Windows, and WSL2; the inheritance semantics (os.environ propagates across subprocess.Popen on every supported platform) are what the fix relies on.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (ran the env-loader / bitwarden / secret-source slice — 57 passed, 2 skipped)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS arm64 (local)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A (no user-visible config or behavior added; the marker env var is internal, leading underscore)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no new config keys)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guideos.environ propagation is the same on every supported platform; scripts/check-windows-footguns.py clean
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

`hermes` startup spawns child Python processes (gateway, TUI server, ACP
adapter) that each call `load_hermes_dotenv()` at import time. The in-process
`_APPLIED_HOMES` guard short-circuits repeats within one process, but module
state does not survive a subprocess boundary — so users with
`secrets.bitwarden.enabled: true` but no `BWS_ACCESS_TOKEN` saw the same
"BWS_ACCESS_TOKEN is not set" stderr line 2-3x per startup (NousResearch#32715).

Gate the Bitwarden status prints on a new `_HERMES_BWS_STATUS_PRINTED` env
var, set on first emit. Subprocesses inherit `os.environ` and skip the
prints; sanitization + credential-origin tracking still run unchanged.
`reset_secret_source_cache()` clears the marker so tests and long-running
processes can force a re-emit.

Fixes NousResearch#32715
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 29, 2026
@konsisumer

Copy link
Copy Markdown
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.

@konsisumer konsisumer closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Startup noise: repeated Bitwarden Secrets Manager warning when BWS_ACCESS_TOKEN is unset

2 participants