🛡️ Sentinel: [HIGH] Fix Git argument injection#62
Conversation
* Validated branch names in `_checkout_branch` and `_rebase_onto_base` to prevent Git argument injection vulnerabilities where a branch starting with a hyphen could be treated as an option by the `git` command. * Added documentation about this learning in `.jules/sentinel.md`. Co-authored-by: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR addresses a Git argument injection vulnerability by adding input validation to reject untrusted branch and base names that start with "-", and documents the vulnerability in a sentinel advisory. ChangesGit Argument Injection Prevention
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d76e2a6cd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if branch.startswith("-"): | ||
| raise CommandError(f"Invalid branch name: {branch}") |
There was a problem hiding this comment.
In the normal single-PR path I inspected, main() calls _ensure_pr_worktree(... branch=branch) before _rebase_onto_base, and _ensure_pr_worktree immediately calls _fetch_pr_branch_or_head, which runs git fetch origin <branch>. git fetch -h shows refspecs are parsed after options (git fetch [<options>] [<repository> [<refspec>...]]) and supports options such as -o/--server-option, so a PR head named like -o... still reaches git as an option before this new guard can fire. Move/centralize branch validation at PR metadata/worktree entry, or make the first fetch use an unambiguous full refspec/option terminator, so the first mutating git command is protected.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
🚨 Severity: HIGH
💡 Vulnerability: Untrusted branch and base names fetched from remote sources were passed directly to
gitcommands without validation. If these names start with a hyphen (e.g.,-o), they can be interpreted as arguments bygit, leading to command injection vulnerabilities.🎯 Impact: An attacker who can control branch names on a remote repository could execute arbitrary arguments against the underlying
gitcommand, potentially leading to information disclosure, arbitrary code execution, or denial of service depending on the arguments injected.🔧 Fix: Added validation in
_checkout_branchand_rebase_onto_baseto check ifbranchorbasestrings start with a hyphen, and raise aCommandErrorif they do.✅ Verification: Ran syntax checks and the test suite.
PR created automatically by Jules for task 12719901167017156289 started by @xbmc4lyfe