Skip to content

fix(subprocess): block /proc/environ env-blocklist bypass (#4427) - #19568

Closed
EthanGuo-coder wants to merge 4 commits into
NousResearch:mainfrom
EthanGuo-coder:fix/issue-4427
Closed

fix(subprocess): block /proc/environ env-blocklist bypass (#4427)#19568
EthanGuo-coder wants to merge 4 commits into
NousResearch:mainfrom
EthanGuo-coder:fix/issue-4427

Conversation

@EthanGuo-coder

@EthanGuo-coder EthanGuo-coder commented May 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

Closes #4427. On Linux a same-UID child can recover the parent's stripped
env vars by reading /proc/<ppid>/environ even when the spawn used a
filtered env= dict, because dumpable=1 leaves /proc/<self>/environ
world-readable to same-UID processes. Clear PR_SET_DUMPABLE on the parent
across the secret-stripping spawn sites so the kernel marks the file
0400 root:root. Hardening is best-effort: a one-shot logger.warning
fires if prctl is unavailable.

Related Issue

Fixes #4427

Related work

PR #4609 fixes a different layer (the read_file tool's own guard against
reading other processes' environ); the two are complementary and neither
subsumes the other.

Type of Change

  • 🔒 Security fix

Changes Made

  • Clear PR_SET_DUMPABLE via a ctypes prctl(2) call, wired into every
    secret-stripping spawn path: the helpers in tools/environments/local.py,
    the inline child_env builder in tools/code_execution_tool.py, and
    _build_safe_env() in tools/mcp_tool.py (MCP stdio servers).
  • Resolve libc via ctypes.util.find_library("c") with a CDLL(None)
    fallback so musl-based distros (Alpine) don't silently skip the
    protection.
  • Read PR_GET_DUMPABLE before every set so the helper self-corrects when
    the flag is reset by a fork / extension / multiprocessing path.
  • One-shot logger.warning if prctl is unavailable; the spawn still
    proceeds.

How to Test

  • Linux only — the hardening is a no-op on macOS/Windows where
    /proc/<pid>/environ doesn't exist.
  • python -m pytest tests/test_subprocess_proc_environ_hardening.py -v
  • test_child_cannot_read_parent_environ_after_hardening is skipped under
    root (root bypasses dumpable=0); the upstream CI runner is non-root so
    this test executes there.
  • Manual repro: before the fix, a child spawned from
    tools/environments/local.py could read /proc/<ppid>/environ and
    recover stripped secrets; after the fix, the same open() raises
    PermissionError.

Checklist

Code

Documentation & Housekeeping

  • N/A — internal hardening, no user-facing config or tool-schema change
  • N/A — no new config keys
  • N/A — no architecture change
  • Considered cross-platform impact: prctl call is gated behind
    sys.platform == "linux"; on other platforms the function is a no-op.
  • N/A — no tool-behavior change

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround backend/local Local shell execution tool/code-exec execute_code sandbox labels May 4, 2026
EthanGuo-coder and others added 3 commits May 14, 2026 14:53
@EthanGuo-coder @claude
fix(subprocess): clear PR_SET_DUMPABLE to block /proc/environ leak
05469ee 
Fixes NousResearch#4427

Same-UID children could read the parent's stripped env vars by opening
/proc/<ppid>/environ when dumpable=1. Clear PR_SET_DUMPABLE on Linux via
a prctl ctypes call at module load and inside the env-sanitizing helpers,
plus an explicit call before code_execution_tool's subprocess.Popen since
it builds child_env inline. argtypes/restype are declared on prctl, the
errno path emits a one-shot logger.warning so a silent fallback to the
vulnerable state stays observable, and the call is a no-op on non-Linux.

Co-Authored-By: Claude <noreply@anthropic.com>
@EthanGuo-coder @claude
test(subprocess): cover /proc/environ hardening for spawn sites
485c95b 
Adds three Linux-only unit tests verifying _sanitize_subprocess_env(),
_make_run_env(), and the one-shot _harden_against_proc_environ_leak()
gate plus an integration test that spawns a child and asserts
/proc/<ppid>/environ is unreadable after hardening (skipped on root,
where dumpable=0 is bypassed).

Co-Authored-By: Claude <noreply@anthropic.com>
@EthanGuo-coder @claude
fix(subprocess): close MCP stdio leak + harden ABI/state for NousRese…
4a8391a 
…arch#4427

Independent codex review surfaced one blocker and three important findings
on the original /proc/<ppid>/environ hardening. This follow-up addresses
all four:

1. MCP stdio servers spawned by mcp_tool._build_safe_env() were not
   covered by the chokepoint — the helper strips secrets via _SAFE_ENV_KEYS
   but never invoked the prctl harden, and it can run before
   tools.environments.local is imported via CLI MCP discovery. Same
   /proc/<ppid>/environ leak as the issue describes, just at a different
   spawn site. Add an explicit _harden_against_proc_environ_leak() call
   at the top of _build_safe_env().

2. libc.so.6 was hardcoded; on musl-based distros (Alpine) and systems
   without that soname, CDLL raised OSError and the helper silently
   returned, leaving the leak open. Resolve via ctypes.util.find_library
   with a CDLL(None) fallback so the bound symbols come from whatever
   libc the interpreter is already using.

3. _PROC_ENVIRON_HARDENED was a write-once boolean cache. If the dumpable
   flag was reset externally (e.g., via prctl from another extension or
   by a fork that re-enabled it), subsequent sanitizer calls would skip
   the prctl and leave the parent's /proc/<pid>/environ readable again.
   Switch to state-based: read PR_GET_DUMPABLE on every call and reapply
   PR_SET_DUMPABLE=0 when needed. The boolean is kept as informational
   "have we ever successfully hardened" telemetry.

4. Test fixture set dumpable=1 before importing tools.environments.local;
   the first import would then re-fire module-load harden and clobber
   the test setup. Tests passed only because conftest's import chain
   pre-loaded the module. Reorder so the import (and any module-load
   side effect) happens before _set_dumpable(1). Replace
   test_hardening_runs_only_once with test_hardening_reapplies_when_dumpable_resets
   reflecting the new state-based contract. Add test_mcp_build_safe_env_clears_dumpable
   as direct regression coverage for finding 1.

Co-Authored-By: Claude <noreply@anthropic.com>
@austinpickett
austinpickett requested a review from Copilot May 18, 2026 15:30
Copilot AI reviewed May 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Linux subprocess launches against a /proc/<ppid>/environ bypass where same-UID child processes can recover secrets stripped from the child’s env= by reading the parent’s proc environ. It does this by clearing PR_SET_DUMPABLE (via prctl(2) through ctypes) before secret-scrubbed subprocess spawns, and adds pytest coverage to validate the hardening.

Changes:

  • Add a Linux-only prctl(PR_SET_DUMPABLE, 0) hardening helper to prevent /proc/<ppid>/environ reads from recovering stripped secrets.
  • Invoke the hardening helper in multiple subprocess env construction/spawn paths (local env helpers, MCP stdio safe env, code execution tool).
  • Add a new test suite validating dumpable flag behavior and the /proc/<ppid>/environ access denial.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
tools/mcp_tool.py Clears dumpability when building MCP stdio subprocess environments to prevent /proc/<ppid>/environ recovery.
tools/environments/local.py Introduces the hardening helper (and wires it into env construction helpers) plus warning-once logging.
tools/code_execution_tool.py Applies the hardening helper before spawning the execute_code sandbox subprocess.
tests/test_subprocess_proc_environ_hardening.py Adds Linux-only tests asserting dumpable is cleared and /proc/<ppid>/environ reads fail post-hardening.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/environments/local.py Outdated
Comment thread tools/environments/local.py Outdated
Comment thread tools/environments/local.py Outdated
Comment thread tests/test_subprocess_proc_environ_hardening.py Outdated
@EthanGuo-coder
fix(subprocess): cache prctl + drop import-time harden (review feedback)
a713f9b 
Cache libc.prctl after first resolution, remove module-level harden call so import is side-effect-free, drop duplicate logger, and route the test libc resolution through the production helper so musl-based distros work.
@EthanGuo-coder

EthanGuo-coder commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@Copilot thanks for the review — all four points addressed in a713f9bca:

  1. Duplicate logger assignment — removed the second one at tools/environments/local.py:20.
  2. _PROC_ENVIRON_HARDENED written-but-never-read — replaced with a real _resolve_prctl() cache: libc + bound prctl are resolved once, a _PRCTL_UNAVAILABLE sentinel skips retries when libc is missing, and _harden_against_proc_environ_leak() still re-reads PR_GET_DUMPABLE on every call so fork/extension resets are self-corrected. Steady-state cost is now one syscall, no ctypes re-import.
  3. Module-level harden call — removed. All four secret-stripping spawn sites already invoke the helper (_sanitize_subprocess_env, _make_run_env, code_execution_tool inline child_env, mcp_tool._build_safe_env), so a bare import tools.environments.local no longer flips dumpability/core-dumps for the host process.
  4. Tests hardcoding libc.so.6 — _get_dumpable/_set_dumpable now route through the production _resolve_prctl() and pytest.skip if libc/prctl can't be resolved, so musl-based hosts (Alpine) won't spuriously fail.

pytest tests/test_subprocess_proc_environ_hardening.py -v → 4 passed, 1 skipped (the non-root child-leak test correctly skips under root); neighbor suites (tests/environments/, tests/test_subprocess_home_isolation.py) still green.

@egilewski

egilewski commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

obsolete

The issue this PR closes appears to be resolved already. Please reopen with a fresh target if this still covers a distinct gap.

Signed: GPT-5.5-low in Codex

@EthanGuo-coder

EthanGuo-coder commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing — issue #4427 was resolved NOT_PLANNED by maintainers, so this subprocess-layer hardening will not be accepted upstream.

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

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

backend/local Local shell execution P1 High — major feature broken, no workaround tool/code-exec execute_code sandbox type/security Security vulnerability or hardening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Security: Subprocess env blocklist bypassed via /proc/environ

4 participants

@EthanGuo-coder @egilewski @alt-glitch