types/commit: skip parent line for root commits in payload #300

Currently, root commits are not showing up as "verified" in the AppView. See example Repo. This is because the Payload produced is incorrect, causing the signature verification to fail. The payload is always including the parent field, even when it should not be present (such as for the root commit).

First, I confirmed the expected Payload, based on the commits to that Repo:

Good commit, verified in UI:

subzidion@mizithra ~/w/ssh-signing (mainline)> git cat-file commit 06f191fa
tree ec947e3dd7a7752d078f1ed0cfde7457b21fef58
parent 048662fa0dbc061740a324028c96773ad8f342b6
author Carl 'Subzidion' Hiltbrunner <git@carl.hiltbrunner.email> 1777700993 -0700
committer Carl 'Subzidion' Hiltbrunner <git@carl.hiltbrunner.email> 1777700993 -0700
gpgsig -----BEGIN SSH SIGNATURE-----
 U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAg0z270OwauAUjqsiJWwAgrTZlTm
 34RukB+S3sMKtZn5UAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
 AAAAQIDHf3FzPh/bOF+5wVavdcCibtG3yDJdgg4DKkmMTeFX4wXW00Zb5ld+aKtazd/0wA
 R5c06R/HsYAq4Y6VO4dgc=
 -----END SSH SIGNATURE-----

Add hello.txt.

Root commit, not verified in UI (Note, no parent field):

subzidion@mizithra ~/w/ssh-signing (mainline)> git cat-file commit 048662fa
tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
author Carl 'Subzidion' Hiltbrunner <git@carl.hiltbrunner.email> 1777700337 -0700
committer Carl 'Subzidion' Hiltbrunner <git@carl.hiltbrunner.email> 1777700337 -0700
gpgsig -----BEGIN SSH SIGNATURE-----
 U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAg0z270OwauAUjqsiJWwAgrTZlTm
 34RukB+S3sMKtZn5UAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
 AAAAQF/EZB0DWCgjUFt0IPp5QiTEAdVMjkAGTbe44gVGx4uwMyBRygQ+iofrjYZpzZhh77
 gs0Nv5fwsV2nVTThpkXQg=
 -----END SSH SIGNATURE-----

Initial commit.

Then verified a unit test for a root commit failed. Again, note the presence of the parent in the Payload output:

=== RUN   TestPayloadRootCommit
    commit_test.go:27: root commit payload must not contain a parent line, got: "parent "
        full payload:
        tree abc123
        parent
        author Alice <alice@example.com> 1735732800 +0000
        committer Alice <alice@example.com> 1735732800 +0000

Removing parent should cause the Payload to match the one in the signature and produce a Verified commit in the AppView.

+70 -1

2 changed files

Diff round #0

expand all

types

commit.go

commit_test.go

+1 -1

types/commit.go

··· 149 149 for _, p := range c.ParentHashes { 150 150 fmt.Fprintf(&payload, "parent %s\n", p.String()) 151 151 } 152 - } else { 152 + } else if c.Parent != "" { 153 153 // present for backwards compatibility 154 154 fmt.Fprintf(&payload, "parent %s\n", c.Parent) 155 155 }

+69

types/commit_test.go

··· 1 + package types 2 + 3 + import ( 4 + "strings" 5 + "testing" 6 + "time" 7 + 8 + "github.com/go-git/go-git/v5/plumbing" 9 + "github.com/go-git/go-git/v5/plumbing/object" 10 + ) 11 + 12 + func TestPayloadRootCommit(t *testing.T) { 13 + when := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) 14 + sig := object.Signature{Name: "Alice", Email: "alice@example.com", When: when} 15 + 16 + c := Commit{ 17 + Author: sig, 18 + Committer: sig, 19 + Message: "initial commit\n", 20 + Tree: "abc123", 21 + } 22 + 23 + payload := c.Payload() 24 + 25 + for _, line := range strings.Split(payload, "\n") { 26 + if strings.HasPrefix(line, "parent ") || line == "parent" { 27 + t.Errorf("root commit payload must not contain a parent line, got: %q\nfull payload:\n%s", line, payload) 28 + } 29 + } 30 + } 31 + 32 + func TestPayloadWithParentHashes(t *testing.T) { 33 + when := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) 34 + sig := object.Signature{Name: "Alice", Email: "alice@example.com", When: when} 35 + parent := plumbing.NewHash("0000000000000000000000000000000000000001") 36 + 37 + c := Commit{ 38 + Author: sig, 39 + Committer: sig, 40 + Message: "second commit\n", 41 + Tree: "abc123", 42 + ParentHashes: []plumbing.Hash{parent}, 43 + } 44 + 45 + payload := c.Payload() 46 + want := "parent " + parent.String() 47 + if !strings.Contains(payload, want) { 48 + t.Errorf("payload missing %q\nfull payload:\n%s", want, payload) 49 + } 50 + } 51 + 52 + func TestPayloadLegacyParentField(t *testing.T) { 53 + when := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) 54 + sig := object.Signature{Name: "Alice", Email: "alice@example.com", When: when} 55 + 56 + c := Commit{ 57 + Author: sig, 58 + Committer: sig, 59 + Message: "second commit\n", 60 + Tree: "abc123", 61 + Parent: "0000000000000000000000000000000000000001", 62 + } 63 + 64 + payload := c.Payload() 65 + want := "parent 0000000000000000000000000000000000000001" 66 + if !strings.Contains(payload, want) { 67 + t.Errorf("payload missing %q\nfull payload:\n%s", want, payload) 68 + } 69 + }

History

1 round 0 comments

subzidion.com submitted #0 6w

1 commit

expand

87bb31bc

types/commit: skip parent line for root commits in payload

merge conflicts detected

expand

types/commit.go:149

expand 0 comments

Configure Feed

Configure Feed

History