fix(core): prevent git option injection via CI branch names (GHSA-v58q-fvq4-9vh4) - #378
Merged
Merged
Conversation
…q-fvq4-9vh4) The git helpers pass the branch and commit of the CI environment to git as positional arguments. Git parses any argument starting with a dash as an option, so a pull request from a branch named `--upload-pack=<command>` (a valid, pushable ref name) made `git fetch` run `<command>` through a shell on ssh:// and file:// origins whenever the merge base is resolved locally, i.e. for projects without a connected Git provider. The fix for GHSA-4x45-gxvp-6283 removed the shell but left this option parsing. Pass every CI-provided value after `--end-of-options` (git 2.24+) in the fetch, merge-base, rev-parse, log and rev-list invocations, so git always treats it as a ref. The fetch helper now assembles the argument list itself so no caller can leave the separator out. Add regression tests feeding the advisory's payload through the merge base, the ancestor listing and the commit parents, plus one checking that a branch legitimately named with a leading dash is still fetched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
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.
Description
Fixes GHSA-v58q-fvq4-9vh4 (CWE-88, argument injection) in
@argos-ci/core.The git helpers of the CI environment pass the branch and commit they get from CI to git as positional arguments. Git parses any argument starting with a dash as an option: a pull request opened from a branch named
--upload-pack=<command>(a valid, pushable ref name) madegit fetchrun<command>through a shell onssh://andfile://origins, whenever the merge base is resolved locally (projects without a connected Git provider). The fix for GHSA-4x45-gxvp-6283 (#319) removed the shell but left this option parsing in place.Changes
--end-of-options(git 2.24+, November 2019), so git always treats it as a ref: fetch, merge-base, rev-parse, log and rev-list.--end-of-options origin, then refs), so no caller can leave the separator out.git.tsrecords both hardening rules with the advisory IDs.Rejecting names starting with a dash was the alternative. The terminator was preferred because it keeps legitimately named branches working. On git older than 2.24 the commands fail on the unknown option before contacting the remote, so the failure is closed rather than exploitable.
Tests
The single shell-injection test is replaced with a block covering both advisories:
--upload-packpayload through the merge base, the ancestor listing and the commit parents, and assert the injected command never runs;-dashedto origin and checks it is fetched and used as the merge-base head.Against the unfixed code, the four new tests fail and the injected command runs from all three sinks. With the fix, the full
@argos-ci/coresuite passes (148 tests, 2 pre-existing skips), andtsc,eslintandprettierare clean.Type of changes
bugChecklist
Optional checks:
Further comments
Reported by @manus-use. Once merged,
@argos-ci/coreshould be released and the new version set as the patched version on the advisory, which currently lists none.🤖 Generated with Claude Code