fix(security): strip dynamic Hermes secrets (AUXILIARY_*/GATEWAY_RELAY_*) from all subprocess spawn env + route codex through hermes_subprocess_env - #213
Conversation
…Y_*) from all subprocess spawn env + route codex through hermes_subprocess_env
|
Review Complete Files Reviewed: 8 By Severity:
The PR adds new Tier-1 secret keys (GATEWAY_RELAY_*) to the unconditional strip list and improves env passthrough hygiene across subprocess spawn paths. Two medium-priority gaps found: the Docker forward_env path bypasses _is_hermes_internal_secret filtering for explicit keys, and the FORCE_PREFIX injection path doesn't enforce _ALWAYS_STRIP_KEYS for non-suffixed Tier-1 keys like GATEWAY_RELAY_ID. Files Reviewed (8 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟡 Medium (48/100) — 2 medium findings · 407 LOC across 8 files
Summary
This PR strengthens credential isolation across subprocess spawn paths by adding new Tier-1 secrets (GATEWAY_RELAY_ID, GATEWAY_RELAY_SECRET, GATEWAY_RELAY_DELIVERY_KEY) to the unconditional _ALWAYS_STRIP_KEYS set and expanding the _is_hermes_internal_secret predicate to cover GATEWAY_RELAY_*_SECRET/_KEY/_TOKEN patterns. Changes span agent/transports/codex_app_server.py, tools/env_passthrough.py, tools/environments/docker.py, and tools/environments/local.py, with corresponding test updates.
Findings
finding-001: Docker forward_env bypasses secret filtering (tools/environments/docker.py:1004)
Explicit docker_forward_env keys are unioned directly into the Docker container's env without _is_hermes_internal_secret checking. A user who adds a relay-auth secret to docker_forward_env config would inject it into the container's init snapshot. Fix: filter the union of explicit and implicit keys through _is_hermes_internal_secret.
finding-002: FORCE_PREFIX path doesn't enforce _ALWAYS_STRIP_KEYS (tools/environments/local.py:286)
The _sanitize_subprocess_env and _make_run_env FORCE_PREFIX handling only checks _is_hermes_internal_secret, but Tier-1 keys like GATEWAY_RELAY_ID lack a secret suffix and pass that check. Since GATEWAY_RELAY_ID is in _ALWAYS_STRIP_KEYS, the FORCE_PREFIX path must also skip keys present in that set.
Risk Assessment
Both findings require specific misconfiguration to be exploitable, but they violate the documented invariant that Tier-1 secrets are stripped unconditionally on every spawn path. Medium severity, low exploitability.
| _implicit_forward = { | ||
| k for k in passthrough_keys if not _is_hermes_internal_secret(k) | ||
| } | ||
| forward_keys = explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST) |
There was a problem hiding this comment.
🟡 Explicit docker_forward_env keys bypass _is_hermes_internal_secret filtering (security)
In tools/environments/docker.py, _build_init_env_args applies _is_hermes_internal_secret filtering only to implicit passthrough keys derived from get_all_passthrough(), but NOT to explicit forward keys from self._forward_env (docker_forward_env config). The is_hermes_internal_secret predicate (tools/environments/local.py:211-253) is documented as the single source of truth for stripping AUXILIARY_API_KEY/BASE_URL and GATEWAY_RELAY_SECRET/_KEY/_TOKEN secrets 'unconditionally regardless of env_passthrough skill registration or inherit_credentials' across every spawn path. The local subprocess sanitizer (_sanitize_subprocess_env, local.py:280-281) enforces this by checking every key against _is_hermes_internal_secret before passthrough logic. The Docker path breaks this contract for explicit forward keys — line 1004 unions explicit_forward_keys directly into forward_keys without any _is_hermes_internal_secret check. A user who adds e.g. GATEWAY_RELAY_SESSION_TOKEN to docker_forward_env in config.yaml would have that relay-auth secret injected via docker exec -e into the container's init_session snapshot.
💡 Suggestion: Apply _is_hermes_internal_secret to both explicit and implicit forward keys by filtering the union through a set comprehension that checks _is_hermes_internal_secret on every key. Update the comment at lines 996-1000 to note that explicit opt-in wins over the name-based blocklist (_HERMES_PROVIDER_ENV_BLOCKLIST) but NOT over the unconditional internal-secret predicate.
| forward_keys = explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST) | |
| forward_keys = {k for k in (explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST)) if not _is_hermes_internal_secret(k)} |
📋 Prompt for AI Agents
In tools/environments/docker.py, _build_init_env_args method (line 1004), change:
forward_keys = explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST)
to:
forward_keys = {k for k in (explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST)) if not _is_hermes_internal_secret(k)}
Also update the comment at lines 996-1000 to note that explicit opt-in does not override _is_hermes_internal_secret stripping.
| if key.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): | ||
| real_key = key[len(_HERMES_PROVIDER_ENV_FORCE_PREFIX):] | ||
| if _is_hermes_internal_secret(real_key): | ||
| continue | ||
| sanitized[real_key] = value |
There was a problem hiding this comment.
🟡 _ALWAYS_STRIP_KEYS Tier-1 policy not enforced in FORCE_PREFIX path of _sanitize_subprocess_env and _make_run_env (security)
The patch adds GATEWAY_RELAY_ID, GATEWAY_RELAY_SECRET, and GATEWAY_RELAY_DELIVERY_KEY to _ALWAYS_STRIP_KEYS (Tier-1: stripped unconditionally on every spawn path). In hermes_subprocess_env, this is correctly enforced — _ALWAYS_STRIP_KEYS entries are explicitly popped AND all FORCE_PREFIX keys are unconditionally stripped. However, in _sanitize_subprocess_env extra_env path (line 285-290) and _make_run_env (line 681-685), the FORCE_PREFIX handling uses only _is_hermes_internal_secret(real_key) as its guard before injecting the real key into the child environment. Since _is_hermes_internal_secret('GATEWAY_RELAY_ID') returns False (the key lacks a secret suffix — confirmed intentional by test at test_local_env_blocklist.py:662), a _HERMES_FORCE_GATEWAY_RELAY_ID entry in extra_env or the terminal env dict would inject GATEWAY_RELAY_ID into the sandboxed subprocess, violating the patch's own Tier-1 guarantee. Other Tier-1 secrets have overlapping coverage (e.g. TELEGRAM_BOT_TOKEN is also in _HERMES_PROVIDER_ENV_BLOCKLIST), but GATEWAY_RELAY_ID relies solely on _ALWAYS_STRIP_KEYS for its unconditional-strip guarantee on the non-terminal path.
💡 Suggestion: Add a check against _ALWAYS_STRIP_KEYS in the FORCE_PREFIX branch of both _sanitize_subprocess_env (line 286-290) and _make_run_env (line 681-685), so that Tier-1 secrets cannot be injected via FORCE_PREFIX. After the _is_hermes_internal_secret check, also skip the key if real_key in _ALWAYS_STRIP_KEYS. Additionally, harmonize with _sanitize_subprocess_env base_env path (line 278-279) which unconditionally skips all FORCE_PREFIX keys from os.environ without unwrapping.
📋 Prompt for AI Agents
In tools/environments/local.py, inside _sanitize_subprocess_env at the FORCE_PREFIX branch (lines 285-290) and _make_run_env at the FORCE_PREFIX branch (lines 681-685), after the if _is_hermes_internal_secret(real_key): continue check, also add if real_key in _ALWAYS_STRIP_KEYS: continue to enforce the Tier-1 unconditional-strip policy consistently across all spawn paths.
1 participant
Summary
Model-authored subprocesses can no longer read Hermes-internal secrets that are injected into
os.environunder dynamic names the static blocklist can't enumerate, and the codex app-server no longer inherits the operator's full secret environment.Root cause:
_HERMES_PROVIDER_ENV_BLOCKLISTmatches by exact name, but the gateway/CLI inject secrets at runtime under names no registry knows about —AUXILIARY_<TASK>_API_KEY/_BASE_URL(per-task side-LLM creds) andGATEWAY_RELAY_*auth. Separately,agent/transports/codex_app_server.pybuilt its spawn env from a rawos.environ.copy(), bypassing the centralizedhermes_subprocess_env()helper entirely (the NousResearch#29157 sibling spawn-site gap —copilot_acp_clientalready routed through it).Changes
tools/environments/local.py: add_is_hermes_internal_secret()— the single source of truth for the dynamic patterns (AUXILIARY_*_API_KEY/_BASE_URL,GATEWAY_RELAY_*_SECRET/_KEY/_TOKEN). Wire it unconditionally (ignores skillenv_passthroughopt-in ANDinherit_credentials) into_sanitize_subprocess_env,_make_run_env, andhermes_subprocess_env. Add theGATEWAY_RELAY_*names to the static blocklist, and theGATEWAY_RELAY_ID/_SECRET/_DELIVERY_KEYtriplet to_ALWAYS_STRIP_KEYSso_ID(no secret suffix) is also stripped on the model-driving-CLI inherit path.tools/environments/docker.py: strip the same dynamic patterns from the Dockerforward_envfilter.tools/env_passthrough.py:_is_hermes_provider_credential()consults the shared predicate — defense in depth so a skill can't register these names as passthrough and tunnel them into anexecute_code/terminal child.agent/transports/codex_app_server.py: route the spawn env throughhermes_subprocess_env(inherit_credentials=True)— strips Tier-1 + dynamic-internal secrets while keeping the provider creds codex needs to authenticate. Non-secretAUXILIARY_*_PROVIDER/_MODELandGATEWAY_RELAYrouting hints (_URL,_PLATFORMS,_ROUTE_KEYS, …) remain visible.Validation
_sanitize_subprocess_env_make_run_envhermes_subprocess_env(inherit=True)(codex/copilot)env_passthroughregistration243 targeted tests pass (
tests/tools/test_local_env_blocklist.py,test_hermes_subprocess_env.py,test_env_passthrough.py,test_docker_environment.py,tests/agent/transports/test_codex_app_server_runtime.py+_session.py); ruff clean; ty shows zero new diagnostics vsorigin/main.Consolidation / credit
Consolidates three PRs into one architectural fix on current
main:_is_hermes_internal_secretpredicate backbone + Docker filter.env_passthroughdefense-in-depth guard.hermes_subprocess_env(inherit_credentials=True)routing.Supersedes NousResearch#52348 (@claudlos): its copilot half is already on
main, and its codex half used the full-strip_sanitize_subprocess_env— which would break codex provider auth. The correct tier isinherit_credentials=True(codex needs provider creds). All four contributors credited viaCo-authored-by.Closes NousResearch#53715
Closes NousResearch#53503
Closes NousResearch#55709
Closes NousResearch#52348
Mirror-of: NousResearch#56202
NousResearch#56202