diff --git a/internal/prinfo/prinfo.go b/internal/prinfo/prinfo.go index 64bf29142..a1f79d7e5 100644 --- a/internal/prinfo/prinfo.go +++ b/internal/prinfo/prinfo.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,20 +19,27 @@ 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.0/pr-info.schema.json" + EvidenceSchemaURL = "https://schemas.chainloop.dev/prinfo/1.1/pr-info.schema.json" ) +// Reviewer represents a reviewer of the PR/MR +type Reviewer struct { + Login string `json:"login" jsonschema:"required,description=Username of the reviewer"` + Type string `json:"type" jsonschema:"required,enum=User,enum=Bot,enum=unknown,description=Account type of the reviewer"` +} + // Data represents the data payload of the PR/MR info evidence type Data struct { - Platform string `json:"platform" jsonschema:"required,enum=github,enum=gitlab,description=The CI/CD platform"` - Type string `json:"type" jsonschema:"required,enum=pull_request,enum=merge_request,description=The type of change request"` - Number string `json:"number" jsonschema:"required,description=The PR/MR number or identifier"` - Title string `json:"title" jsonschema:"description=The PR/MR title"` - Description string `json:"description" jsonschema:"description=The PR/MR description or body"` - 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"` + Platform string `json:"platform" jsonschema:"required,enum=github,enum=gitlab,description=The CI/CD platform"` + Type string `json:"type" jsonschema:"required,enum=pull_request,enum=merge_request,description=The type of change request"` + Number string `json:"number" jsonschema:"required,description=The PR/MR number or identifier"` + Title string `json:"title" jsonschema:"description=The PR/MR title"` + Description string `json:"description" jsonschema:"description=The PR/MR description or body"` + 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"` + Reviewers []Reviewer `json:"reviewers,omitempty" jsonschema:"description=List of reviewers who reviewed or were requested to review"` } // Evidence represents the complete evidence structure for PR/MR metadata diff --git a/internal/prinfo/prinfo_test.go b/internal/prinfo/prinfo_test.go index e0a309ebb..cfdf1e78b 100644 --- a/internal/prinfo/prinfo_test.go +++ b/internal/prinfo/prinfo_test.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -105,6 +105,57 @@ func TestValidatePRInfo(t *testing.T) { }`, wantErr: true, }, + { + name: "valid PR with reviewers", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "User"}, + {"login": "coderabbitai", "type": "Bot"} + ] + }`, + wantErr: false, + }, + { + name: "valid PR with empty reviewers", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [] + }`, + wantErr: false, + }, + { + name: "invalid reviewer missing login", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"type": "User"} + ] + }`, + wantErr: true, + }, + { + name: "invalid reviewer type", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "789", + "url": "https://github.com/owner/repo/pull/789", + "reviewers": [ + {"login": "reviewer1", "type": "InvalidType"} + ] + }`, + wantErr: true, + }, { name: "invalid JSON", data: `{invalid json}`, @@ -124,6 +175,52 @@ func TestValidatePRInfo(t *testing.T) { require.NoError(t, err) } + err = schemavalidators.ValidatePRInfo(data, schemavalidators.PRInfoVersion1_1) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidatePRInfoV1_0BackwardCompat(t *testing.T) { + testCases := []struct { + name string + data string + wantErr bool + }{ + { + name: "v1.0 valid PR without reviewers", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "author": "username" + }`, + wantErr: false, + }, + { + name: "v1.0 rejects reviewers field", + data: `{ + "platform": "github", + "type": "pull_request", + "number": "123", + "url": "https://github.com/owner/repo/pull/123", + "reviewers": [{"login": "reviewer1", "type": "User"}] + }`, + wantErr: true, + }, + } + + 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_0) if tc.wantErr { assert.Error(t, err) diff --git a/internal/prinfo/schemas/generate.go b/internal/prinfo/schemas/generate.go index e67b557d0..dfb21d03c 100644 --- a/internal/prinfo/schemas/generate.go +++ b/internal/prinfo/schemas/generate.go @@ -23,13 +23,13 @@ import ( "github.com/chainloop-dev/chainloop/internal/prinfo" ) -//go:generate go run ./generate.go --output-dir ../../../internal/schemavalidators/internal_schemas/prinfo --version 1.0 +//go:generate go run ./generate.go --output-dir ../../../internal/schemavalidators/internal_schemas/prinfo --version 1.1 func main() { var outputDir string var version string flag.StringVar(&outputDir, "output-dir", "../../../internal/schemavalidators/internal_schemas/prinfo", "Directory to output the schema files") - flag.StringVar(&version, "version", "1.0", "Schema version") + flag.StringVar(&version, "version", "1.1", "Schema version") flag.Parse() generator := prinfo.NewGenerator() diff --git a/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.1.schema.json b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.1.schema.json new file mode 100644 index 000000000..c7fed5d42 --- /dev/null +++ b/internal/schemavalidators/internal_schemas/prinfo/pr-info-1.1.schema.json @@ -0,0 +1,88 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://schemas.chainloop.dev/prinfo/1.1/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": { + "type": "string", + "description": "Username of the PR/MR author" + }, + "reviewers": { + "items": { + "properties": { + "login": { + "type": "string", + "description": "Username of the reviewer" + }, + "type": { + "type": "string", + "enum": [ + "User", + "Bot", + "unknown" + ], + "description": "Account type of the reviewer" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "login", + "type" + ] + }, + "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" +} \ No newline at end of file diff --git a/internal/schemavalidators/schemavalidators.go b/internal/schemavalidators/schemavalidators.go index d500d7711..1356f6da7 100644 --- a/internal/schemavalidators/schemavalidators.go +++ b/internal/schemavalidators/schemavalidators.go @@ -48,6 +48,8 @@ const ( RunnerContextVersion0_1 RunnerContextVersion = "0.1" // PRInfoVersion1_0 represents PR/MR Info version 1.0 schema. PRInfoVersion1_0 PRInfoVersion = "1.0" + // PRInfoVersion1_1 represents PR/MR Info version 1.1 schema (adds reviewers). + PRInfoVersion1_1 PRInfoVersion = "1.1" // CycloneDXVersion1_5 represents CycloneDX version 1.5 schema. CycloneDXVersion1_5 CycloneDXVersion = "1.5" // CycloneDXVersion1_6 represents CycloneDX version 1.6 schema. @@ -92,6 +94,8 @@ var ( // PR/MR Info schemas //go:embed internal_schemas/prinfo/pr-info-1.0.schema.json prInfoSpecVersion1_0 string + //go:embed internal_schemas/prinfo/pr-info-1.1.schema.json + prInfoSpecVersion1_1 string // AI Agent Config schemas //go:embed internal_schemas/aiagentconfig/ai-agent-config-0.1.schema.json @@ -115,6 +119,7 @@ var schemaURLMapping = map[string]string{ "https://www.first.org/cvss/cvss-v4.0.json": cvssSpecVersion4_0, "https://chainloop.dev/schemas/runner-context-response-0.1.schema.json": runnerContextSpecVersion0_1, "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/aiagentconfig/0.1/ai-agent-config.schema.json": aiAgentConfigSpecVersion0_1, } @@ -143,6 +148,7 @@ func init() { compiledPRInfoSchemas = make(map[PRInfoVersion]*jsonschema.Schema) 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") compiledAIAgentConfigSchemas = make(map[AIAgentConfigVersion]*jsonschema.Schema) compiledAIAgentConfigSchemas[AIAgentConfigVersion0_1] = compiler.MustCompile("https://schemas.chainloop.dev/aiagentconfig/0.1/ai-agent-config.schema.json") @@ -240,7 +246,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_0 + version = PRInfoVersion1_1 } schema, ok := compiledPRInfoSchemas[version] diff --git a/pkg/attestation/crafter/collector_prmetadata.go b/pkg/attestation/crafter/collector_prmetadata.go index 2d75cc8d5..8426a2479 100644 --- a/pkg/attestation/crafter/collector_prmetadata.go +++ b/pkg/attestation/crafter/collector_prmetadata.go @@ -39,7 +39,7 @@ func NewPRMetadataCollector(runner SupportedRunner) *PRMetadataCollector { func (c *PRMetadataCollector) ID() string { return "pr-metadata" } func (c *PRMetadataCollector) Collect(ctx context.Context, cr *Crafter, attestationID string, casBackend *casclient.CASBackend) error { - isPR, metadata, err := DetectPRContext(c.runner) + isPR, metadata, err := DetectPRContext(ctx, c.runner) if err != nil { return fmt.Errorf("detecting PR/MR context: %w", err) } @@ -61,6 +61,7 @@ func (c *PRMetadataCollector) Collect(ctx context.Context, cr *Crafter, attestat TargetBranch: metadata.TargetBranch, URL: metadata.URL, Author: metadata.Author, + Reviewers: metadata.Reviewers, }) jsonData, err := json.Marshal(evidenceData) diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info.go b/pkg/attestation/crafter/materials/chainloop_pr_info.go index 8abf702ce..552a2b124 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_0); err != nil { + if err := schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_1); 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 94e710bb1..339b4498d 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info_test.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info_test.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -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_0) + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_1) if tc.wantErr { require.Error(t, err) @@ -129,6 +129,27 @@ 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", + } + + dataBytes, err := json.Marshal(data) + require.NoError(t, err) + + var rawData interface{} + err = json.Unmarshal(dataBytes, &rawData) + require.NoError(t, err) + + err = schemavalidators.ValidatePRInfo(rawData, schemavalidators.PRInfoVersion1_0) + require.NoError(t, err) +} + func TestChainloopPRInfoCrafter_InvalidJSON(t *testing.T) { tmpDir := t.TempDir() tmpFile := filepath.Join(tmpDir, "invalid.json") diff --git a/pkg/attestation/crafter/prmetadata.go b/pkg/attestation/crafter/prmetadata.go index d8766723e..dec0e1f8f 100644 --- a/pkg/attestation/crafter/prmetadata.go +++ b/pkg/attestation/crafter/prmetadata.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,11 +16,16 @@ package crafter import ( + "context" "encoding/json" "fmt" + "net/http" + "net/url" "os" + "time" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/chainloop-dev/chainloop/internal/prinfo" ) // PRMetadata holds extracted PR/MR information @@ -34,10 +39,11 @@ type PRMetadata struct { TargetBranch string URL string Author string + Reviewers []prinfo.Reviewer } // DetectPRContext checks if we're in a PR/MR context and extracts metadata -func DetectPRContext(runner SupportedRunner) (bool, *PRMetadata, error) { +func DetectPRContext(ctx context.Context, runner SupportedRunner) (bool, *PRMetadata, error) { if runner == nil { return false, nil, fmt.Errorf("runner is nil") } @@ -55,7 +61,7 @@ func DetectPRContext(runner SupportedRunner) (bool, *PRMetadata, error) { case schemaapi.CraftingSchema_Runner_GITHUB_ACTION: return extractGitHubPRMetadata(envVars) case schemaapi.CraftingSchema_Runner_GITLAB_PIPELINE: - return extractGitLabMRMetadata(envVars) + return extractGitLabMRMetadata(ctx, envVars) case schemaapi.CraftingSchema_Runner_DAGGER_PIPELINE: // When running in Dagger, check for parent CI context passed through as env vars // Try Github first @@ -64,7 +70,7 @@ func DetectPRContext(runner SupportedRunner) (bool, *PRMetadata, error) { } // Then try Gitlab if envVars["CI_PIPELINE_SOURCE"] != "" { - return extractGitLabMRMetadata(envVars) + return extractGitLabMRMetadata(ctx, envVars) } return false, nil, nil default: @@ -101,6 +107,10 @@ func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, erro User struct { Login string `json:"login"` } `json:"user"` + RequestedReviewers []struct { + Login string `json:"login"` + Type string `json:"type"` + } `json:"requested_reviewers"` } `json:"pull_request"` } @@ -108,6 +118,18 @@ func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, erro return false, nil, fmt.Errorf("failed to parse event JSON: %w", err) } + var reviewers []prinfo.Reviewer + for _, r := range event.PullRequest.RequestedReviewers { + reviewerType := r.Type + if reviewerType == "" { + reviewerType = "unknown" + } + reviewers = append(reviewers, prinfo.Reviewer{ + Login: r.Login, + Type: reviewerType, + }) + } + metadata := &PRMetadata{ Platform: "github", Type: "pull_request", @@ -118,13 +140,14 @@ func extractGitHubPRMetadata(envVars map[string]string) (bool, *PRMetadata, erro TargetBranch: envVars["GITHUB_BASE_REF"], URL: event.PullRequest.HTMLURL, Author: event.PullRequest.User.Login, + Reviewers: reviewers, } return true, metadata, nil } // extractGitLabMRMetadata extracts from GitLab environment variables -func extractGitLabMRMetadata(envVars map[string]string) (bool, *PRMetadata, error) { +func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bool, *PRMetadata, error) { pipelineSource := envVars["CI_PIPELINE_SOURCE"] // Check if this is a merge request event if pipelineSource != "merge_request_event" { @@ -140,6 +163,15 @@ func extractGitLabMRMetadata(envVars map[string]string) (bool, *PRMetadata, erro projectURL := envVars["CI_MERGE_REQUEST_PROJECT_URL"] mrURL := fmt.Sprintf("%s/-/merge_requests/%s", projectURL, mrIID) + // Fetch 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")) + metadata := &PRMetadata{ Platform: "gitlab", Type: "merge_request", @@ -150,7 +182,56 @@ func extractGitLabMRMetadata(envVars map[string]string) (bool, *PRMetadata, erro TargetBranch: envVars["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"], URL: mrURL, Author: envVars["GITLAB_USER_LOGIN"], + Reviewers: reviewers, } return true, metadata, nil } + +// fetchGitLabReviewers fetches MR reviewers from the GitLab API. +// Returns nil on any failure (best-effort). +func fetchGitLabReviewers(ctx context.Context, baseURL, projectPath, mrIID, token string) []prinfo.Reviewer { + if baseURL == "" || projectPath == "" || token == "" { + return nil + } + + encodedProject := url.PathEscape(projectPath) + apiURL := fmt.Sprintf("%s/api/v4/projects/%s/merge_requests/%s", baseURL, encodedProject, mrIID) + + client := &http.Client{Timeout: 10 * time.Second} + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return nil + } + req.Header.Set("JOB-TOKEN", token) + + resp, err := client.Do(req) + if err != nil { + return nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil + } + + var mrResponse struct { + Reviewers []struct { + Username string `json:"username"` + } `json:"reviewers"` + } + if err := json.NewDecoder(resp.Body).Decode(&mrResponse); err != nil { + return nil + } + + var reviewers []prinfo.Reviewer + for _, r := range mrResponse.Reviewers { + reviewers = append(reviewers, prinfo.Reviewer{ + Login: r.Username, + Type: "unknown", + }) + } + + return reviewers +} diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index 1e6636839..9a32d9df7 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.go @@ -16,9 +16,15 @@ package crafter import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" "path/filepath" "testing" + "github.com/chainloop-dev/chainloop/internal/prinfo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -32,7 +38,7 @@ func TestExtractGitHubPRMetadata(t *testing.T) { validate func(t *testing.T, metadata *PRMetadata) }{ { - name: "valid pull_request event", + name: "valid pull_request event with reviewers", envVars: map[string]string{ "GITHUB_EVENT_NAME": "pull_request", "GITHUB_EVENT_PATH": filepath.Join("testdata", "github_pr_event.json"), @@ -51,6 +57,28 @@ func TestExtractGitHubPRMetadata(t *testing.T) { assert.Equal(t, "main", metadata.TargetBranch) assert.Equal(t, "https://github.com/owner/repo/pull/123", metadata.URL) assert.Equal(t, "johndoe", metadata.Author) + require.Len(t, metadata.Reviewers, 2) + assert.Equal(t, "reviewer1", metadata.Reviewers[0].Login) + assert.Equal(t, "User", metadata.Reviewers[0].Type) + assert.Equal(t, "coderabbitai", metadata.Reviewers[1].Login) + assert.Equal(t, "Bot", metadata.Reviewers[1].Type) + }, + }, + { + name: "valid pull_request event without reviewers", + envVars: map[string]string{ + "GITHUB_EVENT_NAME": "pull_request", + "GITHUB_EVENT_PATH": filepath.Join("testdata", "github_pr_event_no_reviewers.json"), + "GITHUB_HEAD_REF": "fix-branch", + "GITHUB_BASE_REF": "main", + }, + expectPR: true, + expectError: false, + 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.Empty(t, metadata.Reviewers) }, }, { @@ -134,6 +162,8 @@ func TestExtractGitLabMRMetadata(t *testing.T) { 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) + // No reviewers without API access in tests + assert.Empty(t, metadata.Reviewers) }, }, { @@ -156,7 +186,7 @@ func TestExtractGitLabMRMetadata(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - isMR, metadata, err := extractGitLabMRMetadata(tc.envVars) + isMR, metadata, err := extractGitLabMRMetadata(context.Background(), tc.envVars) if tc.expectError { require.Error(t, err) @@ -173,3 +203,136 @@ func TestExtractGitLabMRMetadata(t *testing.T) { }) } } + +func TestFetchGitLabReviewers(t *testing.T) { + testCases := []struct { + name string + handler http.HandlerFunc + baseURL string // override if empty, use server URL + projectPath string + mrIID string + token string + expected []prinfo.Reviewer + }{ + { + 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"}]}`) + }, + projectPath: "group/project", + mrIID: "10", + token: "test-token", + expected: []prinfo.Reviewer{ + {Login: "alice", Type: "unknown"}, + {Login: "bot-reviewer", Type: "unknown"}, + }, + }, + { + name: "empty reviewers", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"reviewers": []}`) + }, + projectPath: "group/project", + mrIID: "10", + token: "test-token", + expected: nil, + }, + { + name: "API returns error", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + }, + projectPath: "group/project", + mrIID: "10", + token: "test-token", + expected: nil, + }, + { + name: "missing token", + handler: nil, + projectPath: "group/project", + mrIID: "10", + token: "", + expected: nil, + }, + { + name: "missing base URL", + handler: nil, + baseURL: "", + projectPath: "group/project", + mrIID: "10", + token: "test-token", + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var serverURL string + if tc.handler != nil { + server := httptest.NewServer(tc.handler) + defer server.Close() + serverURL = server.URL + } + + baseURL := tc.baseURL + if baseURL == "" && tc.handler != nil { + baseURL = serverURL + } + + reviewers := fetchGitLabReviewers(context.Background(), baseURL, tc.projectPath, tc.mrIID, tc.token) + + if tc.expected == nil { + assert.Nil(t, reviewers) + } else { + assert.Equal(t, tc.expected, reviewers) + } + }) + } +} + +func TestFetchGitLabReviewersRequestPath(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") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"reviewers": []any{}})) + })) + defer server.Close() + + fetchGitLabReviewers(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"}]}`) + })) + defer server.Close() + + envVars := map[string]string{ + "CI_PIPELINE_SOURCE": "merge_request_event", + "CI_MERGE_REQUEST_IID": "10", + "CI_MERGE_REQUEST_TITLE": "MR with reviewers", + "CI_MERGE_REQUEST_PROJECT_URL": "https://gitlab.com/group/project", + "GITLAB_USER_LOGIN": "author", + "CI_MERGE_REQUEST_SOURCE_BRANCH_NAME": "feature", + "CI_MERGE_REQUEST_TARGET_BRANCH_NAME": "main", + "CI_SERVER_URL": server.URL, + "CI_MERGE_REQUEST_PROJECT_PATH": "group/project", + } + + // CI_JOB_TOKEN is read via os.Getenv (not from envVars) to avoid persisting it in attestations. + t.Setenv("CI_JOB_TOKEN", "test-token") + + isMR, metadata, err := extractGitLabMRMetadata(context.Background(), envVars) + require.NoError(t, err) + 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, "coderabbitai", metadata.Reviewers[1].Login) +} diff --git a/pkg/attestation/crafter/runners/daggerpipeline.go b/pkg/attestation/crafter/runners/daggerpipeline.go index bb335d9cd..7dd21b029 100644 --- a/pkg/attestation/crafter/runners/daggerpipeline.go +++ b/pkg/attestation/crafter/runners/daggerpipeline.go @@ -68,6 +68,9 @@ func (r *DaggerPipeline) ListEnvVars() []*EnvVarDefinition { {"CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_TARGET_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_PROJECT_URL", true}, + {"CI_MERGE_REQUEST_PROJECT_PATH", true}, + {"CI_SERVER_URL", true}, + {"CI_PROJECT_PATH", true}, {"GITLAB_USER_LOGIN", true}, } } diff --git a/pkg/attestation/crafter/runners/daggerpipeline_test.go b/pkg/attestation/crafter/runners/daggerpipeline_test.go index a812440d6..7bc3b8640 100644 --- a/pkg/attestation/crafter/runners/daggerpipeline_test.go +++ b/pkg/attestation/crafter/runners/daggerpipeline_test.go @@ -78,6 +78,9 @@ func (s *daggerPipelineSuite) TestListEnvVars() { {"CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_TARGET_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_PROJECT_URL", true}, + {"CI_MERGE_REQUEST_PROJECT_PATH", true}, + {"CI_SERVER_URL", true}, + {"CI_PROJECT_PATH", true}, {"GITLAB_USER_LOGIN", true}, } assert.Equal(s.T(), expected, s.runner.ListEnvVars()) diff --git a/pkg/attestation/crafter/runners/gitlabpipeline.go b/pkg/attestation/crafter/runners/gitlabpipeline.go index 44ac37e87..5fc84baba 100644 --- a/pkg/attestation/crafter/runners/gitlabpipeline.go +++ b/pkg/attestation/crafter/runners/gitlabpipeline.go @@ -78,6 +78,7 @@ func (r *GitlabPipeline) ListEnvVars() []*EnvVarDefinition { {"CI_RUNNER_VERSION", false}, {"CI_RUNNER_DESCRIPTION", true}, {"CI_COMMIT_REF_NAME", false}, + {"CI_PROJECT_PATH", false}, // MR-specific variables (optional - only present in MR contexts) {"CI_PIPELINE_SOURCE", true}, {"CI_MERGE_REQUEST_IID", true}, @@ -86,6 +87,7 @@ func (r *GitlabPipeline) ListEnvVars() []*EnvVarDefinition { {"CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_TARGET_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_PROJECT_URL", true}, + {"CI_MERGE_REQUEST_PROJECT_PATH", true}, } } diff --git a/pkg/attestation/crafter/runners/gitlabpipeline_test.go b/pkg/attestation/crafter/runners/gitlabpipeline_test.go index 32e5ba742..b14c25e29 100644 --- a/pkg/attestation/crafter/runners/gitlabpipeline_test.go +++ b/pkg/attestation/crafter/runners/gitlabpipeline_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023-2025 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -91,6 +91,7 @@ func (s *gitlabPipelineSuite) TestListEnvVars() { {"CI_RUNNER_VERSION", false}, {"CI_RUNNER_DESCRIPTION", true}, {"CI_COMMIT_REF_NAME", false}, + {"CI_PROJECT_PATH", false}, {"CI_PIPELINE_SOURCE", true}, {"CI_MERGE_REQUEST_IID", true}, {"CI_MERGE_REQUEST_TITLE", true}, @@ -98,6 +99,7 @@ func (s *gitlabPipelineSuite) TestListEnvVars() { {"CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_TARGET_BRANCH_NAME", true}, {"CI_MERGE_REQUEST_PROJECT_URL", true}, + {"CI_MERGE_REQUEST_PROJECT_PATH", true}, }, s.runner.ListEnvVars()) } @@ -115,6 +117,7 @@ func (s *gitlabPipelineSuite) TestResolveEnvVars() { "CI_RUNNER_DESCRIPTION": "chainloop-runner", "CI_COMMIT_REF_NAME": "main", "CI_SERVER_URL": "https://gitlab.com", + "CI_PROJECT_PATH": "chainloop/chainloop", "CI_PIPELINE_SOURCE": "merge_request_event", "CI_MERGE_REQUEST_IID": "42", "CI_MERGE_REQUEST_TITLE": "Add new feature", @@ -122,6 +125,7 @@ func (s *gitlabPipelineSuite) TestResolveEnvVars() { "CI_MERGE_REQUEST_SOURCE_BRANCH_NAME": "feature/awesome", "CI_MERGE_REQUEST_TARGET_BRANCH_NAME": "main", "CI_MERGE_REQUEST_PROJECT_URL": "https://gitlab.com/chainloop/chainloop/-/merge_requests/42", + "CI_MERGE_REQUEST_PROJECT_PATH": "chainloop/chainloop", }, resolvedEnvVars) } @@ -135,6 +139,7 @@ func (s *gitlabPipelineSuite) TestResolveEnvVarsWithoutRunnerDescription() { s.T().Setenv("CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", "") s.T().Setenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "") s.T().Setenv("CI_MERGE_REQUEST_PROJECT_URL", "") + s.T().Setenv("CI_MERGE_REQUEST_PROJECT_PATH", "") resolvedEnvVars, errors := s.runner.ResolveEnvVars() s.Empty(errors, "Should not error when optional variables are missing") @@ -149,6 +154,7 @@ func (s *gitlabPipelineSuite) TestResolveEnvVarsWithoutRunnerDescription() { "CI_RUNNER_VERSION": "13.10.0", "CI_COMMIT_REF_NAME": "main", "CI_SERVER_URL": "https://gitlab.com", + "CI_PROJECT_PATH": "chainloop/chainloop", } s.Equal(expected, resolvedEnvVars) } @@ -184,6 +190,8 @@ func (s *gitlabPipelineSuite) SetupTest() { t.Setenv("CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", "feature/awesome") t.Setenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "main") t.Setenv("CI_MERGE_REQUEST_PROJECT_URL", "https://gitlab.com/chainloop/chainloop/-/merge_requests/42") + t.Setenv("CI_PROJECT_PATH", "chainloop/chainloop") + t.Setenv("CI_MERGE_REQUEST_PROJECT_PATH", "chainloop/chainloop") } // Run the tests diff --git a/pkg/attestation/crafter/testdata/github_pr_event.json b/pkg/attestation/crafter/testdata/github_pr_event.json index 0275cbcb8..78450c297 100644 --- a/pkg/attestation/crafter/testdata/github_pr_event.json +++ b/pkg/attestation/crafter/testdata/github_pr_event.json @@ -6,6 +6,16 @@ "html_url": "https://github.com/owner/repo/pull/123", "user": { "login": "johndoe" - } + }, + "requested_reviewers": [ + { + "login": "reviewer1", + "type": "User" + }, + { + "login": "coderabbitai", + "type": "Bot" + } + ] } } diff --git a/pkg/attestation/crafter/testdata/github_pr_event_no_reviewers.json b/pkg/attestation/crafter/testdata/github_pr_event_no_reviewers.json new file mode 100644 index 000000000..6a0ddf5a4 --- /dev/null +++ b/pkg/attestation/crafter/testdata/github_pr_event_no_reviewers.json @@ -0,0 +1,11 @@ +{ + "pull_request": { + "number": 456, + "title": "Fix bug", + "body": "This PR fixes a bug.", + "html_url": "https://github.com/owner/repo/pull/456", + "user": { + "login": "janedoe" + } + } +}