From a8d2715f16b68fc6265b73aa90df0cf5a055298a Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Mon, 15 Jun 2026 15:46:20 +0100 Subject: [PATCH 1/5] feat: record commit signature and PR base_ref for GitHub PR attestations Adds the GitHub producer side for the V2 pull_request attestation fields added on the server (kosli-dev/server#5892): - types.Commit gains Verified (*bool) and SignatureState (*string), mapped from the GraphQL commit signature { isValid, state }. An unsigned commit (nil signature node) leaves both nil, keeping "unsigned" distinct from a present-but-invalid signature (verified=false). - types.PREvidence gains BaseRef, mapped from the PR baseRefName, in both GraphQL paths (PREvidenceByPRNumber and PREvidenceForCommitV2). All new fields are optional/omitempty, so other providers and existing consumers are unaffected. GitLab, Bitbucket and Azure are follow-up slices (Bitbucket and Azure cannot supply per-commit signatures). Requires the server schema change to deploy first, since base_ref lands on FoundPullRequestV2 (extra="forbid"). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/github/build_pr_evidence_test.go | 75 ++++++++++++++++++++++- internal/github/github.go | 26 +++++++- internal/types/types.go | 17 ++--- 3 files changed, 106 insertions(+), 12 deletions(-) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index b15dd7f63..dcb295a69 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -39,6 +39,7 @@ func TestBuildPREvidence_RecordsAuthorNotCommitter(t *testing.T) { "", "Introduce kosli evaluate", "introduce-kosli-evaluate", + "main", []graphqlCommitNode{node}, nil, ) @@ -69,7 +70,7 @@ func TestBuildPREvidence_UsesAuthoredDate(t *testing.T) { "https://github.com/kosli-dev/cli/pull/671", "0e723254516c841126e81f76100be57258ff1386", "MERGED", "tooky", "2026-03-01T09:00:00Z", "", - "Introduce kosli evaluate", "introduce-kosli-evaluate", + "Introduce kosli evaluate", "introduce-kosli-evaluate", "main", []graphqlCommitNode{node}, nil, ) require.NoError(t, err) @@ -95,7 +96,7 @@ func TestBuildPREvidence_FallsBackToCommittedDate(t *testing.T) { "https://github.com/kosli-dev/cli/pull/671", "0e723254516c841126e81f76100be57258ff1386", "MERGED", "tooky", "2026-03-01T09:00:00Z", "", - "title", "branch", + "title", "branch", "main", []graphqlCommitNode{node}, nil, ) require.NoError(t, err) @@ -105,3 +106,73 @@ func TestBuildPREvidence_FallsBackToCommittedDate(t *testing.T) { require.Equal(t, wantCommitted.Unix(), evidence.Commits[0].Timestamp, "timestamp must fall back to the committed date when authored date is absent") } + +// TestBuildPREvidence_RecordsBaseRef verifies the PR's base (target) branch is +// captured, enabling a "merged into main" policy (server#5892). +func TestBuildPREvidence_RecordsBaseRef(t *testing.T) { + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", "tooky", "2026-03-01T09:00:00Z", "", + "title", "feature-branch", "main", + nil, nil, + ) + require.NoError(t, err) + require.Equal(t, "main", evidence.BaseRef, + "base_ref must record the PR target branch") +} + +// TestBuildPREvidence_RecordsCommitSignature verifies a verified commit +// signature is captured (server#5892, control 1.13). +func TestBuildPREvidence_RecordsCommitSignature(t *testing.T) { + node := graphqlCommitNode{} + node.Commit.Oid = "0e723254516c841126e81f76100be57258ff1386" + node.Commit.MessageHeadline = "signed work" + node.Commit.CommittedDate = "2026-03-01T12:00:00Z" + node.Commit.Author.Name = "Steve Tooke" + node.Commit.Author.Email = "tooky@kosli.com" + node.Commit.Signature = &struct { + IsValid graphql.Boolean + State graphql.String + }{IsValid: true, State: "VALID"} + + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", "tooky", "2026-03-01T09:00:00Z", "", + "title", "feature", "main", + []graphqlCommitNode{node}, nil, + ) + require.NoError(t, err) + require.Len(t, evidence.Commits, 1) + c := evidence.Commits[0] + require.NotNil(t, c.Verified, "verified must be populated for a signed commit") + require.True(t, *c.Verified, "verified must be true for a valid signature") + require.NotNil(t, c.SignatureState) + require.Equal(t, "VALID", *c.SignatureState) +} + +// TestBuildPREvidence_UnsignedCommitHasNoSignatureFields verifies an unsigned +// commit (no signature node) leaves verified/signature_state nil, so "unsigned" +// stays distinct from "present-but-invalid" (verified=false). +func TestBuildPREvidence_UnsignedCommitHasNoSignatureFields(t *testing.T) { + node := graphqlCommitNode{} + node.Commit.Oid = "0e723254516c841126e81f76100be57258ff1386" + node.Commit.MessageHeadline = "unsigned work" + node.Commit.CommittedDate = "2026-03-01T12:00:00Z" + node.Commit.Author.Name = "Steve Tooke" + node.Commit.Author.Email = "tooky@kosli.com" + // node.Commit.Signature left nil — unsigned commit + + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", "tooky", "2026-03-01T09:00:00Z", "", + "title", "feature", "main", + []graphqlCommitNode{node}, nil, + ) + require.NoError(t, err) + require.Len(t, evidence.Commits, 1) + require.Nil(t, evidence.Commits[0].Verified, "unsigned commit must leave verified nil") + require.Nil(t, evidence.Commits[0].SignatureState) +} diff --git a/internal/github/github.go b/internal/github/github.go index 973b24a41..858b3eab7 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -272,6 +272,10 @@ type graphqlCommitNode struct { Login graphql.String } } + Signature *struct { + IsValid graphql.Boolean + State graphql.String + } } } @@ -288,7 +292,7 @@ type graphqlReviewNode struct { // raw GraphQL commit/review nodes. mergeCommit must be resolved by the caller // (it differs between commit-SHA queries and PR-number queries). func buildPREvidence( - url, mergeCommit, state, author, createdAtStr, mergedAtStr, title, headRef string, + url, mergeCommit, state, author, createdAtStr, mergedAtStr, title, headRef, baseRef string, commitNodes []graphqlCommitNode, reviewNodes []graphqlReviewNode, ) (*types.PREvidence, error) { @@ -314,6 +318,7 @@ func buildPREvidence( MergedAt: mergedAt, Title: title, HeadRef: headRef, + BaseRef: baseRef, Approvers: []any{}, Commits: []types.Commit{}, } @@ -333,6 +338,17 @@ func buildPREvidence( if n.Commit.Author.User != nil { authorUsername = string(n.Commit.Author.User.Login) } + // Capture the commit signature when present. A nil signature node means + // the commit is unsigned, which must stay distinct from a present but + // invalid signature (verified=false) — so leave the fields nil (server#5892). + var verified *bool + var signatureState *string + if n.Commit.Signature != nil { + v := bool(n.Commit.Signature.IsValid) + s := string(n.Commit.Signature.State) + verified = &v + signatureState = &s + } evidence.Commits = append(evidence.Commits, types.Commit{ SHA: string(n.Commit.Oid), Message: string(n.Commit.MessageHeadline), @@ -341,6 +357,8 @@ func buildPREvidence( Timestamp: timestamp.Unix(), Branch: headRef, URL: string(n.Commit.URL), + Verified: verified, + SignatureState: signatureState, }) } @@ -377,6 +395,7 @@ func (c *GithubConfig) PREvidenceByPRNumber(prNumber int) (*types.PREvidence, er Title graphql.String State graphql.String HeadRefName graphql.String + BaseRefName graphql.String URL graphql.String CreatedAt graphql.String MergedAt graphql.String @@ -442,7 +461,7 @@ func (c *GithubConfig) PREvidenceByPRNumber(prNumber int) (*types.PREvidence, er return buildPREvidence( string(pr.URL), mergeCommit, string(pr.State), string(pr.Author.Login), - string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), + string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), string(pr.BaseRefName), pr.Commits.Nodes, pr.Reviews.Nodes, ) } @@ -469,6 +488,7 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence Title graphql.String State graphql.String HeadRefName graphql.String + BaseRefName graphql.String URL graphql.String CreatedAt graphql.String MergedAt graphql.String @@ -522,7 +542,7 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence // so the commit is by definition the merge commit. evidence, err := buildPREvidence( string(pr.URL), commit, string(pr.State), string(pr.Author.Login), - string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), + string(pr.CreatedAt), string(pr.MergedAt), string(pr.Title), string(pr.HeadRefName), string(pr.BaseRefName), pr.Commits.Nodes, pr.Reviews.Nodes, ) if err != nil { diff --git a/internal/types/types.go b/internal/types/types.go index 84d42f9f3..8e427a3ea 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -10,6 +10,7 @@ type PREvidence struct { MergedAt int64 `json:"merged_at,omitempty"` Title string `json:"title,omitempty"` HeadRef string `json:"head_ref,omitempty"` + BaseRef string `json:"base_ref,omitempty"` Commits []Commit `json:"commits,omitempty"` } @@ -20,13 +21,15 @@ type PRApprovals struct { } type Commit struct { - SHA string `json:"sha1"` - Message string `json:"message"` - Author string `json:"author"` - AuthorUsername string `json:"author_username,omitempty"` - Timestamp int64 `json:"timestamp"` - Branch string `json:"branch"` - URL string `json:"url,omitempty"` + SHA string `json:"sha1"` + Message string `json:"message"` + Author string `json:"author"` + AuthorUsername string `json:"author_username,omitempty"` + Timestamp int64 `json:"timestamp"` + Branch string `json:"branch"` + URL string `json:"url,omitempty"` + Verified *bool `json:"verified,omitempty"` + SignatureState *string `json:"signature_state,omitempty"` } type PRRetriever interface { From 4975af4c2ab3e0604966d69406ebe3cb105474d3 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Mon, 15 Jun 2026 15:54:28 +0100 Subject: [PATCH 2/5] feat: record commit signature and MR base_ref for GitLab PR attestations base_ref is mapped from the MR TargetBranch. Per-commit signatures are fetched via GetGPGSignature; GitLab returns 404 for unsigned commits, which is treated as unsigned (verified left nil) rather than failing the attestation, while other errors propagate. The pure gitlabCommitVerification mapper (verified only when status=="verified") is unit-tested. Part of kosli-dev/server#5892. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/gitlab/commit_mapping_test.go | 21 +++++++++++++++++++ internal/gitlab/gitlab.go | 29 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/internal/gitlab/commit_mapping_test.go b/internal/gitlab/commit_mapping_test.go index 15554426a..a3376c77c 100644 --- a/internal/gitlab/commit_mapping_test.go +++ b/internal/gitlab/commit_mapping_test.go @@ -70,3 +70,24 @@ func TestCommitFromGitlabCommit_FallsBackToCreatedAt(t *testing.T) { require.Equal(t, int64(1772635812), c.Timestamp, "timestamp must fall back to created_at when authored date is absent") } + +// TestGitlabCommitVerification maps GitLab's verification_status to the neutral +// verified/signature_state fields (server#5892). verified is true only for a +// cryptographically valid signature; a non-"verified" status records +// verified=false (distinct from unsigned), and an empty status leaves both nil. +func TestGitlabCommitVerification(t *testing.T) { + verified, state := gitlabCommitVerification("verified") + require.NotNil(t, verified) + require.True(t, *verified) + require.NotNil(t, state) + require.Equal(t, "verified", *state) + + verified, state = gitlabCommitVerification("unverified") + require.NotNil(t, verified) + require.False(t, *verified, "a non-'verified' status must record verified=false, not nil") + require.Equal(t, "unverified", *state) + + verified, state = gitlabCommitVerification("") + require.Nil(t, verified, "empty status (unsigned) must leave verified nil") + require.Nil(t, state) +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index bb901ac67..b06f58ffe 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -2,6 +2,7 @@ package gitlab import ( "fmt" + "net/http" "github.com/kosli-dev/cli/internal/types" gitlab "gitlab.com/gitlab-org/api/client-go" @@ -107,6 +108,7 @@ func (c *GitlabConfig) newPRGitlabEvidenceV2(mr *gitlab.BasicMergeRequest) (*typ MergedAt: mr.MergedAt.Unix(), Title: mr.Title, HeadRef: mr.SourceBranch, + BaseRef: mr.TargetBranch, } approvers, err := c.GetMergeRequestApprovers(mr.IID, 2) if err != nil { @@ -174,7 +176,20 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t return commits, err } for _, commit := range glCommits { - commits = append(commits, commitFromGitlabCommit(commit, mr.SourceBranch)) + mappedCommit := commitFromGitlabCommit(commit, mr.SourceBranch) + // Capture the commit signature. GitLab returns 404 for unsigned commits, + // which must be treated as "unsigned" (fields left nil), not an error — + // otherwise a single unsigned commit would fail the whole attestation + // (server#5892). + sig, resp, sigErr := client.Commits.GetGPGSignature(c.ProjectID(), commit.ID) + if sigErr != nil { + if resp == nil || resp.StatusCode != http.StatusNotFound { + return commits, sigErr + } + } else { + mappedCommit.Verified, mappedCommit.SignatureState = gitlabCommitVerification(sig.VerificationStatus) + } + commits = append(commits, mappedCommit) } return commits, nil } @@ -196,3 +211,15 @@ func commitFromGitlabCommit(commit *gitlab.Commit, branch string) types.Commit { URL: commit.WebURL, } } + +// gitlabCommitVerification maps a GitLab signature verification_status to the +// neutral verified/signature_state fields (server#5892). The status is +// "verified" only when the signature is cryptographically valid; an empty +// status leaves both nil (unsigned commits 404 and are handled by the caller). +func gitlabCommitVerification(status string) (*bool, *string) { + if status == "" { + return nil, nil + } + verified := status == "verified" + return &verified, &status +} From 4fbef988a011d84179be9171e5409830e697570e Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Mon, 15 Jun 2026 15:54:28 +0100 Subject: [PATCH 3/5] feat: record PR base_ref for Bitbucket PR attestations base_ref is mapped from destination.branch.name. Bitbucket Cloud does not expose per-commit signature verification, so verified/signature_state are omitted. Part of kosli-dev/server#5892. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/bitbucket/bitbucket.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index e535d0010..47b05d273 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -183,6 +183,7 @@ func (c *Config) getPullRequestDetailsFromBitbucket(prApiUrl, prHtmlLink, commit evidence.MergedAt = mergedAt evidence.Title = responseData["title"].(string) evidence.HeadRef = responseData["source"].(map[string]any)["branch"].(map[string]any)["name"].(string) + evidence.BaseRef = responseData["destination"].(map[string]any)["branch"].(map[string]any)["name"].(string) prCommits, err := c.getPullRequestCommitsFromBitbucket(int(responseData["id"].(float64))) if err != nil { From b41ef72b9765766792041a9a7a98360b7e2546c6 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Mon, 15 Jun 2026 15:54:28 +0100 Subject: [PATCH 4/5] feat: record PR base_ref for Azure DevOps PR attestations base_ref is mapped from TargetRefName, kept raw (refs/heads/...) to match the existing head_ref. Azure DevOps does not expose per-commit signature verification, so verified/signature_state are omitted. Part of kosli-dev/server#5892. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/azure/azure.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/azure/azure.go b/internal/azure/azure.go index b83cf56ab..803f14b80 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -131,6 +131,7 @@ func (c *AzureConfig) newPRAzureEvidenceV2(pr git.GitPullRequest) (*types.PREvid CreatedAt: pr.CreationDate.Time.Unix(), Title: *pr.Title, HeadRef: *pr.SourceRefName, + BaseRef: *pr.TargetRefName, } if pr.Status != nil && pr.ClosedDate != nil && *pr.Status == git.PullRequestStatusValues.Completed { evidence.MergedAt = pr.ClosedDate.Time.Unix() From 95290386856600e566902f24fd472627eeb541aa Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Mon, 15 Jun 2026 16:15:49 +0100 Subject: [PATCH 5/5] test: cover PR signature resolution and base_ref mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract small pure seams from the provider PR-evidence builders so the new server#5892 mappings are unit-testable, and add the tests. No behaviour change. - GitLab: resolveGitlabSignature isolates the GetGPGSignature result handling — 404 -> unsigned (nil), other errors propagate, otherwise map verification_status. Covered for all four outcomes. gitlabMRRefs maps the head/base branch names. - Bitbucket: bitbucketBranchName extracts the source/destination branch name (used for both head_ref and base_ref); covered. - Azure: azurePRRefs maps the raw head/base ref names; covered. Part of kosli-dev/server#5892. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/azure/azure.go | 9 +++-- internal/azure/pr_refs_test.go | 19 ++++++++++ internal/bitbucket/bitbucket.go | 10 ++++-- internal/bitbucket/branch_mapping_test.go | 22 ++++++++++++ internal/gitlab/commit_mapping_test.go | 44 +++++++++++++++++++++++ internal/gitlab/gitlab.go | 43 +++++++++++++++------- 6 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 internal/azure/pr_refs_test.go create mode 100644 internal/bitbucket/branch_mapping_test.go diff --git a/internal/azure/azure.go b/internal/azure/azure.go index 803f14b80..3c81f5fdf 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -130,9 +130,8 @@ func (c *AzureConfig) newPRAzureEvidenceV2(pr git.GitPullRequest) (*types.PREvid Author: fmt.Sprintf("%s (%s)", *pr.CreatedBy.DisplayName, *pr.CreatedBy.UniqueName), CreatedAt: pr.CreationDate.Time.Unix(), Title: *pr.Title, - HeadRef: *pr.SourceRefName, - BaseRef: *pr.TargetRefName, } + evidence.HeadRef, evidence.BaseRef = azurePRRefs(pr) if pr.Status != nil && pr.ClosedDate != nil && *pr.Status == git.PullRequestStatusValues.Completed { evidence.MergedAt = pr.ClosedDate.Time.Unix() } @@ -148,6 +147,12 @@ func (c *AzureConfig) newPRAzureEvidenceV2(pr git.GitPullRequest) (*types.PREvid return evidence, nil } +// azurePRRefs returns the head (source) and base (target) ref names of an Azure +// DevOps pull request, kept raw (e.g. "refs/heads/main") to match head_ref. +func azurePRRefs(pr git.GitPullRequest) (head, base string) { + return *pr.SourceRefName, *pr.TargetRefName +} + // GetPullRequestCommits returns a list of commits for a given pull request func (c *AzureConfig) GetPullRequestCommits(pr git.GitPullRequest) ([]types.Commit, error) { commits := []types.Commit{} diff --git a/internal/azure/pr_refs_test.go b/internal/azure/pr_refs_test.go new file mode 100644 index 000000000..30e903a1a --- /dev/null +++ b/internal/azure/pr_refs_test.go @@ -0,0 +1,19 @@ +package azure + +import ( + "testing" + + "github.com/microsoft/azure-devops-go-api/azuredevops/git" + "github.com/stretchr/testify/require" +) + +// TestAzurePRRefs verifies head (source) and base (target) ref extraction. The +// refs are kept raw with the refs/heads/ prefix to match head_ref (server#5892). +func TestAzurePRRefs(t *testing.T) { + head, base := azurePRRefs(git.GitPullRequest{ + SourceRefName: azStr("refs/heads/feature-branch"), + TargetRefName: azStr("refs/heads/main"), + }) + require.Equal(t, "refs/heads/feature-branch", head) + require.Equal(t, "refs/heads/main", base) +} diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index 47b05d273..dead8d82e 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -182,8 +182,8 @@ func (c *Config) getPullRequestDetailsFromBitbucket(prApiUrl, prHtmlLink, commit } evidence.MergedAt = mergedAt evidence.Title = responseData["title"].(string) - evidence.HeadRef = responseData["source"].(map[string]any)["branch"].(map[string]any)["name"].(string) - evidence.BaseRef = responseData["destination"].(map[string]any)["branch"].(map[string]any)["name"].(string) + evidence.HeadRef = bitbucketBranchName(responseData, "source") + evidence.BaseRef = bitbucketBranchName(responseData, "destination") prCommits, err := c.getPullRequestCommitsFromBitbucket(int(responseData["id"].(float64))) if err != nil { @@ -197,6 +197,12 @@ func (c *Config) getPullRequestDetailsFromBitbucket(prApiUrl, prHtmlLink, commit return evidence, nil } +// bitbucketBranchName extracts a branch name from a Bitbucket PR response for +// the given side: "source" for the head branch, "destination" for the base. +func bitbucketBranchName(prData map[string]any, side string) string { + return prData[side].(map[string]any)["branch"].(map[string]any)["name"].(string) +} + // getPullRequestCommitsFromBitbucket gets the commits of a pull request from the Bitbucket API func (c *Config) getPullRequestCommitsFromBitbucket(prID int) ([]types.Commit, error) { url, err := url.JoinPath("https://api.bitbucket.org/2.0/repositories", c.Workspace, c.Repository, "pullrequests", strconv.Itoa(prID), "commits") diff --git a/internal/bitbucket/branch_mapping_test.go b/internal/bitbucket/branch_mapping_test.go new file mode 100644 index 000000000..c12a1615e --- /dev/null +++ b/internal/bitbucket/branch_mapping_test.go @@ -0,0 +1,22 @@ +package bitbucket + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestBitbucketBranchName verifies head (source) and base (destination) branch +// extraction from a Bitbucket PR response (server#5892). +func TestBitbucketBranchName(t *testing.T) { + prData := map[string]any{ + "source": map[string]any{ + "branch": map[string]any{"name": "feature-branch"}, + }, + "destination": map[string]any{ + "branch": map[string]any{"name": "main"}, + }, + } + require.Equal(t, "feature-branch", bitbucketBranchName(prData, "source")) + require.Equal(t, "main", bitbucketBranchName(prData, "destination")) +} diff --git a/internal/gitlab/commit_mapping_test.go b/internal/gitlab/commit_mapping_test.go index a3376c77c..a5fdf9417 100644 --- a/internal/gitlab/commit_mapping_test.go +++ b/internal/gitlab/commit_mapping_test.go @@ -1,6 +1,8 @@ package gitlab import ( + "errors" + "net/http" "testing" "time" @@ -91,3 +93,45 @@ func TestGitlabCommitVerification(t *testing.T) { require.Nil(t, verified, "empty status (unsigned) must leave verified nil") require.Nil(t, state) } + +// TestResolveGitlabSignature covers the unsigned (404), fatal-error, and +// verified/unverified outcomes of a GetGPGSignature call (server#5892). +func TestResolveGitlabSignature(t *testing.T) { + notFound := &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}} + verified, state, err := resolveGitlabSignature(nil, notFound, errors.New("404")) + require.NoError(t, err, "404 (unsigned commit) must not be a fatal error") + require.Nil(t, verified, "unsigned commit must leave verified nil") + require.Nil(t, state) + + serverErr := &gitlab.Response{Response: &http.Response{StatusCode: http.StatusInternalServerError}} + _, _, err = resolveGitlabSignature(nil, serverErr, errors.New("boom")) + require.Error(t, err, "a non-404 error must propagate") + + _, _, err = resolveGitlabSignature(nil, nil, errors.New("transport failure")) + require.Error(t, err, "an error with no response must propagate") + + verified, state, err = resolveGitlabSignature( + &gitlab.GPGSignature{VerificationStatus: "verified"}, &gitlab.Response{}, nil, + ) + require.NoError(t, err) + require.NotNil(t, verified) + require.True(t, *verified) + require.Equal(t, "verified", *state) + + verified, _, err = resolveGitlabSignature( + &gitlab.GPGSignature{VerificationStatus: "unverified"}, &gitlab.Response{}, nil, + ) + require.NoError(t, err) + require.NotNil(t, verified) + require.False(t, *verified, "a non-'verified' status must record verified=false") +} + +// TestGitlabMRRefs verifies the head (source) / base (target) branch mapping. +func TestGitlabMRRefs(t *testing.T) { + head, base := gitlabMRRefs(&gitlab.BasicMergeRequest{ + SourceBranch: "feature-branch", + TargetBranch: "main", + }) + require.Equal(t, "feature-branch", head) + require.Equal(t, "main", base) +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index b06f58ffe..fa0d7a07f 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -107,9 +107,8 @@ func (c *GitlabConfig) newPRGitlabEvidenceV2(mr *gitlab.BasicMergeRequest) (*typ CreatedAt: mr.CreatedAt.Unix(), MergedAt: mr.MergedAt.Unix(), Title: mr.Title, - HeadRef: mr.SourceBranch, - BaseRef: mr.TargetBranch, } + evidence.HeadRef, evidence.BaseRef = gitlabMRRefs(mr) approvers, err := c.GetMergeRequestApprovers(mr.IID, 2) if err != nil { return evidence, err @@ -177,18 +176,14 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t } for _, commit := range glCommits { mappedCommit := commitFromGitlabCommit(commit, mr.SourceBranch) - // Capture the commit signature. GitLab returns 404 for unsigned commits, - // which must be treated as "unsigned" (fields left nil), not an error — - // otherwise a single unsigned commit would fail the whole attestation - // (server#5892). - sig, resp, sigErr := client.Commits.GetGPGSignature(c.ProjectID(), commit.ID) - if sigErr != nil { - if resp == nil || resp.StatusCode != http.StatusNotFound { - return commits, sigErr - } - } else { - mappedCommit.Verified, mappedCommit.SignatureState = gitlabCommitVerification(sig.VerificationStatus) + verified, signatureState, err := resolveGitlabSignature( + client.Commits.GetGPGSignature(c.ProjectID(), commit.ID), + ) + if err != nil { + return commits, err } + mappedCommit.Verified = verified + mappedCommit.SignatureState = signatureState commits = append(commits, mappedCommit) } return commits, nil @@ -223,3 +218,25 @@ func gitlabCommitVerification(status string) (*bool, *string) { verified := status == "verified" return &verified, &status } + +// resolveGitlabSignature maps the result of a GetGPGSignature call to the +// neutral verified/signature_state fields. GitLab returns 404 for unsigned +// commits, which is treated as unsigned (nil fields) rather than a fatal error +// (server#5892); other errors propagate so incomplete signature data is never +// silently recorded. +func resolveGitlabSignature(sig *gitlab.GPGSignature, resp *gitlab.Response, err error) (*bool, *string, error) { + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return nil, nil, nil + } + return nil, nil, err + } + verified, signatureState := gitlabCommitVerification(sig.VerificationStatus) + return verified, signatureState, nil +} + +// gitlabMRRefs returns the head (source) and base (target) branch names of a +// merge request. +func gitlabMRRefs(mr *gitlab.BasicMergeRequest) (head, base string) { + return mr.SourceBranch, mr.TargetBranch +}