fix(kanban): scrub the dispatcher's credentials from another profile's worker on any host
_default_spawn gated the scrub on is_multiplex_active(), so on a single-profile host — the common Kanban deployment — build_subprocess_env never reached _sanitize_subprocess_env and profile B's worker env was byte-identical to the dispatcher's, OPENAI_API_KEY and systemd-injected tokens included. The authority test is "is this worker acting for a ROUTED home", exactly as served_profile_child_env decides it, not the gateway-wide flag. Drops install_profile_terminal_scope from the bind_home=False branch: that path has no terminal-scope consumer and it cost a config.yaml parse per spawn. The test now asserts the worker ENV rather than that a scope object was bound.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user