diff --git a/pkg/git/rawobj.go b/pkg/git/rawobj.go index fa34c1636..4e583ed75 100644 --- a/pkg/git/rawobj.go +++ b/pkg/git/rawobj.go @@ -113,7 +113,7 @@ type TagSig struct { // or gpgsig-sha256 headers, because either is ambiguous about which // signature to extract. func SplitCommit(r io.Reader) (*CommitSig, error) { - scanner := bufio.NewScanner(r) + scanner := newRawObjectScanner(r) var ( payloadBuf bytes.Buffer @@ -245,7 +245,7 @@ func JoinCommit(c *CommitSig) ([]byte, error) { // signatures (gpgsig, gpgsig-sha256) are stripped from the header section // the same way SplitCommit does. func SplitTag(r io.Reader) (*TagSig, error) { - scanner := bufio.NewScanner(r) + scanner := newRawObjectScanner(r) var ( payloadBuf bytes.Buffer @@ -392,6 +392,26 @@ func sigOrNil(b *bytes.Buffer) []byte { return b.Bytes() } +// newRawObjectScanner splits on '\n' like bufio.ScanLines but keeps a trailing +// '\r' in the token. git treats the '\r' as object content, so dropping it +// would make what we split diverge from the bytes git hashes. +func newRawObjectScanner(r io.Reader) *bufio.Scanner { + s := bufio.NewScanner(r) + s.Split(func(data []byte, atEOF bool) (int, []byte, error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\n'); i >= 0 { + return i + 1, data[:i], nil + } + if atEOF { + return len(data), data, nil + } + return 0, nil, nil + }) + return s +} + // commitSingletons / tagSingletons name the headers a well-formed object // carries at most once. parent is intentionally absent from the commit set // (merge commits have several); mergetag and encoding are intentionally @@ -453,7 +473,7 @@ func checkUniqueHeaders(obj plumbing.EncodedObject, singletons map[string]bool) defer r.Close() // nolint:errcheck seen := make(map[string]bool, len(singletons)) - scanner := bufio.NewScanner(r) + scanner := newRawObjectScanner(r) for scanner.Scan() { line := scanner.Bytes() if len(line) == 0 { diff --git a/pkg/git/rawobj_test.go b/pkg/git/rawobj_test.go index cb44787e4..df7f77781 100644 --- a/pkg/git/rawobj_test.go +++ b/pkg/git/rawobj_test.go @@ -415,6 +415,36 @@ func TestSplitCommit_LenientContinuation(t *testing.T) { } } +// TestSplitCommit_TrailingCR confirms a trailing CR stays in the payload. git +// hashes it as object content, so dropping it would collapse two distinct +// commits onto one payload and let a single signature verify for both. +func TestSplitCommit_TrailingCR(t *testing.T) { + sig := "gpgsig -----BEGIN SIGNED MESSAGE-----\n ZmFrZQ==\n -----END SIGNED MESSAGE-----\n" + base := "tree b333504b8cf3d9c314fed2cc242c5c38e89534a5\n" + + "author Alice 1700000000 +0000\n" + + "committer Alice 1700000000 +0000\n" + sig + "\n" + raw := []byte(base + "release v1.0\n") + rawCR := []byte(base + "release v1.0\r\n") + + hash, _ := ObjectHash(raw) + hashCR, _ := ObjectHash(rawCR) + if hash == hashCR { + t.Fatalf("setup: expected distinct objects, both hashed to %s", hash) + } + + c, err := SplitCommit(bytes.NewReader(raw)) + if err != nil { + t.Fatalf("SplitCommit: %v", err) + } + cCR, err := SplitCommit(bytes.NewReader(rawCR)) + if err != nil { + t.Fatalf("SplitCommit (CR variant): %v", err) + } + if bytes.Equal(c.Payload, cCR.Payload) { + t.Errorf("payloads should differ, both are %q", c.Payload) + } +} + // TestJoinCommit_RoundTrip uses the real signed HEAD commit: split it, join // it back, and confirm the bytes are identical. func TestJoinCommit_RoundTrip(t *testing.T) { @@ -623,6 +653,29 @@ func TestSplitTag_NoHeaderTerminator(t *testing.T) { } } +// TestSplitTag_TrailingCR is the tag counterpart to +// TestSplitCommit_TrailingCR. +func TestSplitTag_TrailingCR(t *testing.T) { + sig := "-----BEGIN SIGNED MESSAGE-----\nZmFrZQ==\n-----END SIGNED MESSAGE-----\n" + base := "object 2d9cff2bad7132c586e128bcc23322dbb5297e8e\n" + + "type commit\ntag v1\n" + + "tagger Alice 1700000000 +0000\n\n" + raw := []byte(base + "release v1.0\n" + sig) + rawCR := []byte(base + "release v1.0\r\n" + sig) + + tag, err := SplitTag(bytes.NewReader(raw)) + if err != nil { + t.Fatalf("SplitTag: %v", err) + } + tagCR, err := SplitTag(bytes.NewReader(rawCR)) + if err != nil { + t.Fatalf("SplitTag (CR variant): %v", err) + } + if bytes.Equal(tag.Payload, tagCR.Payload) { + t.Errorf("payloads should differ, both are %q", tag.Payload) + } +} + func TestJoinTag_RoundTrip(t *testing.T) { raw := loadObject(t, "tag.txt") tag, err := SplitTag(bytes.NewReader(raw)) @@ -663,6 +716,7 @@ func TestValidateCommit(t *testing.T) { {"duplicate gpgsig-sha256", base + "gpgsig-sha256 sig1\ngpgsig-sha256 sig2\n\nmsg\n", true}, {"gpgsig with continuation lines does not count as duplicate", base + "gpgsig line1\n line2\n line3\n\nmsg\n", false}, {"duplicate headers in message body are ignored", base + "\ntree fake\ntree alsofake\n", false}, + {"duplicate tree behind a bare-CR line", "tree aaaa\n" + authorLine + commLine + "\r\ntree bbbb\n\nmsg\n", true}, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { @@ -693,6 +747,7 @@ func TestValidateTag(t *testing.T) { {"duplicate tag", "object aaaa\ntype commit\ntag v1\ntag v2\n" + taggerLine + "\nmsg\n", true}, {"duplicate tagger", "object aaaa\ntype commit\ntag v1\n" + taggerLine + taggerLine + "\nmsg\n", true}, {"duplicate headers in message body are ignored", base + "\nobject fake\nobject alsofake\n", false}, + {"duplicate object behind a bare-CR line", "object aaaa\ntype commit\ntag v1\n" + taggerLine + "\r\nobject bbbb\n\nmsg\n", true}, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) {