From b4382d8c96efc2773a35991c4774bf07a0a64340 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:48:52 +0000 Subject: [PATCH 1/2] Fix MCP CLI newline escaping in coerceToolArgValue Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/mcp_cli_bridge.cjs | 43 ++++++++++++++ actions/setup/js/mcp_cli_bridge.test.cjs | 76 +++++++++++++++++++++++- 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index 2186b18eeb6..e307450518d 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -778,6 +778,40 @@ function resolveSchemaPropertyKey(key, schemaProperties, normalizedSchemaKeyMap, return normalizedSchemaKeyMap.get(normalized) || key; } +/** + * Unescape standard escape sequences in a CLI string argument. + * + * Converts the same escape sequences that JSON string parsing recognises so + * that agents can write `--body "Hello\nWorld"` and get an actual newline, + * matching the behaviour of JSON stdin mode where `JSON.parse` handles `\n`. + * + * Supported sequences: + * `\n` → newline + * `\t` → tab + * `\r` → carriage return + * `\\` → single backslash + * Any other `\X` is left unchanged (the backslash is preserved). + * + * @param {string} str - Raw CLI string argument + * @returns {string} String with escape sequences replaced by their literal characters + */ +function unescapeCliStringArg(str) { + return str.replace(/\\([\s\S])/g, (match, char) => { + switch (char) { + case "n": + return "\n"; + case "t": + return "\t"; + case "r": + return "\r"; + case "\\": + return "\\"; + default: + return match; + } + }); +} + /** * Parse and coerce a CLI argument value based on the MCP tool schema property type. * @@ -857,6 +891,14 @@ function coerceToolArgValue(key, rawValue, schemaProperty, existingValue, allowN } } + // Unescape standard escape sequences in string-typed values so that agents + // can write `--body "Hello\nWorld"` and get an actual newline. This mirrors + // the behaviour of JSON stdin mode where JSON.parse interprets `\n` as a + // newline character. + if (types.includes("string")) { + 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 +1418,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..5251da408db 100644 --- a/actions/setup/js/mcp_cli_bridge.test.cjs +++ b/actions/setup/js/mcp_cli_bridge.test.cjs @@ -3,7 +3,7 @@ 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 +750,80 @@ 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 \\\\ to a single backslash", () => { + expect(unescapeCliStringArg("path\\\\to\\\\file")).toBe("path\\to\\file"); + }); + + 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 — string escape unescaping", () => { + it("unescapes \\n in string-typed 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 string-typed --key=value arguments", () => { + const schemaProperties = { body: { type: "string" } }; + const { args } = parseToolArgs(["--body=Hello\\nWorld"], schemaProperties); + expect(args).toEqual({ body: "Hello\nWorld" }); + }); + + it("unescapes \\n when type is ['string', 'null']", () => { + const schemaProperties = { body: { type: ["string", "null"] } }; + const { args } = parseToolArgs(["--body", "line1\\nline2"], schemaProperties); + expect(args).toEqual({ body: "line1\nline2" }); + }); + + 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 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" } }; + + 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). From da0a061c54275888885f22ac1bf0de8e998977fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:24:06 +0000 Subject: [PATCH 2/2] Scope CLI string unescaping to body fields Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/mcp_cli_bridge.cjs | 50 +++++++++++++++++------- actions/setup/js/mcp_cli_bridge.test.cjs | 50 +++++++++++++++++++----- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index e307450518d..17fa51c16c6 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -778,25 +778,34 @@ function resolveSchemaPropertyKey(key, schemaProperties, normalizedSchemaKeyMap, return normalizedSchemaKeyMap.get(normalized) || key; } +const CLI_UNESCAPED_TEXT_ARG_KEYS = new Set(["body", "draftbody"]); + /** - * Unescape standard escape sequences in a CLI string argument. + * Unescape a conservative subset of JSON-style escape sequences in a CLI text argument. * - * Converts the same escape sequences that JSON string parsing recognises so - * that agents can write `--body "Hello\nWorld"` and get an actual newline, - * matching the behaviour of JSON stdin mode where `JSON.parse` handles `\n`. + * 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 - * `\\` → single backslash + * `\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 escape sequences replaced by their literal characters + * @returns {string} String with supported escape sequences replaced by literal characters */ function unescapeCliStringArg(str) { - return str.replace(/\\([\s\S])/g, (match, char) => { + 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"; @@ -804,6 +813,10 @@ function unescapeCliStringArg(str) { return "\t"; case "r": return "\r"; + case "b": + return "\b"; + case "f": + return "\f"; case "\\": return "\\"; default: @@ -812,6 +825,14 @@ function unescapeCliStringArg(str) { }); } +/** + * @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. * @@ -891,11 +912,10 @@ function coerceToolArgValue(key, rawValue, schemaProperty, existingValue, allowN } } - // Unescape standard escape sequences in string-typed values so that agents - // can write `--body "Hello\nWorld"` and get an actual newline. This mirrors - // the behaviour of JSON stdin mode where JSON.parse interprets `\n` as a - // newline character. - if (types.includes("string")) { + // 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); } diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs index 5251da408db..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, unescapeCliStringArg, 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; @@ -763,10 +775,22 @@ describe("mcp_cli_bridge.cjs", () => { 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"); @@ -785,23 +809,29 @@ describe("mcp_cli_bridge.cjs", () => { }); }); - describe("parseToolArgs — string escape unescaping", () => { - it("unescapes \\n in string-typed CLI flag arguments", () => { + 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 string-typed --key=value arguments", () => { + 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 when type is ['string', 'null']", () => { - const schemaProperties = { body: { type: ["string", "null"] } }; - const { args } = parseToolArgs(["--body", "line1\\nline2"], schemaProperties); - expect(args).toEqual({ body: "line1\nline2" }); + 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", () => { @@ -812,10 +842,10 @@ describe("mcp_cli_bridge.cjs", () => { expect(args).toEqual({ count: "5\\n" }); }); - it("produces actual newlines matching JSON stdin mode behaviour", () => { + 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" } }; + const schemaProperties = { body: { type: ["string", "null"] } }; const { args: cliArgs } = parseToolArgs(["--body", "title\\n\\nbody"], schemaProperties); const { args: jsonArgs } = parseToolArgs(["."], schemaProperties, '{"body":"title\\n\\nbody"}');