Harden safe-output field validation

actions/setup/js/collect_ndjson_output.cjs

@@ -112,12 +112,12 @@ async function main() {
     }
     function validateItemWithSafeJobConfig(item, jobConfig, lineNum) {
       const errors = [];
-      const normalizedItem = { ...item };
+      const normalizedItem = { type: item.type };
       if (!jobConfig.inputs) {
         return {
           isValid: true,
           errors: [],
-          normalizedItem: item,
+          normalizedItem,
         };
       }
       for (const [fieldName, inputSchema] of Object.entries(jobConfig.inputs)) {
@@ -380,16 +380,13 @@ async function main() {
             continue;
           }
           const safeJobConfig = jobOutputType;
-          if (safeJobConfig && safeJobConfig.inputs) {
-            const validation = validateItemWithSafeJobConfig(item, safeJobConfig, i + 1);
-            if (!validation.isValid) {
-              errors.push(...validation.errors);
-              continue;
-            }
-            Object.assign(item, validation.normalizedItem);
+          const validation = validateItemWithSafeJobConfig(item, safeJobConfig, i + 1);
+          if (!validation.isValid) {
+            errors.push(...validation.errors);
+            continue;
           }
           core.info(`Line ${i + 1}: Valid ${itemType} item`);
-          parsedItems.push(item);
+          parsedItems.push(validation.normalizedItem);
         }
       } catch (error) {
         const errorMsg = getErrorMessage(error);

actions/setup/js/collect_ndjson_output.test.cjs

@@ -61,6 +61,12 @@ describe("collect_ndjson_output.cjs", () => {
             },
           },
           add_comment: { defaultMax: 1, fields: { body: { required: !0, type: "string", sanitize: !0, maxLength: 65e3 }, item_number: { issueOrPRNumber: !0 } } },
+          add_labels: { defaultMax: 5, fields: { labels: { required: !0, type: "array" }, item_number: { issueNumberOrTemporaryId: !0 } } },
+          assign_milestone: {
+            defaultMax: 1,
+            customValidation: "requiresOneOf:milestone_number,milestone_title",
+            fields: { issue_number: { issueNumberOrTemporaryId: !0 }, milestone_number: { optionalPositiveInteger: !0 }, milestone_title: { type: "string", sanitize: !0, maxLength: 128 } },
+          },
           create_pull_request: {
             defaultMax: 1,
             fields: {
@@ -258,7 +264,7 @@ describe("collect_ndjson_output.cjs", () => {
     it("should preserve Slack mrkdwn links in custom safe-job string inputs", async () => {
       const testFile = "/tmp/gh-aw/test-ndjson-output.txt";
       const slackText = "Tracking issue: <https://github.com/octo-org/octo-repo/issues/123|Build failure — Build github/gh-aw#456>";
-      const ndjsonContent = JSON.stringify({ type: "post_to_slack", text: slackText });
+      const ndjsonContent = JSON.stringify({ type: "post_to_slack", text: slackText, channel: "security-alerts" });
       fs.writeFileSync(testFile, ndjsonContent);
       process.env.GH_AW_SAFE_OUTPUTS = testFile;
       fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", JSON.stringify({ post_to_slack: { inputs: { text: { type: "string", required: true } } } }));
@@ -881,9 +887,9 @@ describe("collect_ndjson_output.cjs", () => {
           const parsedOutput = JSON.parse(outputCall[1]);
           (expect(parsedOutput.items).toHaveLength(1),
             expect(parsedOutput.items[0].type).toBe("create_issue"),
-            expect(parsedOutput.items[0].priority).toBe(5),
-            expect(parsedOutput.items[0].urgent).toBe(!0),
-            expect(parsedOutput.items[0].assignee).toBe(null),
+            expect(parsedOutput.items[0]).not.toHaveProperty("priority"),
+            expect(parsedOutput.items[0]).not.toHaveProperty("urgent"),
+            expect(parsedOutput.items[0]).not.toHaveProperty("assignee"),
             expect(parsedOutput.errors).toHaveLength(0));
         }),
         it("should attempt repair but fail gracefully with excessive malformed JSON", async () => {
@@ -928,12 +934,7 @@ describe("collect_ndjson_output.cjs", () => {
             outputCall = setOutputCalls.find(call => "output" === call[0]);
           expect(outputCall).toBeDefined();
           const parsedOutput = JSON.parse(outputCall[1]);
-          (expect(parsedOutput.items).toHaveLength(1),
-            expect(parsedOutput.items[0].type).toBe("create_issue"),
-            expect(parsedOutput.items[0].metadata).toBeDefined(),
-            expect(parsedOutput.items[0].metadata.project).toBe("test"),
-            expect(parsedOutput.items[0].metadata.tags).toEqual(["important", "urgent"]),
-            expect(parsedOutput.errors).toHaveLength(0));
+          (expect(parsedOutput.items).toHaveLength(1), expect(parsedOutput.items[0].type).toBe("create_issue"), expect(parsedOutput.items[0]).not.toHaveProperty("metadata"), expect(parsedOutput.errors).toHaveLength(0));
         }),
         it("should handle complex backslash scenarios with graceful failure", async () => {
           const testFile = "/tmp/gh-aw/test-ndjson-output.txt",
@@ -1052,7 +1053,7 @@ describe("collect_ndjson_output.cjs", () => {
           (expect(parsedOutput.items).toHaveLength(1),
             expect(parsedOutput.items[0].type).toBe("create_issue"),
             expect(parsedOutput.items[0].title).toBe("Combined issues"),
-            expect(parsedOutput.items[0].priority).toBe(1),
+            expect(parsedOutput.items[0]).not.toHaveProperty("priority"),
             expect(parsedOutput.errors).toHaveLength(0));
         }));
     }),

actions/setup/js/commit_sha_helpers.cjs

@@ -15,6 +15,22 @@ function normalizeCommitSHA(value) {
   return GIT_COMMIT_SHA_PATTERN.test(normalized) ? normalized : "";
 }
 
+/**
+ * Extract the trusted base commit embedded in the generated patch artifact.
+ *
+ * @param {unknown} patchContent
+ * @returns {string}
+ */
+function extractPatchBaseCommit(patchContent) {
+  if (typeof patchContent !== "string") {
+    return "";
+  }
+  const headerBlock = patchContent.split(/\r?\n\r?\n/, 1)[0];
+  const match = headerBlock.match(/^X-GH-AW-Base-Commit:\s*([0-9a-fA-F]{7,40})\s*$/m);
+  return normalizeCommitSHA(match?.[1]);
+}
+
 module.exports = {
+  extractPatchBaseCommit,
   normalizeCommitSHA,
 };

actions/setup/js/commit_sha_helpers.test.cjs

@@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest";
 import { createRequire } from "module";
 
 const require = createRequire(import.meta.url);
-const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
+const { extractPatchBaseCommit, normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
 
 describe("normalizeCommitSHA", () => {
   it("accepts valid commit SHAs and trims whitespace", () => {
@@ -11,6 +11,21 @@ describe("normalizeCommitSHA", () => {
     expect(normalizeCommitSHA("a".repeat(40))).toBe("a".repeat(40));
   });
 
+  describe("extractPatchBaseCommit", () => {
+    it("extracts a validated base commit from patch metadata", () => {
+      expect(extractPatchBaseCommit("From abc123 Mon Sep 17 00:00:00 2001\nX-GH-AW-Base-Commit: deadbeef\nFrom: Test\n")).toBe("deadbeef");
+    });
+
+    it("ignores missing or malformed patch metadata", () => {
+      expect(extractPatchBaseCommit("From abc123 Mon Sep 17 00:00:00 2001\nFrom: Test\n")).toBe("");
+      expect(extractPatchBaseCommit("X-GH-AW-Base-Commit: main\n")).toBe("");
+    });
+
+    it("ignores metadata-like lines outside the patch header block", () => {
+      expect(extractPatchBaseCommit("From abc123 Mon Sep 17 00:00:00 2001\nFrom: Test\n\nX-GH-AW-Base-Commit: deadbeef\n")).toBe("");
+    });
+  });
+
   it("rejects invalid commit references", () => {
     expect(normalizeCommitSHA("main")).toBe("");
     expect(normalizeCommitSHA("--upload-pack=/bin/echo")).toBe("");

actions/setup/js/create_pull_request.cjs

@@ -33,7 +33,7 @@ const { renderTemplateFromFile, renderFilesList, buildProtectedFileList, getProm
 const { withGitHubHostToken } = require("./git_auth_helpers.cjs");
 const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL } = require("./constants.cjs");
 const { isStagedMode } = require("./safe_output_helpers.cjs");
-const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
+const { extractPatchBaseCommit } = require("./commit_sha_helpers.cjs");
 const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
 const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs");
 const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, getBundlePrerequisites, isShallowOrSparseCheckout, linearizeRangeAsCommit } = require("./git_helpers.cjs");
@@ -1882,10 +1882,10 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead
 
         // Handle branch creation/checkout
         let branchBaseRef = baseBranch;
-        const recordedBaseCommit = normalizeCommitSHA(pullRequestItem.base_commit);
+        const recordedBaseCommit = extractPatchBaseCommit(patchContent);
         if (recordedBaseCommit) {
           core.info(`Patch route base_commit resolved: ${recordedBaseCommit}`);
-          core.info(`Using base_commit from safe output entry for patch apply: ${recordedBaseCommit}`);
+          core.info(`Using base_commit embedded in the patch for patch apply: ${recordedBaseCommit}`);
           try {
             try {
               await exec.exec("git", ["fetch", "origin", recordedBaseCommit, "--depth=1"]);
@@ -1901,8 +1901,6 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead
           } catch (baseCommitError) {
             core.warning(`Recorded base_commit ${recordedBaseCommit} is not available in this checkout (${getErrorMessage(baseCommitError)}); falling back to ${baseBranch}`);
           }
-        } else if (String(pullRequestItem.base_commit ?? "").trim()) {
-          core.warning(`Ignoring invalid base_commit value for patch apply: ${String(pullRequestItem.base_commit).trim()}`);
         }
         core.info(`Branch should not exist locally, creating new branch from base: ${branchName} (${branchBaseRef})`);
         await exec.exec("git", ["checkout", "-b", branchName, branchBaseRef]);
@@ -1997,9 +1995,9 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead
                 // Use the base commit recorded at patch generation time.
                 // The From <sha> header in format-patch output contains the agent's new commit SHA
                 // which does not exist in this checkout, so we cannot derive the base from it.
-                const originalBaseCommit = normalizeCommitSHA(pullRequestItem.base_commit);
+                const originalBaseCommit = extractPatchBaseCommit(patchContent);
                 if (!originalBaseCommit) {
-                  core.warning("No base_commit recorded in safe output entry - fallback not possible");
+                  core.warning("No base_commit embedded in patch - fallback not possible");
                 } else {
                   core.info(`Original base commit from patch generation: ${originalBaseCommit}`);
 

actions/setup/js/create_pull_request.test.cjs

@@ -55,6 +55,7 @@ function ensureDefaultDisclosureHeaderPrompt() {
 beforeEach(() => {
   cleanupCanonicalTransports();
   ensureDefaultDisclosureHeaderPrompt();
+  process.env.GH_AW_PROMPTS_DIR = promptsSourceDir;
 });
 afterEach(() => {
   cleanupCanonicalTransports();
@@ -2848,6 +2849,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
   // Minimal valid format-patch output
   const PATCH_CONTENT =
     `From a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 Mon Sep 17 00:00:00 2001\n` +
+    `X-GH-AW-Base-Commit: ${MOCK_BASE_COMMIT_SHA}\n` +
     `From: Test Author <test@example.com>\n` +
     `Date: Wed, 26 Mar 2026 12:00:00 +0000\n` +
     `Subject: [PATCH] Test change\n\n` +
@@ -2868,7 +2870,6 @@ describe("create_pull_request - patch apply fallback to original base commit", (
     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-fallback-test-"));
     patchFilePath = path.join(tempDir, "test.patch");
     fs.writeFileSync(patchFilePath, PATCH_CONTENT, "utf8");
@@ -2963,7 +2964,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
     return false;
   }
 
-  it("should create the PR branch from normalized base_commit before applying the patch when available", async () => {
+  it("should create the PR branch from the patch-embedded base commit when available", async () => {
     global.exec = {
       exec: vi.fn().mockResolvedValue(0),
       getExecOutput: vi.fn().mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" }),
@@ -2979,7 +2980,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
     expect(checkoutWithBaseCommit).toBeTruthy();
   });
 
-  it("should ignore invalid base_commit values when creating the branch", async () => {
+  it("should ignore agent-supplied base_commit values when creating the branch", async () => {
     global.exec = {
       exec: vi.fn().mockResolvedValue(0),
       getExecOutput: vi.fn().mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" }),
@@ -2991,7 +2992,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
 
     expect(result.success).toBe(true);
     expect(global.exec.exec).not.toHaveBeenCalledWith("git", ["cat-file", "-e", "not-a-sha --bad"]);
-    expect(global.core.warning).toHaveBeenCalledWith("Ignoring invalid base_commit value for patch apply: not-a-sha --bad");
+    expect(global.core.warning).not.toHaveBeenCalledWith(expect.stringContaining("base_commit"));
   });
 
   it("should fall back to base branch when base_commit is unavailable", async () => {
@@ -3221,6 +3222,8 @@ describe("create_pull_request - patch apply fallback to original base commit", (
   });
 
   it("should return error when no base_commit is provided and git am --3way fails", async () => {
+    const patchWithoutBaseCommit = PATCH_CONTENT.replace(`X-GH-AW-Base-Commit: ${MOCK_BASE_COMMIT_SHA}\n`, "");
+    fs.writeFileSync(canonicalPatchPath("test-branch"), patchWithoutBaseCommit, "utf8");
     global.exec = {
       exec: vi.fn().mockImplementation((cmd, args) => {
         if (isGitAm3Way(cmd, args)) {
@@ -3241,7 +3244,7 @@ describe("create_pull_request - patch apply fallback to original base commit", (
 
     expect(result.success).toBe(false);
     expect(result.error).toBe("Failed to apply patch");
-    expect(global.core.warning).toHaveBeenCalledWith("No base_commit recorded in safe output entry - fallback not possible");
+    expect(global.core.warning).toHaveBeenCalledWith("No base_commit embedded in patch - fallback not possible");
   });
 
   it("should reuse existing remote branch when preserve-branch-name and recreate-ref are true (force-delete then recreate)", async () => {

actions/setup/js/generate_git_patch.cjs

@@ -29,6 +29,17 @@ function debugLog(message) {
   }
 }
 
+function embedBaseCommit(patchContent, baseCommitSha) {
+  if (!baseCommitSha || typeof patchContent !== "string") {
+    return patchContent;
+  }
+  const firstNewline = patchContent.indexOf("\n");
+  if (firstNewline < 0) {
+    return patchContent;
+  }
+  return `${patchContent.slice(0, firstNewline + 1)}X-GH-AW-Base-Commit: ${baseCommitSha}\n${patchContent.slice(firstNewline + 1)}`;
+}
+
 /**
  * Generates a git patch file for the current changes
  * @param {string} branchName - The branch name to generate patch for
@@ -290,7 +301,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
           const patchContent = execGitSync(["format-patch", `${baseRef}..${tipRef}`, "--stdout", ...excludeArgs()], { cwd });
 
           if (patchContent && patchContent.trim()) {
-            fs.writeFileSync(patchPath, patchContent, "utf8");
+            fs.writeFileSync(patchPath, embedBaseCommit(patchContent, baseCommitSha), "utf8");
             patchGenerated = true;
             debugLog(`Strategy 1: SUCCESS - Generated patch with ${patchContent.split("\n").length} lines`);
           }
@@ -407,7 +418,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
               const patchContent = execGitSync(["format-patch", `${githubSha}..HEAD`, "--stdout", ...excludeArgs()], { cwd });
 
               if (patchContent && patchContent.trim()) {
-                fs.writeFileSync(patchPath, patchContent, "utf8");
+                fs.writeFileSync(patchPath, embedBaseCommit(patchContent, baseCommitSha), "utf8");
                 patchGenerated = true;
                 debugLog(`Strategy 2: SUCCESS - Generated patch with ${patchContent.split("\n").length} lines`);
               }
@@ -490,7 +501,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
                 const patchContent = execGitSync(["format-patch", `${bestBaseCommit}..${branchName}`, "--stdout", ...excludeArgs()], { cwd });
 
                 if (patchContent && patchContent.trim()) {
-                  fs.writeFileSync(patchPath, patchContent, "utf8");
+                  fs.writeFileSync(patchPath, embedBaseCommit(patchContent, baseCommitSha), "utf8");
                   patchGenerated = true;
                   debugLog(`Strategy 3: SUCCESS - Generated patch with ${patchContent.split("\n").length} lines`);
                 }