Commit 1030a916f9

1030a916f95d6fd434574c3fe59be9c9aac0fe21

parent: d99ece54b6

Verified · cmc ci/build: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-03 18:22 UTC

gitutil: end option parsing before every ref

Refs reaching gitutil come from URL segments and command arguments, and
the call sites passed them as bare arguments after a subcommand's
options. A leading-dash ref was only harmless because every caller
happened to glue it to :path or ^{commit} or resolve it first. Each
invocation now passes --end-of-options before the ref (rev-parse with
--verify, since it otherwise echoes the flag), and blame, which has no
such flag, resolves the ref to a sha first.

TestRefsAreNotOptions calls every ref-taking helper with an
option-shaped ref and asserts nothing was parsed as a flag.

Closes #135

Layout: unified · split

internal/gitutil/blame.go +6 −1
@@ -23,8 +23,13 @@ type BlameHunk struct {
2323// Blame attributes lines start..end (1-based, inclusive) of path at ref,
2424// merging consecutive same-commit lines into hunks.
2525func Blame(dir, ref, path string, start, end int) ([]BlameHunk, error) {
26 // blame has no --end-of-options; a resolved sha cannot be an option.
27 sha, err := ResolveRef(dir, ref)
28 if err != nil {
29 return nil, err
30 }
2631 cmd := exec.Command("git", "-C", dir, "blame", "--porcelain",
27 fmt.Sprintf("-L%d,%d", start, end), ref, "--", path)
32 fmt.Sprintf("-L%d,%d", start, end), sha, "--", path)
2833 out, err := cmd.Output()
2934 if err != nil {
3035 return nil, fmt.Errorf("git blame %s at %s: %w", path, ref, err)
internal/gitutil/endofoptions_test.go added +54
@@ -0,0 +1,54 @@
1package gitutil
2
3import (
4 "bytes"
5 "os"
6 "path/filepath"
7 "testing"
8)
9
10// A ref reaching gitutil is user input: a URL segment or an argument. Every
11// git invocation ends option parsing before it, so a ref shaped like an
12// option is a bad revision, never a flag (#135). The payload here is an
13// option several subcommands accept, whose effect would be a file.
14func TestRefsAreNotOptions(t *testing.T) {
15 dir := t.TempDir()
16 git(t, dir, "init", "-q", "-b", "main")
17 write(t, dir, "f.txt", "hi\n")
18 git(t, dir, "add", ".")
19 git(t, dir, "commit", "-q", "-m", "base")
20 pwn := filepath.Join(t.TempDir(), "pwned")
21 ref := "--output=" + pwn
22
23 var sink bytes.Buffer
24 calls := map[string]func() error{
25 "CountCommits": func() error { CountCommits(dir, ref); return nil },
26 "Contributors": func() error { Contributors(dir, ref); return nil },
27 "Languages": func() error { Languages(dir, ref, func(string) string { return "x" }); return nil },
28 "RevList": func() error { _, err := RevList(dir, ref, 1); return err },
29 "RevListPath": func() error { _, err := RevListPath(dir, ref, "f.txt", 1); return err },
30 "PeelToCommit": func() error { _, err := PeelToCommit(dir, ref); return err },
31 "ReadCommit": func() error { _, err := ReadCommit(dir, ref); return err },
32 "ListTree": func() error { _, err := ListTree(dir, ref, ""); return err },
33 "ReadBlob": func() error { _, err := ReadBlob(dir, ref, "f.txt", 1<<20); return err },
34 "ResolveRef": func() error { _, err := ResolveRef(dir, ref); return err },
35 "Archive": func() error { return Archive(dir, ref, "x", &sink) },
36 "Grep": func() error { _, err := Grep(dir, ref, "hi", 10); return err },
37 "MergeBase": func() error { _, err := MergeBase(dir, ref, "main"); return err },
38 "Diff": func() error { _, err := Diff(dir, ref, "main", 1<<20); return err },
39 "DiffFiles": func() error { _, err := DiffFiles(dir, ref, "main"); return err },
40 "RevListRange": func() error { _, err := RevListRange(dir, "main", ref); return err },
41 "Blame": func() error { _, err := Blame(dir, ref, "f.txt", 1, 1); return err },
42 "TipCommit": func() error { TipCommit(dir, ref); return nil },
43 "StatPath": func() error { StatPath(dir, ref, "f.txt"); return nil },
44 }
45 for name, call := range calls {
46 call()
47 if _, err := os.Stat(pwn); err == nil {
48 t.Fatalf("%s: ref %q was parsed as an option and wrote a file", name, ref)
49 }
50 }
51 if _, err := ResolveRef(dir, ref); err == nil {
52 t.Fatal("option-shaped ref resolved")
53 }
54}
internal/gitutil/facts.go +3 −3
@@ -10,7 +10,7 @@ import (
1010// CountCommits returns the number of commits reachable from ref, or 0 when
1111// the ref does not resolve (an empty repository).
1212func CountCommits(dir, ref string) int {
13 out, err := exec.Command("git", "-C", dir, "rev-list", "--count", ref).Output()
13 out, err := exec.Command("git", "-C", dir, "rev-list", "--count", "--end-of-options", ref).Output()
1414 if err != nil {
1515 return 0
1616 }
@@ -31,7 +31,7 @@ type Contributor struct {
3131// say — a bare repo resolves that from HEAD:.mailmap with no config.
3232func Contributors(dir, ref string) []Contributor {
3333 out, err := exec.Command("git", "-C", dir, "log",
34 "--use-mailmap", "--format=%aN%x01%aE", ref).Output()
34 "--use-mailmap", "--format=%aN%x01%aE", "--end-of-options", ref).Output()
3535 if err != nil {
3636 return nil
3737 }
@@ -62,7 +62,7 @@ func Contributors(dir, ref string) []Contributor {
6262// largest first, keyed by the extension map the caller supplies. Only
6363// blobs count; git's own metadata does not.
6464func Languages(dir, ref string, lang func(path string) string) []Language {
65 out, err := exec.Command("git", "-C", dir, "ls-tree", "-r", "-l", "--full-name", ref).Output()
65 out, err := exec.Command("git", "-C", dir, "ls-tree", "-r", "-l", "--full-name", "--end-of-options", ref).Output()
6666 if err != nil {
6767 return nil
6868 }
internal/gitutil/gitutil.go +4 −4
@@ -87,7 +87,7 @@ func ZeroSHA(s string) bool {
8787
8888// RevList returns up to limit commit SHAs reachable from ref, newest first.
8989func RevList(dir, ref string, limit int) ([]string, error) {
90 cmd := exec.Command("git", "-C", dir, "rev-list", fmt.Sprintf("--max-count=%d", limit), ref)
90 cmd := exec.Command("git", "-C", dir, "rev-list", fmt.Sprintf("--max-count=%d", limit), "--end-of-options", ref)
9191 out, err := cmd.Output()
9292 if err != nil {
9393 return nil, fmt.Errorf("rev-list %s: %w", ref, err)
@@ -106,7 +106,7 @@ func RevList(dir, ref string, limit int) ([]string, error) {
106106// read as an option or ref.
107107func RevListPath(dir, ref, filePath string, limit int) ([]string, error) {
108108 cmd := exec.Command("git", "-C", dir, "rev-list",
109 fmt.Sprintf("--max-count=%d", limit), ref, "--", filePath)
109 fmt.Sprintf("--max-count=%d", limit), "--end-of-options", ref, "--", filePath)
110110 out, err := cmd.Output()
111111 if err != nil {
112112 return nil, fmt.Errorf("rev-list %s -- %s: %w", ref, filePath, err)
@@ -123,7 +123,7 @@ func RevListPath(dir, ref, filePath string, limit int) ([]string, error) {
123123// PeelToCommit resolves a ref or object to its commit — annotated tags
124124// peel to the commit they point at.
125125func PeelToCommit(dir, ref string) (string, error) {
126 out, err := exec.Command("git", "-C", dir, "rev-parse", ref+"^{commit}").Output()
126 out, err := exec.Command("git", "-C", dir, "rev-parse", "--verify", "--end-of-options", ref+"^{commit}").Output()
127127 if err != nil {
128128 return "", fmt.Errorf("rev-parse %s^{commit}: %w", ref, err)
129129 }
@@ -132,7 +132,7 @@ func PeelToCommit(dir, ref string) (string, error) {
132132
133133// ReadCommit returns the raw commit object bytes.
134134func ReadCommit(dir, sha string) ([]byte, error) {
135 cmd := exec.Command("git", "-C", dir, "cat-file", "commit", sha)
135 cmd := exec.Command("git", "-C", dir, "cat-file", "commit", "--end-of-options", sha)
136136 out, err := cmd.Output()
137137 if err != nil {
138138 return nil, fmt.Errorf("cat-file commit %s: %w", sha, err)
internal/gitutil/grep.go +1 −1
@@ -23,7 +23,7 @@ func Grep(dir, ref, query string, max int) ([]GrepMatch, error) {
2323 defer cancel()
2424 // -z: NUL after the path and the line number, so paths containing
2525 // ':' parse unambiguously (format: "ref:path\0line\0text\n").
26 cmd := exec.CommandContext(ctx, "git", "-C", dir, "grep", "-nIiF", "-z", "-e", query, ref)
26 cmd := exec.CommandContext(ctx, "git", "-C", dir, "grep", "-nIiF", "-z", "-e", query, "--end-of-options", ref)
2727 out, err := cmd.Output()
2828 if err != nil {
2929 if ee, ok := err.(*exec.ExitError); ok && ee.ExitCode() == 1 {
internal/gitutil/lastcommit.go +3 −3
@@ -45,7 +45,7 @@ func LastCommits(dir, ref, path string, names []string) map[string]EntryCommit {
4545 }
4646
4747 args := []string{"-C", dir, "log", "--first-parent", "--name-only",
48 "--format=%x1e%H%x1f%ct%x1f%an%x1f%ae%x1f%s", "-n", strconv.Itoa(lastCommitScan), ref}
48 "--format=%x1e%H%x1f%ct%x1f%an%x1f%ae%x1f%s", "-n", strconv.Itoa(lastCommitScan), "--end-of-options", ref}
4949 if prefix != "" {
5050 args = append(args, "--", strings.TrimSuffix(prefix, "/"))
5151 }
@@ -108,7 +108,7 @@ func parseCommitHeader(s string) EntryCommit {
108108// answers "who touched this repository last".
109109func TipCommit(dir, ref string) EntryCommit {
110110 out, err := exec.Command("git", "-C", dir, "log", "-1",
111 "--format=%H%x1f%ct%x1f%an%x1f%ae%x1f%s", ref).Output()
111 "--format=%H%x1f%ct%x1f%an%x1f%ae%x1f%s", "--end-of-options", ref).Output()
112112 if err != nil {
113113 return EntryCommit{}
114114 }
@@ -137,7 +137,7 @@ func entryName(changed, prefix string) (string, bool) {
137137// page can report the facts the file listing no longer carries: its size,
138138// and whether it is executable or a symlink.
139139func StatPath(dir, ref, path string) (TreeEntry, bool) {
140 out, err := exec.Command("git", "-C", dir, "ls-tree", "-l", ref, "--", path).Output()
140 out, err := exec.Command("git", "-C", dir, "ls-tree", "-l", "--end-of-options", ref, "--", path).Output()
141141 if err != nil {
142142 return TreeEntry{}, false
143143 }
internal/gitutil/merge.go +10 −10
@@ -44,7 +44,7 @@ func DeleteRef(dir, ref string) error {
4444
4545// RevListRange returns commits in old..new, newest first.
4646func RevListRange(dir, old, new string) ([]string, error) {
47 cmd := exec.Command("git", "-C", dir, "rev-list", new, "^"+old)
47 cmd := exec.Command("git", "-C", dir, "rev-list", "--end-of-options", new, "^"+old)
4848 out, err := cmd.Output()
4949 if err != nil {
5050 return nil, fmt.Errorf("rev-list %s..%s: %w", old, new, err)
@@ -61,7 +61,7 @@ func RevListRange(dir, old, new string) ([]string, error) {
6161// MergeTree performs a real merge of ours and theirs, returning the merged
6262// tree id. conflict=true means the merge cannot be done automatically.
6363func MergeTree(dir, ours, theirs string) (tree string, conflict bool, err error) {
64 cmd := exec.Command("git", "-C", dir, "merge-tree", "--write-tree", ours, theirs)
64 cmd := exec.Command("git", "-C", dir, "merge-tree", "--write-tree", "--end-of-options", ours, theirs)
6565 out, runErr := cmd.Output()
6666 tree = strings.TrimSpace(strings.SplitN(string(out), "\n", 2)[0])
6767 if runErr != nil {
@@ -95,7 +95,7 @@ func CommitTree(dir, tree string, parents []string, name, email, message string)
9595// Diff returns the patch for old..new (three-dot semantics are the caller's
9696// job: pass the merge base as old).
9797func Diff(dir, old, new string, limit int64) (string, error) {
98 cmd := exec.Command("git", "-C", dir, "diff", "--stat", "--patch", old, new)
98 cmd := exec.Command("git", "-C", dir, "diff", "--stat", "--patch", "--end-of-options", old, new)
9999 out, err := cmd.Output()
100100 if err != nil {
101101 return "", fmt.Errorf("diff: %w", err)
@@ -108,7 +108,7 @@ func Diff(dir, old, new string, limit int64) (string, error) {
108108
109109// MergeBase returns the best common ancestor, or an error if none exists.
110110func MergeBase(dir, a, b string) (string, error) {
111 cmd := exec.Command("git", "-C", dir, "merge-base", a, b)
111 cmd := exec.Command("git", "-C", dir, "merge-base", "--end-of-options", a, b)
112112 out, err := cmd.Output()
113113 if err != nil {
114114 return "", fmt.Errorf("no common history between %s and %s", a, b)
@@ -175,7 +175,7 @@ func CommitFileChange(dir, branch, path string, content []byte, name, email, mes
175175
176176// CommitParents returns the parent SHAs of a commit.
177177func CommitParents(dir, sha string) ([]string, error) {
178 out, err := exec.Command("git", "-C", dir, "rev-list", "--parents", "-n1", sha).Output()
178 out, err := exec.Command("git", "-C", dir, "rev-list", "--parents", "-n1", "--end-of-options", sha).Output()
179179 if err != nil {
180180 return nil, fmt.Errorf("rev-list --parents %s: %w", sha, err)
181181 }
@@ -188,7 +188,7 @@ func CommitParents(dir, sha string) ([]string, error) {
188188
189189// AuthorIdent returns a commit's author name, email, and ISO date.
190190func AuthorIdent(dir, sha string) (name, email, date string, err error) {
191 out, err := exec.Command("git", "-C", dir, "log", "-1", "--format=%an%x1f%ae%x1f%aI", sha).Output()
191 out, err := exec.Command("git", "-C", dir, "log", "-1", "--format=%an%x1f%ae%x1f%aI", "--end-of-options", sha).Output()
192192 if err != nil {
193193 return "", "", "", fmt.Errorf("log %s: %w", sha, err)
194194 }
@@ -201,7 +201,7 @@ func AuthorIdent(dir, sha string) (name, email, date string, err error) {
201201
202202// CommitMessage returns a commit's full message.
203203func CommitMessage(dir, sha string) (string, error) {
204 out, err := exec.Command("git", "-C", dir, "log", "-1", "--format=%B", sha).Output()
204 out, err := exec.Command("git", "-C", dir, "log", "-1", "--format=%B", "--end-of-options", sha).Output()
205205 if err != nil {
206206 return "", fmt.Errorf("log %s: %w", sha, err)
207207 }
@@ -211,7 +211,7 @@ func CommitMessage(dir, sha string) (string, error) {
211211// MergeTreeOnto replays commit's changes (relative to base) onto onto,
212212// returning the resulting tree. conflict=true when it cannot apply cleanly.
213213func MergeTreeOnto(dir, base, onto, commit string) (tree string, conflict bool, err error) {
214 cmd := exec.Command("git", "-C", dir, "merge-tree", "--write-tree", "--merge-base="+base, onto, commit)
214 cmd := exec.Command("git", "-C", dir, "merge-tree", "--write-tree", "--merge-base="+base, "--end-of-options", onto, commit)
215215 out, runErr := cmd.Output()
216216 tree = strings.TrimSpace(strings.SplitN(string(out), "\n", 2)[0])
217217 if runErr != nil {
@@ -249,7 +249,7 @@ func CommitTreeIdent(dir, tree string, parents []string,
249249
250250// ResolveTree returns the tree id of a commit.
251251func ResolveTree(dir, sha string) (string, error) {
252 out, err := exec.Command("git", "-C", dir, "rev-parse", sha+"^{tree}").Output()
252 out, err := exec.Command("git", "-C", dir, "rev-parse", "--verify", "--end-of-options", sha+"^{tree}").Output()
253253 if err != nil {
254254 return "", fmt.Errorf("rev-parse %s^{tree}: %w", sha, err)
255255 }
@@ -258,7 +258,7 @@ func ResolveTree(dir, sha string) (string, error) {
258258
259259// DiffFiles lists the paths changed between old and new.
260260func DiffFiles(dir, old, new string) ([]string, error) {
261 out, err := exec.Command("git", "-C", dir, "diff", "--name-only", old, new).Output()
261 out, err := exec.Command("git", "-C", dir, "diff", "--name-only", "--end-of-options", old, new).Output()
262262 if err != nil {
263263 return nil, fmt.Errorf("diff --name-only: %w", err)
264264 }
internal/gitutil/read.go +4 −4
@@ -26,7 +26,7 @@ func ListTree(dir, ref, path string) ([]TreeEntry, error) {
2626 if path != "" {
2727 spec = ref + ":" + path
2828 }
29 cmd := exec.Command("git", "-C", dir, "ls-tree", "-l", spec)
29 cmd := exec.Command("git", "-C", dir, "ls-tree", "-l", "--end-of-options", spec)
3030 out, err := cmd.Output()
3131 if err != nil {
3232 return nil, fmt.Errorf("ls-tree %s: %w", spec, err)
@@ -56,7 +56,7 @@ func ListTree(dir, ref, path string) ([]TreeEntry, error) {
5656
5757// ReadBlob returns the contents of ref:path, capped at limit bytes.
5858func ReadBlob(dir, ref, path string, limit int64) ([]byte, error) {
59 cmd := exec.Command("git", "-C", dir, "cat-file", "blob", ref+":"+path)
59 cmd := exec.Command("git", "-C", dir, "cat-file", "blob", "--end-of-options", ref+":"+path)
6060 stdout, err := cmd.StdoutPipe()
6161 if err != nil {
6262 return nil, err
@@ -74,7 +74,7 @@ func ReadBlob(dir, ref, path string, limit int64) ([]byte, error) {
7474
7575// ResolveRef resolves a ref or sha to a full commit sha; errors if absent.
7676func ResolveRef(dir, ref string) (string, error) {
77 cmd := exec.Command("git", "-C", dir, "rev-parse", "--verify", "--quiet", ref+"^{commit}")
77 cmd := exec.Command("git", "-C", dir, "rev-parse", "--verify", "--quiet", "--end-of-options", ref+"^{commit}")
7878 out, err := cmd.Output()
7979 if err != nil {
8080 return "", fmt.Errorf("unknown ref %q", ref)
@@ -118,7 +118,7 @@ var ErrArchiveTooLarge = errors.New("archive exceeds the size limit")
118118func Archive(dir, ref, prefix string, w io.Writer) error {
119119 ctx, cancel := context.WithTimeout(context.Background(), archiveTimeout)
120120 defer cancel()
121 cmd := exec.CommandContext(ctx, "git", "-C", dir, "archive", "--format=tar.gz", "--prefix="+prefix+"/", ref)
121 cmd := exec.CommandContext(ctx, "git", "-C", dir, "archive", "--format=tar.gz", "--prefix="+prefix+"/", "--end-of-options", ref)
122122 lw := &cappedWriter{w: w, left: MaxArchiveBytes, stop: cancel}
123123 cmd.Stdout = lw
124124 err := cmd.Run()