From 551f0db217647ddb07d3a60695a7306b0259977c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 04:38:19 +0000 Subject: [PATCH 1/3] Initial plan From d2f70e17fa2a6db3e0bcfffdc36a0bf537bd32b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 04:52:22 +0000 Subject: [PATCH 2/3] Add validation test to prevent github-token in safe output types Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../js/safe_output_types_validation.test.cjs | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 actions/setup/js/safe_output_types_validation.test.cjs diff --git a/actions/setup/js/safe_output_types_validation.test.cjs b/actions/setup/js/safe_output_types_validation.test.cjs new file mode 100644 index 0000000000..33433f024a --- /dev/null +++ b/actions/setup/js/safe_output_types_validation.test.cjs @@ -0,0 +1,141 @@ +/** + * @file safe_output_types_validation.test.cjs + * @description Validates that safe output JSONL item types do not contain github-token field. + * This test ensures the separation between output items (what agents produce) and + * output configuration (how outputs are handled at workflow level). + * + * Key principle: Individual output items should NOT specify authentication tokens. + * Tokens belong in the configuration layer, not in the data layer. + */ + +import { describe, it, expect } from "vitest"; +import fs from "fs"; +import path from "path"; + +describe("Safe Output Types Validation", () => { + const typeDefsPath = path.join(__dirname, "types", "safe-outputs.d.ts"); + const configDefsPath = path.join( + __dirname, + "types", + "safe-outputs-config.d.ts" + ); + + it("safe-outputs.d.ts should NOT contain github-token field", () => { + const content = fs.readFileSync(typeDefsPath, "utf-8"); + + // Check for various forms of github-token that might be added accidentally + expect(content).not.toMatch(/[\s"]github-token["\s:]/); + expect(content).not.toMatch(/[\s"]GitHub-token["\s:]/); + expect(content).not.toMatch(/[\s"]githubToken["\s:]/); + + // Verify the file exists and is not empty + expect(content.length).toBeGreaterThan(0); + }); + + it("safe-outputs-config.d.ts SHOULD contain github-token field", () => { + const content = fs.readFileSync(configDefsPath, "utf-8"); + + // Configuration SHOULD have github-token + expect(content).toMatch(/"github-token"\?:/); + + // Verify it's in the right places (base config and safe job config) + const lines = content.split("\n"); + const githubTokenLines = lines.filter((line) => + line.includes('"github-token"') + ); + + // Should appear at least twice: once in SafeOutputConfig, once in SafeJobConfig + expect(githubTokenLines.length).toBeGreaterThanOrEqual(2); + }); + + it("safe-outputs.d.ts should define output item interfaces", () => { + const content = fs.readFileSync(typeDefsPath, "utf-8"); + + // Verify core output types exist + expect(content).toMatch(/interface CreateIssueItem/); + expect(content).toMatch(/interface CreatePullRequestItem/); + expect(content).toMatch(/interface AddCommentItem/); + expect(content).toMatch(/type: "create_issue"/); + expect(content).toMatch(/type: "create_pull_request"/); + }); + + it("safe-outputs-config.d.ts should define configuration interfaces", () => { + const content = fs.readFileSync(configDefsPath, "utf-8"); + + // Verify configuration types exist + expect(content).toMatch(/interface SafeOutputConfig/); + expect(content).toMatch(/interface CreateIssueConfig/); + expect(content).toMatch(/interface SafeJobConfig/); + + // Configuration should have max, min, type fields + expect(content).toMatch(/max\?:/); + expect(content).toMatch(/type:/); + }); + + it("output items should extend BaseSafeOutputItem, not SafeOutputConfig", () => { + const content = fs.readFileSync(typeDefsPath, "utf-8"); + + // Output items extend BaseSafeOutputItem + expect(content).toMatch(/interface CreateIssueItem extends BaseSafeOutputItem/); + expect(content).toMatch(/interface AddCommentItem extends BaseSafeOutputItem/); + + // Should NOT extend SafeOutputConfig + expect(content).not.toMatch(/extends SafeOutputConfig/); + }); + + it("configuration interfaces should extend SafeOutputConfig", () => { + const content = fs.readFileSync(configDefsPath, "utf-8"); + + // Config interfaces extend SafeOutputConfig + expect(content).toMatch(/interface CreateIssueConfig extends SafeOutputConfig/); + expect(content).toMatch(/interface AddCommentConfig extends SafeOutputConfig/); + }); + + it("BaseSafeOutputItem should only have type field", () => { + const content = fs.readFileSync(typeDefsPath, "utf-8"); + + // Extract BaseSafeOutputItem definition + const baseInterfaceMatch = content.match( + /interface BaseSafeOutputItem\s*{([^}]*)}/ + ); + expect(baseInterfaceMatch).toBeTruthy(); + + if (baseInterfaceMatch) { + const interfaceBody = baseInterfaceMatch[1]; + + // Should only contain type field and comments + expect(interfaceBody).toMatch(/type:\s*string/); + + // Should NOT contain any auth-related fields + expect(interfaceBody).not.toMatch(/token/i); + expect(interfaceBody).not.toMatch(/auth/i); + expect(interfaceBody).not.toMatch(/credential/i); + expect(interfaceBody).not.toMatch(/secret/i); + } + }); + + it("SafeOutputConfig should contain only configuration fields", () => { + const content = fs.readFileSync(configDefsPath, "utf-8"); + + // Extract SafeOutputConfig definition + const baseInterfaceMatch = content.match( + /interface SafeOutputConfig\s*{([^}]*)}/ + ); + expect(baseInterfaceMatch).toBeTruthy(); + + if (baseInterfaceMatch) { + const interfaceBody = baseInterfaceMatch[1]; + + // Should contain configuration fields + expect(interfaceBody).toMatch(/type:\s*string/); + expect(interfaceBody).toMatch(/max\?:/); + expect(interfaceBody).toMatch(/min\?:/); + expect(interfaceBody).toMatch(/"github-token"\?:/); + + // Should NOT contain output-specific fields like title, body, etc. + expect(interfaceBody).not.toMatch(/title:/); + expect(interfaceBody).not.toMatch(/body:/); + expect(interfaceBody).not.toMatch(/message:/); + } + }); +}); From 0015d9fe9c1eaf0e36c13159b3575cf05247dc0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 05:14:01 +0000 Subject: [PATCH 3/3] Fix tests: Remove github-token from individual safe output types Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...ithub_token_precedence_integration_test.go | 49 ++++++------ ...ndividual_github_token_integration_test.go | 79 ++++++------------- 2 files changed, 49 insertions(+), 79 deletions(-) diff --git a/pkg/workflow/github_token_precedence_integration_test.go b/pkg/workflow/github_token_precedence_integration_test.go index 2bf1cac9c5..a230c4da8a 100644 --- a/pkg/workflow/github_token_precedence_integration_test.go +++ b/pkg/workflow/github_token_precedence_integration_test.go @@ -117,9 +117,9 @@ Test that safe-outputs github-token overrides top-level. } }) - t.Run("individual safe-output token overrides both", func(t *testing.T) { + t.Run("safe-outputs token overrides top-level", func(t *testing.T) { testContent := `--- -name: Test Individual Override +name: Test Safe Outputs Override on: issues: types: [opened] @@ -128,12 +128,12 @@ github-token: ${{ secrets.TOPLEVEL_PAT }} safe-outputs: github-token: ${{ secrets.SAFE_OUTPUTS_PAT }} create-issue: - github-token: ${{ secrets.INDIVIDUAL_PAT }} + title-prefix: "[AUTO] " --- -# Test Individual Override +# Test Safe Outputs Override -Test that individual safe-output github-token has highest precedence. +Test that safe-outputs github-token overrides top-level token. ` testFile := filepath.Join(tmpDir, "test-individual-override.md") @@ -155,29 +155,28 @@ Test that individual safe-output github-token has highest precedence. yamlContent := string(content) - // Verify that individual token is used in the safe_outputs job - if !strings.Contains(yamlContent, "github-token: ${{ secrets.INDIVIDUAL_PAT }}") { - t.Error("Expected individual safe-output github-token to be used in safe_outputs job") + // Verify that safe-outputs token is used in the safe_outputs job + if !strings.Contains(yamlContent, "github-token: ${{ secrets.SAFE_OUTPUTS_PAT }}") { + t.Error("Expected safe-outputs github-token to be used in safe_outputs job") t.Logf("Generated YAML:\n%s", yamlContent) } - // Count occurrences of each token to verify precedence - individualCount := strings.Count(yamlContent, "github-token: ${{ secrets.INDIVIDUAL_PAT }}") - safeOutputsCount := strings.Count(yamlContent, "github-token: ${{ secrets.SAFE_OUTPUTS_PAT }}") - toplevelCount := strings.Count(yamlContent, "github-token: ${{ secrets.TOPLEVEL_PAT }}") - - if individualCount == 0 { - t.Error("Individual token should be present at least once") - } - - // Note: safe-outputs global token might appear in other safe-output jobs or contexts - // but should not appear more frequently than the individual token in the safe_outputs job - // The test is primarily checking that the individual token is used where it should be - if individualCount == 0 && safeOutputsCount > 0 { - t.Error("Individual token should take precedence over safe-outputs token") - } - if individualCount == 0 && toplevelCount > 0 { - t.Error("Individual token should take precedence over top-level token") + // Verify top-level token is not used in safe_outputs job + // (it may appear in other jobs, but not in safe_outputs) + lines := strings.Split(yamlContent, "\n") + inSafeOutputsJob := false + for i, line := range lines { + if strings.Contains(line, "safe_outputs:") && !strings.Contains(line, "#") { + inSafeOutputsJob = true + continue + } + // Check if we've moved to a new top-level job + if inSafeOutputsJob && len(line) > 0 && !strings.HasPrefix(line, " ") && strings.Contains(line, ":") { + inSafeOutputsJob = false + } + if inSafeOutputsJob && strings.Contains(line, "github-token: ${{ secrets.TOPLEVEL_PAT }}") { + t.Errorf("Top-level token should not be used in safe_outputs job (found at line %d)", i+1) + } } }) diff --git a/pkg/workflow/individual_github_token_integration_test.go b/pkg/workflow/individual_github_token_integration_test.go index f76986d9e6..bff801c80b 100644 --- a/pkg/workflow/individual_github_token_integration_test.go +++ b/pkg/workflow/individual_github_token_integration_test.go @@ -15,9 +15,9 @@ func TestIndividualGitHubTokenIntegration(t *testing.T) { // Create temporary directory for test files tmpDir := testutil.TempDir(t, "individual-github-token-test") - t.Run("create-issue uses individual github-token in generated workflow", func(t *testing.T) { + t.Run("create-issue uses safe-outputs global github-token", func(t *testing.T) { testContent := `--- -name: Test Individual GitHub Token for Issues +name: Test Global GitHub Token for Issues on: issues: types: [opened] @@ -25,12 +25,12 @@ engine: claude safe-outputs: github-token: ${{ secrets.GLOBAL_PAT }} create-issue: - github-token: ${{ secrets.ISSUE_SPECIFIC_PAT }} + title-prefix: "[AUTO] " --- -# Test Individual GitHub Token for Issues +# Test Global GitHub Token for Issues -This workflow tests that create-issue uses its own github-token. +This workflow tests that create-issue uses the safe-outputs global github-token. ` testFile := filepath.Join(tmpDir, "test-issue-token.md") @@ -60,36 +60,16 @@ This workflow tests that create-issue uses its own github-token. t.Error("Expected safe_outputs job to be generated") } - // Verify that the specific token is used for create_issue - if !strings.Contains(yamlContent, "github-token: ${{ secrets.ISSUE_SPECIFIC_PAT }}") { - t.Error("Expected safe_outputs job to use the issue-specific GitHub token") + // Verify that the global token is used for create_issue + if !strings.Contains(yamlContent, "github-token: ${{ secrets.GLOBAL_PAT }}") { + t.Error("Expected safe_outputs job to use the global GitHub token") t.Logf("Generated YAML:\n%s", yamlContent) } - - // Verify that the global token is not used in create_issue - if strings.Contains(yamlContent, "github-token: ${{ secrets.GLOBAL_PAT }}") { - // Check if it's in the safe_outputs job section specifically - lines := strings.Split(yamlContent, "\n") - inCreateIssueJob := false - for _, line := range lines { - if strings.Contains(line, "create_issue:") { - inCreateIssueJob = true - continue - } - if inCreateIssueJob && strings.HasPrefix(line, " ") && strings.Contains(line, ":") && !strings.HasPrefix(line, " ") { - // We've moved to a new job - inCreateIssueJob = false - } - if inCreateIssueJob && strings.Contains(line, "github-token: ${{ secrets.GLOBAL_PAT }}") { - t.Error("safe_outputs job should not use the global GitHub token when individual token is specified") - } - } - } }) - t.Run("create-pull-request fallback to global github-token when no individual token specified", func(t *testing.T) { + t.Run("create-pull-request uses safe-outputs global github-token", func(t *testing.T) { testContent := `--- -name: Test GitHub Token Fallback for PRs +name: Test GitHub Token for PRs on: issues: types: [opened] @@ -98,14 +78,13 @@ safe-outputs: github-token: ${{ secrets.GLOBAL_PAT }} create-pull-request: draft: true - # No github-token specified, should use global create-issue: - github-token: ${{ secrets.ISSUE_SPECIFIC_PAT }} + title-prefix: "[AUTO] " --- -# Test GitHub Token Fallback +# Test GitHub Token for PRs -This workflow tests that create-pull-request falls back to global github-token. +This workflow tests that create-pull-request uses the safe-outputs global github-token. ` testFile := filepath.Join(tmpDir, "test-pr-fallback.md") @@ -131,28 +110,20 @@ This workflow tests that create-pull-request falls back to global github-token. yamlContent := string(content) // Verify that both jobs exist and use correct tokens - if !strings.Contains(yamlContent, "safe_outputs:") { - t.Error("Expected create_pull_request job to be generated") - } if !strings.Contains(yamlContent, "safe_outputs:") { t.Error("Expected safe_outputs job to be generated") } - // Use simple string checks like the other working tests + // Verify that the global token is used if !strings.Contains(yamlContent, "github-token: ${{ secrets.GLOBAL_PAT }}") { - t.Error("Expected create_pull_request job to use global GitHub token (fallback)") - t.Logf("Generated YAML:\n%s", yamlContent) - } - - if !strings.Contains(yamlContent, "github-token: ${{ secrets.ISSUE_SPECIFIC_PAT }}") { - t.Error("Expected safe_outputs job to use individual GitHub token") + t.Error("Expected safe_outputs job to use global GitHub token") t.Logf("Generated YAML:\n%s", yamlContent) } }) - t.Run("add-labels uses individual github-token", func(t *testing.T) { + t.Run("add-labels uses safe-outputs global github-token", func(t *testing.T) { testContent := `--- -name: Test Individual GitHub Token for Labels +name: Test Global GitHub Token for Labels on: issues: types: [opened] @@ -161,12 +132,11 @@ safe-outputs: github-token: ${{ secrets.GLOBAL_PAT }} add-labels: allowed: [bug, feature, enhancement] - github-token: ${{ secrets.LABELS_PAT }} --- -# Test Individual GitHub Token for Labels +# Test Global GitHub Token for Labels -This workflow tests that add-labels uses its own github-token. +This workflow tests that add-labels uses the safe-outputs global github-token. ` testFile := filepath.Join(tmpDir, "test-labels-token.md") @@ -191,13 +161,14 @@ This workflow tests that add-labels uses its own github-token. yamlContent := string(content) - // Verify that the safe_outputs job is generated with add_labels step - if !strings.Contains(yamlContent, "id: add_labels") { - t.Error("Expected safe_outputs job with add_labels step to be generated") + // Verify that the safe_outputs job is generated + if !strings.Contains(yamlContent, "safe_outputs:") { + t.Error("Expected safe_outputs job to be generated") } - if !strings.Contains(yamlContent, "github-token: ${{ secrets.LABELS_PAT }}") { - t.Error("Expected safe_outputs job to use the labels-specific GitHub token") + // Verify the github token is used + if !strings.Contains(yamlContent, "github-token: ${{ secrets.GLOBAL_PAT }}") { + t.Error("Expected safe_outputs job to use the global GitHub token") t.Logf("Generated YAML:\n%s", yamlContent) } })