From e128019c890de27cb009cdeb8e275aae82445670 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Fri, 3 Apr 2026 13:37:39 +0800 Subject: [PATCH 1/4] fix(permissions): match env-prefixed shell commands Fixes #2846 --- .../permissions/permission-manager.test.ts | 9 ++++ packages/core/src/permissions/rule-parser.ts | 49 +++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 5b1048d5cb..db1851666a 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -228,6 +228,15 @@ describe('matchesCommandPattern', () => { expect(matchesCommandPattern('npm run *', 'npm run build')).toBe(true); }); + it('matches commands with leading env var assignments', async () => { + expect( + matchesCommandPattern( + 'python3 *', + 'PYTHONPATH=/tmp/lib python3 -c "print(1)"', + ), + ).toBe(true); + }); + it('space-star requires word boundary (ls * does not match lsof)', async () => { expect(matchesCommandPattern('ls *', 'ls -la')).toBe(true); expect(matchesCommandPattern('ls *', 'lsof')).toBe(false); diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 6ca9e83632..0dac38c345 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -7,6 +7,7 @@ import path from 'node:path'; import os from 'node:os'; import picomatch from 'picomatch'; +import { parse } from 'shell-quote'; /** * Normalize a filesystem path to use POSIX-style forward slashes. @@ -606,6 +607,7 @@ export function matchesCommandPattern( ): boolean { // This function matches a single pattern against a single simple command. // Compound command splitting is handled by the caller (PermissionManager). + const normalizedCommand = stripLeadingVariableAssignments(command); // Special case: lone `*` matches any single command if (pattern === '*') { @@ -616,7 +618,10 @@ export function matchesCommandPattern( // No wildcards: prefix matching (backward compat). // "git commit" matches "git commit" and "git commit -m test" // but NOT "gitcommit". - return command === pattern || command.startsWith(pattern + ' '); + return ( + normalizedCommand === pattern || + normalizedCommand.startsWith(pattern + ' ') + ); } // Build regex from glob pattern with word-boundary semantics. @@ -665,9 +670,9 @@ export function matchesCommandPattern( regex += '$'; try { - return new RegExp(regex).test(command); + return new RegExp(regex).test(normalizedCommand); } catch { - return command === pattern; + return normalizedCommand === pattern; } } @@ -678,6 +683,44 @@ function escapeRegex(s: string): string { return s.replace(/[.+?^${}()|[\]\\]/g, '\\$&'); } +const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=/; + +function stripLeadingVariableAssignments(command: string): string { + const trimmed = command.trim(); + if (!trimmed) { + return trimmed; + } + + try { + const tokens: string[] = []; + + for (const token of parse(trimmed)) { + if (typeof token === 'string') { + tokens.push(token); + } else if ( + token && + typeof token === 'object' && + 'op' in token && + typeof token.op === 'string' + ) { + tokens.push(token.op); + } + } + + let firstCommandToken = 0; + while ( + firstCommandToken < tokens.length && + ENV_ASSIGNMENT_REGEX.test(tokens[firstCommandToken]!) + ) { + firstCommandToken++; + } + + return tokens.slice(firstCommandToken).join(' '); + } catch { + return trimmed; + } +} + // ───────────────────────────────────────────────────────────────────────────── // File path matching (gitignore-style) // ───────────────────────────────────────────────────────────────────────────── From d5f6e9fe46c2b62afbc5763012df1ed43ed76824 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Fri, 3 Apr 2026 16:42:34 +0800 Subject: [PATCH 2/4] fix(core): improve shell command parsing for env vars and multiline commands - Add dotAll flag to matchesCommandPattern for matching commands with embedded newlines - Support newline operators in SHELL_OPERATORS for splitCompoundCommand - Refactor getCommandRoot to skip leading VAR=value assignments - Add test coverage for multiline commands and env var prefixed commands Co-authored-by: Qwen-Coder --- .../permissions/permission-manager.test.ts | 22 ++++++++++++ packages/core/src/permissions/rule-parser.ts | 4 +-- packages/core/src/utils/shell-utils.test.ts | 8 +++++ packages/core/src/utils/shell-utils.ts | 34 ++++++++----------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index db1851666a..34a065bc00 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -237,6 +237,15 @@ describe('matchesCommandPattern', () => { ).toBe(true); }); + it('matches commands containing embedded newlines (dotAll)', async () => { + expect( + matchesCommandPattern( + 'python3 *', + 'python3 -c "\nimport sys\nprint(sys.version)\n"', + ), + ).toBe(true); + }); + it('space-star requires word boundary (ls * does not match lsof)', async () => { expect(matchesCommandPattern('ls *', 'ls -la')).toBe(true); expect(matchesCommandPattern('ls *', 'lsof')).toBe(false); @@ -416,6 +425,19 @@ describe('splitCompoundCommand', () => { 'rm -rf /', ]); }); + + it('splits on newline', async () => { + expect(splitCompoundCommand('export FOO=bar\npython3 script.py')).toEqual([ + 'export FOO=bar', + 'python3 script.py', + ]); + }); + + it('does not split on newline inside quotes', async () => { + expect(splitCompoundCommand('echo "line1\nline2"')).toEqual([ + 'echo "line1\nline2"', + ]); + }); }); // ─── resolvePathPattern ────────────────────────────────────────────────────── diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 0dac38c345..03caf74652 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -514,7 +514,7 @@ export function buildHumanReadableRuleLabel(rules: string[]): string { * Shell operator tokens that act as command boundaries. * Ordered by length (longest first) for correct multi-char operator detection. */ -const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';']; +const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';', '\r\n', '\n']; /** * Split a compound shell command into its individual simple commands @@ -670,7 +670,7 @@ export function matchesCommandPattern( regex += '$'; try { - return new RegExp(regex).test(normalizedCommand); + return new RegExp(regex, 's').test(normalizedCommand); } catch { return normalizedCommand === pattern; } diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 0cdeaee88e..bd939cbdfb 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -453,6 +453,14 @@ describe('getCommandRoots', () => { const result = getCommandRoots('echo hello >| out.txt'); expect(result).toEqual(['echo']); }); + + it('should skip leading env var assignments', async () => { + expect( + getCommandRoots( + 'PYTHONPATH=/Users/jinjing/.qwen/skills/scripts python3 -c "print(1)"', + ), + ).toEqual(['python3']); + }); }); describe('stripShellWrapper', () => { diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 8eeb19eaad..d012259546 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -289,11 +289,8 @@ export function splitCommands(command: string): string[] { /** * Extracts the root command from a given shell command string. - * This is used to identify the base command for permission checks. - * @param command The shell command string to parse - * @returns The root command name, or undefined if it cannot be determined - * @example getCommandRoot("ls -la /tmp") returns "ls" - * @example getCommandRoot("git status && npm test") returns "git" + * Skips leading env var assignments (VAR=value) so that + * `PYTHONPATH=/tmp python3 -c "..."` returns `python3`. */ export function getCommandRoot(command: string): string | undefined { const trimmedCommand = command.trim(); @@ -301,22 +298,21 @@ export function getCommandRoot(command: string): string | undefined { return undefined; } - // This regex is designed to find the first "word" of a command, - // while respecting quotes. It looks for a sequence of non-whitespace - // characters that are not inside quotes. - const match = trimmedCommand.match(/^"([^"]+)"|^'([^']+)'|^(\S+)/); - if (match) { - // The first element in the match array is the full match. - // The subsequent elements are the capture groups. - // We prefer a captured group because it will be unquoted. - const commandRoot = match[1] || match[2] || match[3]; - if (commandRoot) { - // If the command is a path, return the last component. - return commandRoot.split(/[\\/]/).pop(); - } + // Split on whitespace and skip leading VAR=value tokens. + const tokens = trimmedCommand.split(/\s+/); + let idx = 0; + while (idx < tokens.length && /^[A-Za-z_]\w*=/.test(tokens[idx]!)) { + idx++; + } + + const firstToken = tokens[idx]; + if (!firstToken) { + return undefined; } - return undefined; + // Strip quotes and extract last path component. + const unquoted = firstToken.replace(/^["']|["']$/g, ''); + return unquoted ? unquoted.split(/[\\/]/).pop() : undefined; } export function getCommandRoots(command: string): string[] { From 0c17b20ceb88cf2defac062d8c74cacef68df5cb Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Sun, 5 Apr 2026 15:14:58 +0800 Subject: [PATCH 3/4] fix(permissions): tighten shell command parsing Handle env-prefixed commands and quoted Windows paths consistently. Keep newline splitting heredoc-aware and avoid false heredoc detection in comments or arithmetic expressions. --- .../permissions/permission-manager.test.ts | 40 +++ packages/core/src/permissions/rule-parser.ts | 317 +++++++++++++++++- packages/core/src/utils/shell-utils.test.ts | 21 +- packages/core/src/utils/shell-utils.ts | 35 +- 4 files changed, 381 insertions(+), 32 deletions(-) diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 34a065bc00..9c88c2a24c 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -438,6 +438,46 @@ describe('splitCompoundCommand', () => { 'echo "line1\nline2"', ]); }); + + it('does not split heredoc bodies on newlines', async () => { + const command = ["python - <<'PY'", 'print(1)', 'PY', 'echo done'].join( + '\n', + ); + + expect(splitCompoundCommand(command)).toEqual([ + ["python - <<'PY'", 'print(1)', 'PY'].join('\n'), + 'echo done', + ]); + }); + + it('keeps heredoc bodies attached to the command that declared them', async () => { + const command = ["cat <<'EOF' | grep foo", 'foo', 'EOF'].join('\n'); + + expect(splitCompoundCommand(command)).toEqual([ + ["cat <<'EOF'", 'foo', 'EOF'].join('\n'), + 'grep foo', + ]); + }); + + it('does not treat comment text containing << as a heredoc', async () => { + const command = ["# <<'EOF'", 'rm -rf /', 'EOF', 'echo done'].join('\n'); + + expect(splitCompoundCommand(command)).toEqual([ + "# <<'EOF'", + 'rm -rf /', + 'EOF', + 'echo done', + ]); + }); + + it('does not treat arithmetic shifts as heredoc operators', async () => { + const command = ['echo $((1 << 2))', 'echo done'].join('\n'); + + expect(splitCompoundCommand(command)).toEqual([ + 'echo $((1 << 2))', + 'echo done', + ]); + }); }); // ─── resolvePathPattern ────────────────────────────────────────────────────── diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 03caf74652..7883d52f03 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -510,11 +510,214 @@ export function buildHumanReadableRuleLabel(rules: string[]): string { // Shell command matching // ───────────────────────────────────────────────────────────────────────────── +type PendingHeredoc = { + delimiter: string; + segmentIndex: number; + stripLeadingTabs: boolean; +}; + /** * Shell operator tokens that act as command boundaries. * Ordered by length (longest first) for correct multi-char operator detection. + * + * Newlines are handled separately so heredoc bodies can consume their own + * embedded line breaks without being split into standalone commands. */ -const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';', '\r\n', '\n']; +const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';']; + +function isShellWordBoundary(char: string): boolean { + if (char === ' ' || char === '\t' || char === '\n' || char === '\r') { + return true; + } + + return [';', '&', '|', '<', '>', '(', ')'].includes(char); +} + +function isCommentStart(command: string, index: number): boolean { + if (command[index] !== '#') { + return false; + } + if (index === 0) { + return true; + } + + const prev = command[index - 1]!; + if (prev === ' ' || prev === '\t' || prev === '\n' || prev === '\r') { + return true; + } + + return [';', '&', '|', '(', ')', '<', '>'].includes(prev); +} + +function isArithmeticCommandStart(command: string, index: number): boolean { + if (command[index] !== '(' || command[index + 1] !== '(') { + return false; + } + if (index === 0) { + return true; + } + + const prev = command[index - 1]!; + return isShellWordBoundary(prev); +} + +function parseHeredocOperator( + command: string, + startIndex: number, + segmentIndex: number, +): { nextIndex: number; heredoc: PendingHeredoc } | null { + if (command[startIndex] !== '<' || command[startIndex + 1] !== '<') { + return null; + } + + let i = startIndex + 2; + const stripLeadingTabs = command[i] === '-'; + if (stripLeadingTabs) { + i++; + } + + while (i < command.length && (command[i] === ' ' || command[i] === '\t')) { + i++; + } + + let delimiter = ''; + let inSingleQuotes = false; + let inDoubleQuotes = false; + + while (i < command.length) { + const char = command[i]!; + + if (!inSingleQuotes && !inDoubleQuotes && isShellWordBoundary(char)) { + break; + } + + if (!inSingleQuotes && !inDoubleQuotes) { + if (char === "'") { + inSingleQuotes = true; + i++; + continue; + } + + if (char === '"') { + inDoubleQuotes = true; + i++; + continue; + } + + if (char === '\\') { + i++; + if (i >= command.length) { + break; + } + delimiter += command[i]!; + i++; + continue; + } + + delimiter += char; + i++; + continue; + } + + if (inSingleQuotes) { + if (char === "'") { + inSingleQuotes = false; + i++; + continue; + } + + delimiter += char; + i++; + continue; + } + + if (char === '"') { + inDoubleQuotes = false; + i++; + continue; + } + + if (char === '\\') { + i++; + if (i >= command.length) { + break; + } + delimiter += command[i]!; + i++; + continue; + } + + delimiter += char; + i++; + } + + if (delimiter.length === 0) { + return null; + } + + return { + nextIndex: i, + heredoc: { + delimiter, + segmentIndex, + stripLeadingTabs, + }, + }; +} + +function consumePendingHeredocs( + command: string, + startIndex: number, + pendingHeredocs: PendingHeredoc[], + commands: string[], +): number { + let i = startIndex; + + for (const heredoc of pendingHeredocs) { + let heredocText = ''; + + while (i <= command.length) { + const lineStart = i; + while (i < command.length && command[i] !== '\n' && command[i] !== '\r') { + i++; + } + const lineEnd = i; + + const rawLine = command.slice(lineStart, lineEnd); + const effectiveLine = heredoc.stripLeadingTabs + ? rawLine.replace(/^\t+/, '') + : rawLine; + heredocText += `\n${rawLine}`; + + let newlineLength = 0; + if ( + i < command.length && + command[i] === '\r' && + command[i + 1] === '\n' + ) { + newlineLength = 2; + } else if ( + i < command.length && + (command[i] === '\n' || command[i] === '\r') + ) { + newlineLength = 1; + } + + commands[heredoc.segmentIndex] = + (commands[heredoc.segmentIndex] ?? '') + heredocText; + + if (effectiveLine === heredoc.delimiter || newlineLength === 0) { + i = lineEnd + newlineLength; + break; + } + + heredocText = ''; + i = lineEnd + newlineLength; + } + } + + return i; +} /** * Split a compound shell command into its individual simple commands @@ -531,54 +734,142 @@ const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';', '\r\n', '\n']; */ export function splitCompoundCommand(command: string): string[] { const commands: string[] = []; + const pendingHeredocs: PendingHeredoc[] = []; + let currentCommand = ''; let inSingle = false; let inDouble = false; + let inComment = false; let escaped = false; - let lastSplit = 0; + let arithmeticParenDepth = 0; + + const pushCurrentCommand = () => { + const segment = currentCommand.trim(); + if (segment) { + commands.push(segment); + } + currentCommand = ''; + }; for (let i = 0; i < command.length; i++) { const ch = command[i]!; + if (inComment) { + if (ch === '\r' || ch === '\n') { + inComment = false; + } else { + currentCommand += ch; + continue; + } + } + if (escaped) { + currentCommand += ch; escaped = false; continue; } if (ch === '\\') { + currentCommand += ch; escaped = true; continue; } if (ch === "'" && !inDouble) { + currentCommand += ch; inSingle = !inSingle; continue; } if (ch === '"' && !inSingle) { + currentCommand += ch; inDouble = !inDouble; continue; } if (inSingle || inDouble) { + currentCommand += ch; + continue; + } + + if (arithmeticParenDepth > 0) { + currentCommand += ch; + if (ch === '(') { + arithmeticParenDepth++; + } else if (ch === ')') { + arithmeticParenDepth--; + } continue; } + if (ch === '\r' || ch === '\n') { + const newlineLength = ch === '\r' && command[i + 1] === '\n' ? 2 : 1; + + if (pendingHeredocs.length > 0) { + pushCurrentCommand(); + const nextIndex = consumePendingHeredocs( + command, + i + newlineLength, + pendingHeredocs, + commands, + ); + pendingHeredocs.length = 0; + i = nextIndex - 1; + continue; + } + + pushCurrentCommand(); + if (newlineLength === 2) { + i++; + } + continue; + } + + if (ch === '#' && isCommentStart(command, i)) { + currentCommand += ch; + inComment = true; + continue; + } + + if (ch === '$' && command[i + 1] === '(' && command[i + 2] === '(') { + currentCommand += '$(('; + arithmeticParenDepth = 2; + i += 2; + continue; + } + + if (ch === '(' && isArithmeticCommandStart(command, i)) { + currentCommand += '(('; + arithmeticParenDepth = 2; + i += 1; + continue; + } + + if (ch === '<' && command[i + 1] === '<') { + const parsed = parseHeredocOperator(command, i, commands.length); + if (parsed) { + currentCommand += command.slice(i, parsed.nextIndex); + pendingHeredocs.push(parsed.heredoc); + i = parsed.nextIndex - 1; + continue; + } + } + // Check for shell operators (longest match first) + let matchedOperator = false; for (const op of SHELL_OPERATORS) { if (command.substring(i, i + op.length) === op) { - const segment = command.substring(lastSplit, i).trim(); - if (segment) { - commands.push(segment); - } - lastSplit = i + op.length; - i = lastSplit - 1; // -1 because the loop will i++ + pushCurrentCommand(); + i += op.length - 1; + matchedOperator = true; break; } } - } - // Add the last segment - const lastSegment = command.substring(lastSplit).trim(); - if (lastSegment) { - commands.push(lastSegment); + if (matchedOperator) { + continue; + } + + currentCommand += ch; } + pushCurrentCommand(); + return commands.length > 0 ? commands : [command]; } diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index bd939cbdfb..b95e95c7b1 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -29,11 +29,15 @@ vi.mock('os', () => ({ })); const mockQuote = vi.hoisted(() => vi.fn()); -const mockParse = vi.hoisted(() => vi.fn()); -vi.mock('shell-quote', () => ({ - quote: mockQuote, - parse: mockParse, -})); +vi.mock('shell-quote', async () => { + const actual = + await vi.importActual('shell-quote'); + + return { + ...actual, + quote: mockQuote, + }; +}); let config: Config; @@ -42,7 +46,6 @@ beforeEach(() => { mockQuote.mockImplementation((args: string[]) => args.map((arg) => `'${arg}'`).join(' '), ); - mockParse.mockImplementation((cmd: string) => cmd.split(' ')); config = { getCoreTools: () => [], getPermissionsDeny: () => [], @@ -461,6 +464,12 @@ describe('getCommandRoots', () => { ), ).toEqual(['python3']); }); + + it('should preserve quoted Windows paths with spaces', async () => { + expect(getCommandRoots('"C:\\Program Files\\foo\\bar.exe" arg1')).toEqual([ + 'bar.exe', + ]); + }); }); describe('stripShellWrapper', () => { diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index d012259546..ac4a2d0d51 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -8,7 +8,7 @@ import type { AnyToolInvocation } from '../index.js'; import type { Config } from '../config/config.js'; import os from 'node:os'; import path from 'node:path'; -import { quote } from 'shell-quote'; +import { parse, quote } from 'shell-quote'; import { doesToolInvocationMatch } from './tool-utils.js'; import { isShellCommandReadOnly } from './shellReadOnlyChecker.js'; import { @@ -40,6 +40,7 @@ export interface ShellConfiguration { } let cachedBashPath: string | undefined; +const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=/; /** * Attempts to find the Git Bash executable path on Windows. @@ -298,21 +299,29 @@ export function getCommandRoot(command: string): string | undefined { return undefined; } - // Split on whitespace and skip leading VAR=value tokens. - const tokens = trimmedCommand.split(/\s+/); - let idx = 0; - while (idx < tokens.length && /^[A-Za-z_]\w*=/.test(tokens[idx]!)) { - idx++; - } + try { + const tokens = parse(trimmedCommand).filter( + (token): token is string => typeof token === 'string', + ); + + let idx = 0; + while (idx < tokens.length && ENV_ASSIGNMENT_REGEX.test(tokens[idx]!)) { + idx++; + } + + const firstToken = tokens[idx]; + return firstToken ? firstToken.split(/[\\/]/).pop() : undefined; + } catch { + const match = trimmedCommand.match(/^"([^"]+)"|^'([^']+)'|^(\S+)/); + if (match) { + const commandRoot = match[1] || match[2] || match[3]; + if (commandRoot) { + return commandRoot.split(/[\\/]/).pop(); + } + } - const firstToken = tokens[idx]; - if (!firstToken) { return undefined; } - - // Strip quotes and extract last path component. - const unquoted = firstToken.replace(/^["']|["']$/g, ''); - return unquoted ? unquoted.split(/[\\/]/).pop() : undefined; } export function getCommandRoots(command: string): string[] { From 8b30b7ac433c712062618ebdafcd7d416e59d2a1 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Sun, 5 Apr 2026 15:28:22 +0800 Subject: [PATCH 4/4] refactor(permissions): simplify fix by reverting splitCompoundCommand rewrite Remove ~350 lines of heredoc/comment/arithmetic parsing from splitCompoundCommand that were not needed to fix #2846. Revert to the original main version, keeping only the core env-var stripping logic in matchesCommandPattern and getCommandRoot. This addresses both reviewer concerns: - heredoc breakage: no longer an issue since splitCompoundCommand is unchanged - Windows quoted paths: handled correctly by shell-quote parse in getCommandRoot --- .../permissions/permission-manager.test.ts | 53 --- packages/core/src/permissions/rule-parser.ts | 315 +----------------- 2 files changed, 12 insertions(+), 356 deletions(-) diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 9c88c2a24c..505a80d15b 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -425,59 +425,6 @@ describe('splitCompoundCommand', () => { 'rm -rf /', ]); }); - - it('splits on newline', async () => { - expect(splitCompoundCommand('export FOO=bar\npython3 script.py')).toEqual([ - 'export FOO=bar', - 'python3 script.py', - ]); - }); - - it('does not split on newline inside quotes', async () => { - expect(splitCompoundCommand('echo "line1\nline2"')).toEqual([ - 'echo "line1\nline2"', - ]); - }); - - it('does not split heredoc bodies on newlines', async () => { - const command = ["python - <<'PY'", 'print(1)', 'PY', 'echo done'].join( - '\n', - ); - - expect(splitCompoundCommand(command)).toEqual([ - ["python - <<'PY'", 'print(1)', 'PY'].join('\n'), - 'echo done', - ]); - }); - - it('keeps heredoc bodies attached to the command that declared them', async () => { - const command = ["cat <<'EOF' | grep foo", 'foo', 'EOF'].join('\n'); - - expect(splitCompoundCommand(command)).toEqual([ - ["cat <<'EOF'", 'foo', 'EOF'].join('\n'), - 'grep foo', - ]); - }); - - it('does not treat comment text containing << as a heredoc', async () => { - const command = ["# <<'EOF'", 'rm -rf /', 'EOF', 'echo done'].join('\n'); - - expect(splitCompoundCommand(command)).toEqual([ - "# <<'EOF'", - 'rm -rf /', - 'EOF', - 'echo done', - ]); - }); - - it('does not treat arithmetic shifts as heredoc operators', async () => { - const command = ['echo $((1 << 2))', 'echo done'].join('\n'); - - expect(splitCompoundCommand(command)).toEqual([ - 'echo $((1 << 2))', - 'echo done', - ]); - }); }); // ─── resolvePathPattern ────────────────────────────────────────────────────── diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 7883d52f03..dd3641f785 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -510,215 +510,12 @@ export function buildHumanReadableRuleLabel(rules: string[]): string { // Shell command matching // ───────────────────────────────────────────────────────────────────────────── -type PendingHeredoc = { - delimiter: string; - segmentIndex: number; - stripLeadingTabs: boolean; -}; - /** * Shell operator tokens that act as command boundaries. * Ordered by length (longest first) for correct multi-char operator detection. - * - * Newlines are handled separately so heredoc bodies can consume their own - * embedded line breaks without being split into standalone commands. */ const SHELL_OPERATORS = ['&&', '||', ';;', '|&', '|', ';']; -function isShellWordBoundary(char: string): boolean { - if (char === ' ' || char === '\t' || char === '\n' || char === '\r') { - return true; - } - - return [';', '&', '|', '<', '>', '(', ')'].includes(char); -} - -function isCommentStart(command: string, index: number): boolean { - if (command[index] !== '#') { - return false; - } - if (index === 0) { - return true; - } - - const prev = command[index - 1]!; - if (prev === ' ' || prev === '\t' || prev === '\n' || prev === '\r') { - return true; - } - - return [';', '&', '|', '(', ')', '<', '>'].includes(prev); -} - -function isArithmeticCommandStart(command: string, index: number): boolean { - if (command[index] !== '(' || command[index + 1] !== '(') { - return false; - } - if (index === 0) { - return true; - } - - const prev = command[index - 1]!; - return isShellWordBoundary(prev); -} - -function parseHeredocOperator( - command: string, - startIndex: number, - segmentIndex: number, -): { nextIndex: number; heredoc: PendingHeredoc } | null { - if (command[startIndex] !== '<' || command[startIndex + 1] !== '<') { - return null; - } - - let i = startIndex + 2; - const stripLeadingTabs = command[i] === '-'; - if (stripLeadingTabs) { - i++; - } - - while (i < command.length && (command[i] === ' ' || command[i] === '\t')) { - i++; - } - - let delimiter = ''; - let inSingleQuotes = false; - let inDoubleQuotes = false; - - while (i < command.length) { - const char = command[i]!; - - if (!inSingleQuotes && !inDoubleQuotes && isShellWordBoundary(char)) { - break; - } - - if (!inSingleQuotes && !inDoubleQuotes) { - if (char === "'") { - inSingleQuotes = true; - i++; - continue; - } - - if (char === '"') { - inDoubleQuotes = true; - i++; - continue; - } - - if (char === '\\') { - i++; - if (i >= command.length) { - break; - } - delimiter += command[i]!; - i++; - continue; - } - - delimiter += char; - i++; - continue; - } - - if (inSingleQuotes) { - if (char === "'") { - inSingleQuotes = false; - i++; - continue; - } - - delimiter += char; - i++; - continue; - } - - if (char === '"') { - inDoubleQuotes = false; - i++; - continue; - } - - if (char === '\\') { - i++; - if (i >= command.length) { - break; - } - delimiter += command[i]!; - i++; - continue; - } - - delimiter += char; - i++; - } - - if (delimiter.length === 0) { - return null; - } - - return { - nextIndex: i, - heredoc: { - delimiter, - segmentIndex, - stripLeadingTabs, - }, - }; -} - -function consumePendingHeredocs( - command: string, - startIndex: number, - pendingHeredocs: PendingHeredoc[], - commands: string[], -): number { - let i = startIndex; - - for (const heredoc of pendingHeredocs) { - let heredocText = ''; - - while (i <= command.length) { - const lineStart = i; - while (i < command.length && command[i] !== '\n' && command[i] !== '\r') { - i++; - } - const lineEnd = i; - - const rawLine = command.slice(lineStart, lineEnd); - const effectiveLine = heredoc.stripLeadingTabs - ? rawLine.replace(/^\t+/, '') - : rawLine; - heredocText += `\n${rawLine}`; - - let newlineLength = 0; - if ( - i < command.length && - command[i] === '\r' && - command[i + 1] === '\n' - ) { - newlineLength = 2; - } else if ( - i < command.length && - (command[i] === '\n' || command[i] === '\r') - ) { - newlineLength = 1; - } - - commands[heredoc.segmentIndex] = - (commands[heredoc.segmentIndex] ?? '') + heredocText; - - if (effectiveLine === heredoc.delimiter || newlineLength === 0) { - i = lineEnd + newlineLength; - break; - } - - heredocText = ''; - i = lineEnd + newlineLength; - } - } - - return i; -} - /** * Split a compound shell command into its individual simple commands * by splitting on unquoted shell operators (&&, ||, ;, |, etc.). @@ -734,141 +531,53 @@ function consumePendingHeredocs( */ export function splitCompoundCommand(command: string): string[] { const commands: string[] = []; - const pendingHeredocs: PendingHeredoc[] = []; - let currentCommand = ''; let inSingle = false; let inDouble = false; - let inComment = false; let escaped = false; - let arithmeticParenDepth = 0; - - const pushCurrentCommand = () => { - const segment = currentCommand.trim(); - if (segment) { - commands.push(segment); - } - currentCommand = ''; - }; + let lastSplit = 0; for (let i = 0; i < command.length; i++) { const ch = command[i]!; - if (inComment) { - if (ch === '\r' || ch === '\n') { - inComment = false; - } else { - currentCommand += ch; - continue; - } - } - if (escaped) { - currentCommand += ch; escaped = false; continue; } if (ch === '\\') { - currentCommand += ch; escaped = true; continue; } if (ch === "'" && !inDouble) { - currentCommand += ch; inSingle = !inSingle; continue; } if (ch === '"' && !inSingle) { - currentCommand += ch; inDouble = !inDouble; continue; } if (inSingle || inDouble) { - currentCommand += ch; - continue; - } - - if (arithmeticParenDepth > 0) { - currentCommand += ch; - if (ch === '(') { - arithmeticParenDepth++; - } else if (ch === ')') { - arithmeticParenDepth--; - } continue; } - if (ch === '\r' || ch === '\n') { - const newlineLength = ch === '\r' && command[i + 1] === '\n' ? 2 : 1; - - if (pendingHeredocs.length > 0) { - pushCurrentCommand(); - const nextIndex = consumePendingHeredocs( - command, - i + newlineLength, - pendingHeredocs, - commands, - ); - pendingHeredocs.length = 0; - i = nextIndex - 1; - continue; - } - - pushCurrentCommand(); - if (newlineLength === 2) { - i++; - } - continue; - } - - if (ch === '#' && isCommentStart(command, i)) { - currentCommand += ch; - inComment = true; - continue; - } - - if (ch === '$' && command[i + 1] === '(' && command[i + 2] === '(') { - currentCommand += '$(('; - arithmeticParenDepth = 2; - i += 2; - continue; - } - - if (ch === '(' && isArithmeticCommandStart(command, i)) { - currentCommand += '(('; - arithmeticParenDepth = 2; - i += 1; - continue; - } - - if (ch === '<' && command[i + 1] === '<') { - const parsed = parseHeredocOperator(command, i, commands.length); - if (parsed) { - currentCommand += command.slice(i, parsed.nextIndex); - pendingHeredocs.push(parsed.heredoc); - i = parsed.nextIndex - 1; - continue; - } - } - // Check for shell operators (longest match first) - let matchedOperator = false; for (const op of SHELL_OPERATORS) { if (command.substring(i, i + op.length) === op) { - pushCurrentCommand(); - i += op.length - 1; - matchedOperator = true; + const segment = command.substring(lastSplit, i).trim(); + if (segment) { + commands.push(segment); + } + lastSplit = i + op.length; + i = lastSplit - 1; // -1 because the loop will i++ break; } } - - if (matchedOperator) { - continue; - } - - currentCommand += ch; } - pushCurrentCommand(); + // Add the last segment + const lastSegment = command.substring(lastSplit).trim(); + if (lastSegment) { + commands.push(lastSegment); + } return commands.length > 0 ? commands : [command]; }