From c91aa32e9fe787614b74eea72a5f1e7b67a169a1 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 24 Mar 2026 13:10:06 +0100 Subject: [PATCH 1/5] feat(prinfo): expand author field to include account type Add PR info schema v1.3 where author accepts both a string (backwards compatible) and an object with login and type fields (User/Bot/unknown). GitHub populates the type from the event payload's user.type field. GitLab sets type to "unknown" since CI env vars don't expose it. This enables policies to check author type directly instead of relying on naming conventions like the [bot] suffix. Signed-off-by: Javier Rodriguez --- internal/prinfo/prinfo.go | 10 +- .../prinfo/pr-info-1.3.schema.json | 133 ++++++++++++++++++ internal/schemavalidators/schemavalidators.go | 8 +- .../crafter/materials/chainloop_pr_info.go | 2 +- .../materials/chainloop_pr_info_test.go | 42 ++++-- pkg/attestation/crafter/prmetadata.go | 26 +++- pkg/attestation/crafter/prmetadata_test.go | 6 +- 7 files changed, 201 insertions(+), 26 deletions(-) create mode 100644 internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 0c69efb30..210a6b298 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -19,9 +19,15 @@ const ( // EvidenceID is the identifier for the PR/MR info material type EvidenceID = "CHAINLOOP_PR_INFO" // EvidenceSchemaURL is the URL to the JSON schema for PR/MR info - EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json" + EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json" ) +// Author represents the author of the PR/MR with account type information +type Author struct { + Login string `json:"login" jsonschema:"required,description=Username of the PR/MR author"` + Type string `json:"type" jsonschema:"required,enum=User,enum=Bot,enum=unknown,description=Account type of the PR/MR author"` +} + // Reviewer represents a reviewer of the PR/MR type Reviewer struct { Login string `json:"login" jsonschema:"required,description=Username of the reviewer"` @@ -40,7 +46,7 @@ type Data struct { SourceBranch string `json:"source_branch" jsonschema:"description=The source branch name"` TargetBranch string `json:"target_branch" jsonschema:"description=The target branch name"` URL string `json:"url" jsonschema:"required,format=uri,description=Direct URL to the PR/MR"` - Author string `json:"author" jsonschema:"description=Username of the PR/MR author"` + Author *Author `json:"author,omitempty" jsonschema:"description=The PR/MR author"` Reviewers []Reviewer `json:"reviewers,omitempty" jsonschema:"description=List of reviewers who reviewed or were requested to review"` } diff --git a/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json new file mode 100644 index 000000000..3e185d6ee --- /dev/null +++ b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.3.schema.json @@ -0,0 +1,133 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json", + "properties": { + "platform": { + "type": "string", + "enum": [ + "github", + "gitlab" + ], + "description": "The CI/CD platform" + }, + "type": { + "type": "string", + "enum": [ + "pull_request", + "merge_request" + ], + "description": "The type of change request" + }, + "number": { + "type": "string", + "description": "The PR/MR number or identifier" + }, + "title": { + "type": "string", + "description": "The PR/MR title" + }, + "description": { + "type": "string", + "description": "The PR/MR description or body" + }, + "source_branch": { + "type": "string", + "description": "The source branch name" + }, + "target_branch": { + "type": "string", + "description": "The target branch name" + }, + "url": { + "type": "string", + "format": "uri", + "description": "Direct URL to the PR/MR" + }, + "author": { + "oneOf": [ + { + "type": "string", + "description": "Username of the PR/MR author (deprecated, use object form)" + }, + { + "type": "object", + "properties": { + "login": { + "type": "string", + "description": "Username of the PR/MR author" + }, + "type": { + "type": "string", + "enum": [ + "User", + "Bot", + "unknown" + ], + "description": "Account type of the PR/MR author" + } + }, + "required": [ + "login", + "type" + ], + "additionalProperties": false, + "description": "The PR/MR author with account type" + } + ], + "description": "The PR/MR author (string for backwards compatibility, or object with login and type)" + }, + "reviewers": { + "items": { + "properties": { + "login": { + "type": "string", + "description": "Username of the reviewer" + }, + "type": { + "type": "string", + "enum": [ + "User", + "Bot", + "unknown" + ], + "description": "Account type of the reviewer" + }, + "requested": { + "type": "boolean", + "description": "Whether the reviewer was explicitly requested to review" + }, + "review_status": { + "type": "string", + "enum": [ + "APPROVED", + "CHANGES_REQUESTED", + "COMMENTED", + "DISMISSED", + "PENDING" + ], + "description": "The reviewer's current review state if they have submitted a review" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "login", + "type", + "requested" + ] + }, + "type": "array", + "description": "List of reviewers who reviewed or were requested to review" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "platform", + "type", + "number", + "url" + ], + "title": "Pull Request / Merge Request Information", + "description": "Schema for Pull Request or Merge Request metadata collected during attestation" +} diff --git a/internal/schemavalidators/schemavalidators.go b/internal/schemavalidators/schemavalidators.go index e95473c6d..594f9d4b5 100644 --- a/internal/schemavalidators/schemavalidators.go +++ b/internal/schemavalidators/schemavalidators.go @@ -52,6 +52,8 @@ const ( PRInfoVersion1_1 PRInfoVersion = "1.1" // PRInfoVersion1_2 represents PR/MR Info version 1.2 schema (adds requested and review_status to reviewers). PRInfoVersion1_2 PRInfoVersion = "1.2" + // PRInfoVersion1_3 represents PR/MR Info version 1.3 schema (author as object with type). + PRInfoVersion1_3 PRInfoVersion = "1.3" // CycloneDXVersion1_5 represents CycloneDX version 1.5 schema. CycloneDXVersion1_5 CycloneDXVersion = "1.5" // CycloneDXVersion1_6 represents CycloneDX version 1.6 schema. @@ -100,6 +102,8 @@ var ( prInfoSpecVersion1_1 string //go:embed internal_schemas/prinfo/pr-info-1.2.schema.json prInfoSpecVersion1_2 string + //go:embed internal_schemas/prinfo/pr-info-1.3.schema.json + prInfoSpecVersion1_3 string // AI Agent Config schemas //go:embed internal_schemas/aiagentconfig/ai-agent-config-0.1.schema.json @@ -125,6 +129,7 @@ var schemaURLMapping = map[string]string{ "https://schemas.chainloop.dev/prinfo/1.0/pr-info.schema.json": prInfoSpecVersion1_0, "https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json": prInfoSpecVersion1_1, "https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json": prInfoSpecVersion1_2, + "https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json": prInfoSpecVersion1_3, "https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json": aiAgentConfigSpecVersion0_1, } @@ -155,6 +160,7 @@ func init() { compiledPRInfoSchemas[PRInfoVersion1_0] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.0/pr-info.schema.json") compiledPRInfoSchemas[PRInfoVersion1_1] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json") compiledPRInfoSchemas[PRInfoVersion1_2] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.2/pr-info.schema.json") + compiledPRInfoSchemas[PRInfoVersion1_3] = compiler.MustCompile("https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json") compiledAIAgentConfigSchemas = make(map[AIAgentConfigVersion]*jsonschema.Schema) compiledAIAgentConfigSchemas[AIAgentConfigVersion0_1] = compiler.MustCompile("https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json") @@ -252,7 +258,7 @@ func ValidateChainloopRunnerContext(data interface{}, version RunnerContextVersi // ValidatePRInfo validates the PR/MR info schema. func ValidatePRInfo(data interface{}, version PRInfoVersion) error { if version == "" { - version = PRInfoVersion1_2 + version = PRInfoVersion1_3 } schema, ok := compiledPRInfoSchemas[version] diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info.go b/pkg/attestation/crafter/materials/chainloop_pr_info.go index f13759fd0..e5a2c421d 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info.go @@ -75,7 +75,7 @@ func (i *ChainloopPRInfoCrafter) Craft(ctx context.Context, artifactPath string) } // Validate the data against JSON schema - if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_2); err != nil { + if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3); err != nil { i.logger.Debug().Err(err).Msg("schema validation failed") return nil, fmt.Errorf("PR info validation failed: %w", err) } diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go index 6daebb3cd..972528738 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go @@ -47,7 +47,7 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { SourceBranch: "feature", TargetBranch: "main", URL: "https://github.com/org/repo/pull/123", - Author: "testuser", + Author: &prinfo.Author{Login: "testuser", Type: "User"}, }), wantErr: false, }, @@ -118,7 +118,7 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { require.NoError(t, err) // Validate the data against JSON schema - err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_2) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) if tc.wantErr { require.Error(t, err) @@ -129,24 +129,36 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { } } -func TestChainloopPRInfoCrafter_V1_0BackwardCompat(t *testing.T) { - // Data without reviewers should still validate against v1.0 - data := prinfo.Data{ - Platform: "github", - Type: "pull_request", - Number: "123", - URL: "https://github.com/org/repo/pull/123", - Author: "testuser", - } +func TestChainloopPRInfoCrafter_BackwardCompat(t *testing.T) { + // Old string author format should still validate against v1.3 + oldFormatJSON := `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/org/repo/pull/123", + "author": "testuser" + }` - dataBytes, err := json.Marshal(data) + var rawData interface{} + err := json.Unmarshal([]byte(oldFormatJSON), &rawData) require.NoError(t, err) - var rawData interface{} - err = json.Unmarshal(dataBytes, &rawData) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) + require.NoError(t, err) + + // New object author format should also validate against v1.3 + newFormatJSON := `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/org/repo/pull/123", + "author": {"login": "dependabot[bot]", "type": "Bot"} + }` + + err = json.Unmarshal([]byte(newFormatJSON), &rawData) require.NoError(t, err) - err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_0) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_3) require.NoError(t, err) } diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index 72d6daef7..8dfe79527 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -39,7 +39,7 @@ type PRMetadata struct { SourceBranch string TargetBranch string URL string - Author string + Author *prinfo.Author Reviewers []prinfo.Reviewer } @@ -105,8 +105,9 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo Title string `json:"title"` Body string `json:"body"` HTMLURL string `json:"html_url"` - User struct { + User struct { Login string `json:"login"` + Type string `json:"type"` } `json:"user"` RequestedReviewers []struct { Login string `json:"login"` @@ -181,7 +182,10 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo SourceBranch: envVars["GITHUB_HEAD_REF"], TargetBranch: envVars["GITHUB_BASE_REF"], URL: event.PullRequest.HTMLURL, - Author: event.PullRequest.User.Login, + Author: &prinfo.Author{ + Login: event.PullRequest.User.Login, + Type: normalizeAuthorType(event.PullRequest.User.Type), + }, Reviewers: reviewers, } @@ -363,7 +367,10 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo SourceBranch: envVars["CI_MERGE_REQUEST_SOURCE_BRANCH_NAME"], TargetBranch: envVars["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"], URL: mrURL, - Author: envVars["GITLAB_USER_LOGIN"], + Author: &prinfo.Author{ + Login: envVars["GITLAB_USER_LOGIN"], + Type: "unknown", + }, Reviewers: reviewers, } @@ -418,3 +425,14 @@ func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, toke return reviewers } + +// normalizeAuthorType normalizes the author type string from the CI platform +// to one of the allowed values: "User", "Bot", or "unknown". +func normalizeAuthorType(authorType string) string { + switch authorType { + case "User", "Bot": + return authorType + default: + return "unknown" + } +} diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index f305267b3..2b2ef6923 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.go @@ -56,7 +56,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { assert.Equal(t, "feature-branch", metadata.SourceBranch) assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://github.com/owner/repo/pull/123", metadata.URL) - assert.Equal(t, "johndoe", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "johndoe", Type: "unknown"}, metadata.Author) // Event file provides requested_reviewers without needing a token. require.Len(t, metadata.Reviewers, 2) assert.Equal(t, "reviewer1", metadata.Reviewers[0].Login) @@ -80,7 +80,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { validate: func(t *testing.T, metadata *PRMetadata) { assert.Equal(t, "github", metadata.Platform) assert.Equal(t, "456", metadata.Number) - assert.Equal(t, "janedoe", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "janedoe", Type: "unknown"}, metadata.Author) assert.Empty(t, metadata.Reviewers) }, }, @@ -164,7 +164,7 @@ func TestExtractGitLabMRMetadata(t *testing.T) { assert.Equal(t, "feature-branch", metadata.SourceBranch) assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://gitlab.com/owner/repo/-/merge_requests/42", metadata.URL) - assert.Equal(t, "testuser", metadata.Author) + assert.Equal(t, &prinfo.Author{Login: "testuser", Type: "unknown"}, metadata.Author) // No reviewers without API access in tests assert.Empty(t, metadata.Reviewers) }, From 3f3bed04e3e953cca07b10bfb34075d18133e756 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 24 Mar 2026 13:21:41 +0100 Subject: [PATCH 2/5] fix: add custom UnmarshalJSON for backwards-compatible author field Add Data.UnmarshalJSON to handle both string (v1.0-v1.2) and object (v1.3) author formats at the Go struct level. String authors are converted to Author{Login: str, Type: "unknown"}. Also fixes gofmt formatting issues in prmetadata.go and test files. Signed-off-by: Javier Rodriguez --- internal/prinfo/prinfo.go | 43 ++++++ internal/prinfo/prinfo_test.go | 125 ++++++++++++++++++ .../materials/chainloop_pr_info_test.go | 2 +- pkg/attestation/crafter/prmetadata.go | 6 +- 4 files changed, 172 insertions(+), 4 deletions(-) diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 210a6b298..082a59ae0 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -15,6 +15,11 @@ package prinfo +import ( + "encoding/json" + "fmt" +) + const ( // EvidenceID is the identifier for the PR/MR info material type EvidenceID = "CHAINLOOP_PR_INFO" @@ -50,6 +55,44 @@ type Data struct { Reviewers []Reviewer `json:"reviewers,omitempty" jsonschema:"description=List of reviewers who reviewed or were requested to review"` } +// UnmarshalJSON implements custom JSON unmarshaling for Data to handle +// backwards compatibility where author can be either a string (v1.0-v1.2) +// or an object (v1.3+). +func (d *Data) UnmarshalJSON(b []byte) error { + // Use an alias to avoid infinite recursion + type Alias Data + aux := &struct { + Author json.RawMessage `json:"author,omitempty"` + *Alias + }{ + Alias: (*Alias)(d), + } + + if err := json.Unmarshal(b, aux); err != nil { + return err + } + + if len(aux.Author) == 0 { + return nil + } + + // Try object format first + var author Author + if err := json.Unmarshal(aux.Author, &author); err == nil { + d.Author = &author + return nil + } + + // Fall back to string format + var login string + if err := json.Unmarshal(aux.Author, &login); err == nil { + d.Author = &Author{Login: login, Type: "unknown"} + return nil + } + + return fmt.Errorf("author field must be a string or an object with login and type fields") +} + // Evidence represents the complete evidence structure for PR/MR metadata type Evidence struct { ID string `json:"chainloop.material.evidence.id"` diff --git a/internal/prinfo/prinfo_test.go b/internal/prinfo/prinfo_test.go index d3f206730..45508e380 100644 --- a/internal/prinfo/prinfo_test.go +++ b/internal/prinfo/prinfo_test.go @@ -308,3 +308,128 @@ func TestValidatePRInfoV1_0BackwardCompat(t *testing.T) { }) } } + +func TestDataUnmarshalJSON(t *testing.T) { + testCases := []struct { + name string + input string + wantAuthor *Author + wantErr bool + }{ + { + name: "string author (v1.0-v1.2 backwards compat)", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":"dependabot[bot]"}`, + wantAuthor: &Author{Login: "dependabot[bot]", Type: "unknown"}, + }, + { + name: "object author (v1.3)", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":{"login":"dependabot[bot]","type":"Bot"}}`, + wantAuthor: &Author{Login: "dependabot[bot]", Type: "Bot"}, + }, + { + name: "no author field", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1"}`, + wantAuthor: nil, + }, + { + name: "invalid author type", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":123}`, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var data Data + err := json.Unmarshal([]byte(tc.input), &data) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.wantAuthor, data.Author) + }) + } +} + +func TestValidatePRInfoV1_3(t *testing.T) { + testCases := []struct { + name string + data string + wantErr bool + }{ + { + name: "v1.3 author as object", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "dependabot[bot]", "type": "Bot"} + }`, + wantErr: false, + }, + { + name: "v1.3 author as string (backwards compat)", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": "username" + }`, + wantErr: false, + }, + { + name: "v1.3 author object missing type", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "username"} + }`, + wantErr: true, + }, + { + name: "v1.3 author object invalid type", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": {"login": "username", "type": "InvalidType"} + }`, + wantErr: true, + }, + { + name: "v1.3 with reviewers and object author", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "author": {"login": "renovate[bot]", "type": "Bot"}, + "reviewers": [ + {"login": "reviewer1", "type": "User", "requested": true, "review_status": "APPROVED"} + ] + }`, + wantErr: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var data interface{} + err := json.Unmarshal([]byte(tc.data), &data) + require.NoError(t, err) + + err = schemavalidators.ValidatePRInfo(data, schemavalidators.PRInfoVersion1_3) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go index 972528738..8e433819e 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go @@ -47,7 +47,7 @@ func TestChainloopPRInfoCrafter_Validation(t *testing.T) { SourceBranch: "feature", TargetBranch: "main", URL: "https://github.com/org/repo/pull/123", - Author: &prinfo.Author{Login: "testuser", Type: "User"}, + Author: &prinfo.Author{Login: "testuser", Type: "User"}, }), wantErr: false, }, diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index 8dfe79527..1b8b8b22c 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -105,7 +105,7 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo Title string `json:"title"` Body string `json:"body"` HTMLURL string `json:"html_url"` - User struct { + User struct { Login string `json:"login"` Type string `json:"type"` } `json:"user"` @@ -186,7 +186,7 @@ func extractGitHubPRMetadata(ctx context.Context, envVars map[string]string) (bo Login: event.PullRequest.User.Login, Type: normalizeAuthorType(event.PullRequest.User.Type), }, - Reviewers: reviewers, + Reviewers: reviewers, } return true, metadata, nil @@ -371,7 +371,7 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo Login: envVars["GITLAB_USER_LOGIN"], Type: "unknown", }, - Reviewers: reviewers, + Reviewers: reviewers, } return true, metadata, nil From cca7d20f7b18a0c3e251eb8346d9f718c5649a0e Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 24 Mar 2026 13:38:35 +0100 Subject: [PATCH 3/5] feat: detect GitLab MR author bot status via API Refactor fetchGitLabReviewers into fetchGitLabMRDetails to extract both author and reviewer information from the MR API response in a single call. Uses the GitLab `bot` boolean field to set author/reviewer type to "User" or "Bot". Falls back to GITLAB_USER_LOGIN with type "unknown" if the API call fails (best-effort). Signed-off-by: Javier Rodriguez --- pkg/attestation/crafter/prmetadata.go | 65 +++++++++++++++++----- pkg/attestation/crafter/prmetadata_test.go | 40 ++++++++----- 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index 1b8b8b22c..44a312803 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -349,14 +349,27 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo projectURL := envVars["CI_MERGE_REQUEST_PROJECT_URL"] mrURL := fmt.Sprintf("%s/-/merge_requests/%s", projectURL, mrIID) - // Fetch reviewers from GitLab API (best-effort). + // Fetch MR details (author + reviewers) from GitLab API (best-effort). // Prefer CI_MERGE_REQUEST_PROJECT_PATH for fork-based MRs where CI_PROJECT_PATH points to the fork. projectPath := envVars["CI_MERGE_REQUEST_PROJECT_PATH"] if projectPath == "" { projectPath = envVars["CI_PROJECT_PATH"] } // CI_JOB_TOKEN is read via os.Getenv to avoid persisting it in the attestation envVars map. - reviewers := fetchGitLabReviewers(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, os.Getenv("CI_JOB_TOKEN")) + mrDetails := fetchGitLabMRDetails(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, os.Getenv("CI_JOB_TOKEN")) + + // Use API author if available (includes bot detection), fall back to env var + author := &prinfo.Author{ + Login: envVars["GITLAB_USER_LOGIN"], + Type: "unknown", + } + var reviewers []prinfo.Reviewer + if mrDetails != nil { + if mrDetails.Author != nil { + author = mrDetails.Author + } + reviewers = mrDetails.Reviewers + } metadata := &PRMetadata{ Platform: "gitlab", @@ -367,19 +380,22 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo SourceBranch: envVars["CI_MERGE_REQUEST_SOURCE_BRANCH_NAME"], TargetBranch: envVars["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"], URL: mrURL, - Author: &prinfo.Author{ - Login: envVars["GITLAB_USER_LOGIN"], - Type: "unknown", - }, - Reviewers: reviewers, + Author: author, + Reviewers: reviewers, } return true, metadata, nil } -// fetchGitLabReviewers fetches MR reviewers from the GitLab API. +// gitlabMRDetails holds the author and reviewer data extracted from the GitLab MR API. +type gitlabMRDetails struct { + Author *prinfo.Author + Reviewers []prinfo.Reviewer +} + +// fetchGitLabMRDetails fetches MR details (author + reviewers) from the GitLab API. // Returns nil on any failure (best-effort). -func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, token string) []prinfo.Reviewer { +func fetchGitLabMRDetails(ctx context.Context, baseURL, projectPath, mrIID, token string) *gitlabMRDetails { if baseURL == "" || projectPath == "" || token == "" { return nil } @@ -406,24 +422,47 @@ func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, toke } var mrResponse struct { + Author struct { + Username string `json:"username"` + Bot bool `json:"bot"` + } `json:"author"` Reviewers []struct { Username string `json:"username"` + Bot bool `json:"bot"` } `json:"reviewers"` } if err := json.NewDecoder(resp.Body).Decode(&mrResponse); err != nil { return nil } - var reviewers []prinfo.Reviewer + details := &gitlabMRDetails{} + + // Extract author with bot detection + if mrResponse.Author.Username != "" { + authorType := "User" + if mrResponse.Author.Bot { + authorType = "Bot" + } + details.Author = &prinfo.Author{ + Login: mrResponse.Author.Username, + Type: authorType, + } + } + + // Extract reviewers with bot detection for _, r := range mrResponse.Reviewers { - reviewers = append(reviewers, prinfo.Reviewer{ + reviewerType := "User" + if r.Bot { + reviewerType = "Bot" + } + details.Reviewers = append(details.Reviewers, prinfo.Reviewer{ Login: r.Username, - Type: "unknown", + Type: reviewerType, Requested: true, }) } - return reviewers + return details } // normalizeAuthorType normalizes the author type string from the CI platform diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index 2b2ef6923..017996a91 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.go @@ -207,7 +207,7 @@ func TestExtractGitLabMRMetadata(t *testing.T) { } } -func TestFetchGitLabReviewers(t *testing.T) { +func TestFetchGitLabMRDetails(t *testing.T) { testCases := []struct { name string handler http.HandlerFunc @@ -215,33 +215,38 @@ func TestFetchGitLabReviewers(t *testing.T) { projectPath string mrIID string token string - expected []prinfo.Reviewer + expected *gitlabMRDetails }{ { name: "successful response with reviewers", handler: func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "test-token", r.Header.Get("JOB-TOKEN")) w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": [{"username": "alice"}, {"username": "bot-reviewer"}]}`) + fmt.Fprint(w, `{"author": {"username": "mr-author", "bot": false}, "reviewers": [{"username": "alice", "bot": false}, {"username": "bot-reviewer", "bot": true}]}`) }, projectPath: "group/project", mrIID: "10", token: "test-token", - expected: []prinfo.Reviewer{ - {Login: "alice", Type: "unknown", Requested: true}, - {Login: "bot-reviewer", Type: "unknown", Requested: true}, + expected: &gitlabMRDetails{ + Author: &prinfo.Author{Login: "mr-author", Type: "User"}, + Reviewers: []prinfo.Reviewer{ + {Login: "alice", Type: "User", Requested: true}, + {Login: "bot-reviewer", Type: "Bot", Requested: true}, + }, }, }, { name: "empty reviewers", handler: func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": []}`) + fmt.Fprint(w, `{"author": {"username": "author", "bot": false}, "reviewers": []}`) }, projectPath: "group/project", mrIID: "10", token: "test-token", - expected: nil, + expected: &gitlabMRDetails{ + Author: &prinfo.Author{Login: "author", Type: "User"}, + }, }, { name: "API returns error", @@ -286,18 +291,18 @@ func TestFetchGitLabReviewers(t *testing.T) { baseURL = serverURL } - reviewers := fetchGitLabReviewers(context.Background(), baseURL, tc.projectPath, tc.mrIID, tc.token) + details := fetchGitLabMRDetails(context.Background(), baseURL, tc.projectPath, tc.mrIID, tc.token) if tc.expected == nil { - assert.Nil(t, reviewers) + assert.Nil(t, details) } else { - assert.Equal(t, tc.expected, reviewers) + assert.Equal(t, tc.expected, details) } }) } } -func TestFetchGitLabReviewersRequestPath(t *testing.T) { +func TestFetchGitLabMRDetailsRequestPath(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/api/v4/projects/group%2Fproject/merge_requests/42", r.URL.RawPath) w.Header().Set("Content-Type", "application/json") @@ -305,14 +310,14 @@ func TestFetchGitLabReviewersRequestPath(t *testing.T) { })) defer server.Close() - fetchGitLabReviewers(context.Background(), server.URL, "group/project", "42", "token") + fetchGitLabMRDetails(context.Background(), server.URL, "group/project", "42", "token") } func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) { // Set up a mock GitLab API that returns reviewers server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"reviewers": [{"username": "alice"}, {"username": "coderabbitai"}]}`) + fmt.Fprint(w, `{"author": {"username": "author", "bot": false}, "reviewers": [{"username": "alice", "bot": false}, {"username": "coderabbitai", "bot": true}]}`) })) defer server.Close() @@ -336,10 +341,15 @@ func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) { require.True(t, isMR) require.Len(t, metadata.Reviewers, 2) assert.Equal(t, "alice", metadata.Reviewers[0].Login) - assert.Equal(t, "unknown", metadata.Reviewers[0].Type) + assert.Equal(t, "User", metadata.Reviewers[0].Type) assert.True(t, metadata.Reviewers[0].Requested) assert.Equal(t, "coderabbitai", metadata.Reviewers[1].Login) + assert.Equal(t, "Bot", metadata.Reviewers[1].Type) assert.True(t, metadata.Reviewers[1].Requested) + // Author should come from the API, not env var + require.NotNil(t, metadata.Author) + assert.Equal(t, "author", metadata.Author.Login) + assert.Equal(t, "User", metadata.Author.Type) } func TestFetchGitHubReviews(t *testing.T) { From 636e1c2fea28e37fcb1ad4611c975454fcafee18 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 24 Mar 2026 13:39:46 +0100 Subject: [PATCH 4/5] fix: handle JSON null author in custom UnmarshalJSON Address review feedback: explicitly check for JSON null in the author field to prevent unmarshaling null into an empty struct. Signed-off-by: Javier Rodriguez --- internal/prinfo/prinfo.go | 2 +- internal/prinfo/prinfo_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 082a59ae0..682be0e20 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -72,7 +72,7 @@ func (d *Data) UnmarshalJSON(b []byte) error { return err } - if len(aux.Author) == 0 { + if len(aux.Author) == 0 || string(aux.Author) == "null" { return nil } diff --git a/internal/prinfo/prinfo_test.go b/internal/prinfo/prinfo_test.go index 45508e380..098d121a8 100644 --- a/internal/prinfo/prinfo_test.go +++ b/internal/prinfo/prinfo_test.go @@ -331,6 +331,11 @@ func TestDataUnmarshalJSON(t *testing.T) { input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1"}`, wantAuthor: nil, }, + { + name: "null author field", + input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":null}`, + wantAuthor: nil, + }, { name: "invalid author type", input: `{"platform":"github","type":"pull_request","number":"1","url":"https://github.com/o/r/pull/1","author":123}`, From d7fa83eb9cdfe2c9d351f9711d1f68d0cbd19604 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 24 Mar 2026 13:51:16 +0100 Subject: [PATCH 5/5] fix: extract User/Bot/unknown author type strings as constants Address goconst lint: extract repeated "User", "Bot", "unknown" strings into AuthorTypeUser, AuthorTypeBot, AuthorTypeUnknown constants in the prinfo package. Signed-off-by: Javier Rodriguez --- internal/prinfo/prinfo.go | 9 ++++++++- pkg/attestation/crafter/prmetadata.go | 12 ++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 682be0e20..d8686f7e5 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -25,6 +25,13 @@ const ( EvidenceID = "CHAINLOOP_PR_INFO" // EvidenceSchemaURL is the URL to the JSON schema for PR/MR info EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.3/pr-info.schema.json" + + // AuthorTypeUser represents a human user account + AuthorTypeUser = "User" + // AuthorTypeBot represents a bot/service account + AuthorTypeBot = "Bot" + // AuthorTypeUnknown represents an account with unknown type + AuthorTypeUnknown = "unknown" ) // Author represents the author of the PR/MR with account type information @@ -86,7 +93,7 @@ func (d *Data) UnmarshalJSON(b []byte) error { // Fall back to string format var login string if err := json.Unmarshal(aux.Author, &login); err == nil { - d.Author = &Author{Login: login, Type: "unknown"} + d.Author = &Author{Login: login, Type: AuthorTypeUnknown} return nil } diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index 44a312803..5b62702d3 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -439,9 +439,9 @@ func fetchGitLabMRDetails(ctx context.Context, baseURL, projectPath, mrIID, toke // Extract author with bot detection if mrResponse.Author.Username != "" { - authorType := "User" + authorType := prinfo.AuthorTypeUser if mrResponse.Author.Bot { - authorType = "Bot" + authorType = prinfo.AuthorTypeBot } details.Author = &prinfo.Author{ Login: mrResponse.Author.Username, @@ -451,9 +451,9 @@ func fetchGitLabMRDetails(ctx context.Context, baseURL, projectPath, mrIID, toke // Extract reviewers with bot detection for _, r := range mrResponse.Reviewers { - reviewerType := "User" + reviewerType := prinfo.AuthorTypeUser if r.Bot { - reviewerType = "Bot" + reviewerType = prinfo.AuthorTypeBot } details.Reviewers = append(details.Reviewers, prinfo.Reviewer{ Login: r.Username, @@ -469,9 +469,9 @@ func fetchGitLabMRDetails(ctx context.Context, baseURL, projectPath, mrIID, toke // to one of the allowed values: "User", "Bot", or "unknown". func normalizeAuthorType(authorType string) string { switch authorType { - case "User", "Bot": + case prinfo.AuthorTypeUser, prinfo.AuthorTypeBot: return authorType default: - return "unknown" + return prinfo.AuthorTypeUnknown } }