diff --git a/internal/azure/azure.go b/internal/azure/azure.go index b83cf56ab..3c81f5fdf 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -130,8 +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, } + evidence.HeadRef, evidence.BaseRef = azurePRRefs(pr) if pr.Status != nil && pr.ClosedDate != nil && *pr.Status == git.PullRequestStatusValues.Completed { evidence.MergedAt = pr.ClosedDate.Time.Unix() } @@ -147,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 e535d0010..dead8d82e 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -182,7 +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.HeadRef = bitbucketBranchName(responseData, "source") + evidence.BaseRef = bitbucketBranchName(responseData, "destination") prCommits, err := c.getPullRequestCommitsFromBitbucket(int(responseData["id"].(float64))) if err != nil { @@ -196,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/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/gitlab/commit_mapping_test.go b/internal/gitlab/commit_mapping_test.go index 15554426a..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" @@ -70,3 +72,66 @@ 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) +} + +// 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 bb901ac67..fa0d7a07f 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" @@ -106,8 +107,8 @@ func (c *GitlabConfig) newPRGitlabEvidenceV2(mr *gitlab.BasicMergeRequest) (*typ CreatedAt: mr.CreatedAt.Unix(), MergedAt: mr.MergedAt.Unix(), Title: mr.Title, - HeadRef: mr.SourceBranch, } + evidence.HeadRef, evidence.BaseRef = gitlabMRRefs(mr) approvers, err := c.GetMergeRequestApprovers(mr.IID, 2) if err != nil { return evidence, err @@ -174,7 +175,16 @@ 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) + 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 } @@ -196,3 +206,37 @@ 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 +} + +// 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 +} 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 {