From 2beed8b53d988f3bf79e522258287e2b822af507 Mon Sep 17 00:00:00 2001 From: SeoYeonKim <28585885+westkite1201@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:16:06 -0700 Subject: [PATCH] fix(mcp): pass secret-source-injected env vars to stdio servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surgical reapply of PR #37523 onto current main (the original branch predates the SecretSource registry refactor). _build_safe_env() now forwards env vars tagged in env_loader._SECRET_SOURCES — widened from Bitwarden-only to any registered secret source (Bitwarden, 1Password, plugin backends), since the provenance map is source-agnostic. Explicit server env: config still wins; untagged secrets stay filtered. Fixes #37499. --- tests/tools/test_mcp_tool.py | 35 +++++++++++++++++++++++++++++++++++ tools/mcp_tool.py | 14 ++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 07e088533049..769a006940ee 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -1763,6 +1763,41 @@ class TestBuildSafeEnv: assert "DATABASE_URL" not in result assert "API_SECRET" not in result + def test_secret_source_injected_vars_are_passed(self, monkeypatch): + """Vars tagged by an external secret source (Bitwarden/1Password) are + deliberately allowed for MCP stdio servers.""" + from hermes_cli import env_loader + from tools.mcp_tool import _build_safe_env + + monkeypatch.setitem(env_loader._SECRET_SOURCES, "ALPACA_API_KEY", "bitwarden") + monkeypatch.setitem(env_loader._SECRET_SOURCES, "NOTION_TOKEN", "onepassword") + fake_env = { + "PATH": "/usr/bin", + "ALPACA_API_KEY": "from-bws-key", + "NOTION_TOKEN": "from-op", + "UNTRACKED_SECRET_KEY": "still-filtered", + } + with patch.dict("os.environ", fake_env, clear=True): + result = _build_safe_env(None) + + assert result["PATH"] == "/usr/bin" + assert result["ALPACA_API_KEY"] == "from-bws-key" + assert result["NOTION_TOKEN"] == "from-op" + assert "UNTRACKED_SECRET_KEY" not in result + + def test_user_env_overrides_secret_source_var(self, monkeypatch): + """Explicit MCP server env config remains the highest-precedence source.""" + from hermes_cli import env_loader + from tools.mcp_tool import _build_safe_env + + monkeypatch.setitem(env_loader._SECRET_SOURCES, "ALPACA_API_KEY", "bitwarden") + with patch.dict( + "os.environ", {"PATH": "/usr/bin", "ALPACA_API_KEY": "from-bws"}, clear=True + ): + result = _build_safe_env({"ALPACA_API_KEY": "from-config"}) + + assert result["ALPACA_API_KEY"] == "from-config" + def test_windows_location_vars_passed_without_secrets(self): """Windows launcher tools need location vars, but secrets stay filtered.""" from tools.mcp_tool import _build_safe_env diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index a4303bdab5c8..eeac9250d5f3 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -439,18 +439,28 @@ def _build_safe_env(user_env: Optional[dict]) -> dict: """Build a filtered environment dict for stdio subprocesses. Only passes through safe baseline variables (PATH, HOME, etc.) and XDG_* - variables from the current process environment, plus any variables + variables from the current process environment, secrets injected by an + external secret source (Bitwarden, 1Password, plugin backends) that + Hermes explicitly tagged during dotenv loading, plus any variables explicitly specified by the user in the server config. This prevents accidentally leaking secrets like API keys, tokens, or - credentials to MCP server subprocesses. + credentials to MCP server subprocesses. Secret-source-injected vars are + an exception: users configured that backend specifically so Hermes and + its subprocesses can consume those credentials without duplicating them + in every MCP server's ``env:`` block. """ + try: + from hermes_cli.env_loader import get_secret_source + except Exception: # pragma: no cover — early bootstrap/import fallback + get_secret_source = None env = {} for key, value in os.environ.items(): if ( key in _SAFE_ENV_KEYS or key.upper() in _SAFE_ENV_KEYS_CASE_INSENSITIVE or key.startswith("XDG_") + or (get_secret_source is not None and get_secret_source(key)) ): env[key] = value if user_env: