fix: harden subprocess env handling (scrub secrets, isolate browser sessions) by ai-ag2026 · Pull Request #59840 · NousResearch/hermes-agent

ai-ag2026 · 2026-07-06T20:53:48Z

Two small, independent hardening fixes for subprocess/env handling:

scrub secrets from terminal env snapshots — captured environment snapshots no longer carry tokens/secrets, so they are safe to log/persist.
isolate browser tool subprocess sessions — concurrent browser-tool runs no longer share subprocess session state (init session respects its own cwd).

Testing

pytest tests/tools/test_browser_tool_process_isolation.py tests/tools/test_init_session_cwd_respect.py → 11 passed, on current main.

teknium1

Thanks for addressing two real hardening gaps. The current main branch still writes unfiltered snapshots (tools/environments/base.py:502,650) and the browser helper Popen paths lack POSIX session isolation (tools/browser_tool.py:1105,2444).

Problems

Blocking: tools/environments/base.py:423 filters values that Docker intentionally injects only during init_session. DockerEnvironment._run_bash() adds _init_env_args only when login=True (tools/environments/docker.py:1066-1069), and its contract says subsequent commands rely on the snapshot (1012-1024). Explicit forwarded credentials would therefore disappear after initialization.
The PR CI run has two failing existing assertions in tests/tools/test_base_environment.py (lines 219 and 236) that still search for export -p >.
tests/tools/test_browser_tool_process_isolation.py:26 is a source-shape test, not behavioral coverage.

Suggested changes

Preserve explicitly forwarded Docker env per command without writing it into the reusable snapshot, and cover that path end-to-end.
Update the remaining assertions and replace the source-count test with mocked-Popen behavioral coverage for both call sites.

Automated hermes-sweeper review.

teknium1 · 2026-07-15T20:59:16Z

@@ -396,7 +420,7 @@ def init_session(self):
        _snap_tmp = shlex.quote(self._snapshot_path + ".tmp.") + "$BASHPID"
        bootstrap = (
            f"umask 077\n"
-            f"export -p > {_snap_tmp}\n"
+            f"{_snapshot_export_command(_snap_tmp)}\n"


Blocking: Docker adds forwarded -e values only for init_session; later commands rely exclusively on this snapshot (tools/environments/docker.py:1012-1024,1066-1069). Filtering here removes explicitly forwarded credentials after the first command. Preserve them per docker exec without persisting them in the reusable snapshot.

teknium1 · 2026-07-15T20:59:16Z

+    assert isinstance(extra["startupinfo"], FakeStartupInfo)
+    assert extra["startupinfo"].dwFlags == 0x100
+    assert "start_new_session" not in extra
+


Please replace this source-count assertion with behavioral coverage that intercepts subprocess.Popen at both execution paths and verifies the kwargs. AGENTS.md prohibits tests that read source text because they do not validate runtime wiring.

ai-ag2026 · 2026-07-16T00:37:54Z

Current head: fe19ece4cdd975a42234a778743a2f1b179260a6

The two active review findings are addressed on the current head:

Explicit docker_forward_env values are forwarded on every docker exec while remaining excluded from the reusable snapshot; static/container and implicit passthrough values retain their existing scope.
Browser process-isolation coverage now intercepts subprocess.Popen and executes both real production launch paths instead of counting source text.

Validation:

python -m pytest tests/tools/test_base_environment.py tests/tools/test_init_session_cwd_respect.py tests/tools/test_browser_tool_process_isolation.py tests/tools/test_docker_environment.py -q -o addopts= — 110 passed
python -m py_compile for all four touched Python files — passed
git diff --check HEAD^ HEAD — passed
Added-line security scan — 0 findings

@teknium1 Fresh review requested on the current head. I have intentionally left the existing threads unresolved for maintainer readback.

ai-ag2026 · 2026-07-16T00:48:14Z

Current repaired head: dab33c018500f88dbdfefdc4461a4d6102089e73

Follow-up to the validation above: independent review found that the scrub pipeline could mask a failing export -p. The current head now captures both Bash PIPESTATUS values in the shared snapshot helper, accepts grep's normal no-match status, and prevents publication when the export or filter fails. Both command-update and real bootstrap regressions make export -p emit partial output and fail, then prove the prior snapshot remains byte-for-byte unchanged.

Current-head validation:

Focused snapshot/Docker/browser selector — 111 passed
Changed-file py_compile — passed
git diff --check 9a4dd068865432a93dd4944a30b2f4c8d280e289..dab33c018500f88dbdfefdc4461a4d6102089e73 — passed
Complete repair added-line security scan — 0 findings

@teknium1 Fresh review requested on dab33c018500f88dbdfefdc4461a4d6102089e73. Existing human threads remain unresolved for maintainer confirmation.

(cherry picked from commit 1b5d0fedda9131f1b56876e0aa369a29d9356e08) (cherry picked from commit cd4eff10c6b4e4509ebce89a06b7c6372a68e015)

(cherry picked from commit 606e5b08621099f221ba9da3b0cb79b2e48fde56) (cherry picked from commit da6f0936134511d6d16b2159d540bf299c0306d1)

The secret-scrubbing snapshot rewrite changed export -p > $tmp.$BASHPID && mv -f $tmp.$BASHPID $snap to export -p | grep -Eiv <SECRET_RE> > $tmp.$BASHPID && mv -f $tmp.$BASHPID $snap The redirect now binds to grep, which runs in its own pipe subshell, so $BASHPID in the temp path expands to grep's PID while the caller's mv expands it to the outer shell PID. The names diverge, the mv finds nothing, and the env snapshot is never updated — exported vars stop persisting between commands (TestSnapshotEndToEnd: exported env / PATH / venv activation all regressed). Fix: wrap the pipe in a brace group so the redirect binds to the group (run by the current shell), keeping $BASHPID consistent with the mv. Secret scrubbing is preserved. Also update the TestWrapCommand/TestAtomicSnapshotWrite string assertions to expect the scrub pipe (export -p | grep ... >) instead of the pre-scrub direct redirect. tests/tools/test_base_environment.py + test_local_shell_init.py: 42/42 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

andrexibiza · 2026-08-11T01:04:43Z

Bound to the child-process credential-inheritance class under #83565 (#83565) — same bug class, different surface. shared _popen_bash boundary + browser session isolation; Wave B — currently dirty; rebase onto the merged boundary. The EPIC carries the live class table, dedup adjudication, and the dependency-driven merge order.

alt-glitch added type/bug Something isn't working tool/browser Browser automation (CDP, Playwright) tool/terminal Terminal execution and process management P3 Low — cosmetic, nice to have labels Jul 6, 2026

teknium1 reviewed Jul 15, 2026

View reviewed changes

ai-ag2026 and others added 5 commits July 16, 2026 11:43

fix: scrub secrets from terminal env snapshots

5938442

(cherry picked from commit 1b5d0fedda9131f1b56876e0aa369a29d9356e08) (cherry picked from commit cd4eff10c6b4e4509ebce89a06b7c6372a68e015)

fix: isolate browser tool subprocess sessions

3e02bf8

(cherry picked from commit 606e5b08621099f221ba9da3b0cb79b2e48fde56) (cherry picked from commit da6f0936134511d6d16b2159d540bf299c0306d1)

fix: preserve explicit env across docker commands

6353702

fix: preserve snapshots when export fails

ff99152

ai-ag2026 force-pushed the upstream/pr-a1-subprocess-hardening branch from dab33c0 to ff99152 Compare July 16, 2026 09:54

ai-ag2026 mentioned this pull request Jul 17, 2026

Hardening fixes for compression, Hindsight, and env snapshots #57612

Closed

teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026

GottZ mentioned this pull request Aug 3, 2026

fix(hindsight): surface retain timeouts clearly #59842

Open

This was referenced Aug 11, 2026

fix(security): strip BWS token and *_PASSWORD from child-process envs #77027

Open

Child-process credential-inheritance closure (tracker) #83565

Open

Navigation Menu

Uh oh!

fix: harden subprocess env handling (scrub secrets, isolate browser sessions) - #59840

fix: harden subprocess env handling (scrub secrets, isolate browser sessions)#59840
ai-ag2026 wants to merge 5 commits into
NousResearch:mainfrom
ai-ag2026:upstream/pr-a1-subprocess-hardening

ai-ag2026 commented Jul 6, 2026

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 15, 2026

Uh oh!

teknium1 Jul 15, 2026

Uh oh!

ai-ag2026 commented Jul 16, 2026

Uh oh!

ai-ag2026 commented Jul 16, 2026

Uh oh!

andrexibiza commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ai-ag2026 commented Jul 6, 2026

Testing

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

ai-ag2026 commented Jul 16, 2026

Uh oh!

ai-ag2026 commented Jul 16, 2026

Uh oh!

andrexibiza commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants