diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 5b1048d5cb..505a80d15b 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -228,6 +228,24 @@ 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('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); diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 6ca9e83632..dd3641f785 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, 's').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) // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 0cdeaee88e..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: () => [], @@ -453,6 +456,20 @@ 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']); + }); + + 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 8eeb19eaad..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. @@ -289,11 +290,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 +299,29 @@ 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(); + 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++; } - } - return undefined; + 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(); + } + } + + return undefined; + } } export function getCommandRoots(command: string): string[] {