diff --git a/hermes_cli/kanban_db_dispatch.py b/hermes_cli/kanban_db_dispatch.py index d19ea4ba25..17b3460447 100644 --- a/hermes_cli/kanban_db_dispatch.py +++ b/hermes_cli/kanban_db_dispatch.py @@ -2579,12 +2579,13 @@ def _worker_profile_scope(hermes_home: str, *, bind_home: bool = True): The dispatcher runs detached from any turn, so nothing binds a profile for it: ``load_config``, the toolset probes' ``get_secret`` reads and ``build_subprocess_env``'s passthrough resolution all fall back to the LAUNCH profile's ambient ``os.environ`` / ``TERMINAL_*``. Binding was - previously conditional on ``is_multiplex_active()`` and skipped the terminal scope entirely, so - a worker for profile B inherited whatever TERMINAL_* the host process happened to carry. + previously conditional on ``is_multiplex_active()``, so on a single-profile host a worker for + profile B was built entirely from the dispatcher's own environment. ``bind_home=False`` for the spawn-env build: which variables may cross into a child is the - DISPATCHER's ``terminal.env_passthrough`` policy (#109494) — only their VALUES come from the - assignee's scope. Toolset resolution does bind the home, as it always has. + DISPATCHER's ``terminal.env_passthrough`` policy (#109494, read through the home override) — + only their VALUES come from the assignee's scope, so that branch binds the secret scope alone. + Toolset resolution binds the home and the terminal policy, as it always has. The secret mapping is never widened: a profile that is not this process's own home gets its own ``.env`` + external sources ONLY, while the launch home keeps its established @@ -2602,11 +2603,12 @@ def _worker_profile_scope(hermes_home: str, *, bind_home: bool = True): secret_token = set_secret_scope( launch_secret_scope(home) if is_launch_home else build_profile_secret_scope(home)) terminal_token = install_profile_terminal_scope( - home, env_overlay=launch_terminal_env() if is_launch_home else None) + home, env_overlay=launch_terminal_env() if is_launch_home else None) if bind_home else None try: yield finally: - reset_terminal_scope(terminal_token) + if terminal_token is not None: + reset_terminal_scope(terminal_token) reset_secret_scope(secret_token) if home_token is not None: reset_hermes_home_override(home_token) @@ -2773,7 +2775,7 @@ def _default_spawn(task: Task, workspace: str, *, board: Optional[str] = None) - profile_arg = normalize_profile_name(task.assignee) from agent.secret_scope import is_multiplex_active - from tools.environments.local import build_subprocess_env, strip_launch_profile_env + from tools.environments.local import _is_routed_home, build_subprocess_env, strip_launch_profile_env try: profile_home = resolve_profile_env(profile_arg) @@ -2782,15 +2784,18 @@ def _default_spawn(task: Task, workspace: str, *, board: Optional[str] = None) - # HERMES_PROFILE (set below) instead. profile_home = None - multiplex_active = is_multiplex_active() - # build_subprocess_env's secret scrub resolves terminal.env_passthrough vars through - # get_secret(), and its TERMINAL_* reads go through the terminal scope. Unscoped, both read the - # LAUNCH profile's ambient environment for a worker spawned on B's behalf — so bind B's secret - # + terminal scope, not just secrets and not only under multiplex. + # Scrub for a ROUTED home, not only under multiplex: the authority test is "does this worker act + # for another profile", exactly as served_profile_child_env decides it (tools/environments/local.py). + # Gating on the gateway-wide flag left B's worker inheriting the dispatcher's own OPENAI_API_KEY and + # systemd-injected tokens on every single-profile host. + routed = bool(profile_home) and _is_routed_home(profile_home) + # build_subprocess_env's secret scrub resolves terminal.env_passthrough vars through get_secret(), + # which without a bound scope reads the LAUNCH profile's ambient environment for a worker spawned + # on B's behalf (and raises under multiplex) — so bind B's secret scope around the build. with (_worker_profile_scope(profile_home, bind_home=False) if profile_home else contextlib.nullcontext()): env = build_subprocess_env( - scrub_secrets=multiplex_active, + scrub_secrets=is_multiplex_active() or routed, inherit_profile_home=True, ) # The dispatcher is detached from every conversation; its worker must never diff --git a/tests/hermes_cli/test_kanban_worker_terminal_scope.py b/tests/hermes_cli/test_kanban_worker_terminal_scope.py index a6974ba541..bec4f0748c 100644 --- a/tests/hermes_cli/test_kanban_worker_terminal_scope.py +++ b/tests/hermes_cli/test_kanban_worker_terminal_scope.py @@ -1,12 +1,12 @@ -"""A kanban worker spawned for profile B builds its env under B's terminal scope, not the ambient one. +"""A kanban worker spawned for profile B must not inherit the DISPATCHER's credentials. -``_default_spawn`` bound only a secret scope, and only when ``is_multiplex_active()``. The dispatcher -runs detached from any turn, so ``build_subprocess_env`` (terminal ``env_passthrough`` resolution) -and ``_resolve_worker_cli_toolsets`` read whatever ``TERMINAL_*`` the host process happened to carry -— the LAUNCH profile's policy applied to another tenant's worker. +``_default_spawn`` gated the credential scrub on ``is_multiplex_active()``, so on a single-profile +host — the common Kanban deployment — B's worker env was byte-identical to the dispatcher's: its +``OPENAI_API_KEY`` and anything systemd injected crossed straight into another profile's worker. +The authority test is "does this worker act for a ROUTED home", exactly as ``served_profile_child_env`` +decides it. The secret scope bound around the env build is what supplies B's OWN values for the +dispatcher's declared ``terminal.env_passthrough`` names. """ -import subprocess # noqa: F401 — imported so a stray real spawn is obvious in a traceback - import pytest from hermes_cli import kanban_db_dispatch @@ -14,7 +14,7 @@ from tools.terminal_scope import get_terminal_scope class _StopSpawn(Exception): - """Abort ``_default_spawn`` at the env-build seam so no worker process is created.""" + """Abort ``_default_spawn`` after the env is built so no worker process is created.""" @pytest.fixture @@ -33,7 +33,7 @@ def profile_b(tmp_path, monkeypatch): def test_worker_profile_scope_installs_the_assigned_profiles_terminal_policy(profile_b): - """The seam both dispatch-side readers use: toolset resolution and the spawn-env build.""" + """The toolset-resolution seam (``bind_home=True``) reads config under B's own policy.""" with kanban_db_dispatch._worker_profile_scope(str(profile_b)): scope = get_terminal_scope() or {} assert scope.get("TERMINAL_ENV") == "docker" @@ -41,19 +41,18 @@ def test_worker_profile_scope_installs_the_assigned_profiles_terminal_policy(pro f"worker inherited the launch profile's terminal policy: {scope}") -def test_default_spawn_builds_the_worker_env_under_the_assigned_profiles_scope( - profile_b, tmp_path, monkeypatch): +def _spawn_env_for_profile_b(monkeypatch, tmp_path): + """Run ``_default_spawn`` far enough to capture the worker env, never spawning anything.""" from hermes_cli.kanban_db import Task + from tools import process_registry - seen: list[dict] = [] + captured: list[dict] = [] - import tools.environments.local as local_env + def _capture(env): + captured.append(dict(env)) + raise _StopSpawn - def _capture(*_args, **_kwargs): - seen.append(dict(get_terminal_scope() or {})) - raise _StopSpawn # the invariant is observed; nothing must actually spawn - - monkeypatch.setattr(local_env, "build_subprocess_env", _capture) + monkeypatch.setattr(process_registry, "systemd_user_bus_env", _capture) task = Task( id="t1", title="t", body=None, assignee="b", status="claimed", priority=0, @@ -62,7 +61,43 @@ def test_default_spawn_builds_the_worker_env_under_the_assigned_profiles_scope( tenant=None) with pytest.raises(_StopSpawn): kanban_db_dispatch._default_spawn(task, str(tmp_path / "ws")) + assert captured, "_default_spawn never built a worker env" + return captured[0] - assert seen, "_default_spawn never reached build_subprocess_env" - assert seen[0].get("TERMINAL_ENV") == "docker", ( - f"spawn env built under the launch profile's terminal policy: {seen[0]}") + +def test_worker_for_another_profile_never_inherits_the_dispatchers_credentials( + profile_b, tmp_path, monkeypatch): + """Single-profile host (multiplex OFF) — the case the gateway-wide flag left unprotected.""" + monkeypatch.setenv("OPENAI_API_KEY", "dispatcher-launch-key") + env = _spawn_env_for_profile_b(monkeypatch, tmp_path) + assert "OPENAI_API_KEY" not in env, ( + "B's worker inherited the dispatcher's provider credential") + + +def test_launch_profiles_own_worker_keeps_its_credentials(tmp_path, monkeypatch): + """Control: a worker for the LAUNCH profile is not acting for another tenant.""" + launch = tmp_path / "fakehome" / ".hermes" + (launch / "profiles").mkdir(parents=True) + monkeypatch.setenv("HOME", str(tmp_path / "fakehome")) + monkeypatch.setenv("HERMES_HOME", str(launch)) + monkeypatch.setenv("OPENAI_API_KEY", "dispatcher-launch-key") + + from hermes_cli.kanban_db import Task + from tools import process_registry + + captured: list[dict] = [] + + def _capture(env): + captured.append(dict(env)) + raise _StopSpawn + + monkeypatch.setattr(process_registry, "systemd_user_bus_env", _capture) + task = Task( + id="t1", title="t", body=None, assignee="default", status="claimed", priority=0, + created_by=None, created_at=0, started_at=None, completed_at=None, + workspace_kind="dir", workspace_path=None, claim_lock=None, claim_expires=None, + tenant=None) + with pytest.raises(_StopSpawn): + kanban_db_dispatch._default_spawn(task, str(tmp_path / "ws")) + + assert captured and captured[0].get("OPENAI_API_KEY") == "dispatcher-launch-key"