Harden safeoutputs CLI review fixes

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
pelikhan · Sep 17, 2026 · Sep 16, 2026 · Sep 16, 2026 · Sep 16, 2026 · Sep 16, 2026
commit 13bdf8c7f1e118aee7fd30b7e068a709648593c2
diff --git a/.github/aw/actions-lock.json b/.github/aw/actions-lock.json
@@ -118,10 +118,10 @@
       "version": "v2.0.5",
       "sha": "22d081ff2d3a40755e97629de92e3bcbfa7cf2ed"
     },
-    "docker/build-push-action@v7.3.0": {
+    "docker/build-push-action@v7.4.0": {
       "repo": "docker/build-push-action",
-      "version": "v7.3.0",
-      "sha": "53b7df96c91f9c12dcc8a07bcb9ccacbed38856a"
+      "version": "v7.4.0",
+      "sha": "c3c9e263c25d99ce0380d002d59b67737d91b0dc"
     },
     "docker/login-action@v4.6.0": {
       "repo": "docker/login-action",
@@ -133,10 +133,10 @@
       "version": "v6.2.0",
       "sha": "dc802804100637a589fabce1cb79ff13a1411302"
     },
-    "docker/setup-buildx-action@v4.3.0": {
+    "docker/setup-buildx-action@v4.4.0": {
       "repo": "docker/setup-buildx-action",
-      "version": "v4.3.0",
-      "sha": "37fe631027851001ddb9b187196cc803df7f5f0e"
+      "version": "v4.4.0",
+      "sha": "594f3bf4285d9ea8dc53c9a0c9c4092420091003"
     },
     "erlef/setup-beam@v1.24.1": {
       "repo": "erlef/setup-beam",

diff --git a/actions/setup/js/handle_agent_failure.cjs b/actions/setup/js/handle_agent_failure.cjs
@@ -3176,11 +3176,44 @@ const SAFEOUTPUTS_CLI_EXCERPT_MAX_LENGTH = 300;
  * @returns {boolean}
  */
 function isDroppedPipeSafeOutputsCommand(line) {
-  const match = /(printf|echo)\b([^\n]*?)safeoutputs\b/.exec(line);
-  if (!match) return false;
-  // A pipe (or command separator) between the writer and `safeoutputs` means the
-  // binary does run; only the separator-free form is silently skipped.
-  return !/[|;&><]/.test(match[2]);
+  let quote = "";
+  let writerSeen = false;
+  for (let i = 0; i < line.length;) {
+    const char = line[i];
+    if (quote) {
+      if (char === "\\" && quote === '"' && i + 1 < line.length) i += 2;
+      else if (char === quote) {
+        quote = "";
+        i++;
+      } else i++;
+      continue;
+    }
+    if (char === "'" || char === '"' || char === "`") {
+      quote = char;
+      i++;
+      continue;
+    }
+    if ("|;&><".includes(char)) return false;
+    if (/\s/.test(char)) {
+      i++;
+      continue;
+    }
+    const start = i;
+    while (i < line.length && !/\s/.test(line[i]) && !"'\"`|;&><".includes(line[i])) i++;
+    const word = line.slice(start, i);
+    if (word === "printf" || word === "echo") writerSeen = true;
+    else if (word === "safeoutputs") return writerSeen;
+  }
+  return false;
+}
+
+/**
+ * Escape command text before inserting it in a fenced Markdown block.
+ * @param {string} command - Redacted command excerpt
+ * @returns {string}
+ */
+function escapeMarkdownCodeFence(command) {
+  return command.replace(/`/g, "\\`");
 }
 
 /**
@@ -3217,13 +3250,14 @@ function buildSafeOutputsCliInvocationContext(stdioLogPathOverride) {
     if (!/(^|[|;&(\s"'`])safeoutputs\s+[a-z_][a-z0-9_]*/i.test(line)) continue;
     const redacted = applyAddMaskRedaction(line, maskedValues);
     const truncated = redacted.length > SAFEOUTPUTS_CLI_EXCERPT_MAX_LENGTH ? `${redacted.slice(0, SAFEOUTPUTS_CLI_EXCERPT_MAX_LENGTH)}…` : redacted;
+    const escaped = escapeMarkdownCodeFence(truncated);
     if (isDroppedPipeSafeOutputsCommand(line)) {
       hasDroppedPipe = true;
     }
-    if (seen.has(truncated)) continue;
-    seen.add(truncated);
+    if (seen.has(escaped)) continue;
+    seen.add(escaped);
     if (commands.length < SAFEOUTPUTS_CLI_EXCERPT_LIMIT) {
-      commands.push(truncated);
+      commands.push(escaped);
     }
   }
 

diff --git a/actions/setup/js/handle_agent_failure.test.cjs b/actions/setup/js/handle_agent_failure.test.cjs
@@ -2598,8 +2598,17 @@ describe("handle_agent_failure", () => {
     it("classifies piped and unpiped invocations", () => {
       expect(isDroppedPipeSafeOutputsCommand(`printf '{"message":"x"}' safeoutputs noop .`)).toBe(true);
       expect(isDroppedPipeSafeOutputsCommand(`printf '{"message":"x"}' | safeoutputs noop .`)).toBe(false);
+      expect(isDroppedPipeSafeOutputsCommand(`printf '{"message":">"}' safeoutputs noop .`)).toBe(true);
+      expect(isDroppedPipeSafeOutputsCommand(`echo "$(safeoutputs noop .)"`)).toBe(false);
       expect(isDroppedPipeSafeOutputsCommand(`safeoutputs noop --message "x"`)).toBe(false);
     });
+
+    it("does not render Markdown fence delimiters from command excerpts", () => {
+      fs.writeFileSync(stdioLogPath, "safeoutputs noop --message '```\\n'");
+      const result = buildSafeOutputsCliInvocationContext();
+      expect(result).toContain("\\`\\`\\`");
+      expect(result).not.toContain("\n```\n`\n");
+    });
   });
 
   describe("buildEngineFailureContext", () => {

diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs
@@ -743,18 +743,27 @@ function tryExtractJsonFieldFromStdin(trimmedStdin, canonicalKey, schemaProperti
  * Detect inline JSON payload mode: a single positional argument that is a JSON
  * object, e.g. `safeoutputs noop '{"message":"done"}'`.
  *
- * Returns the trimmed JSON string when the sole argument looks like a JSON
- * object (starts with `{`), otherwise null.  Detection is intentionally
- * permissive about validity so malformed JSON produces a loud parse error
- * instead of being silently skipped as a non-flag argument.
+ * Returns the trimmed JSON string when the sole positional argument looks like
+ * JSON, otherwise null. Trailing flags (such as `--json`) are ignored when
+ * finding the positional argument. Detection is intentionally permissive about
+ * invalid objects and arrays so they produce a loud parse error instead of
+ * being silently skipped as non-flag arguments.
  *
  * @param {string[]} args - User arguments after the tool name
  * @returns {string | null}
  */
 function findInlineJsonPayloadArg(args) {
-  if (args.length !== 1) return null;
-  const trimmed = args[0].trim();
-  return trimmed.startsWith("{") ? trimmed : null;
+  if (args.some(arg => arg.startsWith("--") && arg !== "--json" && !arg.startsWith("--json="))) return null;
+  const positionalArgs = args.filter(arg => arg !== "--json" && !arg.startsWith("--json="));
+  if (positionalArgs.length !== 1) return null;
+  const trimmed = positionalArgs[0].trim();
+  if (trimmed.startsWith("{") || trimmed.startsWith("[")) return trimmed;
+  try {
+    JSON.parse(trimmed);
+    return trimmed;
+  } catch {
+    return null;
+  }
 }
 
 /**
@@ -812,7 +821,7 @@ function parseToolArgs(args, schemaProperties = {}, stdinContent = null) {
       const canonicalKey = resolveSchemaPropertyKey(key, schemaProperties, normalizedSchemaKeyMap, ambiguousNormalizedSchemaKeys);
       result[canonicalKey] = value;
     }
-    return { args: result, json: false };
+    return { args: result, json: args.some(arg => arg === "--json" || arg.startsWith("--json=")) };
   }
   // Trimmed stdin content used in both JSON payload mode and per-field stdin mode.
   const trimmedStdin = stdinContent !== null ? stdinContent.trim() : null;
@@ -1659,8 +1668,8 @@ async function main() {
   // leave the agent believing a safe output was emitted when none was.  Structured
   // payload modes are exempt: an explicit `{}` payload legitimately yields no arguments.
   const usedStructuredPayload = shouldReadStdin || findInlineJsonPayloadArg(toolUserArgs) !== null;
-  if (serverName === SAFEOUTPUTS_SERVER_NAME && Object.keys(toolArgs).length === 0 && toolUserArgs.length > 0 && !jsonOutput && !usedStructuredPayload) {
-    const message = `no arguments were recognized for '${toolName}' from: ${toolUserArgs.join(" ")}. Pass a JSON object inline (${serverName} ${toolName} '{"key":"value"}'), use --key value flags, or pipe JSON on stdin with '.'.`;
+  if (serverName === SAFEOUTPUTS_SERVER_NAME && Object.keys(toolArgs).length === 0 && toolUserArgs.length > 0 && !usedStructuredPayload) {
+    const message = `no arguments were recognized for '${toolName}'. Pass a JSON object inline (${serverName} ${toolName} '{"key":"value"}'), use --key value flags, or pipe JSON on stdin with '.'.`;
     auditLog(serverName, { event: "unrecognized_args", tool: toolName });
     process.stderr.write(`Error: ${message}\n`);
     core.setFailed(`[${serverName}] ${message}`);

diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs
@@ -913,6 +913,9 @@ describe("mcp_cli_bridge.cjs", () => {
     it("detects a single JSON object argument", () => {
       expect(findInlineJsonPayloadArg(['{"message":"done"}'])).toBe('{"message":"done"}');
       expect(findInlineJsonPayloadArg([' {"message":"done"} '])).toBe('{"message":"done"}');
+      expect(findInlineJsonPayloadArg(['{"message":"done"}', "--json"])).toBe('{"message":"done"}');
+      expect(findInlineJsonPayloadArg(["[1,2]"])).toBe("[1,2]");
+      expect(findInlineJsonPayloadArg(["null"])).toBe("null");
     });
 
     it("does not treat flags, sentinels or plain text as inline JSON", () => {
@@ -947,7 +950,14 @@ describe("mcp_cli_bridge.cjs", () => {
     });
 
     it("throws when the inline JSON argument is not an object", () => {
-      expect(() => parseToolArgs(["{} extra"], {})).toThrow(/inline JSON argument is not valid JSON/i);
+      expect(() => parseToolArgs(["[1,2]"], {})).toThrow(/inline JSON argument must be a JSON object/i);
+      expect(() => parseToolArgs(["null"], {})).toThrow(/inline JSON argument must be a JSON object/i);
+    });
+
+    it("preserves --json when used with an inline payload", () => {
+      const { args, json } = parseToolArgs(['{"message":"done"}', "--json"], {});
+      expect(args).toEqual({ message: "done" });
+      expect(json).toBe(true);
     });
   });
 
@@ -1803,6 +1813,16 @@ describe("mcp_cli_bridge.cjs", () => {
       expect(toolsCallBody.params.arguments).toEqual({ title: "Inline payload" });
     });
 
+    it("calls the tool with inline JSON followed by --json", async () => {
+      setupMainCall(requiredInputTools, ["create_issue", '{"title":"Inline payload"}', "--json"]);
+
+      await main();
+
+      const toolsCallBody = recordedBodies.find(b => b.method === "tools/call");
+      expect(toolsCallBody).toBeDefined();
+      expect(toolsCallBody.params.arguments).toEqual({ title: "Inline payload" });
+    });
+
     it("allows an explicit empty inline JSON payload for a zero-input tool", async () => {
       setupMainCall(zeroInputTools, ["dispatch_code_factory", "{}"]);
 
@@ -1825,6 +1845,16 @@ describe("mcp_cli_bridge.cjs", () => {
       expect(global.core.setFailed).toHaveBeenCalledWith(expect.stringContaining("no arguments were recognized"));
     });
 
+    it("fails when --json accompanies an unrecognized positional argument", async () => {
+      setupMainCall(requiredInputTools, ["create_issue", "--json", "no action needed"]);
+
+      await main();
+
+      const toolsCallBody = recordedBodies.find(b => b.method === "tools/call");
+      expect(toolsCallBody).toBeUndefined();
+      expect(global.core.setFailed).toHaveBeenCalledWith(expect.stringContaining("no arguments were recognized"));
+    });
+
     it("still shows help for no-flag piped stdin when stdin is truly empty", async () => {
       const origIsTTY = process.stdin.isTTY;
       // @ts-ignore