Skip to content

Comments

Prevent argument injection in git CLI calls#446

Merged
pfleidi merged 2 commits intomainfrom
protect-against-invalid-git-refs
Feb 23, 2026
Merged

Prevent argument injection in git CLI calls#446
pfleidi merged 2 commits intomainfrom
protect-against-invalid-git-refs

Conversation

@pfleidi
Copy link
Contributor

@pfleidi pfleidi commented Feb 20, 2026

Entire-Checkpoint: ac699460bd01

  • CheckoutBranch() and performGitResetHard() pass string arguments directly to git subprocesses. A ref starting with - would be interpreted as a git flag instead of a ref, causing unintended behavior (e.g., git checkout -b evil creates a
    branch, git reset --hard -q silently resets to HEAD).
  • Add input validation rejecting refs/hashes that start with - before they reach git, with tests demonstrating both attack vectors.

Test plan

  • TestCheckoutBranch/rejects_ref_starting_with_dash_to_prevent_argument_injection — verifies CheckoutBranch("-b evil") returns an error instead of creating a branch
  • TestPerformGitResetHard_RejectsArgumentInjection — verifies performGitResetHard("-q") returns an error instead of silently resetting to HEAD
  • Full CI suite passes (mise run fmt && mise run lint && mise run test:ci)

@pfleidi pfleidi requested a review from a team as a code owner February 20, 2026 18:23
Copilot AI review requested due to automatic review settings February 20, 2026 18:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a security vulnerability by preventing argument injection in two git CLI wrapper functions. When refs or commit hashes starting with - were passed to git checkout or git reset --hard, they could be interpreted as git flags rather than arguments, leading to unintended behavior (e.g., creating branches, silently resetting to HEAD).

Changes:

  • Added input validation to reject refs/hashes starting with - in CheckoutBranch() and performGitResetHard()
  • Added comprehensive tests demonstrating both attack vectors and validating the fixes

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cmd/entire/cli/git_operations.go Added dash-prefix validation to CheckoutBranch() to prevent argument injection attacks
cmd/entire/cli/rewind.go Added dash-prefix validation to performGitResetHard() to prevent argument injection attacks
cmd/entire/cli/resume_test.go Added security tests validating that both functions reject dash-prefixed inputs, plus imported strings package

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

cmd/entire/cli/resume_test.go:177

  • The test case CheckoutBranch("-b evil") likely doesn’t reproduce the described argument-injection behavior because exec.CommandContext passes the entire string as a single argv element (it won’t split on spaces). Git option parsing typically requires -b and the branch name as separate args (or a single arg form like -B<name> / --orphan=<name>). As written, this test may have already failed even before the fix, so it may not actually prove the vulnerability. Consider changing the injected ref to a single-argument option form that Git will accept (e.g., a long option with = or a short option with an attached value) and assert it would have changed repo state without the new validation.
	t.Run("rejects ref starting with dash to prevent argument injection", func(t *testing.T) {
		// "git checkout -b evil" would create a new branch named "evil" instead
		// of failing, because git interprets "-b" as a flag.
		err := CheckoutBranch("-b evil")
		if err == nil {
			t.Fatal("CheckoutBranch() should reject refs starting with '-', got nil")
		}
		if !strings.Contains(err.Error(), "invalid ref") {
			t.Errorf("CheckoutBranch() error = %q, want error containing 'invalid ref'", err.Error())
		}

@pfleidi pfleidi merged commit 519c8e3 into main Feb 23, 2026
7 checks passed
@pfleidi pfleidi deleted the protect-against-invalid-git-refs branch February 23, 2026 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants