fix(selfhost): isolate subscription cli environments#1392
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
b77f551 to
a75fbc7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1392 +/- ##
=======================================
Coverage 95.43% 95.43%
=======================================
Files 202 202
Lines 21743 21751 +8
Branches 7856 7859 +3
=======================================
+ Hits 20750 20758 +8
Misses 416 416
Partials 577 577
🚀 New features to boost your workflow:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review — safe to merge
✅ Approved — safe to merge Review summary Nits (5)
Nits — 2 non-blocking
Review context
Contributor next steps
Signal definitions
Review detailsGenerated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative. This PR replaces a fragile denylist of specific API-key env vars with a strict allowlist, closing a structural security hole where any new secret added to the worker env would be silently inherited by prompt-injectable CLI subprocesses. The allowlist is appropriately narrow (home/proxy/cert/locale/path only), the OAuth token is threaded through `extra` rather than being allowlisted globally, and the isolated `cwd` adds a meaningful second layer of containment. The implementation is correct, the test coverage is substantive (allowlist copy, undefined-skipping in `extra`, env and cwd assertions in the Codex test), and CI is fully green. Nits (5)
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…ty-in-codex-self-host-ai # Conflicts: # src/selfhost/ai.ts # test/unit/selfhost-ai.test.ts
Motivation
process.env, exposing runtime secrets to prompt-injectable reviewer subprocesses.Description
SUBSCRIPTION_CLI_ENV_ALLOWLISTandsubscriptionCliEnv()to build a minimal env for subscription CLIs instead of spreadingprocess.envinto subprocesses.isolatedCliCwd()and threadcwdthrough the sharedSpawnFn/defaultSpawn()so each CLI runs from an isolated temporary directory.createClaudeCodeAi()andcreateCodexAi()to usesubscriptionCliEnv()and launch subprocesses with the isolatedcwdand the allowlisted env only.test/unit/selfhost-ai.test.tsto assert the subprocess env is allowlisted, that Codex flags/args remain correct, and that the subprocesscwdis isolated.Testing
npx vitest run test/unit/selfhost-ai.test.ts --reporter=dot, which passed (45tests passed).npm run typecheck, which completed successfully.npm run test:coverage, but coverage remapping failed due to a toolchain error (ast-v8-to-istanbulTypeError: jsTokens is not a function), so coverage reporting could not be produced in this environment.Codex Task