Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,10 @@ jobs:
git fetch --no-tags --deepen=1000 origin "${{ github.base_ref }}" HEAD
done
python scripts/ci/check_public_surface.py --base "origin/${{ github.base_ref }}" --head HEAD

# Under gateway multiplexing one process serves every profile off one shared
# os.environ, so a raw os.getenv of a profile-varying key reads whichever
# profile loaded last (cross-profile leak). Reads must go through the
# fail-closed profile scope; see scripts/check_profile_env_scope.py.
- name: Forbid raw reads of profile-varying env vars
run: python scripts/check_profile_env_scope.py
50 changes: 49 additions & 1 deletion agent/file_safety.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,58 @@ def build_write_denied_prefixes(home: str) -> list[str]:
return [os.path.realpath(p) + os.sep for p in paths]


def _safe_write_root_raw() -> str:
"""The active profile's HERMES_WRITE_SAFE_ROOT, scope-aware.

HERMES_WRITE_SAFE_ROOT is a container-wide floor (Dockerfile sets ENV
HERMES_WRITE_SAFE_ROOT=/opt/data, the same for every profile) that a profile
MAY tighten in its own .env. Under gateway multiplexing that per-profile
override leaked: .env loads into the shared os.environ with override=True, so
a plain os.getenv let whichever profile loaded last gate every profile's
writes (proven live: felix gated by jonas's /home/jonas).

Resolve by layer, never cross-profile:
* scope HIT with a NON-EMPTY value (this profile tightened it) -> use it.
Fixes the leak: felix/jonas each set their own, so each hits its own
scope, never the other's.
* scope hit with an EXPLICIT EMPTY value (HERMES_WRITE_SAFE_ROOT= in the
profile's .env) is treated as absent, NOT as allow-all: load_env_file
keeps the empty entry so it reaches the scope, but an empty override must
not erase the container-wide floor (that would fail OPEN). It falls
through to the floor below, exactly like a scope miss.
* scope MISS or unscoped -> the container-wide floor from os.environ. A
profile that sets its own never reaches here, so this is the global
Dockerfile floor, not another profile's secret. Crucially it is NOT the
empty default: returning "" here would drop the /opt/data confinement and
fail OPEN (allow-all) for every profile that relies on the container floor
- worse than the leak.
"""
try:
from agent.secret_scope import current_secret_scope
except ImportError:
# secret_scope unavailable (ACP shim / import order): no multiplex, this
# process's own os.environ value is safe.
return os.environ.get("HERMES_WRITE_SAFE_ROOT", "") or "" # scope-exempt: ImportError fallback, no secret_scope = single-profile
scope = current_secret_scope()
if scope is not None and scope.get("HERMES_WRITE_SAFE_ROOT"):
# Non-empty scoped value only. An explicit empty override (KEY=) is
# retained by load_env_file but must NOT erase the container floor, so it
# falls through to os.environ below - fail closed, never allow-all.
return scope["HERMES_WRITE_SAFE_ROOT"]
# Scope miss / unscoped: fall back to the container-wide floor. Not a leak - a
# profile that set its own value hit the branch above; only profiles on the
# container default reach here. Load-bearing invariant: under multiplex,
# hermes_cli/env_loader.py (load_hermes_dotenv) SKIPS the override=True dotenv
# load for routed profiles, so os.environ never holds a routed profile's
# HERMES_WRITE_SAFE_ROOT - only the global Dockerfile floor. If that ever
# changes, this read could mis-gate one profile's writes with a stale value.
return os.environ.get("HERMES_WRITE_SAFE_ROOT", "") or "" # scope-exempt: container-wide floor, per-profile override handled by scope hit above


def get_safe_write_roots() -> set[str]:
"""Resolved HERMES_WRITE_SAFE_ROOT paths (``os.pathsep``-separated list)."""
roots: set[str] = set()
for path in filter(None, os.getenv("HERMES_WRITE_SAFE_ROOT", "").split(os.pathsep)):
for path in filter(None, _safe_write_root_raw().split(os.pathsep)):
with suppress(OSError, ValueError):
roots.add(os.path.realpath(os.path.expanduser(path)))
return roots
Expand Down
2 changes: 1 addition & 1 deletion agent/runtime_cwd.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def scope_terminal_cwd() -> str:
try:
from tools.terminal_scope import terminal_env
except ImportError:
return os.environ.get("TERMINAL_CWD", "")
return os.environ.get("TERMINAL_CWD", "") # scope-exempt: ImportError fallback for terminal_env
return terminal_env("TERMINAL_CWD", "")


Expand Down
12 changes: 12 additions & 0 deletions agent/secret_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ def get_secret(name: str, default: Optional[str] = None) -> Optional[str]:
return _environ_or(name, default)


def secret_or(name: str, default: str = "") -> str:
"""``get_secret`` for a fail-closed policy read: an unscoped-multiplex miss
returns *default* instead of raising. Use for profile-varying policy toggles
where the empty/default value is the safe (restrictive) one - NOT for
HERMES_WRITE_SAFE_ROOT, whose empty value is permissive (see
agent.file_safety._safe_write_root_raw for that key's layered resolution)."""
try:
return get_secret(name, default) or default
except UnscopedSecretError:
return default


def _strip_inline_comment(value: str) -> str:
"""Strip a dotenv-style inline comment (python-dotenv semantics): quoted values
scan to the matching close quote (backslash-aware for double quotes) and drop a
Expand Down
8 changes: 7 additions & 1 deletion agent/shell_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,13 @@ def _command_script_path(command: str) -> str:

def _resolve_effective_accept(cfg: Dict[str, Any], accept_hooks_arg: bool) -> bool:
"""Any truthy opt-in channel wins: explicit arg, HERMES_ACCEPT_HOOKS, hooks_auto_accept."""
if accept_hooks_arg or os.environ.get("HERMES_ACCEPT_HOOKS", "").strip().lower() in _TRUTHY:
# Scope-aware: under multiplex os.environ holds another profile's value, and a
# leaked truthy here would auto-approve hooks for a profile that never opted in
# (trust-boundary leak). secret_or resolves the bound profile; unscoped-multiplex
# misses fall to "" (fail closed -> require explicit approval).
from agent.secret_scope import secret_or
env_accept = secret_or("HERMES_ACCEPT_HOOKS", "").strip().lower()
if accept_hooks_arg or env_accept in _TRUTHY:
return True
cfg_val = cfg.get("hooks_auto_accept", False)
return cfg_val if isinstance(cfg_val, bool) else isinstance(cfg_val, str) and cfg_val.strip().lower() in _TRUTHY
Expand Down
3 changes: 2 additions & 1 deletion agent/tool_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,8 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) -
elif function_name == "terminal":
command = function_args.get("command", "")
if _is_destructive_command(command):
cwd = function_args.get("workdir") or os.getenv("TERMINAL_CWD", os.getcwd())
from agent.runtime_cwd import scope_terminal_cwd
cwd = function_args.get("workdir") or scope_terminal_cwd() or os.getcwd()
agent._checkpoint_mgr.ensure_checkpoint(cwd, f"before terminal: {command[:60]}")


Expand Down
11 changes: 11 additions & 0 deletions hermes_cli/env_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,17 @@ def _reapply_terminal_config_bridge(home_path: Path) -> None:
try:
if Path(home_path).resolve() != _process_hermes_home().resolve():
return
# An argument-less load_hermes_dotenv() (lazy import mid-tick) resolves home_path from the process
# HERMES_HOME, so it passes the override-immune guard above even while a routed-profile home override
# is active - but apply_terminal_config_to_env() reads config via the override-FOLLOWING
# get_hermes_home(), which would bridge the routed profile's terminal.* into the shared os.environ and
# hijack the launch profile's next unscoped turn (#102769 route 2). Any override must suppress the
# bridge - unlike the multiplex-gated dotenv skip above, since apply_terminal_config_to_env follows
# the override regardless of multiplex.
from hermes_constants import get_hermes_home_override

if get_hermes_home_override() is not None:
return
from hermes_cli.config import apply_terminal_config_to_env

apply_terminal_config_to_env(env=None)
Expand Down
140 changes: 140 additions & 0 deletions scripts/check_profile_env_scope.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
#!/usr/bin/env python3
"""Fail when profile-varying env vars are read raw instead of through the profile scope.

Under gateway multiplexing one process serves every profile off ONE shared
os.environ, into which each profile's .env is loaded with override=True. So a
plain ``os.getenv("TERMINAL_ENV")`` / ``os.environ["HERMES_WRITE_SAFE_ROOT"]``
reads whichever profile loaded last, not the profile whose turn is running - a
cross-profile leak (proven live: felix's write_file gated by jonas's
HERMES_WRITE_SAFE_ROOT). Every profile-varying read must go through the
fail-closed scope helpers instead:

* TERMINAL_* (a global-env prefix, so get_secret would NOT isolate them) ->
``tools.terminal_scope.terminal_env`` / ``agent.runtime_cwd.scope_terminal_cwd``
* HERMES_WRITE_SAFE_ROOT / HERMES_ACCEPT_HOOKS / HERMES_ALLOW_PRIVATE_URLS
(non-global policy vars) -> ``agent.secret_scope.get_secret``

This walks tools/ and agent/ and flags a raw os.getenv / os.environ.get /
os.environ[...] whose key is (or starts with) a banned name. Legitimate raw
readers (the scope machinery itself, and terminal_tool which reads the projected
scope out of its own env) opt out with a trailing ``# scope-exempt: <reason>``
comment on the offending line.

Exit 1 with a file:line list on any hit. Run: python scripts/check_profile_env_scope.py

Scope and limits (best-effort regression nudge, NOT a complete security control):
this matches only literal-key ``os.getenv("K")`` / ``os.environ.get("K")`` /
``os.environ["K"]`` where the module is imported as ``os``. It does NOT catch an
aliased import (``import os as o``), a ``from os import getenv``, a variable/
computed key (``os.getenv(k)``), or ``os.environ.copy()``. It scans only
``tools/`` and ``agent/`` - the modules that run inside the multiplexed gateway
worker; single-profile CLI/oneshot entrypoints under ``hermes_cli/`` are out of
scope by design. The read-time scope helpers, not this guard, are the actual
isolation control; this just stops the most common verbatim regression.
"""
from __future__ import annotations

import argparse
import ast
import sys
from pathlib import Path

ROOT = Path(__file__).resolve().parent.parent
SCAN_DIRS = ("tools", "agent")

# Exact profile-varying keys that must be scoped.
BANNED_EXACT = frozenset({
"HERMES_WRITE_SAFE_ROOT",
"HERMES_ACCEPT_HOOKS",
"HERMES_ALLOW_PRIVATE_URLS",
})
# Whole prefixes that are profile-varying (terminal/sandbox backend policy).
BANNED_PREFIXES = ("TERMINAL_",)

EXEMPT_MARKER = "# scope-exempt"


def _is_banned(key: str) -> bool:
return key in BANNED_EXACT or key.startswith(BANNED_PREFIXES)


def _py_files(root: Path):
for d in SCAN_DIRS:
base = root / d
if not base.is_dir():
continue
for p in base.rglob("*.py"):
if "__pycache__" in p.parts:
continue
yield p


def _first_str_arg(node: ast.Call) -> str | None:
if node.args and isinstance(node.args[0], ast.Constant) and isinstance(node.args[0].value, str):
return node.args[0].value
return None


def _raw_env_key(node: ast.AST) -> str | None:
"""Return the env key if node is a raw os.environ / os.getenv access, else None."""
# os.getenv("KEY") / os.environ.get("KEY")
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
fn = node.func
# os.getenv(...)
if fn.attr == "getenv" and isinstance(fn.value, ast.Name) and fn.value.id == "os":
return _first_str_arg(node)
# os.environ.get(...)
if (fn.attr == "get" and isinstance(fn.value, ast.Attribute)
and fn.value.attr == "environ"
and isinstance(fn.value.value, ast.Name) and fn.value.value.id == "os"):
return _first_str_arg(node)
# os.environ["KEY"] (Subscript)
if isinstance(node, ast.Subscript) and isinstance(node.value, ast.Attribute):
attr = node.value
if (attr.attr == "environ" and isinstance(attr.value, ast.Name) and attr.value.id == "os"):
key = node.slice
if isinstance(key, ast.Constant) and isinstance(key.value, str):
return key.value
return None


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--root", type=Path, default=ROOT, help="repo root to scan (default: this repo)")
args = parser.parse_args()
root = args.root.resolve()
hits: list[str] = []
for path in _py_files(root):
rel = path.relative_to(root)
try:
src = path.read_text(encoding="utf-8", errors="ignore")
tree = ast.parse(src)
except SyntaxError:
continue
lines = src.splitlines()
for node in ast.walk(tree):
key = _raw_env_key(node)
if key is None or not _is_banned(key):
continue
ln = getattr(node, "lineno", 0)
src_line = lines[ln - 1] if 0 < ln <= len(lines) else ""
if EXEMPT_MARKER in src_line:
continue
hits.append(f"{rel}:{ln}: raw os.environ read of profile-varying '{key}'")
if hits:
print("❌ profile-varying env vars read raw (cross-profile leak under multiplexing):")
for h in sorted(set(hits)):
print(" " + h)
print(
f"\n{len(set(hits))} site(s). Read TERMINAL_* via tools.terminal_scope.terminal_env "
"(or agent.runtime_cwd.scope_terminal_cwd for TERMINAL_CWD), and the HERMES_* policy "
"vars via agent.secret_scope.get_secret. A legitimate raw reader opts out with a "
f"trailing '{EXEMPT_MARKER}: <reason>' comment."
)
return 1
print(f"✅ no raw reads of profile-varying env vars in {'/, '.join(SCAN_DIRS)}/")
return 0


if __name__ == "__main__":
sys.exit(main())
93 changes: 93 additions & 0 deletions tests/agent/test_file_safety_write_root_scope.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""HERMES_WRITE_SAFE_ROOT must resolve per-profile, never cross-profile - and
must keep the container-wide floor when a profile does not set its own.

Under gateway multiplexing one process serves many profiles off one shared
os.environ; the write-safety allowlist used to be read with a plain os.getenv,
so profile A's roots gated profile B's writes (proven live: felix saw
/home/jonas). It now resolves by layer: a profile's own scoped value wins; a
scope miss falls back to the container-wide floor (Dockerfile ENV /opt/data),
never to empty (which would fail OPEN) and never to another profile's value.
"""
from __future__ import annotations

import pytest

import agent.secret_scope as ss
import agent.file_safety as fs


@pytest.fixture(autouse=True)
def _reset_scope():
ss.set_secret_scope(None)
ss.set_multiplex_active(False)
try:
yield
finally:
ss.set_secret_scope(None)
ss.set_multiplex_active(False)


def test_scope_hit_wins_over_os_environ(monkeypatch):
# os.environ carries profile B's value (loaded last); scope binds profile A.
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "/home/jonas")
ss.set_multiplex_active(True)
ss.set_secret_scope({"HERMES_WRITE_SAFE_ROOT": "/home/felix"})
roots = fs.get_safe_write_roots()
assert any(r.endswith("/home/felix") for r in roots), roots
assert not any(r.endswith("/home/jonas") for r in roots), roots


def test_scope_miss_keeps_container_floor_not_allow_all(monkeypatch):
# Profile bound a scope but did NOT set its own root: must keep the
# container-wide floor (/opt/data), NOT fail open to an empty allowlist.
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "/opt/data")
ss.set_multiplex_active(True)
ss.set_secret_scope({"OPENAI_API_KEY": "x"}) # no WRITE_SAFE_ROOT
roots = fs.get_safe_write_roots()
assert any(r.endswith("/opt/data") for r in roots), roots
# And the floor actually confines: a path outside it is denied.
assert fs.is_write_denied("/tmp/evil") is True
assert fs.is_write_denied("/opt/data/profiles/x/notes.txt") is False


def test_explicit_empty_scoped_value_keeps_container_floor(monkeypatch):
# BLOCKER regression: a multiplexed profile whose .env sets an EXPLICIT empty
# HERMES_WRITE_SAFE_ROOT= is retained by load_env_file and reaches the scope.
# It must NOT erase the container-wide floor (that would fail OPEN, allow-all);
# the empty override falls through to the /opt/data floor, same as a miss.
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "/opt/data")
ss.set_multiplex_active(True)
ss.set_secret_scope({"HERMES_WRITE_SAFE_ROOT": ""}) # explicit empty override
roots = fs.get_safe_write_roots()
assert any(r.endswith("/opt/data") for r in roots), roots
# The floor still confines: a path outside it stays denied.
assert fs.is_write_denied("/tmp/evil") is True
assert fs.is_write_denied("/opt/data/profiles/x/notes.txt") is False


def test_unscoped_multiplex_keeps_container_floor(monkeypatch):
# No scope + multiplex active: still honor the container floor, never crash,
# never allow-all.
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "/opt/data")
ss.set_multiplex_active(True)
ss.set_secret_scope(None)
roots = fs.get_safe_write_roots() # must not raise
assert any(r.endswith("/opt/data") for r in roots), roots
assert fs.is_write_denied("/tmp/evil") is True


def test_single_profile_unscoped_reads_os_environ(monkeypatch):
# Multiplex OFF, no scope: behaves exactly as before (os.environ value).
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "/srv/data")
ss.set_multiplex_active(False)
ss.set_secret_scope(None)
roots = fs.get_safe_write_roots()
assert any(r.endswith("/srv/data") for r in roots), roots


def test_truly_unset_is_permissive(monkeypatch):
# No var anywhere = the historical unset baseline: no safe-root restriction.
monkeypatch.delenv("HERMES_WRITE_SAFE_ROOT", raising=False)
ss.set_multiplex_active(False)
ss.set_secret_scope(None)
assert fs.get_safe_write_roots() == set()
Loading
Loading