commit: Fix parsing of gpgsig fields with trailing data

In order to extract commit signatures we scan the commit message for
"gpgsig" fields. Starting from any line with such a prefix we then scan
until we find a line that terminates the signature, e.g. an empty line.

The logic to do this is buggy though as we didn't include the trailing
space that separates the field name from its data. Consequentially, we
currently recognize all fields that start with "gpgsig" as signature
data, even if they have an arbitrary suffix like "-garbage". This suffix
is instead appended to the signed commit data that we extract.

Fix this bug by checking for the space. While at it, prepare the code
for SHA256 support already by reformatting it a bit. Signatures in
SHA256 repositories use the "gpgsig-sha256" field instead, which is how
this bug was indeed found.

Changelog: fixed

2 files changed, 25 insertions, 5 deletions

diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go
index 32d455ccc..5fa47f17b 100644
--- a/internal/gitaly/service/commit/commit_signatures.go
+++ b/internal/gitaly/service/commit/commit_signatures.go
@@ -16,8 +16,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/streamio"
 )
 
-var gpgSiganturePrefix = []byte("gpgsig")
-
 func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error {
 	if err := validateGetCommitSignaturesRequest(s.locator, request); err != nil {
 		return structerr.NewInvalidArgument("%w", err)
@@ -93,9 +91,16 @@ func extractSignature(reader io.Reader) ([]byte, []byte, error) {
 			return nil, nil, err
 		}
 
-		if !sawSignature && !inSignature && bytes.HasPrefix(line, gpgSiganturePrefix) {
-			sawSignature, inSignature = true, true
-			line = bytes.TrimPrefix(line, gpgSiganturePrefix)
+		if !sawSignature && !inSignature {
+			for _, signatureField := range [][]byte{[]byte("gpgsig ")} {
+				if !bytes.HasPrefix(line, signatureField) {
+					continue
+				}
+
+				sawSignature, inSignature = true, true
+				line = bytes.TrimPrefix(line, signatureField)
+				break
+			}
 		}
 
 		if inSignature && !bytes.Equal(line, lineBreak) {
diff --git a/internal/gitaly/service/commit/commit_signatures_test.go b/internal/gitaly/service/commit/commit_signatures_test.go
index b9b05a967..05305a010 100644
--- a/internal/gitaly/service/commit/commit_signatures_test.go
+++ b/internal/gitaly/service/commit/commit_signatures_test.go
@@ -239,6 +239,21 @@ func testGetCommitSignatures(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
+			desc: "garbage-signed commit",
+			setup: func(t *testing.T) setupData {
+				commitID, _ := createCommitWithSignature(t, cfg, repoPath, "gpgsig-garbage", sshSignature, "garbage-signed commit message")
+
+				return setupData{
+					request: &gitalypb.GetCommitSignaturesRequest{
+						Repository: repoProto,
+						CommitIds: []string{
+							commitID.String(),
+						},
+					},
+				}
+			},
+		},
+		{
 			desc: "signed by Gitaly",
 			setup: func(t *testing.T) setupData {
 				repo := localrepo.NewTestRepo(t, cfg, repoProto)