From e54ea9f777ae72f8b9c2a98dd4befed08cd88e8b Mon Sep 17 00:00:00 2001 From: Chris Warburton Date: Wed, 4 Mar 2026 13:42:02 +0000 Subject: [PATCH] Include signature type in GpgSig, to support SSH, x509, etc. The existing commit parser/printer were hard-coded for PGP signatures, and stripped off their BEGIN/END lines. That threw an error for commits signed with other schemes like SSH or x509 ("expected first line of sig to be a single space or version"), and it loses information about what type of signature was present; which is bad for those inspecting the IPLD DAG, and it prevents the codec from correctly encoding commit objects. This change generalises the signature parsing, to accept any `-----BEGIN` and `-----END` markers. It also includes those markers in the `GpgSig` value, so signed commits can roundtrip the codec, and the DAG will show what the signature type is. Note that this changes the contents of the `signature` field exposed to IPLD! Here's what a PGP-signed commit used to look like: ``` $ ipfs dag get baf4bcfastgjlbc4cb4pwvazfmxrs6duxojydbsq | jq . { "author": { "date": "1772632573", "email": "chriswarbo@gmail.com", "name": "Chris Warburton", "timezone": "+0000" }, "committer": { "date": "1772632573", "email": "chriswarbo@gmail.com", "name": "Chris Warburton", "timezone": "+0000" }, "encoding": null, "mergetag": [], "message": "Signed commit for testing\n", "other": [], "parents": [ { "/": "baf4bcfafx5b5ehckbgrxs3w5vw4p3l6jnwchxri" } ], "signature": " \n iQEzBAABCgAdFiEEL4FPY3Lp/jQ0Jiq+seTexiNfLroFAmmoOf0ACgkQseTexiNf\n Lrp3QAf9Ee+oNHjHT9aKCRkUyaRn5ZUV9OxmxskD5hDOz7iLnZ84DIx4WNl7QT+x\n kw15VPyrryr16ES/J4HBb9aaMLDb9ns98vmrPUJ6YPKYWoD+0750GM/+RBe9HLsJ\n woTaADmn9ocSym53b32t6tTLvt2ow8J2UbfjijLqsEkqTH+E+T0qzxwlscZ6xWT8\n z40y88lbvIjuyxV8SirnoJtNra91QPuWKmkJCDE7OM4WGA/tU62Za5z4J8q4WZ9A\n +9K0poBMrEpsBRazx8XKNY1w/4v66vKEvY+q8kcH0zGKbX6uYnxsG/kr6bfLIQMU\n 7KdD0xv+QsGw5uhjhD7WLK8ziJ/x0w==\n =vyj/\n", "tree": { "/": "baf4bcfcthputu4uf5w5m66gfj4szgbazp5agqjy" } } ``` With this change, the `signature` field will become: ``` "signature": " -----BEGIN PGP SIGNATURE-----\n iQEzBAABCgAdFiEEL4FPY3Lp/jQ0Jiq+seTexiNfLroFAmmoOf0ACgkQseTexiNf\n Lrp3QAf9Ee+oNHjHT9aKCRkUyaRn5ZUV9OxmxskD5hDOz7iLnZ84DIx4WNl7QT+x\n kw15VPyrryr16ES/J4HBb9aaMLDb9ns98vmrPUJ6YPKYWoD+0750GM/+RBe9HLsJ\n woTaADmn9ocSym53b32t6tTLvt2ow8J2UbfjijLqsEkqTH+E+T0qzxwlscZ6xWT8\n z40y88lbvIjuyxV8SirnoJtNra91QPuWKmkJCDE7OM4WGA/tU62Za5z4J8q4WZ9A\n +9K0poBMrEpsBRazx8XKNY1w/4v66vKEvY+q8kcH0zGKbX6uYnxsG/kr6bfLIQMU\n 7KdD0xv+QsGw5uhjhD7WLK8ziJ/x0w==\n =vyj/\n -----END PGP SIGNATURE-----", ``` --- commit.go | 33 +++------- commit_test.go | 174 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 commit_test.go diff --git a/commit.go b/commit.go index 5ecec68..1d666e5 100644 --- a/commit.go +++ b/commit.go @@ -6,7 +6,6 @@ import ( "encoding/hex" "fmt" "io" - "strings" "github.com/ipld/go-ipld-prime" cidlink "github.com/ipld/go-ipld-prime/linking/cid" @@ -94,7 +93,7 @@ func decodeCommitLine(c Commit, line []byte, rd *bufio.Reader) error { } } case bytes.HasPrefix(line, []byte("gpgsig ")): - sig, err := decodeGpgSig(rd) + sig, err := decodeGpgSig(string(line[len("gpgsig "):]), rd) if err != nil { return err } @@ -112,23 +111,9 @@ func decodeCommitLine(c Commit, line []byte, rd *bufio.Reader) error { return nil } -func decodeGpgSig(rd *bufio.Reader) (_GpgSig, error) { +func decodeGpgSig(firstLine string, rd *bufio.Reader) (_GpgSig, error) { out := _GpgSig{} - - line, _, err := rd.ReadLine() - if err != nil { - return out, err - } - - if string(line) != " " { - if strings.HasPrefix(string(line), " Version: ") || strings.HasPrefix(string(line), " Comment: ") { - out.x += string(line) + "\n" - } else { - return out, fmt.Errorf("expected first line of sig to be a single space or version") - } - } else { - out.x += " \n" - } + out.x = firstLine + "\n" for { line, _, err := rd.ReadLine() @@ -136,11 +121,15 @@ func decodeGpgSig(rd *bufio.Reader) (_GpgSig, error) { return out, err } - if bytes.Equal(line, []byte(" -----END PGP SIGNATURE-----")) { - break + if len(line) == 0 || line[0] != ' ' { + return out, fmt.Errorf("unexpected end of signature") } out.x += string(line) + "\n" + + if bytes.HasPrefix(line, []byte(" -----END")) && bytes.HasSuffix(line, []byte("-----")) { + break + } } return out, nil @@ -172,9 +161,7 @@ func encodeCommit(n ipld.Node, w io.Writer) error { fmt.Fprintf(buf, "%s", mtag.message.x) } if c.signature.m == schema.Maybe_Value { - fmt.Fprintln(buf, "gpgsig -----BEGIN PGP SIGNATURE-----") - fmt.Fprint(buf, c.signature.v.x) - fmt.Fprintln(buf, " -----END PGP SIGNATURE-----") + fmt.Fprintf(buf, "gpgsig %s", c.signature.v.x) } for _, line := range c.other.x { fmt.Fprintln(buf, line.x) diff --git a/commit_test.go b/commit_test.go new file mode 100644 index 0000000..833edbd --- /dev/null +++ b/commit_test.go @@ -0,0 +1,174 @@ +package ipldgit + +import ( + "bytes" + "fmt" + "strings" + "testing" +) + +// buildCommitRaw creates a raw git commit object with the given signature block. +// The sig parameter should be the complete gpgsig header (including the +// "gpgsig " prefix and trailing newline), or empty for an unsigned commit. +func buildCommitRaw(sig string) string { + // Use a fixed tree hash (40 hex chars) + tree := "4b825dc642cb6eb9a060e54bf899d69f7ef9c0b8" + author := "Test User 1234567890 +0000" + committer := author + message := "test commit" + + var b strings.Builder + b.WriteString(fmt.Sprintf("tree %s\n", tree)) + b.WriteString(fmt.Sprintf("author %s\n", author)) + b.WriteString(fmt.Sprintf("committer %s\n", committer)) + b.WriteString(sig) + b.WriteString(fmt.Sprintf("\n%s", message)) + + content := b.String() + return fmt.Sprintf("commit %d\x00%s", len(content), content) +} + +// sigFixtures defines test signatures of various types. Each entry provides a +// complete gpgsig header block and a substring that must appear in the parsed +// GpgSig value. +var sigFixtures = []struct { + name string + sig string + wantInSig string // substring expected in the parsed signature value +}{ + { + name: "PGP", + sig: "gpgsig -----BEGIN PGP SIGNATURE-----\n" + + " \n" + + " iQEzBAABCAAdFiEEtest+test+test+test+test+tE=\n" + + " =ABCD\n" + + " -----END PGP SIGNATURE-----\n", + wantInSig: "-----BEGIN PGP SIGNATURE-----", + }, + { + name: "SSH", + sig: "gpgsig -----BEGIN SSH SIGNATURE-----\n" + + " U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgtestkey1234567890\n" + + " abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOP==\n" + + " -----END SSH SIGNATURE-----\n", + wantInSig: "-----BEGIN SSH SIGNATURE-----", + }, + { + name: "X509", + sig: "gpgsig -----BEGIN SIGNED MESSAGE-----\n" + + " MIIBxjCCAWugAwIBAgIUTestCertData1234567890abcdef==\n" + + " -----END SIGNED MESSAGE-----\n", + wantInSig: "-----BEGIN SIGNED MESSAGE-----", + }, +} + +func TestSignatureRoundTrip(t *testing.T) { + for _, tt := range sigFixtures { + t.Run(tt.name, func(t *testing.T) { + raw := buildCommitRaw(tt.sig) + + nd, err := ParseObject(strings.NewReader(raw)) + if err != nil { + t.Fatalf("ParseObject failed: %v", err) + } + + var buf bytes.Buffer + if err := Encode(nd, &buf); err != nil { + t.Fatalf("Encode failed: %v", err) + } + + if buf.String() != raw { + t.Errorf("round-trip mismatch.\n--- want ---\n%q\n--- got ---\n%q", raw, buf.String()) + } + }) + } +} + +func TestSignatureParsed(t *testing.T) { + for _, tt := range sigFixtures { + t.Run(tt.name, func(t *testing.T) { + raw := buildCommitRaw(tt.sig) + + nd, err := ParseObject(strings.NewReader(raw)) + if err != nil { + t.Fatalf("ParseObject failed: %v", err) + } + + commit, ok := nd.(Commit) + if !ok { + t.Fatalf("expected Commit, got %T", nd) + } + + sigNode, err := commit.LookupByString("signature") + if err != nil { + t.Fatalf("LookupByString(signature) failed: %v", err) + } + if sigNode.IsNull() { + t.Fatal("signature is null") + } + sigStr, err := sigNode.AsString() + if err != nil { + t.Fatalf("AsString failed: %v", err) + } + if !strings.Contains(sigStr, tt.wantInSig) { + t.Errorf("signature does not contain %q; got: %q", tt.wantInSig, sigStr) + } + }) + } +} + +func TestPGPSignatureWithVersionRoundTrip(t *testing.T) { + // PGP signatures sometimes include Version/Comment headers before the + // blank separator line; this is PGP-specific but must still round-trip. + sig := "gpgsig -----BEGIN PGP SIGNATURE-----\n" + + " Version: GnuPG v1\n" + + " Comment: some comment\n" + + " \n" + + " iQEzBAABCAAdFiEEtest+test+test+test+test+tE=\n" + + " =ABCD\n" + + " -----END PGP SIGNATURE-----\n" + + raw := buildCommitRaw(sig) + + nd, err := ParseObject(strings.NewReader(raw)) + if err != nil { + t.Fatalf("ParseObject failed: %v", err) + } + + var buf bytes.Buffer + if err := Encode(nd, &buf); err != nil { + t.Fatalf("Encode failed: %v", err) + } + + if buf.String() != raw { + t.Errorf("round-trip mismatch.\n--- want ---\n%q\n--- got ---\n%q", raw, buf.String()) + } +} + +// TestNoSignatureRoundTrip ensures commits without signatures still work. +func TestNoSignatureRoundTrip(t *testing.T) { + raw := buildCommitRaw("") + + nd, err := ParseObject(strings.NewReader(raw)) + if err != nil { + t.Fatalf("ParseObject failed: %v", err) + } + + var buf bytes.Buffer + if err := Encode(nd, &buf); err != nil { + t.Fatalf("Encode failed: %v", err) + } + + if buf.String() != raw { + t.Errorf("round-trip mismatch.\n--- want ---\n%q\n--- got ---\n%q", raw, buf.String()) + } + + commit := nd.(Commit) + sigNode, err := commit.LookupByString("signature") + if err != nil { + t.Fatalf("LookupByString(signature) failed: %v", err) + } + if !sigNode.IsAbsent() { + t.Error("expected absent signature for unsigned commit") + } +}