diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index ade53c2b20..f56547588a 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -160,6 +160,71 @@ describe("getCommandDecision — multi-line script wrapped in a quoted argument" }) }) +describe("getCommandDecision — chained command next to a multi-line quoted argument", () => { + // Every command in the chain must match the allowlist on its own, also when + // one of them has a quoted argument that spans lines. + const chained = `cd cmd-test && node -e "\nconsole.log('hello world');\n"` + const reversed = `node -e "\nconsole.log('hello world');\n" && cd cmd-test` + + it.each([chained, reversed])("asks user when only cd is allowed: %j", (command) => { + expect(getCommandDecision(command, ["cd"])).toBe("ask_user") + }) + + it.each([chained, reversed])("auto-approves when every command is allowed: %j", (command) => { + expect(getCommandDecision(command, ["cd", "node"])).toBe("auto_approve") + }) + + it.each([chained, reversed])("auto-denies when the chained command is denied: %j", (command) => { + expect(getCommandDecision(command, ["cd"], ["node"])).toBe("auto_deny") + }) +}) + +describe("getCommandDecision — # inside a word and glob patterns", () => { + it.each(["echo safe#blocked && node -v", "echo safe#blocked 'a\nb' && node -v"])( + "applies a deny rule that contains # and checks the chained command: %j", + (command) => { + expect(getCommandDecision(command, ["echo"], ["echo safe#blocked"])).toBe("auto_deny") + expect(getCommandDecision(command, ["echo"])).toBe("ask_user") + }, + ) + + it.each(["rm *", "rm * 'a\nb'"])("applies a deny rule for a glob: %j", (command) => { + expect(getCommandDecision(command, ["rm"], ["rm *"])).toBe("auto_deny") + }) + + it("auto-approves a glob argument when the command is allowed", () => { + expect(getCommandDecision("ls *.ts && echo x", ["ls", "echo"])).toBe("auto_approve") + }) +}) + +describe("getCommandDecision — outer word attached to a command substitution", () => { + it.each(["reboot$(printf 'x')", "reboot$(printf '\n')"])( + "checks the outer command when only the inner command is allowed: %j", + (command) => { + expect(getCommandDecision(command, ["printf"])).toBe("ask_user") + expect(getCommandDecision(command, ["printf"], ["reboot"])).toBe("auto_deny") + expect(getCommandDecision(command, ["printf", "reboot"])).toBe("auto_approve") + }, + ) + + it.each(["$(printf re)boot", "$(printf re)boot 'a\nb'", "`printf re`boot 'a\nb'"])( + "asks the user when a substitution is part of the command name: %j", + (command) => { + expect(getCommandDecision(command, ["printf", "boot"])).toBe("ask_user") + expect(getCommandDecision(command, ["printf", "boot"], ["reboot"])).toBe("ask_user") + }, + ) + + it.each(["echo*$(reboot)", "echo*$(reboot) 'a\nb'", "ls *$(reboot).ts"])( + "checks a substitution inside a glob: %j", + (command) => { + expect(getCommandDecision(command, ["echo", "ls"])).toBe("ask_user") + expect(getCommandDecision(command, ["echo", "ls"], ["reboot"])).toBe("auto_deny") + expect(getCommandDecision(command, ["echo", "ls", "reboot"])).toBe("auto_approve") + }, + ) +}) + describe("getCommandDecisionDetailed", () => { it("names the offending sub-command and matched denied prefix on chained commands", () => { const result = getCommandDecisionDetailed("git status && rm x && npm test", ["git", "npm"], ["rm"]) diff --git a/src/shared/__tests__/parse-command.spec.ts b/src/shared/__tests__/parse-command.spec.ts index fa41d31c6c..7595dff326 100644 --- a/src/shared/__tests__/parse-command.spec.ts +++ b/src/shared/__tests__/parse-command.spec.ts @@ -157,6 +157,72 @@ describe("parseCommand", () => { }) }) + describe("chain operators on a line with a multi-line quoted argument", () => { + it.each([ + { + input: `cd cmd-test && node -e "\nconsole.log('hello world');\n"`, + expected: ["cd cmd-test", `node -e "\nconsole.log('hello world');\n"`], + }, + { + input: `node -e "\nconsole.log(1)\n" || echo failed`, + expected: [`node -e "\nconsole.log(1)\n"`, "echo failed"], + }, + { + input: "cd a; node -e '\nx\n' | tee out.txt", + expected: ["cd a", "node -e '\nx\n'", "tee out.txt"], + }, + { + input: 'git commit -m "line 1\nline 2" && git push', + expected: ['git commit -m "line 1\nline 2"', "git push"], + }, + { + input: `echo $'first\nline' && printf $"second\r\nline"`, + expected: [`echo $'first\nline'`, `printf $"second\r\nline"`], + }, + { + input: "echo 'first\nline' & echo done", + expected: ["echo 'first\nline'", "echo done"], + }, + { + input: "echo 'first\nline';ls", + expected: ["echo 'first\nline'", "ls"], + }, + ])("splits $input into each chained command", ({ input, expected }) => { + expect(parseCommand(input).commands).toEqual(expected) + }) + + it("restores each quote by its full index when a line has more than ten quotes", () => { + const singleLineQuotes = Array.from({ length: 10 }, (_, i) => `'q${i}'`).join(" ") + const input = `echo ${singleLineQuotes} 'q10\nend' && ls` + expect(parseCommand(input).commands).toEqual([`echo ${singleLineQuotes} 'q10\nend'`, "ls"]) + }) + + it("keeps operators inside the multi-line quoted argument", () => { + const input = `sh -c "cd a && node x\necho done" && ls` + expect(parseCommand(input).commands).toEqual([`sh -c "cd a && node x\necho done"`, "ls"]) + }) + }) + + describe("# inside a word and glob patterns", () => { + it.each([ + { input: "echo safe#blocked && node -v", expected: ["echo safe#blocked", "node -v"] }, + { input: "echo safe#blocked 'a\nb' && node -v", expected: ["echo safe#blocked 'a\nb'", "node -v"] }, + { input: "curl http://x/#frag && node y", expected: ["curl http://x/#frag", "node y"] }, + { input: "rm *", expected: ["rm *"] }, + { input: "rm * 'a\nb'", expected: ["rm * 'a\nb'"] }, + { input: "ls *.ts && echo x", expected: ["ls *.ts", "echo x"] }, + ])("keeps the word text in $input", ({ input, expected }) => { + expect(parseCommand(input).commands).toEqual(expected) + }) + + it.each([ + { input: "echo hi # comment && rm x", expected: ["echo hi"] }, + { input: "echo hi 'a\nb' # comment && rm x", expected: ["echo hi 'a\nb'"] }, + ])("drops a comment that starts a word in $input", ({ input, expected }) => { + expect(parseCommand(input).commands).toEqual(expected) + }) + }) + describe("real-world wrapped multi-line script (regression)", () => { it("treats a wrapper command with an embedded multi-line script as a single command", () => { const input = [ @@ -183,6 +249,19 @@ describe("parseCommand", () => { const { commands: result } = parseCommand("echo $(whoami)") expect(result).toContain("whoami") }) + + it.each([ + { input: "reboot$(printf 'x')", expected: ["printf 'x'", "reboot$(printf 'x')"] }, + { input: "reboot$(printf '\n')", expected: ["printf '\n'", "reboot$(printf '\n')"] }, + { input: "$(printf re)boot", expected: ["printf re", "$(printf re)boot"] }, + { input: "$(printf re)boot 'a\nb'", expected: ["printf re", "$(printf re)boot 'a\nb'"] }, + { input: "`printf re`boot", expected: ["printf re", "$(printf re)boot"] }, + { input: "echo*$(reboot)", expected: ["reboot", "echo*$(reboot)"] }, + { input: "ls *$(reboot).ts 'a\nb'", expected: ["reboot", "ls *$(reboot).ts 'a\nb'"] }, + { input: "echo $(a)$(b)", expected: ["echo", "a", "b"] }, + ])("keeps the outer word and every substitution in $input", ({ input, expected }) => { + expect(parseCommand(input).commands).toEqual(expected) + }) }) describe("shell-quote parse-failure fallback", () => { @@ -201,6 +280,13 @@ describe("parseCommand", () => { expect(result.join(" ")).not.toContain("SQUOTE") expect(result.join(" ")).not.toContain("__") }) + + it("splits outer operators and keeps operators inside a multi-line quote in the fallback path", () => { + forceShellQuoteFailure = true + + const { commands: result } = parseCommand(`cd a && node -e "x && y\nz | w" ; ls`) + expect(result).toEqual(["cd a", `node -e "x && y\nz | w"`, "ls"]) + }) }) describe("malformed input with unterminated quotes", () => { diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index 4195d99d4e..68d548d968 100644 --- a/src/shared/parse-command.ts +++ b/src/shared/parse-command.ts @@ -1,6 +1,7 @@ import { parse } from "shell-quote" -export type ShellToken = string | { op: string } | { command: string } +export type ShellToken = + string | { op: "glob"; pattern: string } | { op: string } | { command: string } | { comment: string } /** * The style of quoting that opened a region. @@ -516,14 +517,18 @@ export function parseCommand(command: string): ParseResult { // Restore top-level quote placeholders before per-line parsing so that // parseCommandLine sees the original quoted content and can apply its own // masking for operator splitting. - const restoredLine = line.replace(/__TOPLEVEL_QUOTE_(\d+)__/g, (_, i) => topLevelQuotes[parseInt(i)]) + const restoreQuotes = (text: string) => + text.replace(/__TOPLEVEL_QUOTE_(\d+)__/g, (_, i) => topLevelQuotes[parseInt(i)]) + const restoredLine = restoreQuotes(line) // If the restored line contains embedded newlines it means a top-level - // quote (e.g. a heredoc) spanned multiple lines. The entire restored - // string is a single atomic command -- passing it through parseCommandLine - // would let shell-quote split on the embedded newlines and << operators. + // quote (e.g. a heredoc) spanned multiple lines. Passing the restored text + // to parseCommandLine would let shell-quote split on the embedded newlines + // and << operators, so split the masked line instead and restore each + // quote inside its own sub-command. Each chained command must reach the + // allowlist check on its own. if (restoredLine.includes("\n")) { - allCommands.push(restoredLine) + allCommands.push(...parseCommandLine(line).map(restoreQuotes)) continue } @@ -650,6 +655,12 @@ function parseCommandLine(command: string): string[] { return `__QUOTE_${quotes.length - 1}__` }) + // The shell starts a comment only at a `#` that begins a word. shell-quote + // starts one at any `#`, so a word such as `safe#x` would hide the rest of + // the line, including chained commands, from the allowlist check. Mask a `#` + // that follows a word character so shell-quote keeps it as text. + processedCommand = processedCommand.replace(/(?<=[^\s;&|()<>])#/g, "__HASH__") + let tokens: ShellToken[] try { tokens = parse(processedCommand) as ShellToken[] @@ -681,6 +692,26 @@ function parseCommandLine(command: string): string[] { const commands: string[] = [] let currentCommand: string[] = [] + const addWord = (word: string) => { + const subshellMatches = [...word.matchAll(/__SUBSH_(\d+)__/g)] + if (subshellMatches.length === 0) { + currentCommand.push(word) + return + } + if (word.replace(/__SUBSH_\d+__/g, "")) { + // The substitution output is part of this word, for example + // `$(printf re)boot` runs `reboot`. Keep the substitution text in the + // word: the allowlist check must not see only the literal part. + currentCommand.push(word.replace(/__SUBSH_(\d+)__/g, (_, i) => `$(${subshells[parseInt(i)]})`)) + } else if (currentCommand.length > 0) { + commands.push(currentCommand.join(" ")) + currentCommand = [] + } + for (const match of subshellMatches) { + commands.push(subshells[parseInt(match[1])]) + } + } + for (const token of tokens) { if (typeof token === "object" && "op" in token) { // Chain operator - split command @@ -689,22 +720,16 @@ function parseCommandLine(command: string): string[] { commands.push(currentCommand.join(" ")) currentCommand = [] } + } else if ("pattern" in token) { + // shell-quote returns a glob as { op: "glob", pattern }. Keep the glob + // text (for example `*`) so that deny rules can match it. + addWord(token.pattern) } else { // Other operators (>) are part of the command currentCommand.push(token.op) } } else if (typeof token === "string") { - // Check if it's a subshell placeholder - const subshellMatch = token.match(/__SUBSH_(\d+)__/) - if (subshellMatch) { - if (currentCommand.length > 0) { - commands.push(currentCommand.join(" ")) - currentCommand = [] - } - commands.push(subshells[parseInt(subshellMatch[1])]) - } else { - currentCommand.push(token) - } + addWord(token) } } @@ -755,5 +780,6 @@ function restorePlaceholders( // Restore variable references result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)]) result = result.replace(/__SUBSH_(\d+)__/g, (_, i) => subshells[parseInt(i)]) + result = result.replace(/__HASH__/g, "#") return result }