diff --git a/.changeset/patch-add-allowed-github-references.md b/.changeset/patch-add-allowed-github-references.md new file mode 100644 index 0000000000..eb9ec12d4f --- /dev/null +++ b/.changeset/patch-add-allowed-github-references.md @@ -0,0 +1,6 @@ +--- +"gh-aw": patch +--- + +Add `allowed-github-references` safe-output field to restrict and escape unauthorized GitHub-style markdown references (e.g. `#123`, `owner/repo#456`). Includes backend parsing, JS sanitizer, schema validation, and tests. + diff --git a/.changeset/patch-allowed-github-references.md b/.changeset/patch-allowed-github-references.md new file mode 100644 index 0000000000..52c78d848c --- /dev/null +++ b/.changeset/patch-allowed-github-references.md @@ -0,0 +1,10 @@ +--- +"gh-aw": patch +--- + +Add `allowed-github-references` safe-output configuration to restrict which +GitHub-style markdown references (e.g. `#123` or `owner/repo#456`) are +allowed when rendering safe outputs. Unauthorized references are escaped with +backticks. This change adds backend parsing, a JS sanitizer, schema +validation, and comprehensive tests. + diff --git a/.github/workflows/smoke-copilot-no-firewall.lock.yml b/.github/workflows/smoke-copilot-no-firewall.lock.yml index 3b91cb6a4d..117b7bf1d5 100644 --- a/.github/workflows/smoke-copilot-no-firewall.lock.yml +++ b/.github/workflows/smoke-copilot-no-firewall.lock.yml @@ -884,6 +884,7 @@ jobs: env: GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.npms.io,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,bun.sh,cdn.playwright.dev,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,deb.nodesource.com,deno.land,get.pnpm.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,nodejs.org,npm.pkg.github.com,npmjs.com,npmjs.org,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,playwright.download.prss.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.bower.io,registry.npmjs.com,registry.npmjs.org,registry.yarnpkg.com,repo.yarnpkg.com,s.symcb.com,s.symcd.com,security.ubuntu.com,skimdb.npmjs.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.npmjs.com,www.npmjs.org,yarnpkg.com" + GH_AW_ALLOWED_GITHUB_REFS: "" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} with: diff --git a/.github/workflows/smoke-copilot-no-firewall.md b/.github/workflows/smoke-copilot-no-firewall.md index 2d1a8d869f..bfc4d8e8fe 100644 --- a/.github/workflows/smoke-copilot-no-firewall.md +++ b/.github/workflows/smoke-copilot-no-firewall.md @@ -36,6 +36,7 @@ tools: - github.com serena: ["go"] safe-outputs: + allowed-github-references: [] add-comment: hide-older-comments: true add-labels: diff --git a/actions/setup/js/sanitize_content.cjs b/actions/setup/js/sanitize_content.cjs index 635ec6e4b6..17c2d1327f 100644 --- a/actions/setup/js/sanitize_content.cjs +++ b/actions/setup/js/sanitize_content.cjs @@ -11,9 +11,12 @@ const { clearRedactedDomains, writeRedactedDomainsLog, buildAllowedDomains, + buildAllowedGitHubReferences, + getCurrentRepoSlug, sanitizeUrlProtocols, sanitizeUrlDomains, neutralizeCommands, + neutralizeGitHubReferences, removeXmlComments, convertXmlTags, neutralizeBotTriggers, @@ -62,6 +65,9 @@ function sanitizeContent(content, maxLengthOrOptions) { // Build list of allowed domains (shared with core) const allowedDomains = buildAllowedDomains(); + // Build list of allowed GitHub references from environment + const allowedGitHubRefs = buildAllowedGitHubReferences(); + let sanitized = content; // Remove ANSI escape sequences and control characters early @@ -87,6 +93,9 @@ function sanitizeContent(content, maxLengthOrOptions) { // Apply truncation limits (shared with core) sanitized = applyTruncation(sanitized, maxLength); + // Neutralize GitHub references if restrictions are configured + sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs); + // Neutralize bot triggers sanitized = neutralizeBotTriggers(sanitized); diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index be7435ccfd..aec4075286 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.cjs @@ -22,9 +22,11 @@ describe("sanitize_content.cjs", () => { afterEach(() => { delete global.core; delete process.env.GH_AW_ALLOWED_DOMAINS; + delete process.env.GH_AW_ALLOWED_GITHUB_REFS; delete process.env.GH_AW_COMMAND; delete process.env.GITHUB_SERVER_URL; delete process.env.GITHUB_API_URL; + delete process.env.GITHUB_REPOSITORY; }); describe("basic sanitization", () => { @@ -493,6 +495,219 @@ describe("sanitize_content.cjs", () => { }); }); + describe("GitHub reference neutralization", () => { + beforeEach(() => { + delete process.env.GH_AW_ALLOWED_GITHUB_REFS; + delete process.env.GITHUB_REPOSITORY; + }); + + afterEach(() => { + delete process.env.GH_AW_ALLOWED_GITHUB_REFS; + delete process.env.GITHUB_REPOSITORY; + }); + + it("should allow all references by default (no env var set)", () => { + const result = sanitizeContent("See issue #123 and owner/repo#456"); + // When no env var is set, all references are allowed + expect(result).toBe("See issue #123 and owner/repo#456"); + }); + + it("should restrict to current repo only when 'repo' is specified", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See issue #123 and other/repo#456"); + expect(result).toBe("See issue #123 and `other/repo#456`"); + }); + + it("should allow current repo references with 'repo' keyword", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See myorg/myrepo#123"); + expect(result).toBe("See myorg/myrepo#123"); + }); + + it("should allow specific repos in the list", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo,other/allowed-repo"; + + const result = sanitizeContent("See #123, other/allowed-repo#456, and bad/repo#789"); + expect(result).toBe("See #123, other/allowed-repo#456, and `bad/repo#789`"); + }); + + it("should handle multiple allowed repos", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "myorg/myrepo,other/repo,another/repo"; + + const result = sanitizeContent("Issues: myorg/myrepo#1, other/repo#2, another/repo#3, blocked/repo#4"); + expect(result).toBe("Issues: myorg/myrepo#1, other/repo#2, another/repo#3, `blocked/repo#4`"); + }); + + it("should be case-insensitive for repo names", () => { + process.env.GITHUB_REPOSITORY = "MyOrg/MyRepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("Issues: myorg/myrepo#123, MYORG/MYREPO#456"); + expect(result).toBe("Issues: myorg/myrepo#123, MYORG/MYREPO#456"); + }); + + it("should not escape references inside backticks", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("Already escaped: `other/repo#123`"); + expect(result).toBe("Already escaped: `other/repo#123`"); + }); + + it("should handle issue numbers with alphanumeric characters", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See #abc123 and other/repo#def456"); + expect(result).toBe("See #abc123 and `other/repo#def456`"); + }); + + it("should handle references in different contexts", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("Start #123 middle other/repo#456 end"); + expect(result).toBe("Start #123 middle `other/repo#456` end"); + }); + + it("should trim whitespace in allowed-refs list", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = " repo , other/repo "; + + const result = sanitizeContent("See myorg/myrepo#123 and other/repo#456"); + expect(result).toBe("See myorg/myrepo#123 and other/repo#456"); + }); + + it("should log when escaping references", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + sanitizeContent("See other/repo#123"); + expect(mockCore.info).toHaveBeenCalledWith("Escaped GitHub reference: other/repo#123 (not in allowed list)"); + }); + + it("should escape all references when allowed-refs is empty array", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = ""; + + const result = sanitizeContent("See #123 and myorg/myrepo#456 and other/repo#789"); + expect(result).toBe("See `#123` and `myorg/myrepo#456` and `other/repo#789`"); + }); + + it("should handle empty allowed-refs list (all references escaped)", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = ""; + + const result = sanitizeContent("See #123 and other/repo#456"); + expect(result).toBe("See `#123` and `other/repo#456`"); + }); + + it("should escape references when current repo is not in list", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "other/allowed"; + + const result = sanitizeContent("See #123 and myorg/myrepo#456"); + expect(result).toBe("See `#123` and `myorg/myrepo#456`"); + }); + + it("should handle references with hyphens in repo names", () => { + process.env.GITHUB_REPOSITORY = "my-org/my-repo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See my-org/my-repo#123 and other-org/other-repo#456"); + expect(result).toBe("See my-org/my-repo#123 and `other-org/other-repo#456`"); + }); + + it("should handle references with underscores in repo names", () => { + process.env.GITHUB_REPOSITORY = "myorg/my_repo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See myorg/my_repo#123 and otherorg/other_repo#456"); + expect(result).toBe("See myorg/my_repo#123 and `otherorg/other_repo#456`"); + }); + + it("should handle references with dots in repo names", () => { + process.env.GITHUB_REPOSITORY = "myorg/my.repo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo,other/repo.test"; + + const result = sanitizeContent("See myorg/my.repo#123 and other/repo.test#456"); + expect(result).toBe("See myorg/my.repo#123 and other/repo.test#456"); + }); + + it("should handle multiple references in same sentence", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo,other/allowed"; + + const result = sanitizeContent("Related to #1, #2, other/allowed#3, and blocked/repo#4"); + expect(result).toBe("Related to #1, #2, other/allowed#3, and `blocked/repo#4`"); + }); + + it("should handle references at start and end of string", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("#123 in the middle other/repo#456"); + expect(result).toBe("#123 in the middle `other/repo#456`"); + }); + + it("should not escape references in code blocks", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("Code: `other/repo#123` end"); + expect(result).toBe("Code: `other/repo#123` end"); + }); + + it("should handle mixed case in repo specification", () => { + process.env.GITHUB_REPOSITORY = "MyOrg/MyRepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "myorg/myrepo,Other/Repo"; + + const result = sanitizeContent("See MyOrg/MyRepo#1, myorg/myrepo#2, OTHER/REPO#3, blocked/repo#4"); + expect(result).toBe("See MyOrg/MyRepo#1, myorg/myrepo#2, OTHER/REPO#3, `blocked/repo#4`"); + }); + + it("should handle very long issue numbers", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See #123456789012345 and other/repo#999999999"); + expect(result).toBe("See #123456789012345 and `other/repo#999999999`"); + }); + + it("should handle no GITHUB_REPOSITORY env var with 'repo' keyword", () => { + delete process.env.GITHUB_REPOSITORY; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("See #123 and other/repo#456"); + // When GITHUB_REPOSITORY is not set, #123 targets empty string which won't match "repo", so not escaped + // But since we're trying to restrict to "repo" only, and current repo is unknown, all refs stay as-is + // because the restriction only applies when it can be determined + expect(result).toBe("See #123 and `other/repo#456`"); + }); + + it("should handle specific repo allowed but not current", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "other/specific"; + + const result = sanitizeContent("See #123 and other/specific#456"); + expect(result).toBe("See `#123` and other/specific#456"); + }); + + it("should preserve spacing around escaped references", () => { + process.env.GITHUB_REPOSITORY = "myorg/myrepo"; + process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo"; + + const result = sanitizeContent("Before other/repo#123 after"); + expect(result).toBe("Before `other/repo#123` after"); + }); + }); + describe("content truncation", () => { it("should truncate content exceeding max length", () => { const longContent = "x".repeat(600000); diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index ad24f1fa2b..d47d39c9f9 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -333,6 +333,94 @@ function neutralizeBotTriggers(s) { return s.replace(/\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, (match, action, ref) => `\`${action} #${ref}\``); } +/** + * Builds the list of allowed repositories for GitHub reference filtering + * Returns null if all references should be allowed (default behavior) + * Returns empty array if no references should be allowed (escape all) + * @returns {string[]|null} Array of allowed repository slugs or null if all allowed + */ +function buildAllowedGitHubReferences() { + const allowedRefsEnv = process.env.GH_AW_ALLOWED_GITHUB_REFS; + if (allowedRefsEnv === undefined) { + return null; // All references allowed by default (env var not set) + } + + if (allowedRefsEnv === "") { + return []; // Empty array means escape all references + } + + return allowedRefsEnv + .split(",") + .map(ref => ref.trim().toLowerCase()) + .filter(ref => ref); +} + +/** + * Gets the current repository slug from GitHub context + * @returns {string} Repository slug in "owner/repo" format, or empty string if not available + */ +function getCurrentRepoSlug() { + // Try to get from GITHUB_REPOSITORY env var + const repoSlug = process.env.GITHUB_REPOSITORY; + if (repoSlug) { + return repoSlug.toLowerCase(); + } + return ""; +} + +/** + * Neutralizes GitHub references (#123 or owner/repo#456) by wrapping them in backticks + * if they reference repositories not in the allowed list + * @param {string} s - The string to process + * @param {string[]|null} allowedRepos - List of allowed repository slugs (lowercase), or null to allow all + * @returns {string} The string with unauthorized references neutralized + */ +function neutralizeGitHubReferences(s, allowedRepos) { + // If no restrictions configured (null), allow all references + if (allowedRepos === null) { + return s; + } + + const currentRepo = getCurrentRepoSlug(); + + // Match GitHub references: + // - #123 (current repo reference) + // - owner/repo#456 (cross-repo reference) + // - GH-123 (GitHub shorthand) + // Must not be inside backticks or code blocks + return s.replace(/(^|[^\w`])(?:([a-z0-9](?:[a-z0-9-]{0,38}[a-z0-9])?)\/([a-z0-9._-]+))?#(\w+)/gi, (match, prefix, owner, repo, issueNum) => { + let targetRepo; + + if (owner && repo) { + // Cross-repo reference: owner/repo#123 + targetRepo = `${owner}/${repo}`.toLowerCase(); + } else { + // Current repo reference: #123 + targetRepo = currentRepo; + } + + // Check if "repo" is in allowed list (means current repo) + const allowCurrentRepo = allowedRepos.includes("repo"); + + // Check if this specific repo is in the allowed list + const isAllowed = allowedRepos.includes(targetRepo) || (allowCurrentRepo && targetRepo === currentRepo); + + if (isAllowed) { + return match; // Keep the original reference + } else { + // Escape the reference + const refText = owner && repo ? `${owner}/${repo}#${issueNum}` : `#${issueNum}`; + + // Log when a reference is escaped + if (typeof core !== "undefined" && core.info) { + core.info(`Escaped GitHub reference: ${refText} (not in allowed list)`); + } + + return `${prefix}\`${refText}\``; + } + }); +} + /** * Apply truncation limits to content * @param {string} content - The content to truncate @@ -376,6 +464,9 @@ function sanitizeContentCore(content, maxLength) { // Build list of allowed domains from environment and GitHub context const allowedDomains = buildAllowedDomains(); + // Build list of allowed GitHub references from environment + const allowedGitHubRefs = buildAllowedGitHubReferences(); + let sanitized = content; // Remove ANSI escape sequences and control characters early @@ -406,6 +497,9 @@ function sanitizeContentCore(content, maxLength) { // Apply truncation limits sanitized = applyTruncation(sanitized, maxLength); + // Neutralize GitHub references if restrictions are configured + sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs); + // Neutralize common bot trigger phrases sanitized = neutralizeBotTriggers(sanitized); @@ -421,9 +515,12 @@ module.exports = { writeRedactedDomainsLog, extractDomainsFromUrl, buildAllowedDomains, + buildAllowedGitHubReferences, + getCurrentRepoSlug, sanitizeUrlProtocols, sanitizeUrlDomains, neutralizeCommands, + neutralizeGitHubReferences, removeXmlComments, convertXmlTags, neutralizeBotTriggers, diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index ab0a6a5b18..e86a0316ec 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -132,6 +132,8 @@ SAFE_INPUTS_FILES=( "mcp_http_transport.cjs" "mcp_handler_shell.cjs" "mcp_handler_python.cjs" + "mcp_handler_go.cjs" + "mcp_handler_javascript.cjs" "read_buffer.cjs" "generate_safe_inputs_config.cjs" "setup_globals.cjs" @@ -180,6 +182,8 @@ SAFE_OUTPUTS_FILES=( "mcp_logger.cjs" "mcp_handler_python.cjs" "mcp_handler_shell.cjs" + "mcp_handler_go.cjs" + "mcp_handler_javascript.cjs" "read_buffer.cjs" "safe_inputs_validation.cjs" "messages.cjs" diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 96f4761dd3..4f534b853e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3501,6 +3501,15 @@ "type": "string" } }, + "allowed-github-references": { + "type": "array", + "description": "List of allowed repositories for GitHub references (e.g., #123 or owner/repo#456). Use 'repo' to allow current repository. References to other repositories will be escaped with backticks. If not specified, all references are allowed.", + "items": { + "type": "string", + "pattern": "^(repo|[a-zA-Z0-9][-a-zA-Z0-9]{0,38}/[a-zA-Z0-9._-]+)$" + }, + "examples": [["repo"], ["repo", "octocat/hello-world"], ["microsoft/vscode", "microsoft/typescript"]] + }, "create-issue": { "oneOf": [ { diff --git a/pkg/workflow/allow_github_references_env_test.go b/pkg/workflow/allow_github_references_env_test.go new file mode 100644 index 0000000000..66a314ffb5 --- /dev/null +++ b/pkg/workflow/allow_github_references_env_test.go @@ -0,0 +1,189 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/githubnext/gh-aw/pkg/testutil" +) + +func TestAllowGitHubReferencesEnvVar(t *testing.T) { + tests := []struct { + name string + workflow string + expectedEnvVarPresent bool + expectedEnvVarValue string + }{ + { + name: "env var set with single repo", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + allowed-github-references: ["repo"] + create-issue: {} +--- + +# Test Workflow + +Test workflow with allowed-github-references. +`, + expectedEnvVarPresent: true, + expectedEnvVarValue: "repo", + }, + { + name: "env var set with multiple repos", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + allowed-github-references: ["repo", "org/repo2", "org/repo3"] + create-issue: {} +--- + +# Test Workflow + +Test workflow with multiple allowed repos. +`, + expectedEnvVarPresent: true, + expectedEnvVarValue: "repo,org/repo2,org/repo3", + }, + { + name: "env var not set when allowed-github-references is absent", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + create-issue: {} +--- + +# Test Workflow + +Test workflow without allowed-github-references. +`, + expectedEnvVarPresent: false, + }, + { + name: "env var with repos containing special characters", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + allowed-github-references: ["my-org/my-repo", "test-org/test.repo"] + create-issue: {} +--- + +# Test Workflow + +Test workflow with special characters in repo names. +`, + expectedEnvVarPresent: true, + expectedEnvVarValue: "my-org/my-repo,test-org/test.repo", + }, + { + name: "env var with mix of repo keyword and specific repos", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + allowed-github-references: ["repo", "microsoft/vscode"] + create-issue: {} +--- + +# Test Workflow + +Test workflow mixing repo keyword with specific repos. +`, + expectedEnvVarPresent: true, + expectedEnvVarValue: "repo,microsoft/vscode", + }, + { + name: "env var with only specific repos (no repo keyword)", + workflow: `--- +on: push +engine: copilot +strict: false +permissions: + contents: read + issues: write +safe-outputs: + allowed-github-references: ["octocat/hello-world", "github/copilot"] + create-issue: {} +--- + +# Test Workflow + +Test workflow with only specific repos allowed. +`, + expectedEnvVarPresent: true, + expectedEnvVarValue: "octocat/hello-world,github/copilot", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp directory for test files + tmpDir := testutil.TempDir(t, "allow-github-refs-test") + + // Write workflow file + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.workflow), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read the generated lock file + lockFile := strings.Replace(testFile, ".md", ".lock.yml", 1) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + lockStr := string(lockContent) + + // Check for env var presence + if tt.expectedEnvVarPresent { + if !strings.Contains(lockStr, "GH_AW_ALLOWED_GITHUB_REFS:") { + t.Error("Expected GH_AW_ALLOWED_GITHUB_REFS environment variable in lock file") + } + + // Verify the value + expectedLine := `GH_AW_ALLOWED_GITHUB_REFS: "` + tt.expectedEnvVarValue + `"` + if !strings.Contains(lockStr, expectedLine) { + t.Errorf("Expected GH_AW_ALLOWED_GITHUB_REFS value to be %q, but it was not found in lock file", tt.expectedEnvVarValue) + } + } else { + if strings.Contains(lockStr, "GH_AW_ALLOWED_GITHUB_REFS:") { + t.Error("Expected no GH_AW_ALLOWED_GITHUB_REFS environment variable in lock file") + } + } + }) + } +} diff --git a/pkg/workflow/allow_github_references_test.go b/pkg/workflow/allow_github_references_test.go new file mode 100644 index 0000000000..bbbdbed98b --- /dev/null +++ b/pkg/workflow/allow_github_references_test.go @@ -0,0 +1,111 @@ +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAllowGitHubReferencesConfig(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + expected []string + }{ + { + name: "allow current repo only", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"repo"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"repo"}, + }, + { + name: "allow multiple repos", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"repo", "org/repo2", "org/repo3"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"repo", "org/repo2", "org/repo3"}, + }, + { + name: "no restrictions (empty array)", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{}, // Empty array should be preserved (means escape all) + }, + { + name: "no allowed-github-references field", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "create-issue": map[string]any{}, + }, + }, + expected: nil, + }, + { + name: "allow repos with hyphens", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"my-org/my-repo", "other-org/other-repo"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"my-org/my-repo", "other-org/other-repo"}, + }, + { + name: "allow repos with underscores and dots", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"my-org/my.repo", "test-org/test.repo.v2"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"my-org/my.repo", "test-org/test.repo.v2"}, + }, + { + name: "single specific repo without 'repo' keyword", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"octocat/hello-world"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"octocat/hello-world"}, + }, + { + name: "mix of 'repo' keyword and specific repos", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "allowed-github-references": []any{"repo", "microsoft/vscode", "github/copilot"}, + "create-issue": map[string]any{}, + }, + }, + expected: []string{"repo", "microsoft/vscode", "github/copilot"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewCompiler(false, "", "1.0.0") + config := c.extractSafeOutputsConfig(tt.frontmatter) + require.NotNil(t, config, "extractSafeOutputsConfig() should not return nil") + + if tt.expected == nil { + assert.Nil(t, config.AllowGitHubReferences, "AllowGitHubReferences should be nil") + } else { + require.NotNil(t, config.AllowGitHubReferences, "AllowGitHubReferences should not be nil") + assert.Equal(t, tt.expected, config.AllowGitHubReferences, "AllowGitHubReferences should match expected") + } + }) + } +} diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 659fcf89e0..23830d506e 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -344,13 +344,14 @@ type SafeOutputsConfig struct { Jobs map[string]*SafeJobConfig `yaml:"jobs,omitempty"` // Safe-jobs configuration (moved from top-level) App *GitHubAppConfig `yaml:"app,omitempty"` // GitHub App credentials for token minting AllowedDomains []string `yaml:"allowed-domains,omitempty"` - Staged bool `yaml:"staged,omitempty"` // If true, emit step summary messages instead of making GitHub API calls - Env map[string]string `yaml:"env,omitempty"` // Environment variables to pass to safe output jobs - GitHubToken string `yaml:"github-token,omitempty"` // GitHub token for safe output jobs - MaximumPatchSize int `yaml:"max-patch-size,omitempty"` // Maximum allowed patch size in KB (defaults to 1024) - RunsOn string `yaml:"runs-on,omitempty"` // Runner configuration for safe-outputs jobs - Messages *SafeOutputMessagesConfig `yaml:"messages,omitempty"` // Custom message templates for footer and notifications - Mentions *MentionsConfig `yaml:"mentions,omitempty"` // Configuration for @mention filtering in safe outputs + AllowGitHubReferences []string `yaml:"allowed-github-references,omitempty"` // Allowed repositories for GitHub references (e.g., ["repo", "org/repo2"]) + Staged bool `yaml:"staged,omitempty"` // If true, emit step summary messages instead of making GitHub API calls + Env map[string]string `yaml:"env,omitempty"` // Environment variables to pass to safe output jobs + GitHubToken string `yaml:"github-token,omitempty"` // GitHub token for safe output jobs + MaximumPatchSize int `yaml:"max-patch-size,omitempty"` // Maximum allowed patch size in KB (defaults to 1024) + RunsOn string `yaml:"runs-on,omitempty"` // Runner configuration for safe-outputs jobs + Messages *SafeOutputMessagesConfig `yaml:"messages,omitempty"` // Custom message templates for footer and notifications + Mentions *MentionsConfig `yaml:"mentions,omitempty"` // Configuration for @mention filtering in safe outputs } // SafeOutputMessagesConfig holds custom message templates for safe-output footer and notification messages diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 62d1516e86..e8f17b82a5 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -543,6 +543,12 @@ func (c *Compiler) generateOutputCollectionStep(yaml *strings.Builder, data *Wor fmt.Fprintf(yaml, " GH_AW_ALLOWED_DOMAINS: %q\n", domainsStr) } + // Add allowed GitHub references configuration for reference escaping + if data.SafeOutputs != nil && data.SafeOutputs.AllowGitHubReferences != nil { + refsStr := strings.Join(data.SafeOutputs.AllowGitHubReferences, ",") + fmt.Fprintf(yaml, " GH_AW_ALLOWED_GITHUB_REFS: %q\n", refsStr) + } + // Add GitHub server URL and API URL for dynamic domain extraction // This allows the sanitization code to permit GitHub domains that vary by deployment yaml.WriteString(" GITHUB_SERVER_URL: ${{ github.server_url }}\n") diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index ee376821c0..8f6a3a5109 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -105,6 +105,19 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } + // Parse allowed-github-references configuration + if allowGitHubRefs, exists := outputMap["allowed-github-references"]; exists { + if refsArray, ok := allowGitHubRefs.([]any); ok { + refStrings := []string{} // Initialize as empty slice, not nil + for _, ref := range refsArray { + if refStr, ok := ref.(string); ok { + refStrings = append(refStrings, refStr) + } + } + config.AllowGitHubReferences = refStrings + } + } + // Parse add-labels configuration addLabelsConfig := c.parseAddLabelsConfig(outputMap) if addLabelsConfig != nil {