Skip to content

Commit fe8ed62

Browse files
pjbgfpjbgf
committed
plumbing: object, Align Tag.EncodeWithoutSignature with Commit
Tag verification re-encoded from struct fields, so any non-canonical bytes the signature was computed over (duplicate headers, mutation-only fields like Signature/SignatureSHA256, etc.) were lost from the payload and signatures that were valid in upstream Git failed in go-git. Mirror the approach already used by Commit: retain a reference to the source plumbing.EncodedObject on Decode, add matchesSource() so EncodeWithoutSignature can stream the raw bytes (with the inline trailing PGP block truncated and gpgsig/gpgsig-sha256 headers stripped) when the struct still matches the source. Mutating struct fields still falls back to the struct-encoded payload. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
1 parent 98e337d commit fe8ed62

4 files changed

Lines changed: 371 additions & 84 deletions

File tree

‎plumbing/object/commit.go‎

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"context"
66
"errors"
77
"fmt"
8-
"io"
98
"slices"
109
"strings"
1110

@@ -299,7 +298,7 @@ func (c *Commit) Encode(o plumbing.EncodedObject) error {
299298
// representation prevails.
300299
func (c *Commit) EncodeWithoutSignature(o plumbing.EncodedObject) error {
301300
if c.matchesSource() {
302-
return stripSignaturesFromRaw(o, c.src)
301+
return stripObjectSignatures(o, c.src, plumbing.CommitObject)
303302
}
304303
return c.encode(o, false)
305304
}
@@ -338,69 +337,6 @@ func signatureEqual(a, b Signature) bool {
338337
a.When.Format("-0700") == b.When.Format("-0700")
339338
}
340339

341-
// stripSignaturesFromRaw streams src into dst, dropping the canonical commit
342-
// signature headers ("gpgsig " and "gpgsig-sha256 ") together with their
343-
// continuation lines (those starting with a space). Everything past the
344-
// blank line that ends the header block is copied verbatim, as is any other
345-
// "gpgsig"-prefixed extra header (for example a user-defined "gpgsig-key-id")
346-
// that does not match one of the canonical signature headers.
347-
func stripSignaturesFromRaw(dst, src plumbing.EncodedObject) (err error) {
348-
dst.SetType(plumbing.CommitObject)
349-
350-
r, err := src.Reader()
351-
if err != nil {
352-
return err
353-
}
354-
defer ioutil.CheckClose(r, &err)
355-
356-
w, err := dst.Writer()
357-
if err != nil {
358-
return err
359-
}
360-
defer ioutil.CheckClose(w, &err)
361-
362-
br := sync.GetBufioReader(r)
363-
defer sync.PutBufioReader(br)
364-
365-
var inBody, skipping bool
366-
for {
367-
line, rerr := br.ReadBytes('\n')
368-
if rerr != nil && rerr != io.EOF {
369-
return rerr
370-
}
371-
372-
write := true
373-
if !inBody {
374-
switch {
375-
case skipping && len(line) > 0 && line[0] == ' ':
376-
write = false
377-
case isSignatureHeader(line):
378-
skipping = true
379-
write = false
380-
case len(line) == 1 && line[0] == '\n':
381-
skipping = false
382-
inBody = true
383-
default:
384-
skipping = false
385-
}
386-
}
387-
388-
if write && len(line) > 0 {
389-
if _, werr := w.Write(line); werr != nil {
390-
return werr
391-
}
392-
}
393-
if rerr == io.EOF {
394-
return nil
395-
}
396-
}
397-
}
398-
399-
func isSignatureHeader(line []byte) bool {
400-
return bytes.HasPrefix(line, []byte(headerpgp+" ")) ||
401-
bytes.HasPrefix(line, []byte(headerpgp256+" "))
402-
}
403-
404340
func isStandardHeader(key string) bool {
405341
switch key {
406342
case "tree", "parent", "author", "committer",

‎plumbing/object/signature.go‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
package object
22

3-
import "bytes"
3+
import (
4+
"bytes"
5+
"io"
6+
7+
"github.com/go-git/go-git/v5/plumbing"
8+
"github.com/go-git/go-git/v5/utils/ioutil"
9+
"github.com/go-git/go-git/v5/utils/sync"
10+
)
411

512
const (
613
signatureTypeUnknown signatureType = iota
@@ -100,3 +107,95 @@ func parseSignedBytes(b []byte) (int, signatureType) {
100107
}
101108
return match, t
102109
}
110+
111+
// isSignatureHeader reports whether line is a canonical "gpgsig "/
112+
// "gpgsig-sha256 " header line. Other "gpgsig"-prefixed extra headers
113+
// are intentionally not matched.
114+
func isSignatureHeader(line []byte) bool {
115+
return bytes.HasPrefix(line, []byte(headerpgp+" ")) ||
116+
bytes.HasPrefix(line, []byte(headerpgp256+" "))
117+
}
118+
119+
// stripObjectSignatures streams src into dst, producing the byte sequence
120+
// over which a PGP/GPG signature is computed:
121+
//
122+
// - Canonical "gpgsig" and "gpgsig-sha256" headers (and their
123+
// continuation lines) are dropped, mirroring upstream's
124+
// remove_signature in commit.c.
125+
// - For tag objects, the inline trailing PGP signature is additionally
126+
// truncated, mirroring upstream's parse_signature in gpg-interface.c
127+
// used by gpg_verify_tag.
128+
//
129+
// The returned object's type is set to objType. Used by both
130+
// Commit.EncodeWithoutSignature and Tag.EncodeWithoutSignature to
131+
// reproduce the exact bytes the signature was computed over.
132+
func stripObjectSignatures(dst, src plumbing.EncodedObject, objType plumbing.ObjectType) (err error) {
133+
dst.SetType(objType)
134+
135+
r, err := src.Reader()
136+
if err != nil {
137+
return err
138+
}
139+
defer ioutil.CheckClose(r, &err)
140+
141+
var input io.Reader = r
142+
if objType == plumbing.TagObject {
143+
raw, err := io.ReadAll(r)
144+
if err != nil {
145+
return err
146+
}
147+
if sm, _ := parseSignedBytes(raw); sm >= 0 {
148+
raw = raw[:sm]
149+
}
150+
input = bytes.NewReader(raw)
151+
}
152+
153+
w, err := dst.Writer()
154+
if err != nil {
155+
return err
156+
}
157+
defer ioutil.CheckClose(w, &err)
158+
159+
return stripHeaderSignatures(w, input)
160+
}
161+
162+
// stripHeaderSignatures copies r to w, dropping canonical signature header
163+
// lines (gpgsig and gpgsig-sha256) and their continuation lines. Lines
164+
// past the blank line that closes the header block are copied verbatim.
165+
func stripHeaderSignatures(w io.Writer, r io.Reader) error {
166+
br := sync.GetBufioReader(r)
167+
defer sync.PutBufioReader(br)
168+
169+
var inBody, skipping bool
170+
for {
171+
line, rerr := br.ReadBytes('\n')
172+
if rerr != nil && rerr != io.EOF {
173+
return rerr
174+
}
175+
176+
write := true
177+
if !inBody {
178+
switch {
179+
case skipping && len(line) > 0 && line[0] == ' ':
180+
write = false
181+
case isSignatureHeader(line):
182+
skipping = true
183+
write = false
184+
case len(line) == 1 && line[0] == '\n':
185+
skipping = false
186+
inBody = true
187+
default:
188+
skipping = false
189+
}
190+
}
191+
192+
if write && len(line) > 0 {
193+
if _, werr := w.Write(line); werr != nil {
194+
return werr
195+
}
196+
}
197+
if rerr == io.EOF {
198+
return nil
199+
}
200+
}
201+
}

‎plumbing/object/tag.go‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ type Tag struct {
3939
Target plumbing.Hash
4040

4141
s storer.EncodedObjectStorer
42+
// src holds the encoded object this Tag was decoded from, used by
43+
// EncodeWithoutSignature to recover the canonical signed bytes.
44+
src plumbing.EncodedObject
4245
}
4346

4447
// GetTag gets a tag from an object storer and decodes it.
@@ -84,6 +87,7 @@ func (t *Tag) Decode(o plumbing.EncodedObject) (err error) {
8487
}
8588

8689
t.Hash = o.Hash()
90+
t.src = o
8791

8892
reader, err := o.Reader()
8993
if err != nil {
@@ -162,11 +166,54 @@ func (t *Tag) Encode(o plumbing.EncodedObject) error {
162166
return t.encode(o, true)
163167
}
164168

165-
// EncodeWithoutSignature export a Tag into a plumbing.EncodedObject without the signature (correspond to the payload of the PGP signature).
169+
// EncodeWithoutSignature exports a Tag into a plumbing.EncodedObject without
170+
// any signature data, producing the payload that PGP/GPG signatures are
171+
// computed over.
172+
//
173+
// Behaviour mirrors Commit.EncodeWithoutSignature:
174+
//
175+
// - For Tags populated by Decode whose exported fields still match the
176+
// source object, the payload is streamed from the raw source bytes with
177+
// the inline trailing signature truncated and gpgsig/gpgsig-sha256
178+
// headers (and their continuation lines) stripped verbatim. This
179+
// preserves the exact bytes the signature was computed over, regardless
180+
// of any normalization performed by Decode.
181+
//
182+
// - For Tags constructed in memory, or for decoded Tags whose exported
183+
// fields have been mutated, the payload is derived from the current
184+
// struct fields. Mutation is detected by re-decoding the source object
185+
// and comparing exported fields; if any differ, the in-memory
186+
// representation prevails.
166187
func (t *Tag) EncodeWithoutSignature(o plumbing.EncodedObject) error {
188+
if t.matchesSource() {
189+
return stripObjectSignatures(o, t.src, plumbing.TagObject)
190+
}
167191
return t.encode(o, false)
168192
}
169193

194+
// matchesSource reports whether t.src is set and re-decoding it produces a
195+
// Tag whose payload-affecting exported fields are identical to those of t.
196+
//
197+
// PGPSignature is intentionally excluded from the comparison: neither path
198+
// emits it as part of the verification payload, so mutating it must not
199+
// trigger a switch to struct-encode (which would change the byte layout the
200+
// caller is trying to verify against).
201+
func (t *Tag) matchesSource() bool {
202+
if t.src == nil {
203+
return false
204+
}
205+
fresh := &Tag{}
206+
if err := fresh.Decode(t.src); err != nil {
207+
return false
208+
}
209+
return t.Hash == fresh.Hash &&
210+
t.Name == fresh.Name &&
211+
signatureEqual(t.Tagger, fresh.Tagger) &&
212+
t.Message == fresh.Message &&
213+
t.TargetType == fresh.TargetType &&
214+
t.Target == fresh.Target
215+
}
216+
170217
func (t *Tag) encode(o plumbing.EncodedObject, includeSig bool) (err error) {
171218
o.SetType(plumbing.TagObject)
172219
w, err := o.Writer()

0 commit comments

Comments
 (0)