fix: reject -F/--file git args that can exfiltrate runner files - #759
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change extends ChangesGit argument validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens matchGitArgs to prevent Git argument injection that can read a commit/tag message from a runner filesystem path (via -F / --file) and persist that content into repository history.
Changes:
- Block message-from-file options for git tag/commit messages by rejecting
-F,--file, accepted long-option abbreviations (e.g.--fi), and short-option clusters containingF(e.g.-aF). - Add unit tests covering PoC injection forms, abbreviations, clusters, and safe cases like
v1.0.0 --forceand-f. - Document the restriction in README and partially in
action.yml.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/util.ts |
Adds detection/blocking for message-from-file options and refactors long-option matching helper. |
test/util.test.ts |
Adds regression tests for blocked -F/--file forms and ensures safe args still parse. |
README.md |
Documents newly blocked message-from-file flags and expands “don’t interpolate untrusted data” guidance. |
action.yml |
Notes the new -F/--file restriction for the tag input (needs a small doc follow-up for completeness). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/util.ts`:
- Around line 133-142: Update matchGitArgs and its option validation to track
Git options that consume the following argument, including -m, so a subsequent
value beginning with -F or --file is preserved as message text rather than
classified by isDangerousMessageFileShortOption or matchesDangerousLongOption.
Keep genuine -F and --file message-file options rejected, and add regressions
for -m "-F" and -m "--file=/tmp/value".
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c58431c-3f69-4c44-85b2-95ff8656d019
📒 Files selected for processing (5)
README.mdaction.ymllib/index.jssrc/util.tstest/util.test.ts
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
-F/--file(and Git abbreviations / short-option clusters containingF) inmatchGitArgs, closing tag/commit message file-read argument injection that can push runner filesystem contents into repo history.v1.0.0 --force; document the restriction in the README andaction.yml.Test plan
1.0.0 -F ../secrets),--fi/--fil,-aF/-Fa, and safe cases (v1.0.0 --force,-f)tag: 'v1.0.0 --force'still tags successfullytag: '1.0.0 -F /path'fails the action beforegit tagrunsMade with Cursor
Summary by CodeRabbit
--file, abbreviations,-F, and short-option clusters.