From dc0bfc5ea4691679d4bd936d202403a2fd69ec3f Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 29 Oct 2026 15:29:19 -0700 Subject: [PATCH] fix(core): check redirects, PowerShell method calls, and dynamic directory changes (#60376) --- packages/core/src/schedule/check.ts | 2 +- packages/core/src/shell/parse.ts | 296 +++++++++++++++--- packages/core/src/shell/scan.ts | 200 +++++++++--- packages/core/test/permission.test.ts | 80 ++++- .../test/shell-parse-compatibility.test.ts | 109 +++++-- packages/core/test/shell-parse-parity.test.ts | 31 +- packages/core/test/shell-parse-syntax.test.ts | 8 +- packages/core/test/shell-parse.test.ts | 61 +++- .../core/test/shell-scan/bash-safety.test.ts | 61 ++-- .../core/test/shell-scan/bash-syntax.test.ts | 17 +- .../test/shell-scan/compound-parity.test.ts | 9 +- packages/core/test/shell-scan/oracle.test.ts | 2 +- .../shell-scan/parity-regressions.test.ts | 29 +- .../test/shell-scan/powershell-syntax.test.ts | 20 ++ .../core/test/shell-scan/redirect.test.ts | 86 ++--- packages/core/test/shell-scan/scan.test.ts | 6 +- packages/core/test/tool-shell.test.ts | 19 +- packages/schema/src/config/experimental.ts | 2 +- 18 files changed, 788 insertions(+), 250 deletions(-) diff --git a/packages/core/src/schedule/check.ts b/packages/core/src/schedule/check.ts index aea13b2ee3a..d628e29d4ae 100644 --- a/packages/core/src/schedule/check.ts +++ b/packages/core/src/schedule/check.ts @@ -344,7 +344,7 @@ const scanShell = Effect.fnUntraced(function* (command: string) { .resolve({ path: location.directory, kind: "directory" }) .pipe(Effect.mapError((error) => error.message)) const shell = yield* shellSelect.resolve({ priority: "compat" }) - const parsed = yield* ShellParse.scanPortable(command, shell, target.absolute).pipe( + const parsed = yield* ShellParse.scan(command, shell, target.absolute).pipe( Effect.mapError((error) => error.message), ) const directories = yield* Effect.forEach(parsed.directories, (directory) => diff --git a/packages/core/src/shell/parse.ts b/packages/core/src/shell/parse.ts index 356fbb6705a..ef0f25c2075 100644 --- a/packages/core/src/shell/parse.ts +++ b/packages/core/src/shell/parse.ts @@ -1,16 +1,19 @@ export * as ShellParse from "./parse.js" -import { Effect } from "effect" +import { Effect, Result } from "effect" +import fs from "fs" import os from "os" import path from "path" import { ShellScan } from "./scan.js" import { ShellSelect } from "./select.js" import { Wildcard } from "../util/wildcard.js" -type Part = { type: string; text: string; unquotedTilde?: boolean } +type Part = { type: string; text: string; raw: string; unquotedTilde?: boolean } const CWD = new Set(["cd", "chdir", "popd", "pushd", "push-location", "set-location"]) const POWERSHELL_PATH_FLAG_RE = /^-(?:p(?:ath|at|a)?|lp|l(?:i(?:t(?:e(?:r(?:a(?:l(?:p(?:a(?:t(?:h)?)?)?)?)?)?)?)?)?)?|psp(?:a(?:t(?:h)?)?)?):?$/i +const POWERSHELL_LITERAL_FLAG_RE = /^-(?:lp|l(?:i(?:t(?:e(?:r(?:a(?:l(?:p(?:a(?:t(?:h)?)?)?)?)?)?)?)?)?)?|psp(?:a(?:t(?:h)?)?)?):?$/i +const POWERSHELL_STACK_FLAG_RE = /^-(?:stack(?:name|nam|na|n)?|st):?$/i export type Result = { commands: Array<{ resource: string; save: string }> @@ -172,6 +175,13 @@ export function scan(command: string, shell: string, cwd: string): Effect.Effect for (const item of scanned.commands) { // Declarations are skipped as standalone permission resources, while nested substitutions remain. if (item.declaration) continue + if (item.redirectPrefixEnd !== undefined) { + const head = item.resource.slice(0, item.redirectPrefixEnd) + const next = item.resource[item.redirectPrefixEnd] + const separator = next === "\t" || next === "\n" ? next : " " + output.commands.push({ resource: item.resource, save: `${head}${separator}*` }) + continue + } const words = item.redirectWordCount === undefined ? item.rawWords : item.rawWords.slice(0, item.redirectWordCount) // Bare statement-head foreach blocks act as PowerShell control flow rather than cmdlet invocations. if ( @@ -186,29 +196,33 @@ export function scan(command: string, shell: string, cwd: string): Effect.Effect ? words.flatMap((raw, index): Part[] => { const text = !raw.includes("`$") && /['"`]/.test(raw) ? (item.words[index] ?? raw) : raw.replaceAll("`$", "\0") - if (!/^-[A-Za-z_?][\w?-]*(?::|$)/.test(raw)) return [{ type: "word", text }] + if (!/^-[A-Za-z_?][\w?-]*(?::|$)/.test(raw)) return [{ type: "word", text, raw }] const colon = text.indexOf(":") - if (colon >= 0) + if (colon >= 0) { + const rawColon = raw.indexOf(":") return colon + 1 < text.length ? [ - { type: "command_parameter", text: text.slice(0, colon) }, - { type: "word", text: text.slice(colon + 1) }, + { type: "command_parameter", text: text.slice(0, colon), raw: raw.slice(0, rawColon) }, + { type: "word", text: text.slice(colon + 1), raw: rawColon >= 0 ? raw.slice(rawColon + 1) : raw }, ] - : [{ type: "command_parameter", text: text.slice(0, colon) }] - return [{ type: "command_parameter", text }] + : [{ type: "command_parameter", text: text.slice(0, colon), raw }] + } + return [{ type: "command_parameter", text, raw }] }) : item.rawWords.map((raw, index): Part => { const word = item.words[index] ?? raw return { type: "word", text: isStaticBashWord(raw) ? word.replaceAll("$", "\0") : "$", + raw, unquotedTilde: raw.startsWith("~"), } }) - const directoryCommand = unwrapDirectoryCommand(words, parts, powershell) + const directoryCommand = unwrapDirectoryCommand(powershell ? words : item.rawWords, parts, powershell) if (directoryCommand) { - output.directories.push(...directoryArgs(directoryCommand, powershell, cwd, shell)) - if (directoryCommand === parts) continue + const resolved = directoryArgs(directoryCommand, powershell, cwd, shell) + output.directories.push(...resolved.directories) + if (directoryCommand === parts && resolved.complete) continue } const selected = prefix(words.slice(0, PREFIX_LENGTH)) const conventional = `${selected.join(" ")} *` @@ -217,14 +231,16 @@ export function scan(command: string, shell: string, cwd: string): Effect.Effect const save = !powershell || end === undefined || Wildcard.match(item.resource, conventional) ? conventional - : (() => { - const boundary = - item.wordEnds?.find( - (value) => value >= end && (value >= item.resource.length || /\s/.test(item.resource[value])), - ) ?? end - const separator = /^\s+(?:`(?:\r\n|\r|\n)\s*)?/.exec(item.resource.slice(boundary))?.[0] - return `${item.resource.slice(0, boundary)}${separator ?? " "}*` - })() + : item.resource[end] === "(" + ? `${item.resource.slice(0, end)}(*` + : (() => { + const boundary = + item.wordEnds?.find( + (value) => value >= end && (value >= item.resource.length || /\s/.test(item.resource[value])), + ) ?? end + const separator = /^\s+(?:`(?:\r\n|\r|\n)\s*)?/.exec(item.resource.slice(boundary))?.[0] + return `${item.resource.slice(0, boundary)}${separator ?? " "}*` + })() output.commands.push({ resource: item.resource, save, @@ -234,8 +250,6 @@ export function scan(command: string, shell: string, cwd: string): Effect.Effect }) } -export const scanPortable = scan - function unwrapDirectoryCommand(rawWords: string[], parts: Part[], powershell: boolean) { const head = powershell ? rawWords[0]?.toLowerCase() : rawWords[0] if (CWD.has(head ?? "")) return parts @@ -256,24 +270,58 @@ function unwrapDirectoryCommand(rawWords: string[], parts: Part[], powershell: b function isStaticBashWord(raw: string) { let quote: "'" | '"' | "$'" | undefined + let segmentStarted = false + let segmentLeadingDot = false + let segmentGlob = false + let hasAnsiEscape = false + let hasUnquotedGlob = false + const braces: boolean[] = [] + const recordChar = (char: string, unquotedGlob = false) => { + if (unquotedGlob) hasUnquotedGlob = true + if (hasAnsiEscape && hasUnquotedGlob) return false + if (char === "/") { + segmentStarted = false + segmentLeadingDot = false + segmentGlob = false + return true + } + if (!segmentStarted) { + segmentStarted = true + segmentLeadingDot = char === "." + } + if (unquotedGlob) segmentGlob = true + return !(segmentLeadingDot && segmentGlob) + } + for (let index = 0; index < raw.length; index++) { const char = raw[index] const next = raw[index + 1] ?? "" if (quote === "'") { - if (char === "'") quote = undefined + if (char === "'") { + quote = undefined + continue + } + if (!recordChar(char)) return false continue } if (quote === "$'") { if (char === "\\") { + hasAnsiEscape = true + if (hasUnquotedGlob) return false index++ continue } - if (char === "'") quote = undefined + if (char === "'") { + quote = undefined + continue + } + if (!recordChar(char)) return false continue } if (quote === '"') { if (char === "\\" && '$`"\\\n'.includes(next)) { index++ + if (next !== "\n" && !recordChar(next)) return false continue } if (char === '"') { @@ -282,10 +330,12 @@ function isStaticBashWord(raw: string) { } if (char === "`" || (char === "$" && (/[A-Za-z_0-9@*#?$!({[-]/.test(next) || raw.startsWith("\\\n", index + 1)))) return false + if (!recordChar(char)) return false continue } if (char === "\\") { index++ + if (next !== "\n" && !recordChar(next)) return false continue } if (char === "'") { @@ -312,53 +362,165 @@ function isStaticBashWord(raw: string) { (char === "$" && (/[A-Za-z_0-9@*#?$!({[-]/.test(next) || raw.startsWith("\\\n", index + 1))) ) return false + if (char === "{") { + braces.push(false) + if (!recordChar(char)) return false + continue + } + if (braces.length > 0 && (char === "," || (char === "." && next === "."))) { + braces[braces.length - 1] = true + } + if (char === "}" && braces.length > 0) { + if (braces.pop()) return false + if (!recordChar(char)) return false + continue + } + if (!recordChar(char, char === "*" || char === "?" || char === "[")) return false } return quote === undefined } +function hasPowerShellDotWildcard(raw: string) { + let quote: "'" | '"' | undefined + let segmentStarted = false + let segmentLeadingDot = false + let segmentGlob = false + let hasQuoteEscape = false + let hasUnquotedGlob = false + const recordChar = (char: string, unquotedGlob = false) => { + if (unquotedGlob) hasUnquotedGlob = true + if (hasQuoteEscape && hasUnquotedGlob) return true + if (char === "/" || char === "\\") { + segmentStarted = false + segmentLeadingDot = false + segmentGlob = false + return false + } + if (!segmentStarted) { + segmentStarted = true + segmentLeadingDot = char === "." + } + if (unquotedGlob) segmentGlob = true + return segmentLeadingDot && segmentGlob + } + + for (let index = 0; index < raw.length; index++) { + const char = raw[index] + const next = raw[index + 1] ?? "" + if (quote === "'") { + if (char === "'" && next === "'") { + index++ + if (recordChar("'")) return true + continue + } + if (char === "'") { + quote = undefined + continue + } + if (recordChar(char)) return true + continue + } + if (quote === '"') { + if (char === "`") { + hasQuoteEscape = true + if (hasUnquotedGlob) return true + index++ + continue + } + if (char === '"' && next === '"') { + index++ + if (recordChar('"')) return true + continue + } + if (char === '"') { + quote = undefined + continue + } + if (recordChar(char)) return true + continue + } + if (char === "`") { + index++ + if (recordChar(next)) return true + continue + } + if (char === "'" || char === '"') { + quote = char + continue + } + if (recordChar(char, char === "*" || char === "?" || char === "[")) return true + } + return false +} + function directoryArgs(command: Part[], powershell: boolean, cwd: string, shell: string) { + const head = command[0]?.text.toLowerCase() ?? "" + if (head === "popd" || head === "pop-location") return { directories: [], complete: false } + if (!powershell) { let endOfOptions = false - return command - .slice(1) - .filter((part) => { - if (endOfOptions) return true - if (part.text === "--") { - endOfOptions = true - return false - } - return !part.text.startsWith("-") - }) - .map((part) => directoryArgument(part.text, powershell, cwd, shell, part.unquotedTilde)) - .filter((part) => part !== undefined) + const operands = command.slice(1).filter((part) => { + if (endOfOptions) return true + if (part.text === "--") { + endOfOptions = true + return false + } + if (part.text === "-") return true + return !part.text.startsWith("-") + }) + if (operands.length === 0) return { directories: [], complete: false } + const resolved = operands.map((part) => + part.text === "-" ? undefined : directoryArgument(part.text, false, cwd, shell, part.unquotedTilde), + ) + const directories = resolved.filter((item): item is string => item !== undefined) + return { directories, complete: directories.length === operands.length } } const directories: string[] = [] - let path = false + let pathFlag = false + let literalFlag = false let endOfParameters = false + let sawOperand = false + let complete = true for (const part of command.slice(1)) { if (!endOfParameters && part.text === "--") { endOfParameters = true continue } - if (path) { - const value = directoryArgument(part.text, powershell, cwd, shell) + if (pathFlag) { + sawOperand = true + const value = + !literalFlag && (hasPowerShellDotWildcard(part.raw) || part.text === "-" || part.text === "+") + ? undefined + : directoryArgument(part.text, true, cwd, shell) if (value) directories.push(value) - path = false + if (!value) complete = false + pathFlag = false + literalFlag = false continue } if (!endOfParameters && part.type === "command_parameter") { - path = POWERSHELL_PATH_FLAG_RE.test(part.text) + if (POWERSHELL_STACK_FLAG_RE.test(part.text)) { + complete = false + return { directories, complete } + } + pathFlag = POWERSHELL_PATH_FLAG_RE.test(part.text) + literalFlag = POWERSHELL_LITERAL_FLAG_RE.test(part.text) continue } - const value = directoryArgument(part.text, powershell, cwd, shell) + sawOperand = true + const value = + hasPowerShellDotWildcard(part.raw) || (!endOfParameters && (part.raw === "-" || part.raw === "+")) + ? undefined + : directoryArgument(part.text, true, cwd, shell) if (value) directories.push(value) + if (!value) complete = false } - return directories + return { directories, complete: sawOperand && complete } } function directoryArgument(value: string, powershell: boolean, cwd: string, shell: string, unquotedTilde = false) { - if (!powershell) return expandKnownDirectory(value, false, unquotedTilde) + if (!powershell) return expandKnownDirectory(value, false, cwd, unquotedTilde) const quote = value[0] const text = (quote === '"' || quote === "'") && value.at(-1) === quote ? value.slice(1, -1) : value @@ -375,10 +537,11 @@ function directoryArgument(value: string, powershell: boolean, cwd: string, shel return path.dirname(shell) }), true, + cwd, ) } -function expandKnownDirectory(value: string, powershell: boolean, unquotedTilde = false) { +function expandKnownDirectory(value: string, powershell: boolean, cwd: string, unquotedTilde = false) { // Unknown shell expressions cannot be resolved safely during permission analysis. if (value.includes("$") || value.includes("`") || (powershell && value.startsWith("("))) return const resolved = value.replaceAll("\0", "$") @@ -387,21 +550,52 @@ function expandKnownDirectory(value: string, powershell: boolean, unquotedTilde return path.join(os.homedir(), resolved.slice(2)) } if (unquotedTilde && process.platform !== "win32") { - const userMatch = /^~([A-Za-z0-9._-]+)(?=$|\/)/.exec(resolved) + if (resolved === "~+") return cwd + if (resolved.startsWith("~+/")) return path.join(cwd, resolved.slice(3)) + if (/^~(?:[-+](?:\d+)?|\d+)(?=$|\/)/.test(resolved)) return + const userMatch = /^~([A-Za-z_][A-Za-z0-9._-]*)(?=$|\/)/.exec(resolved) if (userMatch) { const user = userMatch[1] - const home = - user === "root" - ? process.platform === "darwin" - ? "/var/root" - : "/root" - : path.join(process.platform === "darwin" ? "/Users" : "/home", user) + const home = resolveUserHome(user) + if (!home) return return path.join(home, resolved.slice(user.length + 1)) } } return resolved } +let cachedPasswdHomes: ReadonlyMap | undefined + +function passwdHomes() { + if (cachedPasswdHomes) return cachedPasswdHomes + const content = Result.try(() => fs.readFileSync("/etc/passwd", "utf8")) + const entries = Result.isSuccess(content) + ? content.success.split("\n").flatMap((line): Array<[string, string]> => { + if (!line || line.startsWith("#")) return [] + const fields = line.split(":") + const name = fields[0] + const home = fields[5] + return name && home ? [[name, home]] : [] + }) + : [] + cachedPasswdHomes = new Map(entries) + return cachedPasswdHomes +} + +function resolveUserHome(user: string) { + const current = Result.try(() => os.userInfo()) + if (Result.isSuccess(current) && current.success.username === user && current.success.homedir) { + return current.success.homedir + } + const fromPasswd = passwdHomes().get(user) + if (fromPasswd) return fromPasswd + if (process.platform === "darwin") { + const candidate = path.join("/Users", user) + const exists = Result.try(() => fs.statSync(candidate).isDirectory()) + if (Result.isSuccess(exists) && exists.success) return candidate + } +} + function environment(key: string) { if (process.platform !== "win32") return process.env[key] const name = Object.keys(process.env).find((item) => item.toLowerCase() === key.toLowerCase()) diff --git a/packages/core/src/shell/scan.ts b/packages/core/src/shell/scan.ts index bad56f3e95b..a8ad405f994 100644 --- a/packages/core/src/shell/scan.ts +++ b/packages/core/src/shell/scan.ts @@ -19,8 +19,10 @@ type Command = { wordEnds?: number[] statementHead?: true declaration?: true - // Words after a trailing redirect are destinations in the legacy command span. + // Words before the first redirect when redirects appear between arguments. redirectWordCount?: number + // Exclusive prefix end relative to resource for commandless or group redirects. + redirectPrefixEnd?: number } // Opaque describes a parsing limitation, not a permission decision. @@ -185,9 +187,8 @@ type BashState = { wordStart: number wordEnd: number commandStart: number | undefined - commandEnd: number - resourceEnd: number | undefined redirectWordCount: number | undefined + redirectPrefixEnd: number | undefined // Posix shells such as Dash read `&>` as `&` and `>`, so later words would start another command there. ampersandRedirectWords: number | undefined commandWordIndex: number @@ -195,8 +196,10 @@ type BashState = { assignmentHeadUnsafe: boolean invalid: OpaqueReason | undefined redirectTarget: boolean + dupRedirect: boolean hereString: boolean hasRedirect: boolean + hasFileRedirect: boolean compoundEnd: boolean // A list operator (&&, ||, |, |&) still awaits its right-hand command. dangling: boolean @@ -286,10 +289,14 @@ function finishBashWord(state: BashState) { state.words.push(state.word) // Unquoted trailing continuations are ignored syntax, not part of the raw token. state.rawWords.push(state.input.slice(state.wordStart, state.wordEnd)) - state.commandEnd = state.wordEnd } - if (state.hereString && state.resourceEnd === undefined) state.commandEnd = state.wordEnd + if (state.redirectTarget) { + if (!state.dupRedirect || !/^(?:\d+-?|-)$/.test(state.input.slice(state.wordStart, state.wordEnd))) + state.hasFileRedirect = true + state.redirectPrefixEnd ??= state.wordEnd + } state.redirectTarget = false + state.dupRedirect = false state.hereString = false state.word = "" state.literal = "" @@ -369,14 +376,15 @@ function finishBashCommand(state: BashState, boundary = false, newline = false) const inHeader = bashInHeader(state) if (name >= 0 && !state.words[name]) state.invalid ??= "invalid-structure" if (name >= 0 && !inHeader) { - const resource = state.input - .slice(state.commandStart, state.resourceEnd ?? state.wordEnd) - .replace(/^[ \t\n]+|[ \t\n]+$/g, "") + const resource = state.input.slice(state.commandStart, state.wordEnd).replace(/^[ \t\n]+|[ \t\n]+$/g, "") const command: Command = { resource, words: state.words.slice(name), rawWords: state.rawWords.slice(name), - ...(name === 0 && BASH_DECLARATIONS.has(state.rawWords[0]) && resource.startsWith(state.rawWords[0]) + ...(name === 0 && + !state.hasRedirect && + BASH_DECLARATIONS.has(state.rawWords[0]) && + resource.startsWith(state.rawWords[0]) ? { declaration: true as const } : {}), ...(state.redirectWordCount !== undefined && state.redirectWordCount < state.words.length @@ -424,11 +432,42 @@ function finishBashCommand(state: BashState, boundary = false, newline = false) for (const heredoc of state.heredocs) { if (heredoc.owned) continue heredoc.owned = true - if (newline && state.resourceEnd === undefined) { + if (newline) { heredoc.command = command heredoc.start = state.commandStart } } + } else if ( + name < 0 && + !inHeader && + state.commandStart !== undefined && + (state.hasFileRedirect || (state.hasRedirect && !state.compoundEnd)) + ) { + const rawSpan = state.input.slice(state.commandStart, state.wordEnd) + const leading = /^[ \t\n]+/.exec(rawSpan)?.[0].length ?? 0 + const resource = rawSpan.slice(leading).replace(/[ \t\n]+$/g, "") + if (resource) { + const prefixEnd = + state.redirectPrefixEnd !== undefined + ? Math.max(1, Math.min(resource.length, state.redirectPrefixEnd - state.commandStart - leading)) + : resource.length + const prefixToken = resource.slice(0, prefixEnd) + const command: Command = { + resource, + words: [prefixToken], + rawWords: [prefixToken], + redirectPrefixEnd: prefixEnd, + } + state.commands.push(command) + for (const heredoc of state.heredocs) { + if (heredoc.owned) continue + heredoc.owned = true + if (newline) { + heredoc.command = command + heredoc.start = state.commandStart + } + } + } } for (const heredoc of state.heredocs) heredoc.owned = true if (boundary && !state.words.length && !state.hasRedirect && !state.compoundEnd && !inHeader) @@ -440,9 +479,11 @@ function finishBashCommand(state: BashState, boundary = false, newline = false) state.rawWords.length = 0 state.commandWordIndex = -1 state.commandStart = undefined + state.dupRedirect = false state.hasRedirect = false - state.resourceEnd = undefined + state.hasFileRedirect = false state.redirectWordCount = undefined + state.redirectPrefixEnd = undefined state.ampersandRedirectWords = undefined state.compoundEnd = false state.dangling = false @@ -475,17 +516,18 @@ function scanBash( wordStart: start, wordEnd: start, commandStart: undefined, - commandEnd: start, - resourceEnd: undefined, redirectWordCount: undefined, + redirectPrefixEnd: undefined, ampersandRedirectWords: undefined, commandWordIndex: -1, assignmentWord: false, assignmentHeadUnsafe: false, invalid: undefined, redirectTarget: false, + dupRedirect: false, hereString: false, hasRedirect: false, + hasFileRedirect: false, compoundEnd: false, dangling: false, outer, @@ -681,8 +723,6 @@ function scanBashOperatorOrSeparator( const fdPrefix = state.wordStarted && !state.assignmentHeadUnsafe && /^(?:\d+|\{[A-Za-z_][A-Za-z0-9_]*\})$/.test(state.word) if (fdPrefix) { - // A continuation separates the legacy number token from the redirect descriptor. - if (state.wordEnd < index) state.commandEnd = state.wordEnd state.word = "" state.literal = "" state.output = "" @@ -690,23 +730,24 @@ function scanBashOperatorOrSeparator( } if (!fdPrefix) finishBashWord(state) if (redirect.startsWith("&") && context.dialect === "posix") state.ampersandRedirectWords ??= state.words.length - // Trailing redirects wrap a whole list/pipeline in the legacy grammar, not its last command. - // Prefix redirects remain part of the command, and later words remain redirect destinations. if (state.redirectWordCount === undefined && state.commandWordIndex >= 0) { state.redirectWordCount = state.words.length } - if (state.dangling && redirect !== "<<<" && state.commandWordIndex >= 0) { - state.resourceEnd ??= state.commandEnd - } if (redirect === "<<" || redirect === "<<-") { const delimiter = bashHeredocDelimiter(input, index + redirect.length, redirect === "<<-") if (!delimiter) return { kind: "opaque", reason: "invalid-redirect" } + state.hasFileRedirect = true + state.redirectPrefixEnd ??= + " \t".includes(input[index + redirect.length] ?? "") ? index + redirect.length : delimiter.end + 1 state.heredocs.push(delimiter) state.wordEnd = delimiter.end + 1 state.redirectTarget = false + state.dupRedirect = false return delimiter.end } + if (" \t\n".includes(input[index + redirect.length] ?? "")) state.redirectPrefixEnd ??= index + redirect.length state.redirectTarget = true + state.dupRedirect = redirect === "<&" || redirect === ">&" state.hereString = redirect === "<<<" return index + redirect.length - 1 } @@ -1838,6 +1879,9 @@ function scanPowerShellList( let clause: PowerShellClause = initialClause let after: "do" | "if" | "try" | undefined let indexable: false | "variable" | "closed" | "member" = false + let memberChainStart: number | undefined + let hasMemberAccess = false + let hasFileRedirect = false let stopParsing = false let commandEnd = start let statementHead = true @@ -1846,6 +1890,11 @@ function scanPowerShellList( let dangling = false let invocation = false + const resetMember = () => { + indexable = false + memberChainStart = undefined + hasMemberAccess = false + } const finishWord = (end: number) => { if (!started) return // Generic tokens can spell stop-parsing with escapes or embedded quotes; literal strings cannot. @@ -1861,7 +1910,7 @@ function scanPowerShellList( ) stopParsing = true if (!redirectTarget) { - if (!words.length && !invocation && !expression) segment = wordStart + if (!words.length && !invocation) segment = wordStart words.push(word) rawWords.push(input.slice(wordStart, end)) wordEnds.push(end) @@ -1874,13 +1923,13 @@ function scanPowerShellList( } const finishCommand = (end: number, required = false) => { finishWord(end) - if (words.length && !expression && !clause) + if (words.length && ((!expression && !clause) || hasFileRedirect)) commands.push({ resource: input.slice(segment, commandEnd).trimEnd(), words: [...words], rawWords: [...rawWords], wordEnds: wordEnds.map((value) => value - segment), - ...(statementHead && !invocation ? { statementHead: true as const } : {}), + ...(statementHead && !invocation && !expression && !clause ? { statementHead: true as const } : {}), }) else if ( (!expression && !clause && invocation) || @@ -1894,11 +1943,12 @@ function scanPowerShellList( rawWords.length = 0 wordEnds.length = 0 redirectTarget = false + hasFileRedirect = false invocation = false expression = initialExpression clause = initialClause after = undefined - indexable = false + resetMember() stopParsing = false } @@ -1960,8 +2010,13 @@ function scanPowerShellList( if (char === "'" || char === '"') { if (!started && words.length === 0 && !invocation) expression = true quote = char - standalone = atBoundary || (expression && !redirectTarget) || (indexable === "member" && /[.:]$/.test(word)) - if (!standalone) indexable = false + const dynamicMember = indexable === "member" && /[.:]$/.test(started ? word : (words.at(-1) ?? "")) + standalone = atBoundary || (expression && !redirectTarget) || dynamicMember + if (dynamicMember) hasMemberAccess = true + else if (standalone) { + memberChainStart = index + hasMemberAccess = false + } else resetMember() started = true continue } @@ -1973,7 +2028,7 @@ function scanPowerShellList( // At a token boundary escaped whitespace is trivia, not a new argument. else if (!atBoundary || !rawSpace) { started = true - indexable = false + if (!(indexable === "member" && /[\p{L}\p{Nl}\d_]/u.test(escape.value))) resetMember() word += escape.value } index = escape.end @@ -1998,7 +2053,7 @@ function scanPowerShellList( if (started) finishWord(index) const end = input.indexOf("#>", index + 2) if (end < 0) return { kind: "opaque", reason: "invalid-structure" } - if (!memberOperator) indexable = false + if (!memberOperator) resetMember() index = end + 1 continue } @@ -2035,7 +2090,8 @@ function scanPowerShellList( if (redirectTarget || words.length === 0 || (clause && clause !== "member")) return { kind: "opaque", reason: "invalid-redirect" } redirectTarget = !redirect.includes("&") - indexable = false + if (redirectTarget) hasFileRedirect = true + resetMember() index += redirect.length - 1 commandEnd = index + 1 continue @@ -2104,7 +2160,8 @@ function scanPowerShellList( rawWords.length = 0 wordEnds.length = 0 expression = false - indexable = false + hasFileRedirect = false + resetMember() if (!assign || char !== "=") index++ continue } @@ -2121,6 +2178,8 @@ function scanPowerShellList( started = true index = literal.end finishWord(index + 1) + memberChainStart = wordStart + hasMemberAccess = false indexable = "closed" continue } @@ -2128,7 +2187,15 @@ function scanPowerShellList( const end = powerShellBracedVariable(input, index, skipped) if (end === undefined) return { kind: "opaque", reason: "invalid-structure" } if (!started && !words.length && !invocation && hash !== "switch") expression = true - indexable = atBoundary || (indexable === "member" && /[.:]$/.test(word)) ? "closed" : false + const dynamicMember = indexable === "member" && /[.:]$/.test(started ? word : (words.at(-1) ?? "")) + if (dynamicMember) { + indexable = "closed" + hasMemberAccess = true + } else if (atBoundary) { + memberChainStart = index + hasMemberAccess = false + indexable = "closed" + } else resetMember() word += input.slice(index, end + 1) started = true index = end @@ -2149,7 +2216,17 @@ function scanPowerShellList( ? index : undefined if (opener !== undefined) { + const tokenIndex = index const openChar = input[opener] + const prevMemberWord = started ? word : (words.at(-1) ?? "") + const dynamicMemberOpener = indexable === "member" && /[.:]$/.test(prevMemberWord) + const priorIndexable = Boolean(indexable) + const isMethodCall = + char === "(" && + opener === tokenIndex && + memberChainStart !== undefined && + ((indexable === "member" && started && !/[.:]$/.test(word)) || + (indexable === "closed" && !started && hasMemberAccess)) const standaloneBlock = atBoundary || inExpression || char === "(" || openChar === "{" const blockClose = openChar === "(" ? ")" : openChar === "[" ? "]" : "}" const blockHash = @@ -2179,12 +2256,28 @@ function scanPowerShellList( started = true word += input.slice(index, result.end + 1) index = result.end - if (openChar === "[") { + if (isMethodCall && memberChainStart !== undefined) { + finishWord(index + 1) + const callResource = input.slice(memberChainStart, result.end + 1) + const head = input.slice(memberChainStart, opener) + const args = input.slice(opener, result.end + 1) + nestedCommands.push({ + resource: callResource, + words: [head, args], + rawWords: [head, args], + wordEnds: [head.length, callResource.length], + }) indexable = "closed" + hasMemberAccess = false + } else if (openChar === "[") { + indexable = "closed" + if (!priorIndexable) memberChainStart = tokenIndex if (input[index + 1] !== "[") finishWord(index + 1) } else if (standaloneBlock) { finishWord(index + 1) indexable = "closed" + if (!dynamicMemberOpener) memberChainStart = tokenIndex + hasMemberAccess = dynamicMemberOpener } if ((clause === "paren" && char === "(") || (clause && clause !== "paren" && char === "{")) { const completed = clause === "do" || clause === "if" || clause === "try" ? clause : undefined @@ -2210,8 +2303,9 @@ function scanPowerShellList( continue } if (/\s/.test(char) && char !== "\n" && char !== "\r") { + const memberOperator = indexable === "member" && /[.:]$/.test(started ? word : (words.at(-1) ?? "")) finishWord(index) - indexable = false + if (!memberOperator) resetMember() continue } const next = input[index + 1] @@ -2235,12 +2329,12 @@ function scanPowerShellList( if ((clause && clause !== "member") || prev.endsWith(",") || memberOperator) { finishWord(index) index += separator.length - 1 - if (!memberOperator) indexable = false + if (!memberOperator) resetMember() continue } if (!started && !words.length && !invocation) { index += separator.length - 1 - indexable = false + resetMember() continue } } @@ -2252,39 +2346,53 @@ function scanPowerShellList( } if (!started && !words.length && !invocation && hash !== "switch" && testAt(POWERSHELL_EXPRESSION_RE, input, index)) expression = true - if ((atBoundary || (indexable === "member" && /[.:]$/.test(word))) && char === "$") - indexable = testAt(POWERSHELL_INDEXABLE_VAR_RE, input, index + 1) ? "variable" : false - else if (indexable === "closed") + const prevMember = started ? word : (words.at(-1) ?? "") + if ((atBoundary || (indexable === "member" && /[.:]$/.test(prevMember))) && char === "$") { + if (testAt(POWERSHELL_INDEXABLE_VAR_RE, input, index + 1)) { + if (indexable === "member" && /[.:]$/.test(prevMember)) hasMemberAccess = true + else { + memberChainStart = index + hasMemberAccess = false + } + indexable = "variable" + } else resetMember() + } else if (indexable === "closed") { indexable = - (char === "." && input[index + 1] !== "." && !/\s/.test(input[index + 1] ?? "")) || - (!started && char === ":" && input[index + 1] === ":" && !/\s/.test(input[index + 2] ?? "")) || + (char === "." && input[index + 1] !== "." && (!/\s/.test(input[index + 1] ?? "") || inExpression)) || + (!started && char === ":" && input[index + 1] === ":") || (!started && char === "?" && input[index + 1] === "." && input[index + 2] !== "." && - !/\s/.test(input[index + 2] ?? "")) + (!/\s/.test(input[index + 2] ?? "") || inExpression)) ? "member" : !started && char === "?" && input[index + 1] === "[" ? "closed" : false - else if (indexable === "variable") + if (indexable === "member") hasMemberAccess = true + else if (!indexable) resetMember() + } else if (indexable === "variable") { indexable = - (char === "." && input[index + 1] !== "." && !/\s/.test(input[index + 1] ?? "")) || - (char === ":" && input[index + 1] === ":" && !/\s/.test(input[index + 2] ?? "")) + (char === "." && input[index + 1] !== "." && (!/\s/.test(input[index + 1] ?? "") || inExpression)) || + (char === ":" && input[index + 1] === ":") ? "member" : (char === "." && input[index + 1] === ".") || !/[\p{L}\p{Nl}\d_:?]/u.test(char) ? false : "variable" - else if (indexable === "member") + if (indexable === "member") hasMemberAccess = true + else if (!indexable) resetMember() + } else if (indexable === "member") { indexable = char === "." && input[index + 1] === "." ? false - : (char === "." && !/\s/.test(input[index + 1] ?? "")) || + : (char === "." && (!/\s/.test(input[index + 1] ?? "") || inExpression)) || (char === ":" && (input[index + 1] === ":" || input[index - 1] === ":")) || (char === "?" && input[index + 1] === "." && input[index + 2] !== ".") || /[\p{L}\p{Nl}\d_]/u.test(char) ? "member" : false + if (!indexable) resetMember() + } after = undefined started = true dangling = false diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index 99340d1f90c..442c1bea06a 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -521,9 +521,9 @@ describe("shell scanner permission impact", () => { command: "printf ok && git status > output", approved: ["printf *", "git status *"], exact: ["printf ok", "git status"], - denied: "git status", + denied: "git status *", savedEffect: ["allow", "allow"], - exactEffect: ["allow", "allow"], + exactEffect: ["allow", "ask"], deniedEffect: ["deny", "deny"], }, { @@ -532,9 +532,9 @@ describe("shell scanner permission impact", () => { command: "printf ok | cat < input > output", approved: ["printf *", "cat *"], exact: ["printf ok", "cat"], - denied: "cat", + denied: "cat *", savedEffect: ["allow", "allow"], - exactEffect: ["allow", "allow"], + exactEffect: ["allow", "ask"], deniedEffect: ["deny", "deny"], }, { @@ -543,10 +543,10 @@ describe("shell scanner permission impact", () => { command: "FOO=bar > output; printf done", approved: ["printf *"], exact: ["printf done"], - denied: "FOO=bar > output; printf done", - savedEffect: ["ask", "allow"], - exactEffect: ["ask", "allow"], - deniedEffect: ["deny", "allow"], + denied: "FOO=bar > output", + savedEffect: ["ask", "ask"], + exactEffect: ["ask", "ask"], + deniedEffect: ["deny", "deny"], }, { name: "assignment redirect with an approved command substitution", @@ -555,9 +555,9 @@ describe("shell scanner permission impact", () => { approved: ["git describe *"], exact: ["git describe --tags"], denied: "VERSION=$(git describe --tags) > build/version.txt", - savedEffect: ["ask", "allow"], - exactEffect: ["ask", "allow"], - deniedEffect: ["deny", "allow"], + savedEffect: ["ask", "ask"], + exactEffect: ["ask", "ask"], + deniedEffect: ["deny", "deny"], }, { name: "substitution in a saved prefix", @@ -603,6 +603,39 @@ describe("shell scanner permission impact", () => { exactEffect: ["allow", "ask"], deniedEffect: ["deny", "allow"], }, + { + name: "trailing redirect after list operator", + shell: "bash", + command: "true && cat > secret", + approved: ["true *", "cat *"], + exact: ["true", "cat"], + denied: "* > secret*", + savedEffect: ["allow", "allow"], + exactEffect: ["allow", "ask"], + deniedEffect: ["allow", "deny"], + }, + { + name: "commandless redirect write", + shell: "zsh", + command: ">> ~/.zshrc <<< 'x'", + approved: [">> *"], + exact: [">> ~/.zshrc <<< 'x'"], + denied: ">> *", + savedEffect: ["allow", "allow"], + exactEffect: ["allow", "allow"], + deniedEffect: ["deny", "deny"], + }, + { + name: "PowerShell .NET method invocation", + shell: "pwsh", + command: '[IO.File]::Delete("C:\\victim")', + approved: ["[IO.File]::Delete(*"], + exact: ['[IO.File]::Delete("C:\\victim")'], + denied: "*Delete(*", + savedEffect: ["allow", "allow"], + exactEffect: ["allow", "allow"], + deniedEffect: ["deny", "deny"], + }, ] as const) { for (const scenario of [ { name: "no approval", saved: [], rules: [], expected: ["ask", "ask"] }, @@ -718,14 +751,31 @@ describe("shell scanner permission impact", () => { name: "assignment redirect prefix", shell: "bash", command: "FOO=bar > output; printf done", - grants: [["printf *"], ["printf *"]], - // Identical saved rules cover only the native resource, regardless of which parser saved them. + grants: [ + ["printf *"], + ["FOO=bar > *", "printf *"], + ], repeat: [ ["ask", "allow"], ["ask", "allow"], ], - next: "printf next", - nextEffect: ["allow", "allow"], + next: "FOO=bar rm -rf /", + nextEffect: ["ask", "ask"], + }, + { + name: "PowerShell method prefix", + shell: "pwsh", + command: '[IO.File]::Delete("C:\\first")', + grants: [ + ["[IO.File]::Delete(*"], + ["[IO.File]::Delete(*"], + ], + repeat: [ + ["allow", "allow"], + ["allow", "allow"], + ], + next: '[IO.File]::WriteAllText("C:\\first", "x")', + nextEffect: ["ask", "ask"], }, { name: "PowerShell tab prefix", diff --git a/packages/core/test/shell-parse-compatibility.test.ts b/packages/core/test/shell-parse-compatibility.test.ts index bbabc602aa3..e08bd015454 100644 --- a/packages/core/test/shell-parse-compatibility.test.ts +++ b/packages/core/test/shell-parse-compatibility.test.ts @@ -45,14 +45,37 @@ const compatibilityCases: Array<[string, string, Record, ShellPa "bash", "cd $HOME; pwd", { HOME: "/session-home" }, - { commands: [{ resource: "pwd", save: "pwd *" }], directories: [] }, + { + commands: [ + { resource: "cd $HOME", save: "cd *" }, + { resource: "pwd", save: "pwd *" }, + ], + directories: [], + }, + ], + [ + "bash", + "cd; pwd", + { HOME: "/session-home" }, + { + commands: [ + { resource: "cd", save: "cd *" }, + { resource: "pwd", save: "pwd *" }, + ], + directories: [], + }, ], - ["bash", "cd; pwd", { HOME: "/session-home" }, { commands: [{ resource: "pwd", save: "pwd *" }], directories: [] }], [ "bash", 'target=/outside; cd "$target"; pwd', {}, - { commands: [{ resource: "pwd", save: "pwd *" }], directories: [] }, + { + commands: [ + { resource: 'cd "$target"', save: "cd *" }, + { resource: "pwd", save: "pwd *" }, + ], + directories: [], + }, ], [ "bash", @@ -81,7 +104,18 @@ const compatibilityCases: Array<[string, string, Record, ShellPa {}, { commands: [{ resource: "FOO=bar git status", save: "git status *" }], directories: [] }, ], - ["bash", "HOME=/outside; cd; pwd", {}, { commands: [{ resource: "pwd", save: "pwd *" }], directories: [] }], + [ + "bash", + "HOME=/outside; cd; pwd", + {}, + { + commands: [ + { resource: "cd", save: "cd *" }, + { resource: "pwd", save: "pwd *" }, + ], + directories: [], + }, + ], ["bash", "CDPATH=/outside cd child", {}, { commands: [], directories: ["child"] }], [ "bash", @@ -103,7 +137,7 @@ const compatibilityCases: Array<[string, string, Record, ShellPa { commands: [ { resource: "printf ok", save: "printf *" }, - { resource: "git status", save: "git status *" }, + { resource: "git status > output", save: "git status *" }, ], directories: [], }, @@ -115,7 +149,7 @@ const compatibilityCases: Array<[string, string, Record, ShellPa { commands: [ { resource: "printf ok", save: "printf *" }, - { resource: "cat", save: "cat *" }, + { resource: "cat < input > output", save: "cat *" }, ], directories: [], }, @@ -126,7 +160,20 @@ const compatibilityCases: Array<[string, string, Record, ShellPa {}, { commands: [{ resource: "pwd", save: "pwd *" }], directories: ["-/../../../etc"] }, ], - ["bash", "cd -; pushd; popd; pwd", {}, { commands: [{ resource: "pwd", save: "pwd *" }], directories: [] }], + [ + "bash", + "cd -; pushd; popd; pwd", + {}, + { + commands: [ + { resource: "cd -", save: "cd *" }, + { resource: "pushd", save: "pushd *" }, + { resource: "popd", save: "popd *" }, + { resource: "pwd", save: "pwd *" }, + ], + directories: [], + }, + ], [ "bash", "command cd /outside; builtin cd /elsewhere", @@ -171,7 +218,10 @@ const compatibilityCases: Array<[string, string, Record, ShellPa "pwsh", "Set-Location $HOME; Set-Location $PWD; Set-Location $target", { HOME: "/session-home" }, - { commands: [], directories: [process.platform === "win32" ? os.homedir() : "/session-home", "/workspace"] }, + { + commands: [{ resource: "Set-Location $target", save: "Set-Location *" }], + directories: [process.platform === "win32" ? os.homedir() : "/session-home", "/workspace"], + }, ], [ "pwsh", @@ -209,8 +259,7 @@ const compatibilityCases: Array<[string, string, Record, ShellPa const command = ${JSON.stringify(command)} const shell = ${JSON.stringify(shell)} const scanned = await Effect.runPromise(ShellParse.scan(command, shell, "/workspace")) - const native = await Effect.runPromise(ShellParse.scanPortable(command, shell, "/workspace")) - console.log(JSON.stringify([scanned, native])) + console.log(JSON.stringify(scanned)) `, ], cwd: `${import.meta.dir}/..`, @@ -224,14 +273,11 @@ const compatibilityCases: Array<[string, string, Record, ShellPa child.exited, ]) expect(code, error).toBe(0) - const [scanned, native] = JSON.parse(output) - expect(scanned).toEqual(expected) - expect(native).toEqual(scanned) + expect(JSON.parse(output)).toEqual(expected) return } const scanned = await Effect.runPromise(ShellParse.scan(command, shell, "/workspace")) expect(scanned).toEqual(expected) - expect(await Effect.runPromise(ShellParse.scanPortable(command, shell, "/workspace"))).toEqual(scanned) }, ) @@ -243,7 +289,7 @@ const compatibilityCases: Array<[string, string, Record, ShellPa test("extracts inline PowerShell directory flags with case-insensitive names and quoted values", async () => { const result = await Effect.runPromise( - ShellParse.scanPortable( + ShellParse.scan( "Set-Location -LITERALPATH:C:\\outside; Set-Location -pAtH:'../other dir'; Set-Location -p:/short; Set-Location -lp /literal; Set-Location -l:/prefix; Set-Location -lit:/litprefix; Set-Location -pat:/patprefix; Set-Location -PSPath:/pspath; Set-Location '-outside'; Set-Location -/../../outside; Set-Location -Path -- -/../../outside2; Set-Location -- -outside2; Set-Location -`outside3; Set-Location \"../`$a/../outside4\"; Set-Location /out`side; Set-Location 'C:\\quoted'", "pwsh", "/workspace", @@ -274,7 +320,7 @@ const compatibilityCases: Array<[string, string, Record, ShellPa test("retains statement-head ForEach-Object cmdlet calls when non-scriptblock parameters are present", async () => { const result = await Effect.runPromise( - ShellParse.scanPortable("ForEach-Object -InputObject $proc -MemberName Kill", "pwsh", "/workspace"), + ShellParse.scan("ForEach-Object -InputObject $proc -MemberName Kill", "pwsh", "/workspace"), ) expect(result).toEqual({ commands: [{ resource: "ForEach-Object -InputObject $proc -MemberName Kill", save: "ForEach-Object *" }], @@ -286,16 +332,22 @@ const compatibilityCases: Array<[string, string, Record, ShellPa describe("scanner behavior on former tree-sitter gaps", () => { for (const fixture of [ { - name: "omits synthetic command nodes for an assignment with redirection", + name: "retains redirect resource for an assignment with redirection", shell: "bash", command: "FOO=bar > output", - expected: { commands: [], directories: [] }, + expected: { commands: [{ resource: "FOO=bar > output", save: "FOO=bar > *" }], directories: [] }, }, { - name: "retains nested executable commands without empty assignment command nodes", + name: "retains both redirect resource and nested executable commands on assignment redirection", shell: "bash", command: "FOO=$(printf value) > output", - expected: { commands: [{ resource: "printf value", save: "printf *" }], directories: [] }, + expected: { + commands: [ + { resource: "FOO=$(printf value) > output", save: "FOO=$(printf value) > *" }, + { resource: "printf value", save: "printf *" }, + ], + directories: [], + }, }, { name: "keeps numeric arguments in saved prefixes", @@ -357,13 +409,24 @@ describe("scanner behavior on former tree-sitter gaps", () => { command: "Set-Location a,b", expected: { commands: [], directories: ["a,b"] }, }, + { + name: "retains unresolved PowerShell stack, history, and dot-wildcard directory commands", + shell: "pwsh", + command: "Set-Location -StackName old; Set-Location -; Set-Location +; Set-Location .[.] ; Set-Location -LiteralPath '.[.]'", + expected: { + commands: [ + { resource: "Set-Location -StackName old", save: "Set-Location *" }, + { resource: "Set-Location -", save: "Set-Location *" }, + { resource: "Set-Location +", save: "Set-Location *" }, + { resource: "Set-Location .[.]", save: "Set-Location *" }, + ], + directories: [".[.]"], + }, + }, ]) { test(fixture.name, async () => { const scanned = await Effect.runPromise(ShellParse.scan(fixture.command, fixture.shell, "/workspace")) expect(scanned).toEqual(fixture.expected) - expect(await Effect.runPromise(ShellParse.scanPortable(fixture.command, fixture.shell, "/workspace"))).toEqual( - fixture.expected, - ) }) } }) diff --git a/packages/core/test/shell-parse-parity.test.ts b/packages/core/test/shell-parse-parity.test.ts index 1a842d35f03..dc392e2aba7 100644 --- a/packages/core/test/shell-parse-parity.test.ts +++ b/packages/core/test/shell-parse-parity.test.ts @@ -13,7 +13,6 @@ describe("ShellParse native parity", () => { const scanned = shell === "pwsh" ? ShellScan.scanPowerShell(command) : ShellScan.scan(command) expect(scanned.kind, context).toBe("scanned") const parsed = await Effect.runPromise(ShellParse.scan(command, shell, "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(command, shell, "/workspace")), context).toEqual(parsed) for (const item of parsed.commands) { if (!/^[A-Za-z_][A-Za-z0-9_]*=/.test(item.resource)) { expect(Wildcard.match(item.resource, item.save), context).toBe(true) @@ -50,22 +49,28 @@ describe("ShellParse native parity", () => { ["pwsh", "git status; npm run test; docker compose up"], ["pwsh", 'git "status"; npm "run" test; docker "compose" up'], ["pwsh", "Write-Output done # comment\nGet-ChildItem"], - ])("native resources, saved prefixes, and directories match in %s: %s", async (shell, command) => { + ])("extracts resources, saved prefixes, and directories in %s: %s", async (shell, command) => { const scanned = shell === "pwsh" ? ShellScan.scanPowerShell(command) : ShellScan.scan(command) expect(scanned.kind).toBe("scanned") - const native = await Effect.runPromise(ShellParse.scanPortable(command, shell, "/workspace")) - expect(await Effect.runPromise(ShellParse.scan(command, shell, "/workspace"))).toEqual(native) + await Effect.runPromise(ShellParse.scan(command, shell, "/workspace")) }) - test.each(["> output", "FOO=bar", "2>> output"])( - "returns an explicit empty result for statements without executable command nodes: %s", - async (command) => { - expect(ShellScan.scan(command)).toEqual({ kind: "scanned", commands: [] }) - const native = await Effect.runPromise(ShellParse.scanPortable(command, "bash", "/workspace")) - expect(native).toEqual({ commands: [], directories: [] }) - expect(await Effect.runPromise(ShellParse.scan(command, "bash", "/workspace"))).toEqual(native) - }, - ) + test("returns an explicit empty result for pure variable assignments without redirects", async () => { + expect(ShellScan.scan("FOO=bar")).toEqual({ kind: "scanned", commands: [] }) + expect(await Effect.runPromise(ShellParse.scan("FOO=bar", "bash", "/workspace"))).toEqual({ + commands: [], + directories: [], + }) + }) + + test.each([ + ["> output", [{ resource: "> output", save: "> *" }]], + ["2>> output", [{ resource: "2>> output", save: "2>> *" }]], + ] as const)("retains commandless redirects with matching saved prefixes: %s", async (command, commands) => { + const parsed = await Effect.runPromise(ShellParse.scan(command, "bash", "/workspace")) + expect(parsed).toEqual({ commands: [...commands], directories: [] }) + for (const item of parsed.commands) expect(Wildcard.match(item.resource, item.save)).toBe(true) + }) }) describe("ShellParse malformed native syntax", () => { diff --git a/packages/core/test/shell-parse-syntax.test.ts b/packages/core/test/shell-parse-syntax.test.ts index b24ff973fd1..c9711741518 100644 --- a/packages/core/test/shell-parse-syntax.test.ts +++ b/packages/core/test/shell-parse-syntax.test.ts @@ -65,7 +65,7 @@ describe("native shell syntax compatibility", () => { "F='$HOME/bin'; SIZE=4096; N=$(( (SIZE + 1023) / 1024 )); echo $N", ])(`${shell} extracts commands without rejecting ordinary syntax: %s`, async (command) => { const scanned = await Effect.runPromise(ShellParse.scan(command, shell, "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(command, shell, "/workspace"))).toEqual(scanned) + expect(scanned.commands.length).toBeGreaterThanOrEqual(0) }) } @@ -89,7 +89,7 @@ describe("native shell syntax compatibility", () => { ).toEqual({ commands: [ { resource: "true", save: "true *" }, - { resource: "cat", save: "cat *" }, + { resource: "cat <<'EOF'", save: "cat *" }, { resource: "git status", save: "git status *" }, ], directories: [], @@ -125,7 +125,7 @@ describe("native shell syntax compatibility", () => { ).toEqual({ commands: [ { resource: "true", save: "true *" }, - { resource: "git add -p file.rs <<< $'n\\ny\\n'", save: "git add *" }, + { resource: "git add -p file.rs <<< $'n\\ny\\n' >/dev/null", save: "git add *" }, ], directories: [], }) @@ -196,6 +196,6 @@ describe("native shell syntax compatibility", () => { "git st`atus", ])("PowerShell extracts commands without rejecting ordinary syntax: %s", async (command) => { const scanned = await Effect.runPromise(ShellParse.scan(command, "pwsh", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(command, "pwsh", "/workspace"))).toEqual(scanned) + for (const item of scanned.commands) expect(Wildcard.match(item.resource, item.save), item.resource).toBe(true) }) }) diff --git a/packages/core/test/shell-parse.test.ts b/packages/core/test/shell-parse.test.ts index bd1f4bd7937..44e882e4ee3 100644 --- a/packages/core/test/shell-parse.test.ts +++ b/packages/core/test/shell-parse.test.ts @@ -48,7 +48,7 @@ describe("ShellParse", () => { for (const command of commands) { const scanned = await Effect.runPromise(ShellParse.scan(command, "/bin/bash", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(command, "/bin/bash", "/workspace"))).toEqual(scanned) + expect(scanned.commands.length).toBeGreaterThan(0) } }) @@ -165,9 +165,68 @@ describe("ShellParse", () => { process.platform === "darwin" ? "/var/root/secret" : "/root/secret", "~root", ]) + + const tildeForms = await Effect.runPromise( + ShellParse.scan( + "cd ~+/../outside && cd ~- && cd ~+1 && cd ~0 && cd ~__opencode_nonexistent_user__", + "/bin/bash", + "/workspace", + ), + ) + expect(tildeForms.directories).toEqual(["/outside"]) + expect(tildeForms.commands).toEqual([ + { resource: "cd ~-", save: "cd *" }, + { resource: "cd ~+1", save: "cd *" }, + { resource: "cd ~0", save: "cd *" }, + { resource: "cd ~__opencode_nonexistent_user__", save: "cd *" }, + ]) + + const dynamicCd = await Effect.runPromise( + ShellParse.scan("cd {..,x} && cd .[.] && cd .? && cd $'\\x2e'[.] && cd '.[.]'", "/bin/bash", "/workspace"), + ) + expect(dynamicCd.directories).toEqual([".[.]"]) + expect(dynamicCd.commands).toEqual([ + { resource: "cd {..,x}", save: "cd *" }, + { resource: "cd .[.]", save: "cd *" }, + { resource: "cd .?", save: "cd *" }, + { resource: "cd $'\\x2e'[.]", save: "cd *" }, + ]) + + const nobodyHome = await Effect.runPromise( + ShellParse.scan("cd ~nobody", "/bin/bash", "/workspace"), + ) + expect(nobodyHome.directories).toHaveLength(1) + expect(nobodyHome.directories[0]).not.toBe( + path.join(process.platform === "darwin" ? "/Users" : "/home", "nobody"), + ) } }) + test("retains commandless redirects, redirected declarations, and PowerShell method calls as permission resources", async () => { + const redirects = await Effect.runPromise( + ShellParse.scan(">> ~/.zshrc <<< 'x'; export -p > ~/.bashrc", "/bin/zsh", "/workspace"), + ) + expect(redirects).toEqual({ + commands: [ + { resource: ">> ~/.zshrc <<< 'x'", save: ">> *" }, + { resource: "export -p > ~/.bashrc", save: "export *" }, + ], + directories: [], + }) + + const methods = await Effect.runPromise( + ShellParse.scan('[IO.File]::Delete("C:\\victim"); $proc.Kill(); "x" >> ~/.bashrc', "pwsh", "C:\\workspace"), + ) + expect(methods).toEqual({ + commands: [ + { resource: '[IO.File]::Delete("C:\\victim")', save: "[IO.File]::Delete(*" }, + { resource: "$proc.Kill()", save: "$proc.Kill(*" }, + { resource: '"x" >> ~/.bashrc', save: '"x" *' }, + ], + directories: [], + }) + }) + test("extracts PowerShell directory parameters", async () => { const result = await Effect.runPromise( ShellParse.scan("Set-Location -LiteralPath '..\\outside'; Get-ChildItem", "pwsh", "C:\\workspace"), diff --git a/packages/core/test/shell-scan/bash-safety.test.ts b/packages/core/test/shell-scan/bash-safety.test.ts index a6b6802d9f8..d981c627094 100644 --- a/packages/core/test/shell-scan/bash-safety.test.ts +++ b/packages/core/test/shell-scan/bash-safety.test.ts @@ -81,12 +81,19 @@ describe("Bash execution safety", () => { expect(ShellScan.scan(source).kind).toBe("opaque") }) - test.each(["X=value > /dev/null", "X=x 2> output", "X=x < input", "> /dev/null", "2>> output", ">output 2>&1"])( - "scans redirects without inventing an executable command: %s", - (source) => { - expect(ShellScan.scan(source)).toEqual({ kind: "scanned", commands: [] }) - }, - ) + test.each([ + ["X=value > /dev/null", "X=value > /dev/null", "X=value >"], + ["X=x 2> output", "X=x 2> output", "X=x 2>"], + ["X=x < input", "X=x < input", "X=x <"], + ["> /dev/null", "> /dev/null", ">"], + ["2>> output", "2>> output", "2>>"], + [">output 2>&1", ">output 2>&1", ">output"], + ] as const)("retains commandless redirects as explicit permission resources: %s", (source, resource, head) => { + expect(ShellScan.scan(source)).toMatchObject({ + kind: "scanned", + commands: [{ resource, words: [head], rawWords: [head] }], + }) + }) test.each([">", "X=x >", "X=x > # comment\nprintf ok", ">; printf ok", "(printf ok) >", "{ printf ok; } >"])( "still requires redirect targets: %s", @@ -96,26 +103,40 @@ describe("Bash execution safety", () => { ) test.each([ - "{ printf ok; } > output", - "{ printf ok; } 2> output", - "{ printf ok; } &> output", - "(printf ok) > output", - "{ printf ok; }; X=x > output", - ])("scans leading groups and their redirects without synthetic commands: %s", (source) => { - expect(ShellScan.scan(source)).toEqual({ + ["{ printf ok; } > output", "> output", ">"], + ["{ printf ok; } 2> output", "2> output", "2>"], + ["{ printf ok; } &> output", "&> output", "&>"], + ["(printf ok) > output", "> output", ">"], + ["{ printf ok; }; X=x > output", "X=x > output", "X=x >"], + ] as const)("retains file redirects on compound groups and trailing assignments: %s", (source, redirectResource, head) => { + expect(ShellScan.scan(source)).toMatchObject({ kind: "scanned", - commands: [{ resource: "printf ok", words: ["printf", "ok"], rawWords: ["printf", "ok"] }], + commands: [ + { resource: "printf ok", words: ["printf", "ok"], rawWords: ["printf", "ok"] }, + { resource: redirectResource, words: [head], rawWords: [head] }, + ], }) }) + test.each(["{ printf ok; } 2>&1", "(printf ok) 2>&1", "(printf ok) 2>&-"])( + "does not invent redirect commands for pure group descriptor duplications: %s", + (source) => { + expect(ShellScan.scan(source)).toEqual({ + kind: "scanned", + commands: [{ resource: "printf ok", words: ["printf", "ok"], rawWords: ["printf", "ok"] }], + }) + }, + ) + test.each(["X=x >$(printf output)", ">$(printf output)", "{ :; } >$(printf output)", "(:) >$(printf output)"])( - "retains explicit substitutions in otherwise commandless redirects: %s", + "retains both redirect resources and explicit substitutions in commandless redirects: %s", (source) => { const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind === "opaque") return + const redirectHead = source.startsWith("X=x") ? "X=x >$(printf output)" : ">$(printf output)" expect(result.commands.map((command) => command.words[0])).toEqual( - source.includes(":") ? [":", "printf"] : ["printf"], + source.includes(":") ? [":", redirectHead, "printf"] : [redirectHead, "printf"], ) expect(result.commands.at(-1)).toEqual({ resource: "printf output", @@ -126,10 +147,11 @@ describe("Bash execution safety", () => { ) test("scans a list following a group redirect suffix", () => { - expect(ShellScan.scan("(printf ok) >output && pwd")).toEqual({ + expect(ShellScan.scan("(printf ok) >output && pwd")).toMatchObject({ kind: "scanned", commands: [ { resource: "printf ok", words: ["printf", "ok"], rawWords: ["printf", "ok"] }, + { resource: ">output", words: [">output"], rawWords: [">output"] }, { resource: "pwd", words: ["pwd"], rawWords: ["pwd"] }, ], }) @@ -260,7 +282,10 @@ describe("Bash real-shell redirect-only statements", () => { const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind === "opaque") return - expect(result.commands.map((command) => command.words[0]).sort()).toEqual(observed) + expect( + result.commands.filter((command) => command.redirectPrefixEnd === undefined).map((command) => command.words[0]).sort(), + ).toEqual(observed) + expect(result.commands.some((command) => command.redirectPrefixEnd !== undefined)).toBe(true) }) } }) diff --git a/packages/core/test/shell-scan/bash-syntax.test.ts b/packages/core/test/shell-scan/bash-syntax.test.ts index d5bfb2ab004..5ba1d9f462c 100644 --- a/packages/core/test/shell-scan/bash-syntax.test.ts +++ b/packages/core/test/shell-scan/bash-syntax.test.ts @@ -156,7 +156,10 @@ describe("ordinary Bash and Zsh syntax", () => { }) test("scans a Zsh brace group closed after a redirect", () => { - expect(ShellScan.scan("{ a && >f }")).toMatchObject({ kind: "scanned", commands: [{ words: ["a"] }] }) + expect(ShellScan.scan("{ a && >f }")).toMatchObject({ + kind: "scanned", + commands: [{ words: ["a"] }, { resource: ">f", words: [">f"] }], + }) }) test("preserves raw lexical spelling of ANSI-C and locale quoted words", () => { @@ -188,8 +191,8 @@ describe("Bash shared heredoc delimiter grammar", () => { '(cat <<"E\\$OF"\nhello\nE$OF\n)', '(cat <<-"E\\OF"\n\thello\n\tE\\OF\n)', ])("preserves heredoc permission resources and saved prefixes: %s", async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(source, "/bin/bash", "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands[0]?.save).toBe("cat *") }) test.each([ @@ -202,8 +205,8 @@ describe("Bash shared heredoc delimiter grammar", () => { expect(result.kind).toBe("scanned") if (result.kind !== "scanned") throw new Error(result.reason) expect(result.commands.map((command) => command.words[0])).toEqual([...names]) - const legacy = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(source, "/bin/bash", "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands).toHaveLength(names.length) }) test.each([ @@ -291,7 +294,7 @@ describe("Bash dialects", () => { test("reads a reserved word after a redirect as a Zsh keyword", () => { const source = "if true; then >/dev/null fi; next" - expect(heads(source, "zsh")).toEqual(["true", "next"]) + expect(heads(source, "zsh")).toEqual(["true", ">/dev/null", "next"]) expect(heads(source, "bash")).toBe("opaque") expect(heads(source, "posix")).toBe("opaque") }) @@ -313,7 +316,7 @@ describe("Bash dialects", () => { ["/bin/dash", ["x = 1", "y"]], ["/opt/bin/mksh", ["x = 1", "y"]], ] as const)("derives the dialect from the shell executable: %s", async (shell, resources) => { - const result = await Effect.runPromise(ShellParse.scanPortable("(( x = 1 )); y", shell, "/workspace")) + const result = await Effect.runPromise(ShellParse.scan("(( x = 1 )); y", shell, "/workspace")) expect(result.commands.map((command) => command.resource)).toEqual([...resources]) }) }) diff --git a/packages/core/test/shell-scan/compound-parity.test.ts b/packages/core/test/shell-scan/compound-parity.test.ts index 4ddeee5c115..437ab147ce0 100644 --- a/packages/core/test/shell-scan/compound-parity.test.ts +++ b/packages/core/test/shell-scan/compound-parity.test.ts @@ -26,11 +26,10 @@ describe("compound function acceptance", () => { const name = head.includes("probe-name") ? "probe-name" : "probe" const source = context(`${head} ${body}; ${name}`) test(`${shell}: ${source}`, async () => { - // Braces preserve the function's behavior, but avoid Tree-sitter's recovery artifacts. - const legacy = await Effect.runPromise( + const braced = await Effect.runPromise( ShellParse.scan(context(`${head} { ${body}; }; ${name}`), shell, "/workspace"), ) - expect(await Effect.runPromise(ShellParse.scanPortable(source, shell, "/workspace"))).toEqual(legacy) + expect(await Effect.runPromise(ShellParse.scan(source, shell, "/workspace"))).toEqual(braced) }) } } @@ -79,8 +78,8 @@ describe("Zsh parenthesized loop acceptance", () => { for (const context of contexts) { const source = context(fixture.source) test(source, async () => { - const legacy = await Effect.runPromise(ShellParse.scan(context(fixture.equivalent), "zsh", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(source, "zsh", "/workspace"))).toEqual(legacy) + const equivalent = await Effect.runPromise(ShellParse.scan(context(fixture.equivalent), "zsh", "/workspace")) + expect(await Effect.runPromise(ShellParse.scan(source, "zsh", "/workspace"))).toEqual(equivalent) }) } diff --git a/packages/core/test/shell-scan/oracle.test.ts b/packages/core/test/shell-scan/oracle.test.ts index 406759795a4..1dac077b77d 100644 --- a/packages/core/test/shell-scan/oracle.test.ts +++ b/packages/core/test/shell-scan/oracle.test.ts @@ -752,7 +752,7 @@ describe.skipIf(process.platform === "win32")("real-shell directory oracle", () ) expect(changed.length, `Fixture never changed directory in any real shell: ${source}`).toBeGreaterThan(0) for (const executable of changed) { - const parsed = await Effect.runPromiseExit(ShellParse.scanPortable(source, executable, root)) + const parsed = await Effect.runPromiseExit(ShellParse.scan(source, executable, root)) if (Exit.isSuccess(parsed)) expect(parsed.value.directories, `${executable} in: ${source}`).toContain(target) } }, diff --git a/packages/core/test/shell-scan/parity-regressions.test.ts b/packages/core/test/shell-scan/parity-regressions.test.ts index e7f9f5e68db..3150ad02320 100644 --- a/packages/core/test/shell-scan/parity-regressions.test.ts +++ b/packages/core/test/shell-scan/parity-regressions.test.ts @@ -30,17 +30,16 @@ describe("legacy-accepted shell syntax regressions", () => { "preserves deeply indented function definitions: %s", async (source) => { const command = `( ${" ".repeat(32_000)}${source} )` - const legacy = await Effect.runPromise(ShellParse.scan(command, "zsh", "/workspace")) - expect(await Effect.runPromise(ShellParse.scanPortable(command, "zsh", "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(command, "zsh", "/workspace")) + expect(parsed.commands.some((item) => item.resource === "scan_probe")).toBe(true) }, ) test.each(conditions.flatMap((source) => contexts.map((context) => context(source))))( "retains conditional process substitutions and permission resources: %s", async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, "bash", "/workspace")) - expect(legacy.commands.some((command) => command.resource === "scan_probe")).toBe(true) - expect(await Effect.runPromise(ShellParse.scanPortable(source, "bash", "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, "bash", "/workspace")) + expect(parsed.commands.some((command) => command.resource === "scan_probe")).toBe(true) }, ) @@ -52,9 +51,8 @@ describe("legacy-accepted shell syntax regressions", () => { ), ), )(`${shell} preserves line continuations at function boundaries: %s`, async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) - expect(legacy.commands.some((command) => command.resource === "scan_probe")).toBe(true) - expect(await Effect.runPromise(ShellParse.scanPortable(source, shell, "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) + expect(parsed.commands.some((command) => command.resource === "scan_probe")).toBe(true) }) test.each( @@ -64,17 +62,15 @@ describe("legacy-accepted shell syntax regressions", () => { ), ), )(`${shell} preserves comments between a function head and its body: %s`, async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) - expect(legacy.commands.some((command) => command.resource === "scan_probe")).toBe(true) - expect(await Effect.runPromise(ShellParse.scanPortable(source, shell, "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) + expect(parsed.commands.some((command) => command.resource === "scan_probe")).toBe(true) }) test.each(functions)( `${shell} preserves function resources, saved prefixes, and directories: %s`, async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) - expect(legacy.commands.some((command) => command.resource === "scan_probe")).toBe(true) - expect(await Effect.runPromise(ShellParse.scanPortable(source, shell, "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, shell, "/workspace")) + expect(parsed.commands.some((command) => command.resource === "scan_probe")).toBe(true) }, ) @@ -116,9 +112,8 @@ describe("legacy-accepted shell syntax regressions", () => { "while() { scan_probe; break; }", "until() { scan_probe; break; }", ])("preserves Zsh anonymous functions and parenthesized loop permissions: %s", async (source) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, "zsh", "/workspace")) - expect(legacy.commands.some((command) => command.resource === "scan_probe")).toBe(true) - expect(await Effect.runPromise(ShellParse.scanPortable(source, "zsh", "/workspace"))).toEqual(legacy) + const parsed = await Effect.runPromise(ShellParse.scan(source, "zsh", "/workspace")) + expect(parsed.commands.some((command) => command.resource === "scan_probe")).toBe(true) }) test.each([ diff --git a/packages/core/test/shell-scan/powershell-syntax.test.ts b/packages/core/test/shell-scan/powershell-syntax.test.ts index 59524214521..4ed7ade734a 100644 --- a/packages/core/test/shell-scan/powershell-syntax.test.ts +++ b/packages/core/test/shell-scan/powershell-syntax.test.ts @@ -60,6 +60,26 @@ describe("PowerShell practical syntax", () => { ['@"\n$(Write-Output \'\n"@\'; git status)\n"@', ["Write-Output '\n\"@'", "git status"]], ["class C { [int]$x = (git status); [void] M() { git diff } }", ["git status", "git diff"]], ["enum Color { Red; Green = 2 }; git status", ["git status"]], + ["[IO.File]::Delete('C:\\victim')", ["[IO.File]::Delete('C:\\victim')"]], + [ + '[scriptblock]::Create("Remove-Item victim").Invoke()', + ['[scriptblock]::Create("Remove-Item victim")', '[scriptblock]::Create("Remove-Item victim").Invoke()'], + ], + [ + '([scriptblock]::Create("Remove-Item victim")).Invoke()', + ['[scriptblock]::Create("Remove-Item victim")', '([scriptblock]::Create("Remove-Item victim")).Invoke()'], + ], + [ + '$ExecutionContext.InvokeCommand.InvokeScript("Remove-Item victim")', + ['$ExecutionContext.InvokeCommand.InvokeScript("Remove-Item victim")'], + ], + ["Write-Output $proc.Kill()", ["Write-Output $proc.Kill()", "$proc.Kill()"]], + ["$proc.'Kill'(); $proc.('Ki' + 'll')(); $proc?.Dispose()", ["$proc.'Kill'()", "$proc.('Ki' + 'll')()", "$proc?.Dispose()"]], + [ + "[IO.File]::De`lete('x'); [IO.File]:: Delete('y'); $proc.\nKill()", + ["[IO.File]::De`lete('x')", "[IO.File]:: Delete('y')", "$proc.\nKill()"], + ], + ['"payload" >> ~/.bashrc; (git status) > out.txt', ['"payload" >> ~/.bashrc', "(git status) > out.txt", "git status"]], ] as const)("extracts command resources from %s", (source, resources) => { const result = ShellScan.scanPowerShell(source) expect(result.kind).toBe("scanned") diff --git a/packages/core/test/shell-scan/redirect.test.ts b/packages/core/test/shell-scan/redirect.test.ts index 72fbcafdc74..6626b30c81b 100644 --- a/packages/core/test/shell-scan/redirect.test.ts +++ b/packages/core/test/shell-scan/redirect.test.ts @@ -4,29 +4,36 @@ import { ShellParse } from "../../src/shell/parse.js" import { ShellScan } from "../../src/shell/scan.js" import { Wildcard } from "../../src/util/wildcard.js" -async function parity(source: string) { - const legacy = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - const native = await Effect.runPromise(ShellParse.scanPortable(source, "/bin/bash", "/workspace")) - expect(native, source).toEqual(legacy) +async function checkResources(source: string) { + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + for (const command of parsed.commands) { + if ( + !/^[A-Za-z_][A-Za-z0-9_]*=/.test(command.resource) && + !/^(?:\d*>>?|\d*>&|&>>?|\d*<|\d*<>|>\|)/.test(command.resource) && + !command.resource.includes("\\\n") + ) { + expect(Wildcard.match(command.resource, command.save), `${source}: ${JSON.stringify(command)}`).toBe(true) + } + } } describe("Bash redirect resource oracle", () => { test.each([ - ["printf hello | cat > marker", ["printf hello", "cat"]], - ["printf ok && git status > output", ["printf ok", "git status"]], + ["printf hello | cat > marker", ["printf hello", "cat > marker"]], + ["printf ok && git status > output", ["printf ok", "git status > output"]], ["cat > output", ["cat > output"]], ["cat > output | cat", ["cat > output", "cat"]], - ["pwd | > output cat > tail", ["pwd", "> output cat"]], - ["pwd && cat > output file", ["pwd", "cat"]], + ["pwd | > output cat > tail", ["pwd", "> output cat > tail"]], + ["pwd && cat > output file", ["pwd", "cat > output file"]], ["pwd; cat > output", ["pwd", "cat > output"]], ["pwd\ncat > output", ["pwd", "cat > output"]], - ] as const)("matches exact permission resources: %s", async (source, resources) => { - const legacy = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - expect(legacy.commands.map((command) => command.resource)).toEqual([...resources]) - await parity(source) + ] as const)("keeps redirects visible across lists and pipelines: %s", async (source, resources) => { + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands.map((command) => command.resource)).toEqual([...resources]) + await checkResources(source) }) - test("matches redirect positions across generated list and pipeline boundaries", async () => { + test("scans redirect positions across generated list and pipeline boundaries", async () => { const redirects = [">output", ">>output", "err", "2>&1", "<&0", ">|output", "&>output", "&>>output"] const separators = [" | ", " |& ", " && ", " || ", "; ", " & ", "\n"] for (const redirect of redirects) { @@ -43,10 +50,10 @@ describe("Bash redirect resource oracle", () => { expect(ShellScan.scan(command).kind).toBe("opaque") continue } - await parity(command) + await checkResources(command) for (const separator of separators) { - await parity(`printf ok${separator}${command}`) - await parity(`${command}${separator}pwd >last`) + await checkResources(`printf ok${separator}${command}`) + await checkResources(`${command}${separator}pwd >last`) } } } @@ -88,55 +95,54 @@ describe("Bash redirect resource oracle", () => { "time git status", "time -p git status", "coproc git status", - ])("preserves nested commands, prefixes, and context: %s", parity) + ])("preserves nested commands, prefixes, and context: %s", checkResources) - test("keeps lexical words and nested redirect-target commands after narrowing the resource", () => { - const result = ShellScan.scan('pwd | git >"$(printf output)" status') + test("keeps full redirect resource, lexical words, and nested redirect-target commands", async () => { + const source = 'pwd | git >"$(printf output)" status' + const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind !== "scanned") throw new Error(`Unexpected opacity: ${result.reason}`) expect(result.commands[1]).toMatchObject({ - resource: "git", + resource: 'git >"$(printf output)" status', words: ["git", "status"], rawWords: ["git", "status"], redirectWordCount: 1, }) expect(result.commands[2]).toMatchObject({ resource: "printf output", rawWords: ["printf", "output"] }) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands[1]).toEqual({ resource: 'git >"$(printf output)" status', save: "git *" }) + expect(Wildcard.match(parsed.commands[1]!.resource, parsed.commands[1]!.save)).toBe(true) }) - test("excludes ignored trailing continuations from narrowed command prefixes", async () => { + test("retains trailing redirect span after a continuation on pipeline RHS", async () => { const source = "pwd | cat\\\n >out" - const legacy = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind !== "scanned") throw new Error(`Unexpected opacity: ${result.reason}`) - expect(result.commands.map((command) => command.resource)).toEqual( - legacy.commands.map((command) => command.resource), - ) expect(result.commands[1]?.rawWords).toEqual(["cat"]) - expect(legacy.commands[1]).toEqual({ resource: "cat", save: "cat *" }) - const native = await Effect.runPromise(ShellParse.scanPortable(source, "/bin/bash", "/workspace")) - expect(native).toEqual(legacy) - expect(native.commands.every((command) => Wildcard.match(command.resource, command.save))).toBe(true) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands).toEqual([ + { resource: "pwd", save: "pwd *" }, + { resource: "cat\\\n >out", save: "cat *" }, + ]) }) test.each(["cat\\\n", "cat \\\n", "cat\\\n\\\n", "cat\\\n;", "cat >out\\\n", "cat >out \\\n"])( "saved prefixes cover their standalone continuation command: %j", async (source) => { - await parity(source) const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind !== "scanned") throw new Error(result.reason) expect(result.commands[0]?.rawWords).toEqual(["cat"]) - const native = await Effect.runPromise(ShellParse.scanPortable(source, "/bin/bash", "/workspace")) - expect(native.commands[0]).toEqual({ resource: source.includes(">out") ? "cat >out" : "cat", save: "cat *" }) - expect(native.commands.every((command) => Wildcard.match(command.resource, command.save))).toBe(true) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands[0]).toEqual({ resource: source.includes(">out") ? "cat >out" : "cat", save: "cat *" }) + expect(parsed.commands.every((command) => Wildcard.match(command.resource, command.save))).toBe(true) }, ) test.each(["printf 'literal\\\n'\\\n", 'printf "literal\\\n"\\\n', "printf a\\\nb\\\n", 'printf a\\\n""\\\n'])( "preserves meaningful raw syntax before an ignored trailing continuation: %j", async (source) => { - await parity(source) const result = ShellScan.scan(source) expect(result.kind).toBe("scanned") if (result.kind !== "scanned") throw new Error(result.reason) @@ -145,18 +151,18 @@ describe("Bash redirect resource oracle", () => { }, ) - test("assignment then redirect on a pipeline RHS retains the command", async () => { + test("assignment then redirect on a pipeline RHS retains the full command and trailing redirect", async () => { const source = "printf ok | FOO=bar >output git status 3>tail" - const native = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - expect(native.commands).toEqual([ + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed.commands).toEqual([ { resource: "printf ok", save: "printf *" }, - { resource: "FOO=bar >output git status", save: "git status *" }, + { resource: "FOO=bar >output git status 3>tail", save: "git status *" }, ]) }) test("redirect before cd operand retains the target directory", async () => { const source = "pwd && cd >out /outside" - const native = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) - expect(native).toEqual({ commands: [{ resource: "pwd", save: "pwd *" }], directories: ["/outside"] }) + const parsed = await Effect.runPromise(ShellParse.scan(source, "/bin/bash", "/workspace")) + expect(parsed).toEqual({ commands: [{ resource: "pwd", save: "pwd *" }], directories: ["/outside"] }) }) }) diff --git a/packages/core/test/shell-scan/scan.test.ts b/packages/core/test/shell-scan/scan.test.ts index 66ae2d528cf..25bf89054c5 100644 --- a/packages/core/test/shell-scan/scan.test.ts +++ b/packages/core/test/shell-scan/scan.test.ts @@ -166,12 +166,12 @@ describe("ShellScan", () => { }, ) - test("keeps first-command redirects but excludes list-level redirects from resources", () => { + test("keeps redirects in resources across list operators", () => { expect(ShellScan.scan("FOO=bar 2>>err printf ok > out && cat < input")).toMatchObject({ kind: "scanned", commands: [ { resource: "FOO=bar 2>>err printf ok > out", words: ["printf", "ok"] }, - { resource: "cat", words: ["cat"] }, + { resource: "cat < input", words: ["cat"] }, ], }) }) @@ -181,7 +181,7 @@ describe("ShellScan", () => { kind: "scanned", commands: [ { resource: "printf ok>out 2>&1", words: ["printf", "ok"] }, - { resource: "cat", words: ["cat"] }, + { resource: "cat { }), )) - test("pipeline redirect preserves exact approval and denial despite broad saved approval", () => + test("pipeline redirect keeps the redirect target visible to exact approval and denial", () => withScanner((registry, fixture) => Effect.gen(function* () { const saved = yield* PermissionSaved.Service @@ -366,6 +366,13 @@ describe("ShellTool scanner permissions", () => { yield* saved.add({ projectID: location.project.id, action: "shell", resources: ["printf hello", "cat"] }) const marker = path.join(fixture.active, "marker") const command = "printf hello | cat > marker" + const unapprovedRedirect = yield* runPermissionCommand(registry, command, marker, ["reject"]) + expect(unapprovedRedirect.requests).toMatchObject([ + { action: "shell", resources: ["printf hello", "cat > marker"], save: ["printf *", "cat *"] }, + ]) + expect(yield* Effect.promise(() => Bun.file(marker).exists())).toBe(false) + + yield* saved.add({ projectID: location.project.id, action: "shell", resources: ["cat > marker"] }) const exact = yield* runPermissionCommand(registry, command, marker, []) expect(exact.requests).toEqual([]) expect(exact.exit).toMatchObject({ _tag: "Success", value: { status: "completed", metadata: { exit: 0 } } }) @@ -381,7 +388,7 @@ describe("ShellTool scanner permissions", () => { const agents = yield* Agent.Service yield* agents.transform((editor) => editor.update(toolIdentity.agent, (agent) => { - agent.permissions = [{ action: "shell", resource: "cat", effect: "deny" }] + agent.permissions = [{ action: "shell", resource: "* > marker", effect: "deny" }] }), ) yield* Effect.promise(() => fs.rm(marker)) @@ -858,12 +865,16 @@ describe("ShellTool", () => { { sessionID, action: "shell", - resources: [isWindows ? "Start-Sleep -Milliseconds 100" : helloCommand], + resources: isWindows + ? ["[Console]::Out.Write('hello')", "Start-Sleep -Milliseconds 100"] + : [helloCommand], agent: toolIdentity.agent, source: { type: "tool", messageID: toolIdentity.messageID, id: "call-shell" }, }, ]) - expect(assertions[0]?.save).toEqual([isWindows ? "Start-Sleep *" : "printf *"]) + expect(assertions[0]?.save).toEqual( + isWindows ? ["[Console]::Out.Write(*", "Start-Sleep *"] : ["printf *"], + ) }), ) }, diff --git a/packages/schema/src/config/experimental.ts b/packages/schema/src/config/experimental.ts index c9fb5cccc5e..83ad0a9d212 100644 --- a/packages/schema/src/config/experimental.ts +++ b/packages/schema/src/config/experimental.ts @@ -7,7 +7,7 @@ import { ConfigPolicy } from "./policy.js" export class Info extends Schema.Class("ConfigExperimental.Info")({ portable_shell_scanner: Schema.Boolean.pipe(optional).annotate({ - description: "Enable the experimental portable shell permission scanner. Defaults to false.", + description: "Deprecated: the built-in shell permission scanner is now always enabled.", }), todo: Schema.Boolean.pipe(optional).annotate({ description: "Enable the experimental todowrite tool. Defaults to false.",