From 2c3a75beaaf0fabc5a5dcc932d57f4be70efa3d5 Mon Sep 17 00:00:00 2001 From: beardthelion Date: Tue, 22 Sep 2026 14:44:49 -0500 Subject: [PATCH] fix(secret-scope): scoped misses fail closed under a foreign-home scope get_secret returned os.environ on a scoped miss whenever multiplex was off, but non-multiplex hosts serve foreign homes too (dashboard/desktop backend, per-profile cron, MCP owner scopes, kanban spawn-env builds), where os.environ is the launch profile's. Bound scopes now carry the home they were built for; serves_routed_profile detects a foreign scope even when the binder deliberately skips the HERMES_HOME override, and the miss returns the caller's default. Every production binder stamps its home; own-home scopes keep the deliberate env overlay. --- agent/secret_scope.py | 54 ++++++++++--- cron/scheduler.py | 7 +- gateway/kanban_watchers_dispatcher.py | 3 +- gateway/run.py | 2 +- hermes_cli/kanban_db_dispatch.py | 3 +- hermes_cli/web_server_mcp.py | 3 +- hermes_cli/web_server_profiles.py | 2 +- tests/agent/test_secret_scope.py | 112 ++++++++++++++++++++++++++ tools/browser_tool_lifecycle.py | 2 +- tools/connectors/mcp.py | 9 ++- tools/connectors/mcp_oauth.py | 3 +- tools/mcp_tool_discovery.py | 4 +- tools/mcp_tool_lifecycle.py | 2 +- tui_gateway/compute_host.py | 3 +- tui_gateway/launch_profile_policy.py | 2 +- tui_gateway/model_switch.py | 4 +- 16 files changed, 183 insertions(+), 32 deletions(-) diff --git a/agent/secret_scope.py b/agent/secret_scope.py index 634f30e9f1..2dd0728ebe 100644 --- a/agent/secret_scope.py +++ b/agent/secret_scope.py @@ -18,7 +18,7 @@ import threading from collections import OrderedDict from contextvars import ContextVar, Token from pathlib import Path -from typing import Dict, Mapping, Optional, Tuple +from typing import Dict, Mapping, NamedTuple, Optional, Tuple from utils import file_signature @@ -38,19 +38,34 @@ def is_multiplex_active() -> bool: return _MULTIPLEX_ACTIVE +class _BoundScope(NamedTuple): + """An installed secret scope plus the home it was built for, when the binder + declared one — the provenance ``serves_routed_profile`` needs when the binding + deliberately skips the HERMES_HOME override (kanban spawn-env builds, MCP + owner scopes).""" + + mapping: Mapping[str, str] + profile_home: Optional[str] + + def serves_routed_profile() -> bool: """True when the current task runs for a profile other than the process's own: always under multiplexing, else when a HERMES_HOME override names another home (dashboard/desktop backend, - per-profile cron ticker). The MCP registry scope and the check_fn cache key both follow this - predicate so a served profile's view never aliases the launch profile's (#111151).""" + per-profile cron ticker) or a secret scope stamped with a foreign home is bound. The MCP + registry scope and the check_fn cache key both follow this predicate so a served profile's + view never aliases the launch profile's (#111151).""" if is_multiplex_active(): return True from hermes_constants import get_hermes_home_override, get_process_hermes_home, hermes_home_key + own = hermes_home_key(get_process_hermes_home()) + bound = _SECRET_SCOPE.get() + if bound is not None and bound.profile_home and hermes_home_key(bound.profile_home) != own: + return True override = get_hermes_home_override() - return override is not None and hermes_home_key(override) != hermes_home_key(get_process_hermes_home()) + return override is not None and hermes_home_key(override) != own -_SECRET_SCOPE: ContextVar[Optional[Mapping[str, str]]] = ContextVar("_SECRET_SCOPE", default=None) +_SECRET_SCOPE: ContextVar[Optional[_BoundScope]] = ContextVar("_SECRET_SCOPE", default=None) class UnscopedSecretError(RuntimeError): @@ -81,9 +96,15 @@ class UnscopedSecretError(RuntimeError): self.add_note(developer_detail) -def set_secret_scope(secrets: Optional[Mapping[str, str]]) -> Token: - """Install the active profile's secret mapping; ``None`` clears. Returns a reset token.""" - return _SECRET_SCOPE.set(secrets) +def set_secret_scope(secrets: Optional[Mapping[str, str]], *, profile_home: Optional[str] = None) -> Token: + """Install the active profile's secret mapping; ``None`` clears. Returns a reset token. + + ``profile_home`` stamps the home the mapping was built for so + ``serves_routed_profile`` detects a foreign-home scope even when the binder + deliberately skips the HERMES_HOME override.""" + if secrets is None: + return _SECRET_SCOPE.set(None) + return _SECRET_SCOPE.set(_BoundScope(secrets, str(profile_home) if profile_home else None)) def reset_secret_scope(token: Token) -> None: @@ -92,7 +113,14 @@ def reset_secret_scope(token: Token) -> None: def current_secret_scope() -> Optional[Mapping[str, str]]: """The active secret mapping, or None when no scope is installed.""" - return _SECRET_SCOPE.get() + bound = _SECRET_SCOPE.get() + return bound.mapping if bound is not None else None + + +def current_secret_scope_home() -> Optional[str]: + """The home the active scope was stamped with, or None when unstamped/unbound.""" + bound = _SECRET_SCOPE.get() + return bound.profile_home if bound is not None else None # Genuinely-global env vars: process/deployment settings, NOT profile secrets. @@ -156,12 +184,12 @@ def get_secret(name: str, default: Optional[str] = None) -> Optional[str]: """ if _is_global_env(name): return _environ_or(name, default) - scope = _SECRET_SCOPE.get() - if scope is not None: - val = scope.get(name) + bound = _SECRET_SCOPE.get() + if bound is not None: + val = bound.mapping.get(name) if val is not None: return val - return default if _MULTIPLEX_ACTIVE else _environ_or(name, default) + return default if (_MULTIPLEX_ACTIVE or serves_routed_profile()) else _environ_or(name, default) if _MULTIPLEX_ACTIVE: raise UnscopedSecretError( name, diff --git a/cron/scheduler.py b/cron/scheduler.py index 1d156c0d6e..22d51f3e7e 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -3147,7 +3147,8 @@ def _run_one_job_body( # get_secret() fails closed outside a scope; the ticker thread has none. Delivery adapters # resolve credentials, so the scope must span delivery too (reset in the outer finally). - _scope_token = set_secret_scope(build_profile_secret_scope(_get_hermes_home())) + _scope_token = set_secret_scope( + build_profile_secret_scope(_get_hermes_home()), profile_home=str(_get_hermes_home())) # Same for terminal policy (gateway/run.py _profile_runtime_scope): else the ticker reads # process-global TERMINAL_* env a concurrent profile pinned. Resolution failure installs a # refusal scope — terminal execution raises instead of using the launch process's policy. @@ -3484,7 +3485,7 @@ def _launch_external_cron_worker(job: dict) -> bool: profile_home = _get_hermes_home().resolve() hydrate_profile_secret_sources(profile_home) - secret_token = set_secret_scope(build_profile_secret_scope(profile_home)) + secret_token = set_secret_scope(build_profile_secret_scope(profile_home), profile_home=str(profile_home)) try: worker_env = strip_launch_profile_env(build_subprocess_env( scrub_secrets=multiplex_active, @@ -3666,7 +3667,7 @@ def _run_external_worker_payload(payload_path: Path, ack_path: Path) -> bool: multiplex_active = bool(payload.get("multiplex_active", False)) set_multiplex_active(multiplex_active) hydrate_profile_secret_sources(profile_home) - secret_token = set_secret_scope(build_profile_secret_scope(profile_home)) + secret_token = set_secret_scope(build_profile_secret_scope(profile_home), profile_home=str(profile_home)) try: with use_cron_store(profile_home): if adopt_claimed_execution(execution_id) is None: diff --git a/gateway/kanban_watchers_dispatcher.py b/gateway/kanban_watchers_dispatcher.py index 5b3ef08fdb..8c58fcb60c 100644 --- a/gateway/kanban_watchers_dispatcher.py +++ b/gateway/kanban_watchers_dispatcher.py @@ -309,7 +309,8 @@ def _default_profile_secret_scope(): if not is_multiplex_active(): yield return - token = set_secret_scope(build_profile_secret_scope(Path(get_hermes_home()))) + token = set_secret_scope( + build_profile_secret_scope(Path(get_hermes_home())), profile_home=str(get_hermes_home())) try: yield finally: diff --git a/gateway/run.py b/gateway/run.py index 2c07b36de4..9ec33b16c7 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1821,7 +1821,7 @@ def _profile_runtime_scope( else: from agent.secret_scope import build_profile_secret_scope # caller already hydrated off-loop secrets = build_profile_secret_scope(Path(profile_home)) - secret_token = set_secret_scope(secrets) + secret_token = set_secret_scope(secrets, profile_home=str(profile_home)) # Install the routed profile's COMPLETE terminal policy, never ambient TERMINAL_* a prior turn set. # Without it terminal_tool reads the process-global TERMINAL_* vars a previous profile's turn may have # pinned (first-writer-wins backend leak; #68559). diff --git a/hermes_cli/kanban_db_dispatch.py b/hermes_cli/kanban_db_dispatch.py index 17b3460447..91d80951dc 100644 --- a/hermes_cli/kanban_db_dispatch.py +++ b/hermes_cli/kanban_db_dispatch.py @@ -2601,7 +2601,8 @@ def _worker_profile_scope(hermes_home: str, *, bind_home: bool = True): is_launch_home = str(home.resolve()) == str(Path(get_process_hermes_home()).resolve()) home_token = set_hermes_home_override(str(home)) if bind_home else None secret_token = set_secret_scope( - launch_secret_scope(home) if is_launch_home else build_profile_secret_scope(home)) + launch_secret_scope(home) if is_launch_home else build_profile_secret_scope(home), + profile_home=str(home)) terminal_token = install_profile_terminal_scope( home, env_overlay=launch_terminal_env() if is_launch_home else None) if bind_home else None try: diff --git a/hermes_cli/web_server_mcp.py b/hermes_cli/web_server_mcp.py index 24f3cc87b1..1cdbe5e97b 100644 --- a/hermes_cli/web_server_mcp.py +++ b/hermes_cli/web_server_mcp.py @@ -127,7 +127,8 @@ def _run_dashboard_mcp_oauth(flow, cfg: dict) -> None: from tools.mcp_oauth_manager import get_manager home_token = set_hermes_home_override(flow.hermes_home) - secret_token = set_secret_scope(build_profile_secret_scope(Path(flow.hermes_home))) + secret_token = set_secret_scope( + build_profile_secret_scope(Path(flow.hermes_home)), profile_home=flow.hermes_home) try: transaction = _mcp_oauth_transaction(flow) with transaction, force_interactive_oauth(), dashboard_oauth_flow(flow): diff --git a/hermes_cli/web_server_profiles.py b/hermes_cli/web_server_profiles.py index fed42466a1..618f21d477 100644 --- a/hermes_cli/web_server_profiles.py +++ b/hermes_cli/web_server_profiles.py @@ -313,7 +313,7 @@ def _config_profile_scope(profile: Optional[str]): # and an unscoped launch request would then raise ``UnscopedSecretError`` on its next read. secrets = launch_secret_scope(process_home) with (_hermes_home_scope(profile_dir) if profile_dir is not None else nullcontext()): - token = set_secret_scope(secrets) + token = set_secret_scope(secrets, profile_home=str(profile_dir or process_home)) try: yield scoped finally: diff --git a/tests/agent/test_secret_scope.py b/tests/agent/test_secret_scope.py index 372a251621..b812b38db5 100644 --- a/tests/agent/test_secret_scope.py +++ b/tests/agent/test_secret_scope.py @@ -92,6 +92,118 @@ class TestScopedSingleProfile: ss.reset_secret_scope(token) +class TestRoutedForeignHomeScope: + """Multiplex OFF but the bound scope serves a DIFFERENT home (routed profile: + dashboard/desktop backend, per-profile cron ticker). os.environ is the LAUNCH + profile's env there, so a scoped miss must fail closed exactly like multiplex — + the .env-overlay fallthrough is only safe when the scope's home IS ours.""" + + def test_scoped_miss_under_foreign_home_returns_default(self, monkeypatch, tmp_path): + from hermes_constants import set_hermes_home_override, reset_hermes_home_override + + monkeypatch.setenv("OPENAI_API_KEY", "sk-launch-profile") + home_token = set_hermes_home_override(str(tmp_path / "other-profile")) + token = ss.set_secret_scope({}) + try: + assert ss.serves_routed_profile() is True + assert ss.get_secret("OPENAI_API_KEY") is None + assert ss.get_secret("OPENAI_API_KEY", "d") == "d" + finally: + ss.reset_secret_scope(token) + reset_hermes_home_override(home_token) + + def test_scoped_miss_under_own_home_keeps_env_overlay(self, monkeypatch, tmp_path): + """The deliberate single-profile overlay: a scope bound for the process's + own home still falls through to os.environ (systemd / op run credentials).""" + from hermes_constants import get_process_hermes_home, set_hermes_home_override, reset_hermes_home_override + + monkeypatch.setenv("OPENAI_API_KEY", "sk-own-env") + home_token = set_hermes_home_override(str(get_process_hermes_home())) + token = ss.set_secret_scope({}) + try: + assert ss.serves_routed_profile() is False + assert ss.get_secret("OPENAI_API_KEY") == "sk-own-env" + finally: + ss.reset_secret_scope(token) + reset_hermes_home_override(home_token) + + def test_scope_hit_under_foreign_home_still_wins(self, monkeypatch, tmp_path): + """A scoped hit is unaffected: only the miss branch changes.""" + from hermes_constants import set_hermes_home_override, reset_hermes_home_override + + monkeypatch.setenv("OPENAI_API_KEY", "sk-launch-profile") + home_token = set_hermes_home_override(str(tmp_path / "other-profile")) + token = ss.set_secret_scope({"OPENAI_API_KEY": "sk-served-profile"}) + try: + assert ss.get_secret("OPENAI_API_KEY") == "sk-served-profile" + finally: + ss.reset_secret_scope(token) + reset_hermes_home_override(home_token) + + def test_stamped_foreign_scope_miss_fails_closed_without_override(self, monkeypatch, tmp_path): + """The kanban/MCP shape: a foreign-home scope bound WITHOUT the HERMES_HOME + override (deliberate — those paths need the dispatcher's policy reads). + The profile_home stamp makes serves_routed_profile see it anyway.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-launch-profile") + token = ss.set_secret_scope({}, profile_home=str(tmp_path / "other-profile")) + try: + assert ss.serves_routed_profile() is True + assert ss.get_secret("OPENAI_API_KEY") is None + assert ss.get_secret("OPENAI_API_KEY", "d") == "d" + finally: + ss.reset_secret_scope(token) + + def test_stamped_own_home_scope_keeps_env_overlay(self, monkeypatch): + """A scope stamped with the process's own home is not routed: env + fallthrough stays, matching launch_secret_scope's documented precedence.""" + from hermes_constants import get_process_hermes_home + + monkeypatch.setenv("OPENAI_API_KEY", "sk-own-env") + token = ss.set_secret_scope({}, profile_home=str(get_process_hermes_home())) + try: + assert ss.serves_routed_profile() is False + assert ss.get_secret("OPENAI_API_KEY") == "sk-own-env" + finally: + ss.reset_secret_scope(token) + + def test_profile_runtime_scope_binds_foreign_home_e2e(self, monkeypatch, tmp_path): + """End to end through the real binder: ``_profile_runtime_scope`` (the same + guard the desktop backend and routed turns use) installs the home override + + secret scope together. Inside it, a miss must not borrow launch env, while + the foreign profile's own .env resolves and the adapter-facing reader agrees.""" + from gateway.platforms._shared import get_scoped_secret + from gateway.run import _profile_runtime_scope + + foreign = tmp_path / "profiles" / "team_b" + foreign.mkdir(parents=True) + (foreign / ".env").write_text("TEAM_B_KEY=from-b\n", encoding="utf-8") + monkeypatch.setenv("OPENAI_API_KEY", "sk-launch-profile") + + with _profile_runtime_scope(foreign, hydrate_secrets=False): + assert ss.serves_routed_profile() is True + assert ss.get_secret("OPENAI_API_KEY") is None + assert ss.get_secret("TEAM_B_KEY") == "from-b" + # The reader adapters actually call takes the same fail-closed path. + assert get_scoped_secret("OPENAI_API_KEY") is None + + def test_worker_profile_scope_bind_home_false_e2e(self, monkeypatch, tmp_path): + """The kanban spawn-env build binds the assignee's secret scope with + ``bind_home=False`` — no home override, because the passthrough POLICY + belongs to the dispatcher. The profile_home stamp must still make scoped + misses fail closed, or the dispatcher's env leaks into B's worker env.""" + from hermes_cli.kanban_db_dispatch import _worker_profile_scope + + foreign = tmp_path / "profiles" / "assignee" + foreign.mkdir(parents=True) + (foreign / ".env").write_text("ASSIGNEE_KEY=from-assignee\n", encoding="utf-8") + monkeypatch.setenv("OPENAI_API_KEY", "sk-dispatcher") + + with _worker_profile_scope(str(foreign), bind_home=False): + assert ss.serves_routed_profile() is True + assert ss.get_secret("OPENAI_API_KEY") is None + assert ss.get_secret("ASSIGNEE_KEY") == "from-assignee" + + class TestScopeIsolation: """Two scopes never see each other's secrets.""" diff --git a/tools/browser_tool_lifecycle.py b/tools/browser_tool_lifecycle.py index c1ae1ef07d..765bbd5e01 100644 --- a/tools/browser_tool_lifecycle.py +++ b/tools/browser_tool_lifecycle.py @@ -124,7 +124,7 @@ def _session_owner_scope(task_id: str): home_token = set_hermes_home_override(owner_home) try: hydrate_profile_secret_sources(Path(owner_home)) - secret_token = set_secret_scope(build_profile_secret_scope(Path(owner_home))) + secret_token = set_secret_scope(build_profile_secret_scope(Path(owner_home)), profile_home=owner_home) try: yield finally: diff --git a/tools/connectors/mcp.py b/tools/connectors/mcp.py index 0fca8aca39..18c91791a5 100644 --- a/tools/connectors/mcp.py +++ b/tools/connectors/mcp.py @@ -135,7 +135,8 @@ class _CatalogBackend: """Probe the entry's in-memory configuration with ephemeral credentials; save both only after the server answered. A failure writes nothing, so a failed reinstall keeps the previous configuration.""" - from agent.secret_scope import current_secret_scope, reset_secret_scope, set_secret_scope + from agent.secret_scope import ( + current_secret_scope, current_secret_scope_home, reset_secret_scope, set_secret_scope) from hermes_cli.mcp_catalog import _inline_non_secret_value, card_install_config from hermes_cli.mcp_config import _probe_single_server, _save_mcp_server @@ -148,7 +149,11 @@ class _CatalogBackend: for key, value in env.items(): if key not in secret_names and value: cfg = _inline_non_secret_value(cfg, key, value) - token = set_secret_scope({**dict(current_secret_scope() or {}), **env}) + # The merged scope keeps the bound scope's home stamp: dropping it would + # reopen the env fallthrough under a routed profile with multiplex off. + token = set_secret_scope( + {**dict(current_secret_scope() or {}), **env}, + profile_home=current_secret_scope_home()) try: tools = [str(tool[0]) for tool in (_probe_single_server(name, cfg) or [])] finally: diff --git a/tools/connectors/mcp_oauth.py b/tools/connectors/mcp_oauth.py index 79ea23f130..e9c29b7965 100644 --- a/tools/connectors/mcp_oauth.py +++ b/tools/connectors/mcp_oauth.py @@ -163,7 +163,8 @@ def run_worker( from tools.mcp_dashboard_oauth import dashboard_oauth_flow from tools.mcp_oauth import force_interactive_oauth home_token = set_hermes_home_override(hermes_home) - secret_token = set_secret_scope({**build_profile_secret_scope(Path(hermes_home)), **(env or {})}) + secret_token = set_secret_scope( + {**build_profile_secret_scope(Path(hermes_home)), **(env or {})}, profile_home=hermes_home) try: if not (reuse_saved and flow is not None and _reuse_saved_authorization(server_name, cfg, flow, on_commit)): diff --git a/tools/mcp_tool_discovery.py b/tools/mcp_tool_discovery.py index 1dd1eb9b16..0ae4be65a7 100644 --- a/tools/mcp_tool_discovery.py +++ b/tools/mcp_tool_discovery.py @@ -95,7 +95,7 @@ async def _install_owner_secret_scope(): from hermes_cli.env_loader import hydrate_profile_secret_sources # Off-loop: an external source runs a helper subprocess (once per home, then cached). await asyncio.to_thread(hydrate_profile_secret_sources, home) - return set_secret_scope(build_profile_secret_scope(home)) + return set_secret_scope(build_profile_secret_scope(home), profile_home=str(home)) @contextmanager @@ -113,7 +113,7 @@ def _owner_secret_scope(): return from hermes_cli.env_loader import hydrate_profile_secret_sources hydrate_profile_secret_sources(home) - token = set_secret_scope(build_profile_secret_scope(home)) + token = set_secret_scope(build_profile_secret_scope(home), profile_home=str(home)) try: yield finally: diff --git a/tools/mcp_tool_lifecycle.py b/tools/mcp_tool_lifecycle.py index 8896af9314..a727395fe0 100644 --- a/tools/mcp_tool_lifecycle.py +++ b/tools/mcp_tool_lifecycle.py @@ -115,7 +115,7 @@ def _reregister_orphaned_adopters() -> None: from tools.mcp_tool_config import _load_mcp_config for adopter, names in pending.items(): home_token = set_hermes_home_override(adopter) - secret_token = set_secret_scope(build_profile_secret_scope(Path(adopter))) + secret_token = set_secret_scope(build_profile_secret_scope(Path(adopter)), profile_home=adopter) try: servers = {n: c for n, c in (_load_mcp_config() or {}).items() if n in names} if servers: diff --git a/tui_gateway/compute_host.py b/tui_gateway/compute_host.py index d3bdaf8724..b9c221b8ec 100644 --- a/tui_gateway/compute_host.py +++ b/tui_gateway/compute_host.py @@ -322,7 +322,8 @@ class ComputeHost: from agent.secret_scope import build_profile_secret_scope, set_secret_scope from hermes_state_registry import acquire home_token = set_hermes_home_override(profile_home) - secret_token = set_secret_scope(build_profile_secret_scope(Path(profile_home))) + secret_token = set_secret_scope( + build_profile_secret_scope(Path(profile_home)), profile_home=profile_home) # DEDICATED handle — ours only until _make_agent succeeds, then the agent owns # it. A RAISING _make_agent is the one path where nothing takes it (``owns_db``). session_db = acquire(Path(profile_home) / "state.db") diff --git a/tui_gateway/launch_profile_policy.py b/tui_gateway/launch_profile_policy.py index 5a4a3db5ec..a613a09e1e 100644 --- a/tui_gateway/launch_profile_policy.py +++ b/tui_gateway/launch_profile_policy.py @@ -167,7 +167,7 @@ def launch_profile_runtime_scope(launch_home: "str | Path") -> Iterator[None]: home = Path(launch_home) home_token = set_hermes_home_override(str(home)) - secret_token = set_secret_scope(launch_secret_scope(home)) + secret_token = set_secret_scope(launch_secret_scope(home), profile_home=str(home)) terminal_token = install_profile_terminal_scope(home, env_overlay=launch_terminal_env()) try: yield diff --git a/tui_gateway/model_switch.py b/tui_gateway/model_switch.py index 849702344e..0401589560 100644 --- a/tui_gateway/model_switch.py +++ b/tui_gateway/model_switch.py @@ -85,13 +85,13 @@ def _profile_runtime_scope_tokens(profile_home, *, hydrate_secrets: bool = True) from tui_gateway.launch_profile_policy import launch_secret_scope, launch_terminal_env home = Path(_hermes_home) secrets = launch_secret_scope(home) - scopes.secret = set_secret_scope(secrets) + scopes.secret = set_secret_scope(secrets, profile_home=str(home)) if not is_multiplex_active(): return scopes scopes.home = set_hermes_home_override(str(home)) overlay = launch_terminal_env() if scopes.secret is None: - scopes.secret = set_secret_scope(secrets) + scopes.secret = set_secret_scope(secrets, profile_home=str(home)) # Same terminal policy the gateway binds per turn: a docker-configured profile # must never resolve the launch process's pinned env. Failure → refusal scope. from tools.terminal_scope import install_profile_terminal_scope