From c25f9d1d3665355940d3f1c9fd0097831e49746d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 22 May 2026 03:32:58 -0700 Subject: [PATCH] feat(secrets): label detected credentials with their source (Bitwarden) (#30364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Bitwarden Secrets Manager supplies a provider key, 'hermes model' and the setup wizard show 'credentials ✓' with no hint of where the key came from — identical to the .env case. Users assume the integration isn't wired up and re-enter the key (or hit Enter and cancel). env_loader now tracks which env vars were injected by an external secret source and exposes get_secret_source() / format_secret_source_suffix() so the provider flows can render 'Anthropic credentials: sk-ant-... ✓ (from Bitwarden)' instead of an unlabeled checkmark. Wired into _prompt_api_key (kimi, z.ai, minimax, opencode, ...), the Anthropic provider flow, the Bedrock flow, and the GitHub Copilot token display. Future secret sources (Vault, 1Password, etc.) drop in by setting their own label in _SECRET_SOURCES; format_secret_source_suffix() has a generic fallback so no call sites need updating. --- hermes_cli/env_loader.py | 44 +++++++++ hermes_cli/main.py | 30 +++++- tests/test_env_loader_secret_sources.py | 119 ++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 tests/test_env_loader_secret_sources.py diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index 521076af9b4..8ef60f4e07f 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -21,6 +21,44 @@ _CREDENTIAL_SUFFIXES = ("_API_KEY", "_TOKEN", "_SECRET", "_KEY") # tests) don't spam the same warning multiple times. _WARNED_KEYS: set[str] = set() +# Map of env-var name → source label ("bitwarden", etc.) for credentials +# that were injected by an external secret source during load_hermes_dotenv(). +# Used by setup / `hermes model` flows to label detected credentials so +# users understand WHERE a key came from when their .env doesn't contain it +# directly (otherwise the "credentials detected ✓" line looks identical to +# the .env case and they don't know Bitwarden is wired up). +_SECRET_SOURCES: dict[str, str] = {} + + +def get_secret_source(env_var: str) -> str | None: + """Return the label of the secret source that supplied ``env_var``, if any. + + Returns ``"bitwarden"`` for keys pulled from Bitwarden Secrets Manager + during the current process's ``load_hermes_dotenv()`` call. Returns + ``None`` for keys that came from ``.env``, the shell environment, or + aren't tracked. + """ + return _SECRET_SOURCES.get(env_var) + + +def format_secret_source_suffix(env_var: str) -> str: + """Return a human-readable suffix like ``" (from Bitwarden)"`` or ``""``. + + Use this when printing a detected credential so the user can see where + it came from. Empty string when the credential came from ``.env`` or + the shell — those are the implicit / "default" cases users already + understand. + """ + source = get_secret_source(env_var) + if not source: + return "" + if source == "bitwarden": + return " (from Bitwarden)" + # Generic fallback — future-proofing for additional secret sources + # (e.g. 1Password, HashiCorp Vault) without having to update every + # call site. + return f" (from {source})" + def _format_offending_chars(value: str, limit: int = 3) -> str: """Return a compact 'U+XXXX ('c'), ...' summary of non-ASCII codepoints.""" @@ -213,6 +251,12 @@ def _apply_external_secret_sources(home_path: Path) -> None: # and might have the same copy-paste corruption as a manually # edited .env (see #6843). _sanitize_loaded_credentials() + # Remember where these came from so the setup / `hermes model` + # flows can label detected credentials with "(from Bitwarden)" — + # otherwise users see "credentials ✓" with no hint that the value + # came from BSM rather than .env. + for name in result.applied: + _SECRET_SOURCES[name] = "bitwarden" print( f" Bitwarden Secrets Manager: applied {len(result.applied)} " f"secret{'s' if len(result.applied) != 1 else ''} " diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 004771ee3f2..102ee325b6e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -4662,7 +4662,9 @@ def _model_flow_copilot(config, current_model=""): source = creds.get("source", "") else: if source in {"GITHUB_TOKEN", "GH_TOKEN"}: - print(f" GitHub token: {api_key[:8]}... ✓ ({source})") + from hermes_cli.env_loader import format_secret_source_suffix + bw_suffix = format_secret_source_suffix(source) + print(f" GitHub token: {api_key[:8]}... ✓ ({source}{bw_suffix})") elif source == "gh auth token": print(" GitHub token: ✓ (from `gh auth token`)") else: @@ -4919,7 +4921,10 @@ def _prompt_api_key(pconfig, existing_key: str, provider_id: str = "") -> tuple: return new_key, False # Already configured — offer K / R / C ──────────────────────────────── - print(f" {pconfig.name} API key: {existing_key[:8]}... ✓") + from hermes_cli.env_loader import format_secret_source_suffix + + source_suffix = format_secret_source_suffix(key_env) if key_env else "" + print(f" {pconfig.name} API key: {existing_key[:8]}... ✓{source_suffix}") if not key_env: # Nothing we can rewrite; just acknowledge and move on. print() @@ -5202,7 +5207,9 @@ def _model_flow_bedrock_api_key(config, region, current_model=""): # Prompt for API key existing_key = get_env_value("AWS_BEARER_TOKEN_BEDROCK") or "" if existing_key: - print(f" Bedrock API Key: {existing_key[:12]}... ✓") + from hermes_cli.env_loader import format_secret_source_suffix + source_suffix = format_secret_source_suffix("AWS_BEARER_TOKEN_BEDROCK") + print(f" Bedrock API Key: {existing_key[:12]}... ✓{source_suffix}") else: print(f" Endpoint: {mantle_base_url}") print() @@ -5873,7 +5880,22 @@ def _model_flow_anthropic(config, current_model=""): if has_creds: # Show what we found if existing_key: - print(f" Anthropic credentials: {existing_key[:12]}... ✓") + from hermes_cli.env_loader import format_secret_source_suffix + from hermes_cli.auth import PROVIDER_REGISTRY + + # Surface which env var supplied the key so users with + # Bitwarden see "(from Bitwarden)" — without this, a detected + # BSM key looks identical to a key in .env and users assume + # nothing is wired up. + source_suffix = "" + for var in PROVIDER_REGISTRY["anthropic"].api_key_env_vars: + if os.getenv(var, "").strip() == existing_key: + source_suffix = format_secret_source_suffix(var) + if source_suffix: + break + print( + f" Anthropic credentials: {existing_key[:12]}... ✓{source_suffix}" + ) elif cc_available: print(" Claude Code credentials: ✓ (auto-detected)") print() diff --git a/tests/test_env_loader_secret_sources.py b/tests/test_env_loader_secret_sources.py new file mode 100644 index 00000000000..8bd26451d9d --- /dev/null +++ b/tests/test_env_loader_secret_sources.py @@ -0,0 +1,119 @@ +"""Tests for the secret-source tracking in ``hermes_cli.env_loader``. + +These cover the small public surface that lets `hermes model` / `hermes setup` +label detected credentials with their origin ("from Bitwarden") so users +don't see an unexplained "credentials ✓" line when their .env is empty. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from hermes_cli import env_loader # noqa: E402 + + +@pytest.fixture(autouse=True) +def _reset_sources(): + """Each test starts with a clean source map.""" + env_loader._SECRET_SOURCES.clear() + yield + env_loader._SECRET_SOURCES.clear() + + +def test_get_secret_source_returns_none_for_untracked_var(): + assert env_loader.get_secret_source("ANTHROPIC_API_KEY") is None + + +def test_get_secret_source_returns_label_for_tracked_var(): + env_loader._SECRET_SOURCES["ANTHROPIC_API_KEY"] = "bitwarden" + assert env_loader.get_secret_source("ANTHROPIC_API_KEY") == "bitwarden" + + +def test_format_secret_source_suffix_empty_for_untracked(): + # Credentials from .env or the shell shouldn't add noise — the + # implicit case stays unlabeled. + assert env_loader.format_secret_source_suffix("ANTHROPIC_API_KEY") == "" + + +def test_format_secret_source_suffix_bitwarden_uses_proper_name(): + env_loader._SECRET_SOURCES["ANTHROPIC_API_KEY"] = "bitwarden" + assert ( + env_loader.format_secret_source_suffix("ANTHROPIC_API_KEY") + == " (from Bitwarden)" + ) + + +def test_format_secret_source_suffix_generic_label_for_future_sources(): + # Future-proofing: a new secret source (e.g. "vault") should still + # produce a sensible label without needing to edit every call site. + env_loader._SECRET_SOURCES["OPENAI_API_KEY"] = "vault" + assert ( + env_loader.format_secret_source_suffix("OPENAI_API_KEY") + == " (from vault)" + ) + + +def test_apply_external_secret_sources_records_bitwarden_origin(tmp_path, monkeypatch): + """End-to-end: when ``apply_bitwarden_secrets`` returns applied keys, + they end up in ``_SECRET_SOURCES`` so the UI can label them.""" + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config_path = tmp_path / "config.yaml" + config_path.write_text( + "secrets:\n" + " bitwarden:\n" + " enabled: true\n" + " project_id: test-project\n" + " access_token_env: BWS_ACCESS_TOKEN\n", + encoding="utf-8", + ) + + # Stub apply_bitwarden_secrets to return a synthetic FetchResult. + from agent.secret_sources.bitwarden import FetchResult + + fake_result = FetchResult( + secrets={"ANTHROPIC_API_KEY": "sk-ant-test"}, + applied=["ANTHROPIC_API_KEY"], + ) + + def _fake_apply(**_kwargs): + return fake_result + + # The import inside _apply_external_secret_sources is lazy, so we + # patch the *module attribute* it will pull in. + import agent.secret_sources.bitwarden as bw_module + + monkeypatch.setattr(bw_module, "apply_bitwarden_secrets", _fake_apply) + + env_loader._apply_external_secret_sources(tmp_path) + + assert env_loader.get_secret_source("ANTHROPIC_API_KEY") == "bitwarden" + assert ( + env_loader.format_secret_source_suffix("ANTHROPIC_API_KEY") + == " (from Bitwarden)" + ) + + +def test_apply_external_secret_sources_noop_when_disabled(tmp_path, monkeypatch): + """Disabled Bitwarden config must not touch the source map.""" + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config_path = tmp_path / "config.yaml" + config_path.write_text( + "secrets:\n" + " bitwarden:\n" + " enabled: false\n", + encoding="utf-8", + ) + + env_loader._apply_external_secret_sources(tmp_path) + + assert env_loader.get_secret_source("ANTHROPIC_API_KEY") is None