Action brief from the marketplace-wide review (see docs/review-2026-06-20.md). Three shell/path
injection sinks: F-1 CRITICAL zero-interaction RCE in git-forensics.mjs (execSync shell string +
attacker-controlled filenames; reproduced), F-2 HIGH path-containment write in auto-cleaner.mjs,
F-3 HIGH command injection in the supply-chain hook. Common fix: execSync(string) -> spawnSync(array)
/ input containment. DO AFTER the current active session finishes.
Disclosure hold: this brief + review-2026-06-20.md (ff9bd13) are committed locally but NOT pushed;
push both only together with the F-1 fix (the brief carries a working RCE repro payload).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ter3E2JSi1Khgmuf2kady8
4.6 KiB
Security fix brief — 3 shell/path injection sinks (2026-06-20)
Action brief for the NEXT session. Found in the marketplace-wide review (full context:
docs/review-2026-06-20.md). Do this after the current active session finishes — do not interleave with in-flight work.Disclosure hold: the review report (
review-2026-06-20.md, commit738770e) and this brief are committed locally but NOT pushed, because F-1 is a working RCE with a reproduction payload. Push both only together with the F-1 fix — never publish the RCE detail to a public remote while it is still exploitable.
Priority order
| # | Severity | Fix first? |
|---|---|---|
| F-1 | CRITICAL (zero-interaction RCE) | YES — before any /security scan of an untrusted repo URL |
| F-2 | HIGH (arbitrary file write) | yes |
| F-3 | HIGH (command injection, pre-confirmation) | yes |
| F-4 | LOW | optional |
| F-5/F-6 | LOW (hygiene) | optional |
Common root cause
F-1/F-2/F-3 all trust untrusted strings at a subprocess/filesystem sink. The fix family is the same:
execSync(shell-string) → spawnSync('cmd', [...argArray]) (no shell), or input containment.
Every affected call site already has discrete tokens, so the change is mechanical and localized.
F-1 — CRITICAL — scanners/git-forensics.mjs
- Sink:
git()at:59-66runsexecSync(\git ${cmd}`)(shell string). Attacker-controlled filenames from the scanned repo (git ls-files:202,git log --name-only:549) are interpolated at **:211, :223, :231, :564**."${relFile}"quoting at :211 does NOT stop$(...)`/backticks; :223/:231 are unquoted. - Why critical:
gitScanis in the default SCANNERS array (scan-orchestrator.mjs:119), andcommands/scan.mdclones a user-supplied GitHub URL then scans it. A hostile repo containing a file namedcommands/$(touch INJECTED).mdruns arbitrary code on the analyst's machine on/security scan <url>— no install, no confirmation. Forensics runs outside the git-clone OS-sandbox (that wraps only the clone), so it runs unsandboxed on all platforms. Reproduced end-to-end. - Fix: convert
git()tospawnSync('git', [...argArray], {cwd})and pass each call site's tokens as an array (they are already discrete). No shell, no interpolation. - Verify: add a test fixture repo with a file named
$(touch /tmp/pwned).md; assert the scan completes and/tmp/pwnedis NOT created. (TDD: write the failing repro first.)
F-2 — HIGH — scanners/auto-cleaner.mjs
- Sink:
:776resolve(targetPath, f.file)with no containment check; written at:878-881.f.filecomes from findings JSON (untrusted repo filenames, or a fully attacker-chosen--findingsfile), sofile: "../../../.claude/settings.json"writes outside the scanned tree. - Fix: before writing, assert
absPath === targetPath || absPath.startsWith(targetPath + sep); otherwise skip + report. - Verify: a finding with
file: "../escape.txt"must be refused, not written.
F-3 — HIGH — hooks/scripts/pre-install-supply-chain.mjs → supply-chain-data.mjs
- Sink:
pre-install-supply-chain.mjs:254→inspectNpmPackagecallsexecSafe(\npm view ${spec} --json`);execSafe(supply-chain-data.mjs:221-227) isexecSync(shell). A spec likefoo;touch /tmp/XsurvivesextractNpmPackages+parseSpec(name=foo;touch`) and reaches the shell. It fires on PreToolUse(Bash) before the user's npm install — so it executes pre-confirmation even if the user then denies the Bash call. - Fix:
spawnSync('npm', ['view', spec, '--json']), or validatespecagainst^[@/A-Za-z0-9._-]+$before use. (pip/go/OSV paths already use fetch/array args — only the npmnpm viewpath shells out.) - Verify: a package spec
foo;touch /tmp/Xmust not execute thetouch.
F-4 / F-5 / F-6 — LOW
- F-4
mcp-live-inspect.mjs:296spawns the scanned target's declared MCP command (array-arg, no metachar injection). By-design for/security mcp-inspect, but scanning an untrusted target launches attacker-declared processes — add a confirm before spawning servers from a non-home target. - F-5/F-6 two stray committed root artifacts:
--json(0-byte redirect husk) and.orphaned_at. Inert;git rmthem.
Closing gates (when fixing)
- TDD per fix (repro → red → green). Full
node --testsuite green. - gitleaks clean. Re-run the F-1 repro to confirm the sink is closed.
- Then push: the F-1 fix commit +
review-2026-06-20.md+ this brief, together. After that the disclosure hold is lifted. - Fixing F-1/F-2/F-3 lifts the plugin from B− to a solid A−.