diff --git a/.changeset/fix-sdk-shell-comment-permissions.md b/.changeset/fix-sdk-shell-comment-permissions.md new file mode 100644 index 00000000000..d99833ffaa2 --- /dev/null +++ b/.changeset/fix-sdk-shell-comment-permissions.md @@ -0,0 +1,11 @@ +--- +"gh-aw": patch +--- + +Fix false Copilot SDK shell permission denials for commands containing comments while preserving literal hashes and checking every executable command against the workflow allowlist. + +Keep ambiguous nested shell quoting fail-closed and terminate comments only at line feeds so quote confusion and carriage returns cannot hide disallowed commands. + +Reject unsupported unquoted parenthesis contexts, including extended globs, so literal pattern hashes cannot conceal following commands. + +Check commands after background (`&`) and stderr-pipe (`|&`) operators while preserving redirection ampersands, and retain Bash-specific word boundaries across escaped newlines and non-Bash whitespace. diff --git a/actions/setup/js/bash_command_parser.cjs b/actions/setup/js/bash_command_parser.cjs index bcc2b37cc4f..0a47c95ce97 100644 --- a/actions/setup/js/bash_command_parser.cjs +++ b/actions/setup/js/bash_command_parser.cjs @@ -1,13 +1,27 @@ // @ts-check "use strict"; +const SHELL_BLANK_RE = /^[ \t]$/; + +function trimShellWhitespace(value) { + return value.replace(/^(?:[ \t]|\r?\n)+|(?:[ \t]|\r?\n)+$/g, ""); +} + +function findShellExpansionOpener(commandText, dollarIndex) { + let openerIndex = dollarIndex + 1; + while (commandText[openerIndex] === "\\" && commandText[openerIndex + 1] === "\n") { + openerIndex += 2; + } + return ["{", "(", "'"].includes(commandText[openerIndex]) ? openerIndex : -1; +} + /** * bash_command_parser.cjs * * Dedicated bash command parser for permission checking in the Copilot SDK driver. * * Provides utilities to: - * - Split a shell command text on pipeline operators (&&, ||, |, ;) + * - Split a shell command text on pipeline operators (&&, ||, |, |&, ;, &) * - Extract the executable command name from a shell segment * - Extract all command names from a complex piped/chained command * @@ -24,16 +38,59 @@ * the request, ensuring a safe default. */ +/** + * Find the end of a quoted word, rejecting nested syntax this parser cannot + * safely distinguish from the surrounding quote. + * + * @param {string} commandText + * @param {number} start + * @returns {number} Exclusive end offset, or -1 for unsupported/unclosed quoting. + */ +function findShellQuoteEnd(commandText, start) { + const quote = commandText[start]; + let parameterDepth = 0; + for (let i = start + 1; i < commandText.length; i++) { + const ch = commandText[i]; + if (quote === '"') { + if (ch === "\\") { + i++; + continue; + } + if (ch === "`") return -1; + if (ch === "$") { + const openerIndex = findShellExpansionOpener(commandText, i); + if (openerIndex > i + 1) return -1; + if (openerIndex === i + 1 && (commandText[openerIndex] === "(" || commandText[openerIndex] === "'")) return -1; + if (openerIndex === i + 1 && commandText[openerIndex] === "{") { + parameterDepth++; + i = openerIndex; + continue; + } + } + if (ch === "}" && parameterDepth > 0) { + parameterDepth--; + continue; + } + if (parameterDepth > 0 && (ch === "'" || ch === '"')) return -1; + } + if (ch === quote) return i + 1; + } + return -1; +} + /** * Split a shell command text into individual pipeline segments. - * Splits on the following shell operators: &&, ||, |, ; and newlines. + * Splits on the following shell operators: &&, ||, |, |&, ;, & and newlines. * * The split respects: * - Single-quoted strings (no escaping inside) * - Double-quoted strings (backslash-escape aware) + * - Unquoted backslash escapes and shell comments (terminated by LF, not bare CR) * - $(...) subshell expressions (balanced parentheses) * * Operators embedded inside any of these constructs are not treated as separators. + * Unsupported nested quoting, unquoted grouping/glob parentheses, and unclosed + * constructs fail closed. * * @param {string} commandText - Raw bash command text that may contain pipeline operators * @returns {string[]} Non-empty trimmed segments (operators removed) @@ -43,54 +100,96 @@ function splitOnPipelineOperators(commandText) { const segments = []; let current = ""; + let atWordStart = true; + let parameterDepth = 0; let i = 0; const len = commandText.length; while (i < len) { const ch = commandText[i]; - // ── Single-quoted string: no escape sequences, copy verbatim until closing ' ── - if (ch === "'") { - current += ch; - i++; - while (i < len && commandText[i] !== "'") { - current += commandText[i]; - i++; - } - if (i < len) { - current += commandText[i]; // closing ' - i++; - } + if (ch === "#" && atWordStart && parameterDepth === 0) { + while (i < len && commandText[i] !== "\n") i++; continue; } - // ── Double-quoted string: backslash escapes are recognised ── - if (ch === '"') { - current += ch; - i++; - while (i < len && commandText[i] !== '"') { - if (commandText[i] === "\\" && i + 1 < len) { - current += commandText[i] + commandText[i + 1]; - i += 2; - } else { - current += commandText[i]; - i++; + if (ch === "`") return []; + + if (ch === "$") { + const openerIndex = findShellExpansionOpener(commandText, i); + if (openerIndex > i + 1) return []; + if (openerIndex === i + 1 && commandText[openerIndex] === "'") return []; + } + + if (ch === "\\" && i + 1 < len) { + if (commandText[i + 1] === "\n") { + i += 2; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; + if (current && !/[ \t]$/.test(current)) { + current += " "; } + } else { + current += ch + commandText[i + 1]; + i += 2; + atWordStart = false; } - if (i < len) { - current += commandText[i]; // closing " - i++; - } + continue; + } + + if (ch === "$" && commandText[i + 1] === "{") { + current += "${"; + parameterDepth++; + atWordStart = false; + i += 2; + continue; + } + if (ch === "}" && parameterDepth > 0) parameterDepth--; + + if (ch === "'" || ch === '"') { + const end = findShellQuoteEnd(commandText, i); + if (end < 0) return []; + atWordStart = false; + current += commandText.slice(i, end); + i = end; continue; } // ── $(...) subshell: skip balanced parentheses as a unit ── if (ch === "$" && i + 1 < len && commandText[i + 1] === "(") { + atWordStart = false; current += ch; i++; let depth = 0; + let subshellParameterDepth = 0; while (i < len) { const sc = commandText[i]; + if (sc === "#" || sc === "`") return []; + if (sc === "$") { + const openerIndex = findShellExpansionOpener(commandText, i); + if (openerIndex > i + 1) return []; + if (openerIndex === i + 1 && commandText[openerIndex] === "'") return []; + if (openerIndex === i + 1 && commandText[openerIndex] === "{") { + current += commandText.slice(i, openerIndex + 1); + subshellParameterDepth++; + i = openerIndex + 1; + continue; + } + } + if (sc === "\\" && i + 1 < len) { + current += sc + commandText[i + 1]; + i += 2; + continue; + } + if (sc === "'" || sc === '"') { + const end = findShellQuoteEnd(commandText, i); + if (end < 0) return []; + if (subshellParameterDepth > 0 && /[()]/.test(commandText.slice(i, end))) return []; + current += commandText.slice(i, end); + i = end; + continue; + } + if (sc === "}" && subshellParameterDepth > 0) subshellParameterDepth--; + if (subshellParameterDepth > 0 && (sc === "(" || sc === ")")) return []; if (sc === "(") depth++; else if (sc === ")") { depth--; @@ -102,17 +201,45 @@ function splitOnPipelineOperators(commandText) { current += sc; i++; } + if (depth !== 0 || subshellParameterDepth !== 0) return []; + continue; + } + + // Extended globs can contain literal hashes; their parentheses need a full lexer. + if (ch === "(") return []; + + if (parameterDepth > 0) { + current += ch; + i++; continue; } // ── Pipeline operators ── + // Keep redirection ampersands distinct from background separators. + if (((ch === ">" || ch === "<") && commandText[i + 1] === "&") || (ch === "&" && commandText[i + 1] === ">")) { + current += ch + commandText[i + 1]; + atWordStart = true; + i += 2; + continue; + } + // && (AND-then) if (ch === "&" && i + 1 < len && commandText[i + 1] === "&") { segments.push(current); current = ""; + atWordStart = true; i += 2; - while (i < len && commandText[i] !== undefined && /\s/.test(commandText[i])) i++; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; + continue; + } + + if (ch === "&") { + segments.push(current); + current = ""; + atWordStart = true; + i++; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; continue; } @@ -120,17 +247,19 @@ function splitOnPipelineOperators(commandText) { if (ch === "|" && i + 1 < len && commandText[i + 1] === "|") { segments.push(current); current = ""; + atWordStart = true; i += 2; - while (i < len && /\s/.test(commandText[i])) i++; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; continue; } - // | (pipe) + // | and |& (pipe stdout, or stdout and stderr) if (ch === "|") { segments.push(current); current = ""; - i++; - while (i < len && /\s/.test(commandText[i])) i++; + atWordStart = true; + i += commandText[i + 1] === "&" ? 2 : 1; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; continue; } @@ -138,15 +267,31 @@ function splitOnPipelineOperators(commandText) { if (ch === ";") { segments.push(current); current = ""; + atWordStart = true; i++; - while (i < len && /\s/.test(commandText[i])) i++; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; continue; } // Newline (sequential) — treat line breaks as command separators, // except when escaped as a shell line continuation ("\\" + newline). - // Handles LF, CRLF, and CR forms. - if (ch === "\n" || ch === "\r") { + // Handles LF and CRLF; a bare CR remains part of the shell word. + if (ch === "\r" && commandText[i + 1] === "\n") { + let backslashRunLength = 0; + for (let j = current.length - 1; j >= 0 && current[j] === "\\"; j--) { + backslashRunLength++; + } + if (backslashRunLength % 2 === 1) { + current += ch; + i++; + atWordStart = false; + } else { + i++; + } + continue; + } + + if (ch === "\n") { let backslashRunLength = 0; for (let j = current.length - 1; j >= 0 && current[j] === "\\"; j--) { backslashRunLength++; @@ -156,11 +301,8 @@ function splitOnPipelineOperators(commandText) { if (backslashRunLength % 2 === 1) { current = current.slice(0, -1); i++; - if (ch === "\r" && i < len && commandText[i] === "\n") { - i++; - } - while (i < len && (commandText[i] === " " || commandText[i] === "\t")) i++; - if (current && !/\s$/.test(current)) { + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; + if (current && !/[ \t]$/.test(current)) { current += " "; } continue; @@ -168,24 +310,24 @@ function splitOnPipelineOperators(commandText) { segments.push(current); current = ""; + atWordStart = true; i++; - if (ch === "\r" && i < len && commandText[i] === "\n") { - i++; - } - while (i < len && /\s/.test(commandText[i])) i++; + while (i < len && SHELL_BLANK_RE.test(commandText[i])) i++; continue; } current += ch; + atWordStart = /[ \t&<>]/.test(ch); i++; } // Push the final segment - if (current.trim()) { + if (parameterDepth !== 0) return []; + if (trimShellWhitespace(current)) { segments.push(current); } - return segments.map(s => s.trim()).filter(s => s.length > 0); + return segments.map(trimShellWhitespace).filter(s => s.length > 0); } /** @@ -208,7 +350,7 @@ const SHELL_KEYWORDS = new Set([...CLAUSE_KEYWORDS, ...STRUCTURE_KEYWORDS]); // - "(...)" double-quoted text (supports escapes like \") // - '(...)' single-quoted text // - an unquoted non-space token -const ENV_ASSIGNMENT_PREFIX_RE = /^[A-Za-z_][A-Za-z0-9_]*=(?:"(?:\\.|[^"\\])*"|'[^']*'|\S*)\s*/; +const ENV_ASSIGNMENT_PREFIX_RE = /^[A-Za-z_][A-Za-z0-9_]*=(?:"(?:\\.|[^"\\])*"|'[^']*'|[^ \t\n]*)[ \t]*/; /** * Extract the executable command name from a single shell command segment. @@ -228,21 +370,21 @@ const ENV_ASSIGNMENT_PREFIX_RE = /^[A-Za-z_][A-Za-z0-9_]*=(?:"(?:\\.|[^"\\])*"|' function extractCommandName(segment) { if (!segment || typeof segment !== "string") return null; - let remaining = segment.trim(); + let remaining = trimShellWhitespace(segment); if (!remaining) return null; // Skip leading env-var assignments: IDENTIFIER=anything (repeat) for (;;) { const m = remaining.match(ENV_ASSIGNMENT_PREFIX_RE); if (!m) break; - remaining = remaining.slice(m[0].length).trim(); + remaining = trimShellWhitespace(remaining.slice(m[0].length)); } if (!remaining) return null; for (;;) { // Get the first word - const wordMatch = remaining.match(/^(\S+)/); + const wordMatch = remaining.match(/^([^ \t\n]+)/); if (!wordMatch) return null; const word = wordMatch[1]; @@ -254,7 +396,7 @@ function extractCommandName(segment) { // Shell negation / grouping — skip and continue scanning if (word === "!" || word === "{" || word === "}") { - remaining = remaining.slice(word.length).trim(); + remaining = trimShellWhitespace(remaining.slice(word.length)); if (!remaining) return null; continue; } @@ -262,7 +404,7 @@ function extractCommandName(segment) { // Flow-control keywords are not executable commands if (SHELL_KEYWORDS.has(word)) { if (CLAUSE_KEYWORDS.has(word)) { - remaining = remaining.slice(word.length).trim(); + remaining = trimShellWhitespace(remaining.slice(word.length)); if (!remaining) return null; continue; } @@ -276,7 +418,7 @@ function extractCommandName(segment) { /** * Extract all unique command names from a bash pipeline or command sequence. * - * Splits the text on &&, ||, |, and ; and extracts the executable command name + * Splits the text on &&, ||, |, |&, ;, and & and extracts the executable command name * from each resulting segment. Returns a deduplicated array preserving * first-occurrence order. * @@ -290,7 +432,7 @@ function extractCommandName(segment) { function extractCommandNamesFromPipeline(commandText) { if (!commandText || typeof commandText !== "string") return []; - const text = commandText.trim(); + const text = trimShellWhitespace(commandText); if (!text) return []; const segments = splitOnPipelineOperators(text); diff --git a/actions/setup/js/bash_command_parser.test.cjs b/actions/setup/js/bash_command_parser.test.cjs index 1fdb6dab434..038f6d0ee0b 100644 --- a/actions/setup/js/bash_command_parser.test.cjs +++ b/actions/setup/js/bash_command_parser.test.cjs @@ -35,6 +35,25 @@ describe("splitOnPipelineOperators", () => { expect(splitOnPipelineOperators("ls -la | grep pattern")).toEqual(["ls -la", "grep pattern"]); }); + it.each([ + ["echo ok & cat data.json", ["echo ok", "cat data.json"]], + ["echo ok&cat data.json&", ["echo ok", "cat data.json"]], + ["echo ok |& cat data.json", ["echo ok", "cat data.json"]], + ["echo ok 2>&1&cat data.json", ["echo ok 2>&1", "cat data.json"]], + ["echo ok 2<&0 & cat data.json", ["echo ok 2<&0", "cat data.json"]], + ["echo ok &>output & cat data.json", ["echo ok &>output", "cat data.json"]], + ["echo ok &>>output&cat data.json", ["echo ok &>>output", "cat data.json"]], + [String.raw`echo ok \>&cat data.json`, [String.raw`echo ok \>`, "cat data.json"]], + ])("separates background and stderr-pipe commands in %s", (script, segments) => { + expect(splitOnPipelineOperators(script)).toEqual(segments); + expect(extractCommandNamesFromPipeline(script)).toEqual(["echo", "cat"]); + }); + + it.each(["echo ok 2>&1", "echo ok 1>&2", "echo ok 2<&0", "echo ok &>output", "echo ok &>>output", "echo 'a & b |& c'", 'echo "a & b |& c"', String.raw`echo a\&b`])("preserves redirections and literal ampersands in %s", script => { + expect(splitOnPipelineOperators(script)).toEqual([script]); + expect(extractCommandNamesFromPipeline(script)).toEqual(["echo"]); + }); + it("splits on ; (sequential)", () => { expect(splitOnPipelineOperators("echo a; echo b")).toEqual(["echo a", "echo b"]); }); @@ -110,6 +129,35 @@ describe("splitOnPipelineOperators", () => { expect(segments).toEqual(["pwd", "ls -la", "safeoutputs --help"]); }); + it.each(["\n", "\r\n"])("ignores standalone comments with %j line endings", newline => { + const command = `# Check attribution data${newline} # Ignore "quotes, && operators; and $(subshells)${newline}cat data.json${newline}# Done`; + expect(splitOnPipelineOperators(command)).toEqual(["cat data.json"]); + }); + + it("ignores inline comments without hiding the next command", () => { + expect(splitOnPipelineOperators('echo ready # Ignore "quotes && operators; too\ncat data.json')).toEqual(["echo ready", "cat data.json"]); + }); + + it("returns no segments for a comment-only script", () => { + expect(splitOnPipelineOperators(" # Nothing to execute\n\t# Still a comment")).toEqual([]); + }); + + it.each(["echo '# literal'", 'echo "# literal"', "echo value#literal", String.raw`echo \#literal`, String.raw`echo value\ #literal`, String.raw`echo \"#literal`])("preserves literal hashes in %s", command => { + expect(splitOnPipelineOperators(`${command} && cat data.json`)).toEqual([command, "cat data.json"]); + }); + + it("recognizes a comment after an escaped backslash and unescaped whitespace", () => { + expect(splitOnPipelineOperators(String.raw`echo \\ # Ignore this && command` + "\ncat data.json")).toEqual([String.raw`echo \\`, "cat data.json"]); + }); + + it.each(["echo ${value:- # literal}", "echo ${outer:-${inner:- # literal}}", "echo ${value:-'quoted }' # literal}"])("preserves literal hashes inside parameter expansions: %s", command => { + expect(splitOnPipelineOperators(`${command} && cat data.json`)).toEqual([command, "cat data.json"]); + }); + + it("does not continue a comment ending in a backslash", () => { + expect(splitOnPipelineOperators("# Comment \\\ncat data.json")).toEqual(["cat data.json"]); + }); + it("does not split on escaped newline continuations", () => { const segments = splitOnPipelineOperators("git log \\\n --oneline \\\n --max-count=1"); expect(segments).toEqual(["git log --oneline --max-count=1"]); @@ -289,6 +337,10 @@ describe("extractCommandNamesFromPipeline", () => { expect(extractCommandNamesFromPipeline(" ")).toEqual([]); }); + it("extracts only executable commands from a commented script", () => { + expect(extractCommandNamesFromPipeline("# Check counts\njq length data.json # Count entries\n# Read the data\ncat data.json")).toEqual(["jq", "cat"]); + }); + it("handles command with $() subshell — does not split inside subshell", () => { const result = extractCommandNamesFromPipeline("cat $(ls /tmp)"); expect(result).toEqual(["cat"]); diff --git a/actions/setup/js/bash_command_parser_comments.test.cjs b/actions/setup/js/bash_command_parser_comments.test.cjs new file mode 100644 index 00000000000..c0d4c0eacf1 --- /dev/null +++ b/actions/setup/js/bash_command_parser_comments.test.cjs @@ -0,0 +1,434 @@ +// @ts-check + +import { describe, it, expect } from "vitest"; +import { createRequire } from "module"; +import { spawnSync } from "node:child_process"; + +const require = createRequire(import.meta.url); +const { splitOnPipelineOperators, extractCommandNamesFromPipeline } = require("./bash_command_parser.cjs"); +const { buildCopilotSDKPermissionHandler } = require("./copilot_sdk_permissions.cjs"); + +const checkPermission = buildCopilotSDKPermissionHandler({ allowedTools: ["shell(cat)", "shell(echo)", "shell(jq)", "shell(wc)"] }, () => ({ kind: "approve-once" })); + +const lineEndings = [ + { name: "LF", value: "\n" }, + { name: "CRLF", value: "\r\n" }, +]; + +describe("commented Bash script fixtures", () => { + it.each([ + { + name: "shebang and file header", + script: "#!/usr/bin/env bash\n# Read the data\ncat data.json", + segments: ["cat data.json"], + names: ["cat"], + }, + { + name: "inline comment at EOF", + script: "cat data.json # Read only", + segments: ["cat data.json"], + names: ["cat"], + }, + { + name: "comment-only final line", + script: "cat data.json\n# Finished", + segments: ["cat data.json"], + names: ["cat"], + }, + { + name: "indented comments and blank lines", + script: "\n # Header\n\n\t# Another comment\n\tcat data.json\n\n # Footer\n", + segments: ["cat data.json"], + names: ["cat"], + }, + { + name: "empty inline comments", + script: "echo ready #\ncat data.json\t#\n#", + segments: ["echo ready", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "comment immediately after semicolon", + script: "echo ready;# Ignore this\ncat data.json", + segments: ["echo ready", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "comment immediately after pipe", + script: "cat data.json |# Ignore this\nwc -l", + segments: ["cat data.json", "wc -l"], + names: ["cat", "wc"], + }, + { + name: "comment immediately after AND", + script: "cat data.json &&# Ignore this\necho done", + segments: ["cat data.json", "echo done"], + names: ["cat", "echo"], + }, + { + name: "comment immediately after OR", + script: "cat data.json ||# Ignore this\necho missing", + segments: ["cat data.json", "echo missing"], + names: ["cat", "echo"], + }, + { + name: "commented command is not executable", + script: "# python3 -c 'print(1)'\ncat data.json\n# another_tool", + segments: ["cat data.json"], + names: ["cat"], + }, + { + name: "single-quoted hashes and operators", + script: "echo '# literal && not_a_command' # Actual comment\ncat data.json", + segments: ["echo '# literal && not_a_command'", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "double-quoted hashes and operators", + script: 'echo "# literal | not_a_command" # Actual comment\ncat data.json', + segments: ['echo "# literal | not_a_command"', "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "multiline single-quoted text", + script: "echo 'first line\n# literal second line' # Actual comment\ncat data.json", + segments: ["echo 'first line\n# literal second line'", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "multiline double-quoted text", + script: 'echo "first line\n# literal second line" # Actual comment\ncat data.json', + segments: ['echo "first line\n# literal second line"', "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "hash inside filename", + script: "cat report#draft.json # Actual comment\nwc -l report#draft.json", + segments: ["cat report#draft.json", "wc -l report#draft.json"], + names: ["cat", "wc"], + }, + { + name: "hash inside URL", + script: "echo https://example.invalid/page#section # Actual comment\ncat data.json", + segments: ["echo https://example.invalid/page#section", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "hash adjoining empty single quotes", + script: "echo ''#literal # Actual comment\ncat data.json", + segments: ["echo ''#literal", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "hash adjoining empty double quotes", + script: 'echo ""#literal # Actual comment\ncat data.json', + segments: ['echo ""#literal', "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "escaped hash", + script: String.raw`echo \#literal # Actual comment` + "\ncat data.json", + segments: [String.raw`echo \#literal`, "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "escaped space before literal hash", + script: String.raw`echo value\ #literal # Actual comment` + "\ncat data.json", + segments: [String.raw`echo value\ #literal`, "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "escaped tab before literal hash", + script: "echo value\\\t#literal # Actual comment\ncat data.json", + segments: ["echo value\\\t#literal", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "escaped quote before literal hash", + script: String.raw`echo \"#literal # Actual comment` + "\ncat data.json", + segments: [String.raw`echo \"#literal`, "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "escaped operator does not start a comment", + script: String.raw`echo \;#literal # Actual comment` + "\ncat data.json", + segments: [String.raw`echo \;#literal`, "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "parameter length and prefix removal", + script: "echo ${#value} ${path##*/} # Actual comment\ncat data.json", + segments: ["echo ${#value} ${path##*/}", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "space and hash in parameter default", + script: "echo ${value:- # fallback} # Actual comment\ncat data.json", + segments: ["echo ${value:- # fallback}", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "nested parameter defaults", + script: "echo ${outer:-${inner:- # fallback}} # Actual comment\ncat data.json", + segments: ["echo ${outer:-${inner:- # fallback}}", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "quoted closing brace in parameter default", + script: "echo ${value:-'}' # fallback} # Actual comment\ncat data.json", + segments: ["echo ${value:-'}' # fallback}", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "quoted hash inside command substitution", + script: "echo $(printf '%s' '# literal') # Actual comment\ncat data.json", + segments: ["echo $(printf '%s' '# literal')", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "continued command before inline comment", + script: "jq \\\n '.[]' \\\n data.json # Filter entries\nwc -l data.json", + segments: ["jq '.[]' data.json", "wc -l data.json"], + names: ["jq", "wc"], + }, + { + name: "standalone comment after line continuation", + script: "echo \\\n# This ends the logical command\ncat data.json", + segments: ["echo", "cat data.json"], + names: ["echo", "cat"], + }, + { + name: "assignment with inline comment", + script: '# Configuration\nDATA=data.json # Input path\ncat "$DATA" # Read it', + segments: ["DATA=data.json", 'cat "$DATA"'], + names: ["cat"], + }, + { + name: "for loop with comments around structural keywords", + script: '# Iterate over inputs\nfor file in first.json second.json; do # Loop body\n cat "$file" # Read input\ndone # End loop\necho done', + segments: ["for file in first.json second.json", "do", 'cat "$file"', "done", "echo done"], + names: ["cat", "echo"], + }, + { + name: "conditional with comments on every clause", + script: "if # Condition follows\n grep -q community data.json # Search\nthen # Match\n cat data.json # Read\nelse # No match\n echo missing # Report\nfi # End", + segments: ["if", "grep -q community data.json", "then", "cat data.json", "else", "echo missing", "fi"], + names: ["grep", "cat", "echo"], + }, + { + name: "function declaration and invocation", + script: "function report # Define helper\n{ # Start body\n cat data.json # Read\n} # End body\nreport # Invoke", + segments: ["function report", "{", "cat data.json", "}", "report"], + names: ["cat", "report"], + }, + { + name: "attribution summary script", + script: + '#!/usr/bin/env bash\n# Use precomputed data\nDATA_DIR=/tmp/gh-aw/agent/community-data\n# Count authors\njq length "$DATA_DIR/attribution_by_author.json" # Count\n# Summarize issues\njq \'[.[].count] | add\' "$DATA_DIR/attribution_by_author.json" # Sum\necho done # Finished', + segments: ["DATA_DIR=/tmp/gh-aw/agent/community-data", 'jq length "$DATA_DIR/attribution_by_author.json"', "jq '[.[].count] | add' \"$DATA_DIR/attribution_by_author.json\"", "echo done"], + names: ["jq", "echo"], + }, + ])("parses $name", ({ script, segments, names }) => { + expect(splitOnPipelineOperators(script)).toEqual(segments); + expect(extractCommandNamesFromPipeline(script)).toEqual(names); + }); +}); + +describe.each(lineEndings)("comment placement with $name line endings", ({ value: newline }) => { + for (const separator of ["\n", ";", "&&", "||", "|", "&", "|&"]) { + it(`ignores inert comments around ${JSON.stringify(separator)} separators`, () => { + const text = "inert comment"; + const operator = separator === "\n" ? newline : separator; + const script = [`# ${text}`, `cat data.json ${operator} # ${text}`, `jq length ${operator} # ${text}`, `wc -l # ${text}`, `# ${text}`].join(newline); + expect(splitOnPipelineOperators(script)).toEqual(["cat data.json", "jq length", "wc -l"]); + expect(extractCommandNamesFromPipeline(script)).toEqual(["cat", "jq", "wc"]); + expect(checkPermission({ kind: "shell", fullCommandText: script })).toEqual({ kind: "approve-once" }); + }); + } + + it.each(["#", "# Header", " # Indented", "\t# Tabbed", "#!/usr/bin/env bash", '# Unclosed "quotes && $(subshells'])("keeps comment-only requests fail-closed: %s", comment => { + const script = `${comment}${newline}${comment}`; + expect(splitOnPipelineOperators(script)).toEqual([]); + expect(extractCommandNamesFromPipeline(script)).toEqual([]); + expect(checkPermission({ kind: "shell", fullCommandText: script }).kind).toBe("reject"); + }); + + it.each([ + { name: "standalone comments", prefix: "# Allowed read\ncat data.json\n# Next command is not allowed\n" }, + { name: "inline comments with unmatched quotes", prefix: 'cat data.json # Ignore this "quote and && operator\n' }, + { name: "comment ending in backslash", prefix: "cat data.json # This comment does not continue \\\n" }, + { name: "quoted hash", prefix: "echo '# literal' && " }, + { name: "multiline quoted hash", prefix: 'echo "first line\n# literal" && ' }, + { name: "escaped hash", prefix: String.raw`echo \#literal && ` }, + { name: "escaped space", prefix: String.raw`echo value\ #literal && ` }, + { name: "escaped quote", prefix: String.raw`echo \"#literal && ` }, + { name: "parameter default", prefix: "echo ${value:- # literal} && " }, + { name: "nested parameter defaults", prefix: "echo ${outer:-${inner:- # literal}} && " }, + ])("cannot conceal a disallowed command with $name", ({ prefix }) => { + const script = `${prefix}python3 -c 'print(1)' # Final comment`.replace(/\n/g, newline); + expect(extractCommandNamesFromPipeline(script)).toContain("python3"); + expect(checkPermission({ kind: "shell", fullCommandText: script }).kind).toBe("reject"); + }); +}); + +describe("comment parser security boundaries", () => { + it.each([ + "echo ok & forbidden_probe", + "echo ok&forbidden_probe", + "echo ok & # Background command\nforbidden_probe", + "echo ok 2>&1&forbidden_probe", + String.raw`echo ok \>&forbidden_probe`, + 'echo ok "${value:-"x #"}"; forbidden_probe', + '#\recho ok "\nforbidden_probe\n#"', + "echo ok\r#; forbidden_probe", + "echo ok $\\\n{value:-x #}; forbidden_probe", + "shopt -s extglob\necho ok @(x #); forbidden_probe", + ])("rejects scripts whose disallowed command is reached by the Bash oracle: %j", fullCommandText => { + const result = spawnSync("bash", ["--noprofile", "--norc", "-c", 'forbidden_probe() { builtin printf "\\nMOCK_REACHED\\n"; }\n' + fullCommandText], { + env: {}, + encoding: "utf8", + timeout: 2000, + }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toContain("\nMOCK_REACHED\n"); + for (const grant of ["shell(echo)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: ["shell(shopt)", grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + }); + + it.each([ + "echo ok & forbidden_probe", + "echo ok&forbidden_probe", + "echo ok & # Background command\nforbidden_probe", + "echo ok # Commented ampersand &\nforbidden_probe", + "echo ok 2>&1&forbidden_probe", + "echo ok 2<&0 & forbidden_probe", + "echo ok &>output & forbidden_probe", + "echo ok &>>output&forbidden_probe", + String.raw`echo ok \>&forbidden_probe`, + "echo ok |& forbidden_probe", + ])("checks every command around ampersands: %s", fullCommandText => { + expect(extractCommandNamesFromPipeline(fullCommandText)).toContain("forbidden_probe"); + for (const grant of ["shell(echo)", "shell(echo:*)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: [grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + }); + + it.each(["echo ok & echo done &", "echo ok & # Background command\ncat data.json", "echo ok 2>&1", "echo ok &>output", "echo ok |& cat data.json", "echo ${value:-a&b} # Actual comment"])( + "preserves permitted commands and ampersand redirections: %s", + fullCommandText => { + expect(checkPermission({ kind: "shell", fullCommandText }).kind).toBe("approve-once"); + } + ); + + it.each(["echo ok ${value:-foo|bar} && cat data.json", "echo ok ${value:-foo;bar} && cat data.json"])("keeps command separators inside parameter expansions: %s", fullCommandText => { + expect(splitOnPipelineOperators(fullCommandText)).toEqual([fullCommandText.slice(0, fullCommandText.indexOf(" && ")), "cat data.json"]); + expect(extractCommandNamesFromPipeline(fullCommandText)).toEqual(["echo", "cat"]); + expect(checkPermission({ kind: "shell", fullCommandText })).toEqual({ kind: "approve-once" }); + }); + + it.each(["echo ok\r#; forbidden_probe", "echo ok\\\r#; forbidden_probe", "echo ok;\v#; forbidden_probe", "echo ok;\u00a0#; forbidden_probe"])("does not treat non-Bash whitespace as a comment boundary: %j", fullCommandText => { + expect(extractCommandNamesFromPipeline(fullCommandText)).toContain("forbidden_probe"); + for (const grant of ["shell(echo)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: [grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + }); + + it.each(["\r", "\v", "\f", "\u00a0", "\u2003", "\u2028", "\u2029", "\ufeff"])("preserves non-Bash word characters after separators: %j", character => { + for (const separator of [";", "&&", "||", "|", "&", "\n"]) { + const fullCommandText = `echo ok${separator}${character}#; forbidden_probe`; + expect(extractCommandNamesFromPipeline(fullCommandText)).toContain("forbidden_probe"); + expect(checkPermission({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + }); + + it.each(["echo ok $\\\n{value:-x #}; forbidden_probe", "echo ok $\\\n'x #'; forbidden_probe", 'echo ok "$\\\n(echo x)"; forbidden_probe', 'echo ok "$\\\n{value:-x #}"; forbidden_probe'])( + "rejects expansion openers separated by escaped newlines: %s", + fullCommandText => { + for (const grant of ["shell(echo)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: [grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + } + ); + + it("does not reinterpret escaped-newline openers in comments or single quotes", () => { + const fullCommandText = "# $\\\n# {comment text\ncat data.json && echo '$\\\n{literal}'"; + expect(extractCommandNamesFromPipeline(fullCommandText)).toEqual(["cat", "echo"]); + }); + + describe.each(["?", "*", "+", "@", "!"])("unsupported %s(...) patterns", operator => { + it.each(["", "\\\n", "\\\n\\\n"])("rejects glob operators separated from parentheses by %j", continuation => { + for (const prefix of ["", "shopt -s extglob\n"]) { + const fullCommandText = `${prefix}echo ok ${operator}${continuation}(x #); forbidden_probe`; + for (const grant of ["shell(echo)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: ["shell(shopt)", grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + expect(splitOnPipelineOperators(fullCommandText)).toEqual([]); + expect(extractCommandNamesFromPipeline(fullCommandText)).toEqual([]); + } + }); + }); + + it.each([ + "echo '@(x #)' # Actual comment\ncat data.json", + 'echo "!(x #)" # Actual comment\ncat data.json', + String.raw`echo @\(x\ \#\) # Actual comment` + "\ncat data.json", + "# Ignore @(x #) and $(unclosed\necho ok\ncat data.json", + "echo $(printf '%s' '@(x #)') # Actual comment\ncat data.json", + ])("preserves quoted, escaped, and commented glob syntax: %s", fullCommandText => { + expect(extractCommandNamesFromPipeline(fullCommandText)).toEqual(["echo", "cat"]); + expect(checkPermission({ kind: "shell", fullCommandText }).kind).toBe("approve-once"); + }); + + it.each([ + 'echo ok "${value:-"x #"}"; forbidden_probe', + 'echo ok "${value:+"x #"}"; forbidden_probe', + 'echo ok "${outer:-${inner:-"x #"}}"; forbidden_probe', + 'echo ok "$(printf "x #")"; forbidden_probe', + 'echo ok "`printf "x #"`"; forbidden_probe', + "echo ok $(printf ')x #'); forbidden_probe", + "echo ok $(printf %s ${value:-)x #}); forbidden_probe", + "echo ok $'x \\' #'; forbidden_probe", + ])("cannot hide a following command using nested shell quoting: %s", fullCommandText => { + for (const grant of ["shell(echo)", "shell(echo ok:*)"]) { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: [grant] }, () => ({ kind: "approve-once" })); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + const segments = splitOnPipelineOperators(fullCommandText); + if (segments.length > 0) expect(extractCommandNamesFromPipeline(fullCommandText)).toContain("forbidden_probe"); + }); + + it.each(['#\recho ok "\nforbidden_probe\n#"', '# ignored\recho ok "\nforbidden_probe\n#"', 'echo ok # ignored\recho ok "\nforbidden_probe\n#"', '# ignored\r\r\recho ok "\nforbidden_probe\n#"'])( + "does not let carriage returns terminate a Bash comment: %j", + fullCommandText => { + const handler = buildCopilotSDKPermissionHandler({ allowedTools: ["shell(echo ok:*)"] }, () => ({ kind: "approve-once" })); + expect(extractCommandNamesFromPipeline(fullCommandText)).toContain("forbidden_probe"); + expect(handler({ kind: "shell", fullCommandText }).kind).toBe("reject"); + } + ); + + it.each(["# Header\rcat data.json", "# Header\r # Another comment\rcat data.json", '# Header\r"unclosed quote'])("keeps carriage returns inside comment text: %j", script => { + expect(splitOnPipelineOperators(script)).toEqual([]); + expect(extractCommandNamesFromPipeline(script)).toEqual([]); + expect(checkPermission({ kind: "shell", fullCommandText: script }).kind).toBe("reject"); + }); + + it.each(['echo "${value:-#literal}" # Actual comment\ncat data.json', 'echo "${outer:-${inner:-#literal}}" # Actual comment\ncat data.json', "echo $(printf ${value:-fallback}) # Actual comment\ncat data.json"])( + "preserves simple parameter expansions: %s", + script => { + expect(extractCommandNamesFromPipeline(script)).toEqual(["echo", "cat"]); + expect(checkPermission({ kind: "shell", fullCommandText: script }).kind).toBe("approve-once"); + } + ); + + it.each(["echo ok 'unclosed #", 'echo ok "unclosed #', "echo ok ${value:- #", "echo ok $(printf value"])("rejects unclosed contexts instead of approving a partial parse: %s", fullCommandText => { + expect(splitOnPipelineOperators(fullCommandText)).toEqual([]); + expect(checkPermission({ kind: "shell", fullCommandText }).kind).toBe("reject"); + }); +}); diff --git a/actions/setup/js/copilot_sdk_driver.test.cjs b/actions/setup/js/copilot_sdk_driver.test.cjs index a7be1d46a6f..d47ce27f143 100644 --- a/actions/setup/js/copilot_sdk_driver.test.cjs +++ b/actions/setup/js/copilot_sdk_driver.test.cjs @@ -2013,6 +2013,34 @@ describe("copilot_sdk_driver.cjs", () => { expect(result).toEqual({ kind: "approve-once" }); }); + it.each([ + "\n# Query issue #68529 for follow-up references\ngithub issue read 68529 --json body,comments 2>/dev/null | jq '.body, .comments[].body' 2>/dev/null || echo \"Issue details not available via github CLI\"\n", + "\n# Check what data files exist that might have PR closing references\ncat /tmp/gh-aw/agent/community-data/closing_refs_by_issue.json | jq 'keys[] as $k | select($k | test(\"68529|65058|60858\")) | {issue: $k, prs: .[$k]}'\n", + '\n# Count entries in each tier file\necho "=== Tier counts ==="\nwc -l /tmp/gh-aw/agent/community-data/tier0_attributed.json /tmp/gh-aw/agent/community-data/tier1_attributed.json /tmp/gh-aw/agent/community-data/tier2_attributed.json\n\necho ""\necho "=== Checking Tier 1 & 2 ==="\ncat /tmp/gh-aw/agent/community-data/tier1_attributed.json\necho "---"\ncat /tmp/gh-aw/agent/community-data/tier2_attributed.json\n', + "\n# Get count of authors and total issues\njq 'length' /tmp/gh-aw/agent/community-data/attribution_by_author.json\njq '[.[].count] | add' /tmp/gh-aw/agent/community-data/attribution_by_author.json\n", + ])("allows the community-attribution command despite standalone comments: %s", async fullCommandText => { + const handler = await makePermissionHandlerViaSDK(["shell(github:*)", "shell(cat)", "shell(jq)", "shell(echo)", "shell(wc)"]); + expect(handler({ kind: "shell", commands: [], fullCommandText })).toEqual({ kind: "approve-once" }); + }); + + it.each([ + "# An allowed read follows\ncat data.json\n# Python is not allowed\npython3 -c 'print(1)'", + "cat data.json # Ignore \"quotes && operators; only on this line\npython3 -c 'print(1)'", + "cat ${value:- # literal} && python3 -c 'print(1)'", + "cat ${outer:-${inner:- # literal}} && python3 -c 'print(1)'", + String.raw`cat \#literal && python3 -c 'print(1)'`, + String.raw`cat value\ #literal && python3 -c 'print(1)'`, + "# Comment-only requests remain fail-closed", + ])("does not let comments bypass shell permissions: %s", async fullCommandText => { + const handler = await makePermissionHandlerViaSDK(["shell(cat)"]); + expect(handler({ kind: "shell", commands: [], fullCommandText })).toEqual({ kind: "reject", feedback: "Tool invocation is not allowed by workflow tool permissions." }); + }); + + it("still denies the community-attribution Python heredoc", async () => { + const handler = await makePermissionHandlerViaSDK(["shell(github:*)", "shell(cat)", "shell(jq)", "shell(echo)", "shell(wc)"]); + expect(handler({ kind: "shell", commands: [], fullCommandText: "python3 << 'PYEOF'\nimport json\nprint('summary')\nPYEOF\n" })).toEqual({ kind: "reject", feedback: "Tool invocation is not allowed by workflow tool permissions." }); + }); + it("denies a piped command when any stage is not in the allow-list", async () => { const handler = await makePermissionHandlerViaSDK(["shell(ls)", "shell(echo)"]); // cat is NOT in the allow-list @@ -2169,6 +2197,16 @@ describe("copilot_sdk_driver.cjs", () => { }); }); + it("checks background commands even when SDK identifiers omit them", async () => { + const handler = await makePermissionHandlerViaSDK(["shell(echo)"]); + for (const fullCommandText of ["echo ok & forbidden_probe", "echo ok 2>&1&forbidden_probe", "echo ok & # Comment\nforbidden_probe", "echo ok |& forbidden_probe"]) { + expect(handler({ kind: "shell", fullCommandText, commands: [{ identifier: "echo" }] }).kind).toBe("reject"); + expect(handler({ kind: "shell", fullCommandText: "", commands: [{ identifier: fullCommandText }] }).kind).toBe("reject"); + } + expect(handler({ kind: "shell", fullCommandText: "echo ok & echo done &", commands: [] })).toEqual({ kind: "approve-once" }); + expect(handler({ kind: "shell", fullCommandText: "echo ok 2>&1", commands: [] })).toEqual({ kind: "approve-once" }); + }); + it("denies executable content hidden behind control flow or leading redirection", async () => { const handler = await makePermissionHandlerViaSDK(["shell(echo)", "shell(git:*)"]); // A segment whose first token is a redirection yields no extractable diff --git a/actions/setup/js/fuzz_bash_command_parser_harness.test.cjs b/actions/setup/js/fuzz_bash_command_parser_harness.test.cjs index baebcd74316..187d2f78810 100644 --- a/actions/setup/js/fuzz_bash_command_parser_harness.test.cjs +++ b/actions/setup/js/fuzz_bash_command_parser_harness.test.cjs @@ -33,7 +33,7 @@ const { testSplitOnPipelineOperators, testExtractCommandName, testExtractCommand // ───────────────────────────────────────────────────────────────────────────── /** Pipeline operators that should split segments */ -const OPERATORS = ["&&", "||", "|", ";"]; +const OPERATORS = ["&&", "||", "|", "|&", ";", "&"]; /** Commands that should be recognised as safe (typically in the workflow allow-list) */ const SAFE_COMMANDS = ["ls", "cat", "echo", "grep", "wc", "find", "jq", "printf", "pwd", "date", "head", "tail", "sort", "uniq", "yq", "safeoutputs", "gh", "git"];