Use --end-of-options before user-controlled refs in git invocations#290
Open
lohengrin332 wants to merge 1 commit intoopenai:mainfrom
Open
Use --end-of-options before user-controlled refs in git invocations#290lohengrin332 wants to merge 1 commit intoopenai:mainfrom
--end-of-options before user-controlled refs in git invocations#290lohengrin332 wants to merge 1 commit intoopenai:mainfrom
Conversation
User-supplied --base values flow through resolveReviewTarget and buildBranchComparison into git invocations as positional arguments. A flag-shaped value like `--output=/tmp/x` could in principle be parsed by git as an option rather than a ref. The current set of git subcommands the plugin uses (merge-base, show-ref, diff, log) do not have known option-injection paths to arbitrary command execution, but defending the input boundary is cheaper than relying on that staying true. Insert --end-of-options before any user-input-derived positional. This is the documented git idiom (since 2.24, Nov 2019) for "treat the next argument as a value, not a flag." Note: -- is the wrong construct here because in `git log <range>` and `git diff <range>`, -- is a path-spec separator that would silently change semantics. Tested with a hostile --base value of the form --output=<sentinel>; the call now errors as a bad ref instead of either succeeding or creating the sentinel file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a defense-in-depth hardening PR, not a CVE. There is no remotely exploitable bug here in normal use of the plugin.
The behavior being changed:
git.mjspasses the user-supplied--base <ref>value (and values derived from it) into git argv positions without--end-of-options. None of the affected git subcommands have known option-injection paths to arbitrary command execution, but the boundary is easy to harden.Why it's worth tightening: under prompt-injection-adjacent threat models, a hostile
--base "--output=/tmp/x"argument reaches git as a positional. The set of dangerous git options the current subcommands accept may grow over time (or may already include something not yet audited).--end-of-optionsis the documented git idiom for "treat the next argument as a value regardless of leading dashes" and has been supported since 2.24 (November 2019).A note on
--vs--end-of-options:--is a path-spec separator ingit log <range>andgit diff <range>, which would silently change the command's semantics.--end-of-optionsis the correct construct.Changes
lib/git.mjs: insert--end-of-optionsbefore user-input positionals inmerge-base,show-ref, and downstreamdiff/logcommands that take ranges derived from user input. Also adds a_gitCheckedImplinjection point tobuildBranchComparison(threaded fromcollectReviewContext) to support structural testing.tests/git.test.mjs: three new tests:--basevalue of the form--output=<sentinel>errors out as a bad ref and fails to create the sentinel file.feature/has-dashes-in-name) still work, guarding against any future regex-based "is this a flag?" check.--end-of-optionsappears beforebaseRefin themerge-baseargv.Minimum git version: 2.24 (Nov 2019), which introduced
--end-of-options. CI runs ubuntu-latest so this is well below the floor in practice. Happy to add an explicit note inpackage.jsonengines or the README if you'd like.🤖 Generated with Claude Code