Skip to content

fix(selfhost): keep Claude review instructions out of argv#3951

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-argv-exposure-vulnerability
Jul 7, 2026
Merged

fix(selfhost): keep Claude review instructions out of argv#3951
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-argv-exposure-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent disclosure of private per-repo review instructions by stopping their placement in child-process argv where ps//proc can expose them.
  • The prior flow put repoInstructions into a --append-system-prompt flag for the Claude CLI, exposing container-local review guides and skills loaded from GITTENSORY_REPO_CONFIG_DIR.
  • Route the instructions through the existing stdin prompt path so the text remains in the subprocess input stream (not visible in argv) while preserving the duplicate-strip behavior.

Description

  • Stop adding --append-system-prompt <systemAppend> for Claude Code CLI invocations and instead prepend the systemAppend into the CLI stdin prompt path using a shared helper. (src/selfhost/ai.ts)
  • Rename and generalize the Codex-only helper prependCodexSystemAppend to prependCliSystemAppend and use it for both Claude Code and Codex so CLI providers share the same stdin-prepend behavior. (src/selfhost/ai.ts)
  • Preserve the existing logic that strips duplicate system messages before prepending so the prompt contains the systemAppend exactly once. (src/selfhost/ai.ts)
  • Update the regression test to assert that Claude Code no longer places the appended instructions in argv and that stdin contains the appended instructions exactly once. (test/unit/selfhost-ai.test.ts)

Testing

  • Ran targeted unit tests: npx vitest run test/unit/selfhost-ai.test.ts and the focused tests matching Claude Code keeps systemAppend|Codex prepends systemAppend, which passed locally.
  • Ran npm run typecheck which completed successfully.
  • Ran npm run selfhost:env-reference:check which completed successfully.
  • Attempted full local gate npm run test:ci, but it could not complete in this environment due to unrelated environmental/test failures (transient actionlint setup network errors and a pre-existing test/unit/queue.test.ts recursive mock stack overflow encountered during coverage collection), so the full gate was not finished here.
  • Ran npx vitest run test/unit/selfhost-ai.test.ts --coverage where the tests passed but coverage report generation failed with TypeError: jsTokens is not a function (coverage tooling issue in this environment).
  • Note: all automated tests run above that exercised the changed paths passed; broader CI coverage/gate should be re-run in the repository CI environment to validate the full suite and Codecov patch coverage.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.66%. Comparing base (1ce4364) to head (679b937).
⚠️ Report is 15 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3951      +/-   ##
==========================================
- Coverage   93.66%   93.66%   -0.01%     
==========================================
  Files         372      372              
  Lines       34856    34855       -1     
  Branches    12743    12742       -1     
==========================================
- Hits        32649    32648       -1     
  Misses       1588     1588              
  Partials      619      619              
Files with missing lines Coverage Δ
src/selfhost/ai.ts 98.59% <100.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-07 07:54:15 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR closes an argv-disclosure hole by routing the Claude Code CLI's systemAppend through the same stdin-prepend helper already used for Codex, removing the `--append-system-prompt` argv flag entirely. The rename to prependCliSystemAppend and its reuse for both providers generalizes already-proven code rather than introducing new logic, and the updated test correctly asserts both that systemAppend never appears in argv and that it appears exactly once in stdin. CI is fully green and the change is narrowly scoped to the stated security fix.

Nits — 4 non-blocking
  • src/selfhost/ai.ts: a one-line comment on why Claude Code now shares Codex's stdin-prepend path (the argv-disclosure rationale) would help a future reader diffing the two provider branches.
  • test/unit/selfhost-ai.test.ts: confirm `countOccurrences` is defined/imported in this file since the new assertion relies on it and it isn't shown in the diff.
  • If self-host operators track security-relevant fixes separately (changelog/security notes), this argv-disclosure fix is worth flagging there.
  • If another CLI provider is added later, keep using prependCliSystemAppend rather than reintroducing an argv-based system-prompt flag.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 343 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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.

  • Re-run Gittensory review

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

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant