From c038be9850f05d61cfcea22883fb8c1b6afb3bb9 Mon Sep 17 00:00:00 2001 From: Ashutosh Anand Date: Thu, 4 Jun 2026 13:43:56 +0530 Subject: [PATCH] fix(git): protect worktree operands with separator Pass AddWorktree and MoveWorktree positional operands after -- so dash-leading sources or paths stay data instead of being parsed as git options. Keep MoveWorktree force flags before the separator, and cover the exact argv shape with focused mock runner tests. --- internal/git/mock_runner_test.go | 53 +++++++++++++++++++++++++++----- internal/git/worktree.go | 5 +-- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/internal/git/mock_runner_test.go b/internal/git/mock_runner_test.go index 0beaf0c..e66400e 100644 --- a/internal/git/mock_runner_test.go +++ b/internal/git/mock_runner_test.go @@ -349,6 +349,25 @@ func TestMoveWorktreeNoForce(t *testing.T) { } } +func TestMoveWorktreeInsertsDashDash(t *testing.T) { + var captured []string + r := &mockRunner{ + run: func(args ...string) (string, error) { + captured = args + return "", nil + }, + } + + if err := MoveWorktree(r, "/old/path", "/new/path", false); err != nil { + t.Fatalf("MoveWorktree: %v", err) + } + + want := []string{cmdWorktree, "move", "--", "/old/path", "/new/path"} + if !slices.Equal(captured, want) { + t.Errorf("args = %v, want %v", captured, want) + } +} + func TestMoveWorktreeForce(t *testing.T) { var captured []string r := &mockRunner{ @@ -362,14 +381,9 @@ func TestMoveWorktreeForce(t *testing.T) { t.Fatalf("MoveWorktree: %v", err) } // git worktree move requires --force twice to move locked worktrees - forceCount := 0 - for _, a := range captured { - if a == flagForce { - forceCount++ - } - } - if forceCount != 2 { - t.Errorf("expected 2 %s flags, got %d in args %v", flagForce, forceCount, captured) + want := []string{cmdWorktree, "move", flagForce, flagForce, "--", "/old/path", "/new/path"} + if !slices.Equal(captured, want) { + t.Errorf("args = %v, want %v", captured, want) } } @@ -983,6 +997,29 @@ func TestCheckoutInsertsDashDash(t *testing.T) { } } +func TestAddWorktreeInsertsDashDash(t *testing.T) { + const ( + branch = "feature/some-task" + source = "-default-source" + ) + var capturedArgs []string + r := &mockRunner{ + run: func(args ...string) (string, error) { + capturedArgs = args + return "", nil + }, + } + + if err := AddWorktree(r, fakePath, branch, source); err != nil { + t.Fatalf("AddWorktree: %v", err) + } + + want := []string{cmdWorktree, "add", "-b", branch, "--", fakePath, source} + if !slices.Equal(capturedArgs, want) { + t.Errorf("args = %v, want %v", capturedArgs, want) + } +} + func TestAddWorktreeFromBranchInsertsDashDash(t *testing.T) { const branch = "feature/some-task" var capturedArgs []string diff --git a/internal/git/worktree.go b/internal/git/worktree.go index 6c638b9..635f0e2 100644 --- a/internal/git/worktree.go +++ b/internal/git/worktree.go @@ -27,7 +27,7 @@ type WorktreeEntry struct { // AddWorktree creates a new worktree at the given path with a new branch from source. func AddWorktree(r Runner, path, branch, source string) error { - _, err := r.Run(cmdWorktree, "add", "-b", branch, path, source) + _, err := r.Run(cmdWorktree, "add", "-b", branch, "--", path, source) return err } @@ -50,10 +50,11 @@ func RemoveWorktree(r Runner, path string, force bool) error { // MoveWorktree moves the worktree from oldPath to newPath. // When force is true, --force is passed twice so that even locked worktrees can be moved. func MoveWorktree(r Runner, oldPath, newPath string, force bool) error { - args := []string{cmdWorktree, "move", oldPath, newPath} + args := []string{cmdWorktree, "move"} if force { args = append(args, flagForce, flagForce) } + args = append(args, "--", oldPath, newPath) _, err := r.Run(args...) return err }