Skip to content

feat: support Bitwarden env alias mapping - #55886

Closed
Kaluanbernardo wants to merge 1 commit into
NousResearch:mainfrom
Kaluanbernardo:feat/bitwarden-env-map
Closed

feat: support Bitwarden env alias mapping#55886
Kaluanbernardo wants to merge 1 commit into
NousResearch:mainfrom
Kaluanbernardo:feat/bitwarden-env-map

Conversation

@Kaluanbernardo

Copy link
Copy Markdown

Summary\n- Add secrets.bitwarden.env_map to materialize profile-specific BWS secret aliases as runtime env vars\n- Add prune_env_keys/prune_env_prefixes so alias secrets do not remain in long-lived process env after mapping\n- Cover Discord multi-profile token aliasing with env-loader tests\n\n## Tests\n- uv run --with pytest --with pyyaml --with python-dotenv python -m pytest tests/test_bitwarden_secrets.py tests/hermes_cli/test_env_loader_bitwarden_env_map.py -q -o 'addopts='\n

@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: part of the Bitwarden profile-scoped-secrets cluster with the open #51616 (canonical-aliasing in bitwarden.py) and #50686 (key_prefix/strip_prefix). This PR takes a different mechanism — env_loader-level secrets.bitwarden.env_map aliasing + env pruning — so it competes rather than duplicates. Maintainer should pick the preferred layer.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real profile-scoped secret problem. The current implementation needs to move to the active secret-source path before it can be salvaged.

Problems

  • hermes_cli/env_loader.py:341-346 now delegates secret startup to registry.apply_all(), which writes raw fetched names at agent/secret_sources/registry.py:339-368. The added mapping call is attached to the pre-registry direct Bitwarden path and therefore does not reach the current gateway startup flow.
  • The new tests mock legacy apply_bitwarden_secrets; current active-loader tests mock fetch_bitwarden_secrets and exercise _apply_external_secret_sources() (tests/test_env_loader_secret_sources.py:91-105).
  • The proposed settings are not represented in BitwardenSource.config_schema() (agent/secret_sources/bitwarden.py:616-640) or the Bitwarden config docs/example.

Suggested changes

  • Rework mapping in the active BitwardenSource/registry flow, preserving registry precedence, protected variables, and provenance.
  • Add an active-loader regression that proves gateway/config.py:1501-1504 receives canonical DISCORD_BOT_TOKEN and aliases are absent.
  • Coordinate the mapping contract with related open PRs #51616 and #50686 before defining docs/config surface.

Automated hermes-sweeper review.

Comment thread hermes_cli/env_loader.py
@@ -326,6 +423,11 @@ def _apply_external_secret_sources(home_path: Path) -> None:
server_url=str(bw_cfg.get("server_url", "") or "").strip(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main no longer has this direct Bitwarden-result path: hermes_cli/env_loader.py:341-346 calls registry.apply_all(), which owns the actual environment writes. Please move aliasing into the active SecretSource/registry flow so it preserves precedence, protected-variable handling, and provenance.

assert kwargs["project_id"] == "project-123"
secrets = {
"DISCORD_BOT_TOKEN_MEOS_DEV": "dev-token",
"DISCORD_BOT_TOKEN_MEOS_ACADEMIC": "academic-token",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mocks the legacy apply_bitwarden_secrets entry point. Current startup tests stub fetch_bitwarden_secrets and exercise _apply_external_secret_sources() through the registry; use that live path so the test covers the implementation users run.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@Kaluanbernardo Kaluanbernardo closed this by deleting the head repository Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants