Skip to content

[codex] sanitize codex child process env#190

Open
seungpyoson wants to merge 4 commits intoopenai:mainfrom
seungpyoson:fix/issue-525-routing-env
Open

[codex] sanitize codex child process env#190
seungpyoson wants to merge 4 commits intoopenai:mainfrom
seungpyoson:fix/issue-525-routing-env

Conversation

@seungpyoson
Copy link
Copy Markdown

Summary

  • add a shared sanitizeChildEnv helper in plugins/codex/scripts/lib/process.mjs
  • route plugin child-process launches through that sanitizer in the direct app-server, broker, detached task worker, and stop-review gate paths
  • add regression coverage for the routing-env contract and align the deterministic command-doc test with the current quoted "$ARGUMENTS" command form

Why

This was surfaced by seungpyoson/claude-config#525.

The Codex marketplace plugin was forwarding ambient Rust verification routing variables across plugin process boundaries. The plugin did not expose a shared sanitizer, and downstream boundary tests were correctly identifying that drift.

Impact

  • strips routing-intent and cargo target override env vars from plugin child environments
  • makes the process-boundary contract explicit in plugin code and tests
  • keeps existing launch behavior unchanged apart from environment scrubbing

Validation

  • node --test tests/process.test.mjs
  • npm test

Copy link
Copy Markdown
Author

Requesting adversarial review.

Focus areas:

  • child-process env sanitization boundaries
  • whether sanitizeChildEnv is applied at every relevant launch surface
  • regressions from scrubbing RUST_VERIFICATION_*, BOLT_RUST_VERIFICATION_ROOT, and CARGO_*TARGET_DIR
  • whether the new tests actually prove the contract
  • whether aligning tests/commands.test.mjs with the current quoted "$ARGUMENTS" command form is correct and minimal

Validation already run on this branch:

  • node --test tests/process.test.mjs
  • npm test

PR: #190

Copy link
Copy Markdown
Author

Follow-up pushed in response to review:

  • harden sanitizeChildEnv(...) to scrub keys case-insensitively so mixed-case Windows env keys cannot bypass the filter
  • treat null like inherited env input instead of producing an empty env object
  • extend tests to cover:
    • CARGO_TARGET_DIR
    • preserved CARGO_HOME
    • case-insensitive key scrubbing
    • inherited-env sanitization when runCommand(...) is called without an explicit env

Latest validation on branch:

  • node --test tests/process.test.mjs
  • npm test

Both are passing on the updated branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant