Fix shell injection vulnerability in branch name handling (CWE-78)

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
@@ -96,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)
@@ -112,6 +113,18 @@ 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}")`);
+  }
+  // Fail if base branch name changes during normalization (indicates invalid config)
+  if (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)
   const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined;
 
@@ -411,6 +424,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";

actions/setup/js/create_pull_request.test.cjs

@@ -104,3 +104,133 @@ 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-null" }, // Actual null byte, not escaped string
+      { input: "branch\\x00null", expected: "branch-x00null" }, // Escaped string representation
+    ];
+
+    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"); // Actual null byte
+      expect(result).not.toContain("\\x00"); // Escaped string
+    }
+  });
+
+  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");
+  });
+});

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(", ")}`);