fix(security): isolate reviewer GitHub CLI subprocess environment

[seonghobae]

Problem

Protected main fa62a30f08122778d2db4a27fbb9b99a82dbac9a runs reviewer GitHub I/O through reviewer/noema_reviewer/github_io.py::default_runner(). On that protected source, subprocess.run(..., shell=False) has no explicit env, so every gh child inherits the complete parent process environment, and a failing child can echo the delegated GH_TOKEN into retained diagnostics.

That is broader authority than the reviewer GitHub CLI needs. shell=False prevents shell interpretation; it does not constrain inherited environment authority or diagnostic retention.

RCA

First failing boundary: trusted reviewer process environment -> gh subprocess authority and retained failure diagnostics.

Immediate cause: default_runner() omits env=... and retains raw child stderr on non-zero exit.

Root cause: reviewer GitHub I/O has an injectable command seam but no explicit least-authority subprocess-environment / delegated-token-redaction contract.

Systemic risk: unrelated model/provider/App/cloud credentials, proxy configuration, HOME, launcher state, or future workflow secrets can become observable to a child whose intended authority is GitHub API access only; the explicitly delegated GitHub credential can also become retained evidence if a hostile/failing child echoes it.

Remedy selected

Use an explicit allowlisted child environment containing pinned GH_HOST=github.com, deterministic NO_COLOR=1, optional non-empty PATH, and optional explicit GH_TOKEN; pass it to shell-free subprocess.run. Before a non-zero child error becomes retained diagnostics, redact the exact delegated token value. Do not silently forward ambient GITHUB_TOKEN, model/provider secrets, App private material, proxy/HOME/launcher configuration, or other parent state.

A marker-based secret denylist and ambient-inheritance/workflow-hygiene approaches remain rejected because they do not fail closed for novel authority/configuration names.

Current implementation line

Draft PR #180 owns this repair on branch fix/reviewer-gh-cli-environment, rebuilt directly from protected main after the prior github_io.py writer integrated.

Current exact head: 67c3e0fb3d1a19416712cb64d38e40efdac8afe5.

Test-first lineage:

• 1202b9b364791e9829145198e6ec63fd7c00f3a8: hostile-parent / delegated-token diagnostic regression committed before production implementation. Its GitHub Actions runs were cancelled by branch concurrency and are not promoted to terminal RED evidence.
• 0070ccf6ab06c24569e893195afa420bd496f029: production least-authority environment + exact-token redaction.
• 67c3e0fb3d1a19416712cb64d38e40efdac8afe5: additional regression proving that absent optional PATH/GH_TOKEN does not widen the child environment.

At this update the exact-head application ci, reviewer-ci, and protected-base eligible central Security Scan are queued/pending, therefore non-passing. PR #180 remains Draft until exact-head technical evidence is terminal and current findings/reviews are re-read.

Acceptance

• unrelated parent credentials/configuration never cross default_runner() into the gh child;
• only the reviewed GitHub CLI environment is present, with optional launch/token authority not invented when absent;
• delegated GH_TOKEN remains usable under the reviewed variable name but is removed from retained non-zero diagnostics;
• shell-free GitHub I/O semantics remain unchanged;
• reviewer tests/docstrings and protected-base application ci, reviewer-ci, and eligible central Security Scan must pass on the unchanged final head;
• zero valid unresolved findings and current formal-review state are revalidated separately;
• technical checks/model output are never promoted to independent approval, production evidence, release authority, or acquisition readiness.

Related: #27, #29, #108, #109, #136, #137, #165, #166, #180

[seonghobae]

Fresh 2026-08-12 continuation evidence: protected main is now cd40474b258f9512d93ce82f5375071ce1f78762. Current protected-main successor PR #193 is open Draft on exact base cd40474b258f9512d93ce82f5375071ce1f78762, exact head b6ca3011c479616e5f06439a5b662f642ecfef36, with current application ci run 31545961352, reviewer-ci 31545961275, and eligible central Security Scan 31545961212 all terminal-success. Its branch moved beyond the PR body during this invocation, so that branch is frozen for this writer.

Supersession audit found #180 is not yet safe to close merely because #193 touches the same two paths. The current #180 regression at head 2574b8290901f6387d66bb00d4841e12eaf31a79 additionally asserts that hostile failure diagnostics retain the useful sanitized text retry token=[REDACTED]; the current #193 test blob does not preserve that assertion. Production github_io.py behavior appears equivalent on the inspected boundary, but test-contract preservation is incomplete. Before #193 can supersede #180, preserve or independently justify that stronger diagnostic-redaction regression on the current-main successor, then reacquire exact-head gates and current review/thread evidence. No predecessor checks are transferred.

[seonghobae]

Protected-main closure proof: successor #193 merged exact reviewed head 883bf1e1ea3cc0456b9c7f96dcbf003462109c0b as protected main 0715ba52c32cc94b59f91f74e742657aed822421. The protected source now carries the explicit least-authority gh environment plus exact delegated-token redaction, and its regression preserves the stronger retry token=[REDACTED] diagnostic assertion. Exact successor-head application ci 31548875925, reviewer-ci 31548875942, and protected-base eligible central Security Scan 31548875961 all completed success before merge. Post-integration protected-main ci 31549301056 now completed success with exact checkout/toolchain/install/release verify, and protected-main reviewer-ci 31549301050 completed success with exact checkout, 100% reviewer line+branch coverage, 100% docstring coverage, authenticated/scanned sandbox image, and real no-network CodeGraph smoke. Stale predecessor #180 has been closed only after byte-for-byte preservation was proven. This closes the reviewer subprocess environment/diagnostic-retention defect; external App/governance, production, release, KPI, legal/IP and acquisition evidence remain separate.