From daf4f1a7a917b3ff82a6e2f02e45d4d88c63aa2e Mon Sep 17 00:00:00 2001
From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Date: Wed, 1 Jul 2026 15:34:10 +0530
Subject: [PATCH] fix(tools): close the same session leak on the
 hermes_subprocess_env spawn surface (review)

Review of the #50531 salvage found the cross-session HERMES_SESSION_* leak also
survives on the non-terminal spawn helper hermes_subprocess_env (added by #56202
after #50531 was written), which does os.environ.copy() without the guard. Of
its six callers, five re-bind the session identity explicitly (slash_worker/ACP
via --session-key argv) and are safe by accident; but tui_gateway cli.exec
(server.py) spawns a fresh CLI with NO --session-key under the engaged TUI host,
so it inherits a possibly-foreign HERMES_SESSION_* from the last-writer-wins
global and would stamp Kanban rows / telemetry with another session's id.

Route hermes_subprocess_env through the same _inject_session_context_env
chokepoint, restoring the single-uniform-policy-across-every-spawn-surface
invariant the codebase already claims for the internal-secret filter. Safe for
all six callers: bound ContextVars win (re-binders unaffected), _UNSET strips
(closes cli.exec). Adds 3 guard tests; mutation-checked.
---
 tests/tools/test_local_env_session_leak.py | 52 +++++++++++++++++++++-
 tools/environments/local.py                | 10 +++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/tests/tools/test_local_env_session_leak.py b/tests/tools/test_local_env_session_leak.py
index e6af1d2e2a3..924122eee85 100644
--- a/tests/tools/test_local_env_session_leak.py
+++ b/tests/tools/test_local_env_session_leak.py
@@ -29,7 +29,7 @@ import pytest
 
 import gateway.session_context as sc
 from gateway.session_context import _VAR_MAP, clear_session_vars, set_session_vars
-from tools.environments.local import _make_run_env, _sanitize_subprocess_env
+from tools.environments.local import _make_run_env, _sanitize_subprocess_env, hermes_subprocess_env
 
 # The full set of session vars the bridge owns.
 SESSION_VARS = list(_VAR_MAP.keys())
@@ -252,3 +252,53 @@ def test_sanitize_subprocess_env_unengaged_preserves_fallback(monkeypatch):
     sanitized = _sanitize_subprocess_env(stale_base)
 
     assert sanitized.get("HERMES_SESSION_KEY") == "cli-bg-key"
+
+
+# --------------------------------------------------------------------------- #
+# Non-terminal spawn surface (hermes_subprocess_env) — sibling path
+# --------------------------------------------------------------------------- #
+
+def test_hermes_subprocess_env_strips_foreign_session_key_when_engaged(monkeypatch):
+    """hermes_subprocess_env (browser/ACP/CLI/TUI-host spawns) must not leak a
+    foreign session key either. cli.exec spawns via this helper WITHOUT re-binding
+    the session identity, so an UNSET ContextVar under an engaged host must strip
+    the inherited global rather than hand the child another session's identity.
+    """
+    _engage()
+    monkeypatch.setenv(
+        "HERMES_SESSION_KEY",
+        "agent:main:discord:thread:FOREIGN_CONCURRENT:FOREIGN_CONCURRENT",
+    )
+
+    env = hermes_subprocess_env()
+
+    assert "HERMES_SESSION_KEY" not in env, (
+        "Foreign concurrent session key leaked into non-terminal spawn env: "
+        f"{env.get('HERMES_SESSION_KEY')!r}"
+    )
+
+
+def test_hermes_subprocess_env_bound_contextvar_wins(monkeypatch):
+    """A caller that binds the session identity keeps it through this helper."""
+    monkeypatch.setenv(
+        "HERMES_SESSION_KEY",
+        "agent:main:discord:thread:FOREIGN:FOREIGN",
+    )
+    tokens = set_session_vars(
+        session_key="agent:main:discord:group:MINE:111",
+        platform="discord",
+        chat_id="MINE",
+    )
+    try:
+        env = hermes_subprocess_env()
+        assert env.get("HERMES_SESSION_KEY") == "agent:main:discord:group:MINE:111"
+    finally:
+        clear_session_vars(tokens)
+
+
+def test_hermes_subprocess_env_unengaged_preserves_fallback(monkeypatch):
+    """A pure single-process CLI (never engaged) keeps the inherited fallback."""
+    monkeypatch.setenv("HERMES_SESSION_KEY", "cli-fallback-key")
+    # not engaged (autouse fixture leaves _session_context_engaged False)
+    env = hermes_subprocess_env()
+    assert env.get("HERMES_SESSION_KEY") == "cli-fallback-key"
diff --git a/tools/environments/local.py b/tools/environments/local.py
index 3416402d6e4..49c87cd081e 100644
--- a/tools/environments/local.py
+++ b/tools/environments/local.py
@@ -462,6 +462,16 @@ def hermes_subprocess_env(*, inherit_credentials: bool = False) -> dict[str, str
     for _marker in _ACTIVE_VENV_MARKER_VARS:
         env.pop(_marker, None)
 
+    # Cross-session leak guard, same as the terminal spawn paths: this helper
+    # copies os.environ, whose HERMES_SESSION_* mirror is a last-writer-wins
+    # global under a concurrent multi-session host. A caller that re-binds the
+    # session identity explicitly (slash_worker/ACP via --session-key argv) is
+    # unaffected — bound ContextVars win here — but a caller that spawns without
+    # re-binding (e.g. tui_gateway cli.exec) would otherwise inherit a FOREIGN
+    # session's identity. Strip _UNSET session vars when engaged so that can't
+    # happen; single uniform policy across every spawn surface.
+    _inject_session_context_env(env)
+
     return env