fix(scripts): resolve CodeQL command-injection findings in generate-patch - #635
Conversation
The since ref from argv was interpolated into shell command strings, flagged by CodeQL as indirect command-line injection. execFileSync with argument arrays removes the shell, same treatment format-yaml got in #627. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmHrQEQmpYthrHhmV2cB2c
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Limit details: You’ve used the included review currently available. Your 106 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
Description
Fixes the two
js/indirect-command-line-injectionfindings CodeQL raises onscripts/generate-patch.ts: thesinceref taken from argv was interpolated into shell command strings (git show ${since}:${filePath},git diff --name-status ${since} HEAD). All three git invocations in the file now useexecFileSyncwith argument arrays, so no shell is involved — the same treatmentformat-yaml.tsreceived on #627.Verified by re-running the CodeQL
security-extendedsuite locally: both injection alerts are gone. The one remaining finding (js/file-system-raceinenrich-io.ts) is a check-then-write on a local interactive CLI editing a repo file — not a real risk, recommend dismissing it on the code-scanning page rather than contorting the code.Type of Change
Checklist
pnpm validateand it passesAdditional Notes
pnpm typecheck,pnpm lint, andpnpm test(174 tests) all pass; the two lint warnings are pre-existing on main. No changeset: scripts-only, no data change.🤖 Generated with Claude Code
https://claude.ai/code/session_01GmHrQEQmpYthrHhmV2cB2c
Generated by Claude Code