diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index 2186b18eeb6..17fa51c16c6 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -778,6 +778,61 @@ function resolveSchemaPropertyKey(key, schemaProperties, normalizedSchemaKeyMap, return normalizedSchemaKeyMap.get(normalized) || key; } +const CLI_UNESCAPED_TEXT_ARG_KEYS = new Set(["body", "draftbody"]); + +/** + * Unescape a conservative subset of JSON-style escape sequences in a CLI text argument. + * + * This is only applied to body-like text fields where authors commonly expect + * `\n` and similar escapes to become literal formatting characters, matching + * JSON stdin mode more closely without mutating unrelated string arguments + * such as file paths or regex patterns. + * + * Supported sequences: + * `\n` → newline + * `\t` → tab + * `\r` → carriage return + * `\b` → backspace + * `\f` → form feed + * `\\` → single backslash + * `\uXXXX` → Unicode code point + * Any other `\X` is left unchanged (the backslash is preserved). + * + * @param {string} str - Raw CLI string argument + * @returns {string} String with supported escape sequences replaced by literal characters + */ +function unescapeCliStringArg(str) { + return str.replace(/\\(?:u([0-9a-fA-F]{4})|([\s\S]))/g, (match, hex, char) => { + if (hex) { + return String.fromCharCode(Number.parseInt(hex, 16)); + } + switch (char) { + case "n": + return "\n"; + case "t": + return "\t"; + case "r": + return "\r"; + case "b": + return "\b"; + case "f": + return "\f"; + case "\\": + return "\\"; + default: + return match; + } + }); +} + +/** + * @param {string} key + * @returns {boolean} + */ +function shouldUnescapeCliTextArg(key) { + return CLI_UNESCAPED_TEXT_ARG_KEYS.has(normalizeSchemaKey(key)); +} + /** * Parse and coerce a CLI argument value based on the MCP tool schema property type. * @@ -857,6 +912,13 @@ function coerceToolArgValue(key, rawValue, schemaProperty, existingValue, allowN } } + // Only unescape body-like text fields. Shell argv is already decoded, so + // applying a second escape pass to arbitrary string fields would corrupt + // values like Windows paths and regex patterns. + if (types.includes("string") && shouldUnescapeCliTextArg(key)) { + return unescapeCliStringArg(rawValue); + } + // When schema metadata is unavailable (e.g. empty tools cache), apply // conservative numeric coercion fallback for CLI ergonomics. if (allowNumericFallback && types.length === 0) { @@ -1376,6 +1438,7 @@ if (require.main === module) { module.exports = { parseToolArgs, coerceToolArgValue, + unescapeCliStringArg, extractJSONRPCMessages, renderProgressMessages, formatResponse, diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs index 1478efe4b6f..4d60721582c 100644 --- a/actions/setup/js/mcp_cli_bridge.test.cjs +++ b/actions/setup/js/mcp_cli_bridge.test.cjs @@ -3,7 +3,19 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { ensureSafeOutputsTools, formatResponse, getToolCallTimeoutMs, hasStdinJsonPayload, parseToolArgs, readStdinSync, shouldShowToolHelpForEmptyArgs, showHelp, showToolHelp, writeStdoutAndFlush } from "./mcp_cli_bridge.cjs"; +import { + ensureSafeOutputsTools, + formatResponse, + getToolCallTimeoutMs, + hasStdinJsonPayload, + parseToolArgs, + readStdinSync, + shouldShowToolHelpForEmptyArgs, + showHelp, + showToolHelp, + unescapeCliStringArg, + writeStdoutAndFlush, +} from "./mcp_cli_bridge.cjs"; describe("mcp_cli_bridge.cjs", () => { let originalCore; @@ -750,6 +762,98 @@ describe("mcp_cli_bridge.cjs", () => { }); }); + describe("unescapeCliStringArg", () => { + it("converts \\n to an actual newline", () => { + expect(unescapeCliStringArg("Hello\\nWorld")).toBe("Hello\nWorld"); + }); + + it("converts \\t to a tab character", () => { + expect(unescapeCliStringArg("col1\\tcol2")).toBe("col1\tcol2"); + }); + + it("converts \\r to a carriage return", () => { + expect(unescapeCliStringArg("line1\\rline2")).toBe("line1\rline2"); + }); + + it("converts \\b to a backspace character", () => { + expect(unescapeCliStringArg("abc\\bdef")).toBe("abc\bdef"); + }); + + it("converts \\f to a form-feed character", () => { + expect(unescapeCliStringArg("page1\\fpage2")).toBe("page1\fpage2"); + }); + + it("converts \\\\ to a single backslash", () => { + expect(unescapeCliStringArg("path\\\\to\\\\file")).toBe("path\\to\\file"); + }); + + it("converts \\uXXXX escapes to their Unicode code points", () => { + expect(unescapeCliStringArg("quote:\\u2019")).toBe("quote:’"); + }); + + it("converts \\\\n to a literal backslash followed by n (not a newline)", () => { + // \\n in the CLI arg should become \n (backslash + n), not a newline + expect(unescapeCliStringArg("Hello\\\\nWorld")).toBe("Hello\\nWorld"); + }); + + it("leaves unknown escape sequences unchanged", () => { + expect(unescapeCliStringArg("value\\xunknown")).toBe("value\\xunknown"); + }); + + it("handles multiple escape sequences in the same string", () => { + expect(unescapeCliStringArg("line1\\nline2\\nline3")).toBe("line1\nline2\nline3"); + }); + + it("returns a plain string unchanged when no escape sequences are present", () => { + expect(unescapeCliStringArg("no escapes here")).toBe("no escapes here"); + }); + }); + + describe("parseToolArgs — body-like string escape unescaping", () => { + it("unescapes \\n in body CLI flag arguments", () => { + const schemaProperties = { body: { type: "string" } }; + const { args } = parseToolArgs(["--body", "Hello\\nWorld"], schemaProperties); + expect(args).toEqual({ body: "Hello\nWorld" }); + }); + + it("unescapes \\n in body --key=value arguments", () => { + const schemaProperties = { body: { type: "string" } }; + const { args } = parseToolArgs(["--body=Hello\\nWorld"], schemaProperties); + expect(args).toEqual({ body: "Hello\nWorld" }); + }); + + it("unescapes \\n for nullable draft body fields", () => { + const schemaProperties = { draft_body: { type: ["string", "null"] } }; + const { args } = parseToolArgs(["--draft-body", "line1\\nline2"], schemaProperties); + expect(args).toEqual({ draft_body: "line1\nline2" }); + }); + + it("does not unescape generic string fields like paths", () => { + const schemaProperties = { path: { type: "string" } }; + const { args } = parseToolArgs(["--path", "C:\\temp\\new_file"], schemaProperties); + expect(args).toEqual({ path: "C:\\temp\\new_file" }); + }); + + it("does not unescape \\n when schema type is integer", () => { + // A value with \n for an integer field should not be unescaped (it would fail coercion anyway) + const schemaProperties = { count: { type: "integer" } }; + const { args } = parseToolArgs(["--count", "5\\n"], schemaProperties); + // "5\n" is not a valid integer, falls through to rawValue + expect(args).toEqual({ count: "5\\n" }); + }); + + it("produces actual newlines in body fields matching JSON stdin mode behaviour", () => { + // Verify that --body "title\n\nbody" (CLI flags) gives the same result as + // JSON stdin with {"body":"title\n\nbody"} + const schemaProperties = { body: { type: ["string", "null"] } }; + + const { args: cliArgs } = parseToolArgs(["--body", "title\\n\\nbody"], schemaProperties); + const { args: jsonArgs } = parseToolArgs(["."], schemaProperties, '{"body":"title\\n\\nbody"}'); + + expect(cliArgs).toEqual(jsonArgs); + }); + }); + describe("writeStdoutAndFlush", () => { it("resolves immediately when stdout.write returns true (no backpressure)", async () => { // The beforeEach mock captures chunks and returns true (no backpressure).