From b25323c96cfcc25b2bf490a0290fa807aca37665 Mon Sep 17 00:00:00 2001 From: Sam Agarwal Date: Sat, 25 Jul 2026 15:38:44 -0400 Subject: [PATCH 1/2] Fix panic on short 'diff --git' args parseDiffGitArgs compares the two filenames against the 'a/' and 'b/' prefixes via first[:2] and second[:2] without a length guard, while the sibling check just above guards len(first) >= 3. A 'diff --git' line whose unquoted, space-containing args are shorter than two bytes (e.g. 'x ') therefore panics with a slice-bounds error, and this is reachable from the public ParseFileDiff/ParseMultiFileDiff on malformed input. Guard the prefix comparison with len(first) >= 2 (first and second have equal length on this branch), returning the existing ("", "", true) result for such unextractable-but-valid syntax. --- diff/parse.go | 4 +++- diff/parse_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/diff/parse.go b/diff/parse.go index b73e230..66fbd52 100644 --- a/diff/parse.go +++ b/diff/parse.go @@ -466,7 +466,9 @@ func parseDiffGitArgs(diffArgs string) (string, string, bool) { return first, second, true } // If the names don't have the a/ and b/ prefixes and they're equal, proceed. - if !(first[:2] == "a/" && second[:2] == "b/") && first == second { + // first and second have equal length here; guard the prefix slices so a + // name shorter than "a/" cannot panic. + if len(first) >= 2 && !(first[:2] == "a/" && second[:2] == "b/") && first == second { return first, second, true } } diff --git a/diff/parse_test.go b/diff/parse_test.go index ad00c2f..fcf483c 100644 --- a/diff/parse_test.go +++ b/diff/parse_test.go @@ -91,3 +91,16 @@ func TestParseDiffGitArgs_Unsuccessful(t *testing.T) { } } } + +// Short, space-heavy args reach the ambiguous unquoted-with-spaces branch with a +// filename shorter than the "a/"/"b/" prefixes it is compared against. The syntax +// is valid but no filename can be extracted, so the function must return +// ("", "", true) rather than panicking on the prefix slice. +func TestParseDiffGitArgs_ShortAmbiguous(t *testing.T) { + for _, input := range []string{`x `, `a `, `ab `} { + first, second, success := parseDiffGitArgs(input) + if !success || first != "" || second != "" { + t.Errorf("`diff --git %s`: expected (\"\", \"\", true), got (%q, %q, %v)", input, first, second, success) + } + } +} From 5328ae25613dc12e04a874140d8c6a3a889485b4 Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Sun, 26 Jul 2026 17:18:59 +0200 Subject: [PATCH 2/2] fix/parser: preserve short diff arguments safely Use the string prefix API so malformed short arguments follow the parser's existing ambiguous-input path without relying on slice bounds. Exercise the complete public parsing path because the panic is only reachable after an empty-file header is recognized. Amp-Thread-ID: https://ampcode.com/threads/T-019f9ef9-10eb-74aa-a4f3-ff804b81049d Co-authored-by: Amp --- diff/parse.go | 4 +--- diff/parse_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/diff/parse.go b/diff/parse.go index 66fbd52..ca947cf 100644 --- a/diff/parse.go +++ b/diff/parse.go @@ -466,9 +466,7 @@ func parseDiffGitArgs(diffArgs string) (string, string, bool) { return first, second, true } // If the names don't have the a/ and b/ prefixes and they're equal, proceed. - // first and second have equal length here; guard the prefix slices so a - // name shorter than "a/" cannot panic. - if len(first) >= 2 && !(first[:2] == "a/" && second[:2] == "b/") && first == second { + if !(strings.HasPrefix(first, "a/") && strings.HasPrefix(second, "b/")) && first == second { return first, second, true } } diff --git a/diff/parse_test.go b/diff/parse_test.go index fcf483c..4bf302b 100644 --- a/diff/parse_test.go +++ b/diff/parse_test.go @@ -104,3 +104,14 @@ func TestParseDiffGitArgs_ShortAmbiguous(t *testing.T) { } } } + +func TestParseFileDiff_ShortAmbiguousDiffGitArgs(t *testing.T) { + input := []byte("diff --git x \ndeleted file mode 100644\nindex e69de29..0000000\n") + fileDiff, err := ParseFileDiff(input) + if err != nil { + t.Fatalf("ParseFileDiff: expected success, got %s", err) + } + if fileDiff.OrigName != "" || fileDiff.NewName != "/dev/null" { + t.Errorf("ParseFileDiff: expected empty original name and /dev/null new name, got %q and %q", fileDiff.OrigName, fileDiff.NewName) + } +}