fix(tools): strip PYTHONPATH from subprocess env to prevent leak (#74817)

[webtecnica]

Fixes #74817 — added PYTHONPATH to _ACTIVE_VENV_MARKER_VARS in tools/environments/local.py. Covers all 3 spawn paths.

[alt-glitch]

This was generated by AI during triage.

Related to #74871 and #61028. #74871 bundles this same blanket PYTHONPATH strip with an unrelated Signal rewrite; #61028 instead preserves user PYTHONPATH entries while removing only Hermes-venv site-packages.

[teknium1]

Thanks for isolating the current local-environment gap: tools/environments/local.py:349 still omits PYTHONPATH, and the three local builders consume that tuple at lines 493, 622, and 1263.

Problems

• Adding PYTHONPATH to this global tuple drops every user-provided entry, not only Hermes-injected venv paths. The Nix wrapper deliberately appends extraPythonPackages to PYTHONPATH for entry-point plugin discovery (nix/hermes-agent.nix:116-121, 206-208).
• This does not cover execute_code: its scrubber explicitly permits PYTHONPATH (tools/code_execution_tool.py:148-150) and the spawn path preserves it at lines 1399-1403.
• No regression test accompanies the new behavior; current marker tests cover only VIRTUAL_ENV and CONDA_PREFIX (tests/tools/test_local_env_blocklist.py:268-314).

Suggested changes

• Filter only Hermes-injected incompatible site-packages entries, preserving unrelated PYTHONPATH entries, and apply that policy to the execute_code child environment as well.
• Add behavioral coverage for all affected environment builders, including preservation of a user path.

Automated hermes-sweeper review.

[teknium1]

This removes all PYTHONPATH entries, including user/package-manager paths. execute_code intentionally allows PYTHONPATH (tools/code_execution_tool.py:148-150), and the Nix wrapper appends it for extraPythonPackages plugin discovery (nix/hermes-agent.nix:116-121, 206-208). Please filter only Hermes-injected incompatible site-packages entries and cover the separate execute_code path.

[GottZ]

This was generated by AI during triage.

Summary

Two PRs address #74817: #74951 is an isolated one-line blanket PYTHONPATH strip for the three local environment builders, while #74871 contains the same core change plus execute_code handling, regression tests, and an unrelated Signal rewrite. Both prevent the reported Hermes-path leak on covered paths, but neither preserves legitimate user/Nix PYTHONPATH entries as required by the contributor reviews.

Related pull requests

• fix(env): strip PYTHONPATH from subprocess env (#74817) #74871 duplicate — (+567/-125) — partial fix: The current diff strips inherited PYTHONPATH from the three local builders and from the separate execute_code child environment, with regression coverage, but still removes supported user/Nix paths wholesale and bundles an unrelated Signal rewrite. Consistent with the keep_open review on fix(env): strip PYTHONPATH from subprocess env (#74817) #74871, the environment policy must become selective and the Signal work must be split out.
• fix(tools): strip PYTHONPATH from subprocess env to prevent leak (#74817) #74951 best fix — (+1/-1) — partial fix: Adding PYTHONPATH to _ACTIVE_VENV_MARKER_VARS stops the leak through the three local builders, but it drops every user-provided entry, does not sanitize the separate execute_code path, and adds no regression tests. The keep_open review on fix(tools): strip PYTHONPATH from subprocess env to prevent leak (#74817) #74951 therefore remains blocking and provides the required salvage path: selective filtering, execute_code coverage, and behavioral tests.

Duplicates

#74871 and #74951 duplicate the same core blanket _ACTIVE_VENV_MARKER_VARS change; #74871 is a broader superset with execute_code coverage, tests, and unrelated Signal changes.

Suggested consolidation

Keep #74951 open with a salvage path: replace blanket removal with filtering of only Hermes-injected incompatible paths, preserve unrelated user/Nix PYTHONPATH entries, apply the same policy to the actual execute_code child environment, and add behavioral coverage for every affected builder, explicitly satisfying its keep_open contributor review. Author action on #74871: split out the unrelated Signal rewrite; after its useful environment coverage has been carried into the isolated fix, close #74871 as a duplicate of #74951 for #74817.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I74817(["issue #74817 (open)"])
    subgraph Dup74871 ["PRs duplicating each other"]
        P74871["PR #74871 (open)"]
        P74951["PR #74951 (open)"]
    end
    P74951 -->|best fix| I74817
    class I74817 open
    class P74871 open
    class P74951 open
    class P74871 best
    class P74951 best
    class P74951 target
    click I74817 "https://github.com/NousResearch/hermes-agent/issues/74817"
    click P74871 "https://github.com/NousResearch/hermes-agent/pull/74871"
    click P74951 "https://github.com/NousResearch/hermes-agent/pull/74951"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 44 kB of PR diffs, 7 kB of issue/PR text, 9 kB of discussion (9 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[teknium1]

Thanks for the report and the fix @webtecnica. The underlying leak is now fixed on main via #88182, which took the selective-filtering approach (strip only Hermes-owned entries: cross-version site-packages, the Hermes venv's own site-packages, and the repo root) rather than a blanket PYTHONPATH strip — blanket removal would discard legitimate user-set entries (Nix plugin paths, custom library dirs). Your report on #74817 was part of what drove this fix. Closing as superseded.