Skip to content

fix(env): prevent cross-profile TERMINAL_* leak in multi-profile serve (#102769) - #103672

Closed
salch-cred wants to merge 1 commit into
NousResearch:mainfrom
salch-cred:fix/102769-terminal-leak-v2
Closed

salch-cred wants to merge 1 commit into
NousResearch:mainfrom
salch-cred:fix/102769-terminal-leak-v2

Conversation

@salch-cred

Copy link
Copy Markdown

Summary

Cross-profile terminal hijack under multi-profile serve (#102769, P0 — data-integrity + security).

_reapply_terminal_config_bridge()'s guard compared home_path against get_hermes_home() — the context-overridden home. Under multi-profile serve, a routed profile's turn (e.g. its cron tick) runs with the context override pointing at the routed profile's home, so its load_hermes_dotenv(hermes_home=<routed>) call passed the guard and the shared bridge bridged the routed profile's terminal config into the shared os.environ.

The launch profile's subsequent unscoped turns then executed in the routed profile's Docker backend with its volumes — commands run against another profile's containers, mounts, and credentials.

Fix

Compare against get_process_hermes_home() — the true process launch home that ignores context overrides. The bridge may now run only for the launch profile's own dotenv reloads, regardless of any active routing override.

Reproduction (proven red on current main)

New suite tests/hermes_cli/test_terminal_bridge_scope_102769.py (4 tests, invariant contracts — hermes_cli/config.apply_terminal_config_to_env is mock-observed so the test exercises the real guard, not the mock's opinion):

  1. test_context_override_does_not_change_the_verdict — the incident witness. With a HERMES_HOME_OVERRIDE contextvar pointing at the routed home (the exact multi-profile-serve shape), the routed home is still refused. Fails on unpatched main (AssertionError: the bridge fired); passes with the fix.
  2. test_routed_profile_home_does_not_bridge — routed home never bridges (the hijack).
  3. test_launch_home_still_bridges — control: the launch profile's own reload still re-applies the bridge (the stale-.env fix of TERMINAL_ENV in ~/.hermes/.env overrides terminal.backend in config.yaml #29186/Standalone cron runs should reapply terminal config after loading .env #67323 is preserved).
  4. test_fail_open_on_guard_error — a broken guard lookup must not break dotenv loading (fail-open contract).

Testing

  • New suite: 4/4 passed
  • tests/hermes_cli/test_env_loader.py: 26/26 passed (no regressions in the dotenv family)
  • ruff check: clean

Fixes #102769

Companion leak-class fixes: #102752 (authz env reads), #102802 (credential leak, 19 adapters) — this PR closes the terminal-config arm.

NousResearch#102769)

The _reapply_terminal_config_bridge() guard was comparing against
get_hermes_home() (which follows context overrides), allowing a routed
profile's cron tick to bridge its terminal config into the shared
os.environ. This hijacked the launch profile's subsequent unscoped
turns into the routed profile's Docker backend with its volumes.

Fix: compare against get_process_hermes_home() which ignores context
overrides and returns the true process launch home.

Fixes NousResearch#102769
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/profiles Multi-profile isolation, HERMES_HOME scoping duplicate This issue or pull request already exists labels Sep 5, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #103625 (merged). _reapply_terminal_config_bridge on current main already scopes the bridge to get_process_hermes_home() via _process_hermes_home(), so the guard fix here is already shipped; this branch's removed lines reflect pre-#103625 code. Previous attempts #102800 and #102932 were closed for the same reason. If the extra invariant tests are still wanted, consider a test-only follow-up against current main.

@salch-cred

Copy link
Copy Markdown
Author

Closing as duplicate — #103625 (merged, salvage of #97014) shipped the same guard fix to main. Verified: hermes_cli/env_loader.py on current main now imports get_process_hermes_home and the guard compares against it. My branch's regression-test suite (tests/hermes_cli/test_terminal_bridge_scope_102769.py, proven-red on pre-fix main) is available if maintainers want it as a follow-up — happy to rebase it as a test-only PR against the merged fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

2 participants