fix: add -- separator before file paths in git commands#108
fix: add -- separator before file paths in git commands#108marcus merged 2 commits intomarcus:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens git command invocations against pathspec confusion by adding the -- separator before user-selected file paths, preventing filenames that start with - from being parsed as git flags (which can lead to unintended, repo-wide operations).
Changes:
- Add
--before path arguments forgit add/git restoreoperations in the gitstatus plugin. - Add
--before path arguments forgit status,git log, andgit blamein the filebrowser plugin. - Update
.claude/settings.local.jsonto allow additional Bash commands.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/plugins/gitstatus/tree.go | Inserts -- before file path args for stage/unstage/discard git operations. |
| internal/plugins/filebrowser/git_info.go | Inserts -- before the file path for git status and git log. |
| internal/plugins/filebrowser/blame.go | Inserts -- before the file path for git blame. |
| .claude/settings.local.json | Expands allowed Bash permissions (unrelated to git pathspec fix). |
Comments suppressed due to low confidence (1)
.claude/settings.local.json:7
- This change expands allowed shell permissions in a "settings.local.json" file, which appears unrelated to the PR’s stated purpose (adding
--to git pathspec commands). If this file is intended to be developer-local, it should likely not be version-controlled; if it is intended to be shared, the rationale for broadening permissions (git checkout/go build) should be documented and kept in a separate, clearly-scoped PR.
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // StageFile stages a file. | ||
| func (t *FileTree) StageFile(path string) error { | ||
| cmd := exec.Command("git", "add", path) | ||
| cmd := exec.Command("git", "add", "--", path) | ||
| cmd.Dir = t.workDir |
There was a problem hiding this comment.
There’s no regression test covering the unsafe case described in the PR (a tracked filename that begins with -, e.g. --worktree, being interpreted as a flag). Since this file already has tests, consider adding an integration-style test that creates a temp git repo with a file named like --worktree plus another modified file, then verifies DiscardModified/DiscardStaged only affects the targeted file when invoking git restore with the -- pathspec separator.
|
Thanks for catching this — good security fix! Merged and will be in the next release. 🎉 |
Without --, git interprets filenames that start with - as flags. A file named --worktree passed to git restore becomes git restore --worktree, which restores every tracked file in the working directory instead of the one file the user targeted.
This can silently destroy uncommitted work across the entire repo.
The codebase already uses -- correctly in GetDiff, GetFileDiffStats, and GetCommitDiff. This brings the remaining 7 call sites in StageFile, UnstageFile, DiscardModified, DiscardStaged, RunGitBlame, and fetchGitInfo into line