diff --git a/internal/cli/commit.go b/internal/cli/commit.go index 1d3c93b..8aa6cc9 100644 --- a/internal/cli/commit.go +++ b/internal/cli/commit.go @@ -214,7 +214,7 @@ func runCommitWithBranchLookup(ui ui, noPush bool, repoAliases []string, current color.Green(" ✓ Staged %d file(s)", len(selectedFiles)) // 3e. Commit. - out, err := git.Commit(repo.Path, message) + out, err := git.Commit(repo.Path, message, selectedFiles) if err != nil { color.Red(" ✗ git commit failed: %v", err) results = append(results, repoCommitResult{alias: repo.Alias, err: err}) diff --git a/internal/git/git.go b/internal/git/git.go index 2539d65..dd85b05 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -393,22 +393,14 @@ func UntrackedFiles(path string) ([]string, error) { // UntrackFiles removes files from the git index but keeps them on disk. // Equivalent to: git rm --cached -- func UntrackFiles(path string, files []string) error { - cleaned := make([]string, 0, len(files)) - for _, f := range files { - if len(f) > 3 { - cleaned = append(cleaned, strings.TrimSpace(f[3:])) - } else { - cleaned = append(cleaned, strings.TrimSpace(f)) - } - } - args := append([]string{"rm", "--cached", "--"}, cleaned...) + args := append([]string{"rm", "--cached", "--"}, cleanPorcelainPaths(files)...) _, err := run(path, args...) return err } -// StageFiles stages specific files (by their path relative to the repo root). -func StageFiles(path string, files []string) error { - // Strip the porcelain status prefix (e.g. " M ", "?? ") to get the raw path. +// cleanPorcelainPaths strips the two-char porcelain status prefix (e.g. " M ", +// "?? ") from each line, yielding the bare repo-relative path. +func cleanPorcelainPaths(files []string) []string { cleaned := make([]string, 0, len(files)) for _, f := range files { // porcelain format: "XY filename" where XY is two chars + space. @@ -418,14 +410,23 @@ func StageFiles(path string, files []string) error { cleaned = append(cleaned, strings.TrimSpace(f)) } } - args := append([]string{"add", "--"}, cleaned...) + return cleaned +} + +// StageFiles stages specific files (by their path relative to the repo root). +func StageFiles(path string, files []string) error { + args := append([]string{"add", "--"}, cleanPorcelainPaths(files)...) _, err := run(path, args...) return err } -// Commit creates a commit with the given message. -func Commit(path, message string) (string, error) { - return run(path, "commit", "-m", message) +// Commit creates a commit containing only the given files. Scoping the commit to +// an explicit pathspec stops files that were already staged but NOT selected by the +// user from leaking into the commit. Callers must pass a non-empty file list; an +// empty list would degrade to a whole-index commit. +func Commit(path, message string, files []string) (string, error) { + args := append([]string{"commit", "-m", message, "--"}, cleanPorcelainPaths(files)...) + return run(path, args...) } // Push pushes the current branch to origin. diff --git a/internal/git/git_more_test.go b/internal/git/git_more_test.go index 6f2c6d7..47e6dc0 100644 --- a/internal/git/git_more_test.go +++ b/internal/git/git_more_test.go @@ -287,7 +287,7 @@ func TestPullAndPush(t *testing.T) { writeFile(t, repo1, "local.txt", "local\n") mustRunGit(t, repo1, "add", "local.txt") - if _, err := git.Commit(repo1, "local commit"); err != nil { + if _, err := git.Commit(repo1, "local commit", []string{"A local.txt"}); err != nil { t.Fatalf("Commit: %v", err) } if err := git.Push(repo1); err != nil { @@ -355,6 +355,49 @@ func TestStageFilesAndDiscard(t *testing.T) { } } +// TestCommitOnlySelectedFiles guards against the regression where Commit ran a +// bare `git commit`, sweeping the whole index — so a file that was already staged +// but NOT selected by the user leaked into the commit. Commit must include only the +// files passed to it and leave other staged files untouched. +func TestCommitOnlySelectedFiles(t *testing.T) { + repo := initRepo(t) + makeCommit(t, repo, "selected.txt", "v1\n", "add selected") + + // An unselected new file the user never chose, but already staged in the index. + writeFile(t, repo, "leak.txt", "leak\n") + mustRunGit(t, repo, "add", "leak.txt") + + // The file the user actually selects (a tracked, unstaged modification). + writeFile(t, repo, "selected.txt", "v2\n") + + selection := []string{" M selected.txt"} + if err := git.StageFiles(repo, selection); err != nil { + t.Fatalf("StageFiles: %v", err) + } + if _, err := git.Commit(repo, "commit selected only", selection); err != nil { + t.Fatalf("Commit: %v", err) + } + + committed := mustRunGit(t, repo, "diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD") + if !strings.Contains(committed, "selected.txt") { + t.Errorf("expected selected.txt in commit, got: %q", committed) + } + if strings.Contains(committed, "leak.txt") { + t.Errorf("leak.txt was committed but the user never selected it; committed files: %q", committed) + } + + staged := stagedFiles(t, repo) + foundLeak := false + for _, f := range staged { + if f == "leak.txt" { + foundLeak = true + } + } + if !foundLeak { + t.Errorf("expected leak.txt to remain staged (not committed), staged files: %v", staged) + } +} + func TestIsDefaultBranch(t *testing.T) { repo := initRepo(t) branch, err := git.CurrentBranch(repo)