mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
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.
This commit is contained in:
parent
cc395e8050
commit
daf4f1a7a9
2 changed files with 61 additions and 1 deletions
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue