diff --git a/.github/workflows/cloclo.lock.yml b/.github/workflows/cloclo.lock.yml index 98bc81401e..17f0084d54 100644 --- a/.github/workflows/cloclo.lock.yml +++ b/.github/workflows/cloclo.lock.yml @@ -27,7 +27,7 @@ # - shared/jqschema.md # - shared/mcp/serena-go.md # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"f0cbb935127a3229a5af7be73bfbe1e18cd4d83455bfab734bc621259e3a9b8c","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"ca1d4f87f9266a8717fba7feba5d81fcd0712569366788866211a0ac56e0f30a","strict":true} name: "/cloclo" "on": @@ -1647,7 +1647,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,anthropic.com,api.anthropic.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,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,files.pythonhosted.org,ghcr.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,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,pypi.org,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,sentry.io,statsig.anthropic.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"max\":1},\"create_pull_request\":{\"expires\":48,\"labels\":[\"automation\",\"cloclo\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[cloclo] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"max\":1},\"create_pull_request\":{\"excluded_files\":[\".github/workflows/*.lock.yml\"],\"expires\":48,\"labels\":[\"automation\",\"cloclo\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[cloclo] \"},\"missing_data\":{},\"missing_tool\":{}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/cloclo.md b/.github/workflows/cloclo.md index 3df5ee3b71..cb1efe20bc 100644 --- a/.github/workflows/cloclo.md +++ b/.github/workflows/cloclo.md @@ -32,6 +32,8 @@ safe-outputs: expires: 2d title-prefix: "[cloclo] " labels: [automation, cloclo] + excluded-files: + - ".github/workflows/*.lock.yml" add-comment: max: 1 messages: diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 1c9fe47e0d..6959948d87 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -590,3 +590,171 @@ ${diffs} expect(result.error).toContain("protected files"); }); }); + +// excluded-files exclusion list +// ────────────────────────────────────────────────────── + +describe("create_pull_request - excluded-files exclusion list", () => { + let tempDir; + let originalEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + process.env.GITHUB_REPOSITORY = "test-owner/test-repo"; + process.env.GITHUB_BASE_REF = "main"; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-ignored-test-")); + + global.core = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + startGroup: vi.fn(), + endGroup: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(undefined), + }, + }; + global.github = { + rest: { + pulls: { + create: vi.fn().mockResolvedValue({ data: { number: 1, html_url: "https://github.com/test" } }), + }, + repos: { + get: vi.fn().mockResolvedValue({ data: { default_branch: "main" } }), + }, + }, + graphql: vi.fn(), + }; + global.context = { + eventName: "workflow_dispatch", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: {}, + }; + global.exec = { + exec: vi.fn().mockResolvedValue(0), + getExecOutput: vi.fn().mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }), + }; + + // Clear module cache so globals are picked up fresh + delete require.cache[require.resolve("./create_pull_request.cjs")]; + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in originalEnv)) { + delete process.env[key]; + } + } + Object.assign(process.env, originalEnv); + + if (tempDir && fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + + delete global.core; + delete global.github; + delete global.context; + delete global.exec; + vi.clearAllMocks(); + }); + + /** + * Creates a minimal git patch touching the given file paths. + */ + function createPatchWithFiles(...filePaths) { + const diffs = filePaths + .map( + p => `diff --git a/${p} b/${p} +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/${p} +@@ -0,0 +1 @@ ++content +` + ) + .join("\n"); + return `From abc123 Mon Sep 17 00:00:00 2001 +From: Test Author +Date: Mon, 1 Jan 2024 00:00:00 +0000 +Subject: [PATCH] Test commit + +${diffs} +-- +2.34.1 +`; + } + + function writePatch(content) { + const p = path.join(tempDir, "test.patch"); + fs.writeFileSync(p, content); + return p; + } + + it("should ignore files matching excluded-files patterns (not blocked by allowed-files)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: the patch already contains only the non-ignored file. + const patchPath = writePatch(createPatchWithFiles("src/index.js")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + excluded_files: ["auto-generated/**"], + allowed_files: ["src/**"], + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.error || "").not.toContain("outside the allowed-files list"); + }); + + it("should still block non-ignored files that violate the allowed-files list", async () => { + const patchPath = writePatch(createPatchWithFiles("src/index.js", "other/file.txt")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + excluded_files: ["auto-generated/**"], + allowed_files: ["src/**"], + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("other/file.txt"); + expect(result.error).not.toContain("src/index.js"); + }); + + it("should ignore files matching excluded-files patterns (not blocked by protected-files)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: the patch already contains only the non-ignored file. + const patchPath = writePatch(createPatchWithFiles("src/index.js")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + excluded_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.error || "").not.toContain("protected files"); + }); + + it("should allow when all patch files are ignored (even with allowed-files set)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: all files were excluded so the patch file is absent. + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + excluded_files: ["dist/**"], + allowed_files: ["src/**"], + }); + // No patch file — simulates all changes being ignored at generation time + const result = await handler({ patch_path: path.join(tempDir, "nonexistent.patch"), title: "Test PR", body: "" }, {}); + + // No patch → treated as no changes, not an allowlist violation + expect(result.error || "").not.toContain("outside the allowed-files list"); + }); +}); diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index f5531237a6..50684574e2 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -96,6 +96,9 @@ function getPatchPathForRepo(branchName, repoSlug) { * Required for multi-repo scenarios to prevent patch file collisions. * @param {string} [options.token] - GitHub token for git authentication. Falls back to GITHUB_TOKEN env var. * Use this for cross-repo scenarios where a custom PAT with access to the target repo is needed. + * @param {string[]} [options.excludedFiles] - Glob patterns for files to exclude from the patch. + * Each pattern is passed to `git format-patch` as a `:(exclude)` magic pathspec so + * matching files are never included in the generated patch. * @returns {Promise} Object with patch info or error */ async function generateGitPatch(branchName, baseBranch, options = {}) { @@ -103,6 +106,21 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { // Support custom cwd for multi-repo scenarios const cwd = options.cwd || process.env.GITHUB_WORKSPACE || process.cwd(); // Include repo slug in patch path for multi-repo disambiguation + + // Build :(exclude) pathspec arguments from the excludedFiles option. + // These are appended after "--" so git treats them as pathspecs, not revisions. + // Using git's native pathspec magic keeps the exclusions out of the patch entirely + // without any post-processing of the generated patch file. + const excludePathspecs = Array.isArray(options.excludedFiles) && options.excludedFiles.length > 0 ? options.excludedFiles.map(p => `:(exclude)${p}`) : []; + + /** + * Returns the arguments to append to a format-patch call when excludedFiles is set. + * Produces ["--", ":(exclude)pattern1", ":(exclude)pattern2", ...] or []. + * @returns {string[]} + */ + function excludeArgs() { + return excludePathspecs.length > 0 ? ["--", ...excludePathspecs] : []; + } const patchPath = options.repoSlug ? getPatchPathForRepo(branchName, options.repoSlug) : getPatchPath(branchName); // Validate baseBranch early to avoid confusing git errors (e.g., origin/undefined) @@ -235,7 +253,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { if (commitCount > 0) { // Generate patch from the determined base to the branch - const patchContent = execGitSync(["format-patch", `${baseRef}..${branchName}`, "--stdout"], { cwd }); + const patchContent = execGitSync(["format-patch", `${baseRef}..${branchName}`, "--stdout", ...excludeArgs()], { cwd }); if (patchContent && patchContent.trim()) { fs.writeFileSync(patchPath, patchContent, "utf8"); @@ -304,7 +322,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { if (commitCount > 0) { // Generate patch from GITHUB_SHA to HEAD - const patchContent = execGitSync(["format-patch", `${githubSha}..HEAD`, "--stdout"], { cwd }); + const patchContent = execGitSync(["format-patch", `${githubSha}..HEAD`, "--stdout", ...excludeArgs()], { cwd }); if (patchContent && patchContent.trim()) { fs.writeFileSync(patchPath, patchContent, "utf8"); @@ -339,8 +357,8 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { if (remoteRefs.length > 0) { // Find commits on current branch not reachable from any remote ref // This gets commits the agent added that haven't been pushed anywhere - const excludeArgs = remoteRefs.flatMap(ref => ["--not", ref]); - const revListArgs = ["rev-list", "--count", branchName, ...excludeArgs]; + const remoteExcludeArgs = remoteRefs.flatMap(ref => ["--not", ref]); + const revListArgs = ["rev-list", "--count", branchName, ...remoteExcludeArgs]; const commitCount = parseInt(execGitSync(revListArgs, { cwd }).trim(), 10); debugLog(`Strategy 3: Found ${commitCount} commits not reachable from any remote ref`); @@ -362,7 +380,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { } if (baseCommit) { - const patchContent = execGitSync(["format-patch", `${baseCommit}..${branchName}`, "--stdout"], { cwd }); + const patchContent = execGitSync(["format-patch", `${baseCommit}..${branchName}`, "--stdout", ...excludeArgs()], { cwd }); if (patchContent && patchContent.trim()) { fs.writeFileSync(patchPath, patchContent, "utf8"); diff --git a/actions/setup/js/generate_git_patch.test.cjs b/actions/setup/js/generate_git_patch.test.cjs index 1dd690a024..4212f44ea6 100644 --- a/actions/setup/js/generate_git_patch.test.cjs +++ b/actions/setup/js/generate_git_patch.test.cjs @@ -1,4 +1,11 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { execSync } from "child_process"; +import { createRequire } from "module"; +import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; + +const require = createRequire(import.meta.url); describe("generateGitPatch", () => { let originalEnv; @@ -376,3 +383,105 @@ describe("getPatchPath", () => { expect(getPatchPath("Feature/BRANCH")).toBe("/tmp/gh-aw/aw-feature-branch.patch"); }); }); + +// ────────────────────────────────────────────────────── +// excludedFiles option – end-to-end with a real git repo +// ────────────────────────────────────────────────────── + +describe("generateGitPatch – excludedFiles option", () => { + let repoDir; + let originalEnv; + + beforeEach(() => { + originalEnv = { GITHUB_WORKSPACE: process.env.GITHUB_WORKSPACE, GITHUB_SHA: process.env.GITHUB_SHA }; + + // Set up the core global required by git_helpers.cjs + global.core = { debug: () => {}, info: () => {}, warning: () => {}, error: () => {} }; + + // Create an isolated git repo in a temp directory + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-patch-test-")); + execSync("git init -b main", { cwd: repoDir }); + execSync('git config user.email "test@example.com"', { cwd: repoDir }); + execSync('git config user.name "Test"', { cwd: repoDir }); + + // Initial commit so the repo has a base + fs.writeFileSync(path.join(repoDir, "README.md"), "# Repo\n"); + execSync("git add .", { cwd: repoDir }); + execSync('git commit -m "init"', { cwd: repoDir }); + + // Record the initial commit SHA for GITHUB_SHA (Strategy 2 base) + const sha = execSync("git rev-parse HEAD", { cwd: repoDir }).toString().trim(); + process.env.GITHUB_SHA = sha; + // Clear GITHUB_WORKSPACE so the cwd option is used instead + delete process.env.GITHUB_WORKSPACE; + + // Reset module cache so each test gets a fresh module instance + delete require.cache[require.resolve("./generate_git_patch.cjs")]; + }); + + afterEach(() => { + // Restore env + Object.entries(originalEnv).forEach(([k, v]) => { + if (v !== undefined) process.env[k] = v; + else delete process.env[k]; + }); + // Clean up temp repo + if (repoDir && fs.existsSync(repoDir)) { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + delete require.cache[require.resolve("./generate_git_patch.cjs")]; + delete global.core; + }); + + function commitFiles(files) { + for (const [filePath, content] of Object.entries(files)) { + const abs = path.join(repoDir, filePath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + execSync("git add .", { cwd: repoDir }); + execSync('git commit -m "add files"', { cwd: repoDir }); + } + + it("should include all files when excludedFiles is not set", async () => { + commitFiles({ + "src/index.js": "console.log('hello');\n", + "dist/bundle.js": "/* bundled */\n", + }); + + const { generateGitPatch } = require("./generate_git_patch.cjs"); + const result = await generateGitPatch(null, "main", { cwd: repoDir }); + + expect(result.success).toBe(true); + const patch = fs.readFileSync(result.patchPath, "utf8"); + expect(patch).toContain("src/index.js"); + expect(patch).toContain("dist/bundle.js"); + }); + + it("should exclude files matching excludedFiles patterns from the patch", async () => { + commitFiles({ + "src/index.js": "console.log('hello');\n", + "dist/bundle.js": "/* bundled */\n", + }); + + const { generateGitPatch } = require("./generate_git_patch.cjs"); + const result = await generateGitPatch(null, "main", { cwd: repoDir, excludedFiles: ["dist/**"] }); + + expect(result.success).toBe(true); + const patch = fs.readFileSync(result.patchPath, "utf8"); + expect(patch).toContain("src/index.js"); + expect(patch).not.toContain("dist/bundle.js"); + }); + + it("should return no patch when all files are ignored", async () => { + commitFiles({ + "dist/bundle.js": "/* bundled */\n", + }); + + const { generateGitPatch } = require("./generate_git_patch.cjs"); + const result = await generateGitPatch(null, "main", { cwd: repoDir, excludedFiles: ["dist/**"] }); + + // All changes were excluded — patch is empty so generation reports no changes + expect(result.success).toBe(false); + }); +}); diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index 5ed98e04d5..cab489f711 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -126,18 +126,47 @@ function checkAllowedFiles(patchContent, allowedFilePatterns) { return { hasDisallowedFiles: disallowedFiles.length > 0, disallowedFiles }; } +/** + * Identifies which files in a patch match the given list of excluded-file glob patterns. + * Matching is done against the full file path (e.g. `.github/workflows/ci.yml`). + * + * Glob matching supports `*` (matches any characters except `/`) and `**` (matches + * any characters including `/`). + * + * @param {string} patchContent - The git patch content + * @param {string[]} excludedFilePatterns - Glob patterns for files to exclude + * @returns {{ excludedFiles: string[] }} + */ +function checkExcludedFiles(patchContent, excludedFilePatterns) { + if (!excludedFilePatterns || excludedFilePatterns.length === 0) { + return { excludedFiles: [] }; + } + const allPaths = extractPathsFromPatch(patchContent); + if (allPaths.length === 0) { + return { excludedFiles: [] }; + } + const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); + const compiledPatterns = excludedFilePatterns.map(p => globPatternToRegex(p)); + const excludedFiles = allPaths.filter(p => compiledPatterns.some(re => re.test(p))); + return { excludedFiles }; +} + /** * Evaluates a patch against the configured file-protection policy and returns a * single structured result, eliminating nested branching in callers. * - * The two checks are orthogonal and both must pass: - * 1. If `allowed_files` is set → every file must match at least one pattern (deny if not). - * 2. `protected-files` policy applies independently: "allowed" = skip, "fallback-to-issue" - * = create review issue, default ("blocked") = deny. + * The checks are applied in order and all must pass: + * 1. If `allowed_files` is set → every file in the patch must match at least one pattern (deny if not). + * 2. `protected-files` policy applies independently: "allowed" = skip, + * "fallback-to-issue" = create review issue, default ("blocked") = deny. * * To allow an agent to write protected files, set both `allowed-files` (strict scope) and * `protected-files: allowed` (explicit permission) — neither overrides the other implicitly. * + * Note: `excluded-files` are excluded at patch generation time via `git format-patch` + * `:(exclude)` pathspecs (see `generateGitPatch` options), so they will never appear in + * the patch passed to this function. + * * @param {string} patchContent - The git patch content * @param {HandlerConfig} config * @returns {{ action: 'allow' } | { action: 'deny', source: 'allowlist'|'protected', files: string[] } | { action: 'fallback', files: string[] }} @@ -170,4 +199,4 @@ function checkFileProtection(patchContent, config) { return config.protected_files_policy === "fallback-to-issue" ? { action: "fallback", files: allFound } : { action: "deny", source: "protected", files: allFound }; } -module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles, checkFileProtection }; +module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles, checkExcludedFiles, checkFileProtection }; diff --git a/actions/setup/js/manifest_file_helpers.test.cjs b/actions/setup/js/manifest_file_helpers.test.cjs index 1c832bcf87..8b279fbeb9 100644 --- a/actions/setup/js/manifest_file_helpers.test.cjs +++ b/actions/setup/js/manifest_file_helpers.test.cjs @@ -3,7 +3,7 @@ import { describe, it, expect } from "vitest"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const { extractFilenamesFromPatch, checkForManifestFiles, checkAllowedFiles, checkFileProtection } = require("./manifest_file_helpers.cjs"); +const { extractFilenamesFromPatch, checkForManifestFiles, checkAllowedFiles, checkExcludedFiles, checkFileProtection } = require("./manifest_file_helpers.cjs"); describe("manifest_file_helpers", () => { describe("extractFilenamesFromPatch", () => { @@ -336,6 +336,36 @@ index abc..def 100644 }); }); + describe("checkExcludedFiles", () => { + const makePatch = (...filePaths) => filePaths.map(p => `diff --git a/${p} b/${p}\nindex abc..def 100644\n`).join("\n"); + + it("should return empty when patterns is empty", () => { + const result = checkExcludedFiles(makePatch("src/index.js"), []); + expect(result.excludedFiles).toEqual([]); + }); + + it("should return empty for empty patch", () => { + const result = checkExcludedFiles("", ["auto-generated/**"]); + expect(result.excludedFiles).toEqual([]); + }); + + it("should identify files matching ignored patterns", () => { + const result = checkExcludedFiles(makePatch("auto-generated/file.txt", "src/index.js"), ["auto-generated/**"]); + expect(result.excludedFiles).toContain("auto-generated/file.txt"); + expect(result.excludedFiles).not.toContain("src/index.js"); + }); + + it("should return all files when all match ignored patterns", () => { + const result = checkExcludedFiles(makePatch("auto-generated/a.txt", "auto-generated/b.txt"), ["auto-generated/**"]); + expect(result.excludedFiles).toHaveLength(2); + }); + + it("should support ** glob for deep path matching", () => { + const result = checkExcludedFiles(makePatch("dist/deep/nested/bundle.js"), ["dist/**"]); + expect(result.excludedFiles).toContain("dist/deep/nested/bundle.js"); + }); + }); + describe("checkFileProtection", () => { const makePatch = (...filePaths) => filePaths.map(p => `diff --git a/${p} b/${p}\nindex abc..def 100644\n`).join("\n"); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 0a2dc34f42..74351e141f 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1165,6 +1165,103 @@ ${diffs} expect(result.error).not.toContain(".changeset/my-fix.md"); }); }); + + // excluded-files exclusion list + // ────────────────────────────────────────────────────── + + describe("excluded-files exclusion list", () => { + /** + * Helper to create a patch that touches only the given file path(s). + */ + function createPatchWithFiles(...filePaths) { + const diffs = filePaths + .map( + p => `diff --git a/${p} b/${p} +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/${p} +@@ -0,0 +1 @@ ++content +` + ) + .join("\n"); + return `From abc123 Mon Sep 17 00:00:00 2001 +From: Test Author +Date: Mon, 1 Jan 2024 00:00:00 +0000 +Subject: [PATCH] Test commit + +${diffs} +-- +2.34.1 +`; + } + + it("should ignore files matching excluded-files patterns (not blocked by allowed-files)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: the patch already contains only the non-ignored file. + const patchPath = createPatchFile(createPatchWithFiles("src/index.js")); + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); + + const module = await loadModule(); + const handler = await module.main({ + excluded_files: ["auto-generated/**"], + allowed_files: ["src/**"], + }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.error || "").not.toContain("outside the allowed-files list"); + }); + + it("should still block non-ignored files that violate the allowed-files list", async () => { + const patchPath = createPatchFile(createPatchWithFiles("src/index.js", "other/file.txt")); + + const module = await loadModule(); + const handler = await module.main({ + excluded_files: ["auto-generated/**"], + allowed_files: ["src/**"], + }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("other/file.txt"); + expect(result.error).not.toContain("src/index.js"); + }); + + it("should ignore files matching excluded-files patterns (not blocked by protected-files)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: the patch already contains only the non-ignored file. + const patchPath = createPatchFile(createPatchWithFiles("src/index.js")); + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); + + const module = await loadModule(); + const handler = await module.main({ + excluded_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.error || "").not.toContain("protected files"); + }); + + it("should allow when all patch files are ignored (even with allowed-files set)", async () => { + // excluded-files are excluded at patch generation time via git :(exclude) pathspecs. + // Simulate post-generation: all files were excluded so no patch file is produced. + const nonexistentPath = path.join(tempDir, "nonexistent.patch"); + + const module = await loadModule(); + const handler = await module.main({ + excluded_files: ["dist/**"], + allowed_files: ["src/**"], + }); + const result = await handler({ patch_path: nonexistentPath }, {}); + + // No patch → treated as no changes, not an allowlist violation + expect(result.error || "").not.toContain("outside the allowed-files list"); + }); + }); }); // ────────────────────────────────────────────────────── diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 8db48a8574..5dfd2a15b6 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -324,6 +324,10 @@ function createHandlers(server, appendSafeOutput, config = {}) { if (prConfig["github-token"]) { patchOptions.token = prConfig["github-token"]; } + // Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time. + if (Array.isArray(prConfig.excluded_files) && prConfig.excluded_files.length > 0) { + patchOptions.excludedFiles = prConfig.excluded_files; + } const patchResult = await generateGitPatch(entry.branch, baseBranch, patchOptions); if (!patchResult.success) { @@ -434,6 +438,10 @@ function createHandlers(server, appendSafeOutput, config = {}) { if (pushConfig["github-token"]) { pushPatchOptions.token = pushConfig["github-token"]; } + // Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time. + if (Array.isArray(pushConfig.excluded_files) && pushConfig.excluded_files.length > 0) { + pushPatchOptions.excludedFiles = pushConfig.excluded_files; + } const patchResult = await generateGitPatch(entry.branch, baseBranch, pushPatchOptions); if (!patchResult.success) { diff --git a/actions/setup/js/types/handler-factory.d.ts b/actions/setup/js/types/handler-factory.d.ts index 9360b9ab50..b40b336a27 100644 --- a/actions/setup/js/types/handler-factory.d.ts +++ b/actions/setup/js/types/handler-factory.d.ts @@ -10,6 +10,8 @@ interface HandlerConfig { max?: number; /** Strict allowlist of glob patterns for files eligible for push/create. Checked independently of protected-files; both checks must pass. */ allowed_files?: string[]; + /** List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. */ + excluded_files?: string[]; /** List of filenames (basenames) whose presence in a patch triggers protected-file handling */ protected_files?: string[]; /** List of path prefixes that trigger protected-file handling when any changed file matches */ diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 8735062df3..b551d22d6e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1418,12 +1418,12 @@ "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." }, "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", "oneOf": [ { "type": "string", "enum": ["all"], - "description": "Allow any authenticated user to trigger the workflow (\u26a0\ufe0f disables permission checking entirely - use with caution)" + "description": "Allow any authenticated user to trigger the workflow (⚠️ disables permission checking entirely - use with caution)" }, { "type": "array", @@ -2287,7 +2287,7 @@ }, "network": { "$comment": "Strict mode requirements: When strict=true, the 'network' field must be present (not null/undefined) and cannot contain standalone wildcard '*' in allowed domains (but patterns like '*.example.com' ARE allowed). This is validated in Go code (pkg/workflow/strict_mode_validation.go) via validateStrictNetwork().", - "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' \u2014 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", + "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' — 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", "examples": [ "defaults", { @@ -2675,7 +2675,7 @@ ] }, "plugins": { - "description": "\u26a0\ufe0f EXPERIMENTAL: Plugin configuration for installing plugins before workflow execution. Supports array format (list of repos/plugin configs) and object format (repos + custom token). Note: Plugin support is experimental and may change in future releases.", + "description": "⚠️ EXPERIMENTAL: Plugin configuration for installing plugins before workflow execution. Supports array format (list of repos/plugin configs) and object format (repos + custom token). Note: Plugin support is experimental and may change in future releases.", "examples": [ ["github/copilot-plugin", "acme/custom-tools"], [ @@ -2872,7 +2872,7 @@ [ { "name": "Verify Post-Steps Execution", - "run": "echo \"\u2705 Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" + "run": "echo \"✅ Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" }, { "name": "Upload Test Results", @@ -5357,6 +5357,13 @@ "type": "boolean", "description": "When true, the random salt suffix is not appended to the agent-specified branch name. Invalid characters are still replaced for security, and casing is always preserved regardless of this setting. Useful when the target repository enforces branch naming conventions (e.g. Jira keys in uppercase such as 'bugfix/BR-329-red'). Defaults to false.", "default": false + }, + "excluded-files": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of glob patterns for files to exclude from the patch. Each pattern is passed to `git format-patch` as a `:(exclude)` magic pathspec, so matching files are stripped by git at generation time and will not appear in the commit. Excluded files are also not subject to `allowed-files` or `protected-files` checks. Supports * (any characters except /) and ** (any characters including /)." } }, "additionalProperties": false, @@ -5559,7 +5566,7 @@ "oneOf": [ { "type": "object", - "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only \u2014 threads on other PRs cannot be resolved.", + "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only — threads on other PRs cannot be resolved.", "properties": { "max": { "description": "Maximum number of review threads to resolve (default: 10) Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", @@ -6408,7 +6415,14 @@ "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + }, + "excluded-files": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of glob patterns for files to exclude from the patch. Each pattern is passed to `git format-patch` as a `:(exclude)` magic pathspec, so matching files are stripped by git at generation time and will not appear in the commit. Excluded files are also not subject to `allowed-files` or `protected-files` checks. Supports * (any characters except /) and ** (any characters including /)." } }, "additionalProperties": false @@ -7176,8 +7190,8 @@ }, "staged-title": { "type": "string", - "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '\ud83c\udfad Preview: {operation}'", - "examples": ["\ud83c\udfad Preview: {operation}", "## Staged Mode: {operation}"] + "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '🎭 Preview: {operation}'", + "examples": ["🎭 Preview: {operation}", "## Staged Mode: {operation}"] }, "staged-description": { "type": "string", @@ -7191,18 +7205,18 @@ }, "run-success": { "type": "string", - "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.'", - "examples": ["\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.", "\u2705 [{workflow_name}]({run_url}) finished."] + "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '✅ Agentic [{workflow_name}]({run_url}) completed successfully.'", + "examples": ["✅ Agentic [{workflow_name}]({run_url}) completed successfully.", "✅ [{workflow_name}]({run_url}) finished."] }, "run-failure": { "type": "string", - "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", - "examples": ["\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "\u274c [{workflow_name}]({run_url}) {status}."] + "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", + "examples": ["❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "❌ [{workflow_name}]({run_url}) {status}."] }, "detection-failure": { "type": "string", - "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", - "examples": ["\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "\u26a0\ufe0f Detection job failed in [{workflow_name}]({run_url})."] + "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", + "examples": ["⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "⚠️ Detection job failed in [{workflow_name}]({run_url})."] }, "agent-failure-issue": { "type": "string", @@ -8343,7 +8357,7 @@ }, "auth": { "type": "array", - "description": "Authentication bindings \u2014 maps logical roles (e.g. 'api-key') to GitHub Actions secret names", + "description": "Authentication bindings — maps logical roles (e.g. 'api-key') to GitHub Actions secret names", "items": { "type": "object", "properties": { diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 5e8bd41372..941b755cd2 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -487,6 +487,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("protected_files", getAllManifestFiles()). AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). AddStringSlice("allowed_files", c.AllowedFiles). + AddStringSlice("excluded_files", c.ExcludedFiles). AddIfTrue("preserve_branch_name", c.PreserveBranchName) return builder.Build() }, @@ -515,6 +516,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("protected_files", getAllManifestFiles()). AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). AddStringSlice("allowed_files", c.AllowedFiles). + AddStringSlice("excluded_files", c.ExcludedFiles). Build() }, "update_pull_request": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 977788db94..1533f9340f 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -34,6 +34,7 @@ type CreatePullRequestsConfig struct { GithubTokenForExtraEmptyCommit string `yaml:"github-token-for-extra-empty-commit,omitempty"` // Token used to push an empty commit to trigger CI events. Use a PAT or "app" for GitHub App auth. ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" pushes the branch but creates a review issue. AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for create. Checked independently of protected-files; both checks must pass. + ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. PreserveBranchName bool `yaml:"preserve-branch-name,omitempty"` // When true, skips the random salt suffix on agent-specified branch names. Invalid characters are still replaced for security; casing is always preserved. Useful when CI enforces branch naming conventions (e.g. Jira keys in uppercase). } diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index a319d4d132..1837162bcf 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -22,6 +22,7 @@ type PushToPullRequestBranchConfig struct { AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories in format "owner/repo" that push to pull request branch can target ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" creates a review issue instead of pushing. AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for push. Checked independently of protected-files; both checks must pass. + ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. } // buildCheckoutRepository generates a checkout step with optional target repository and custom token @@ -146,6 +147,9 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) // Parse allowed-files: list of glob patterns forming a strict allowlist of eligible files pushToBranchConfig.AllowedFiles = ParseStringArrayFromConfig(configMap, "allowed-files", pushToPullRequestBranchLog) + // Parse excluded-files: list of glob patterns for files to exclude via git :(exclude) pathspecs + pushToBranchConfig.ExcludedFiles = ParseStringArrayFromConfig(configMap, "excluded-files", pushToPullRequestBranchLog) + // Parse common base fields with default max of 0 (no limit) c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0) }