Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/core/src/permissions/permission-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
49 changes: 46 additions & 3 deletions packages/core/src/permissions/rule-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 === '*') {
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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)
// ─────────────────────────────────────────────────────────────────────────────
Expand Down
29 changes: 23 additions & 6 deletions packages/core/src/utils/shell-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import('shell-quote')>('shell-quote');

return {
...actual,
quote: mockQuote,
};
});

let config: Config;

Expand All @@ -42,7 +46,6 @@ beforeEach(() => {
mockQuote.mockImplementation((args: string[]) =>
args.map((arg) => `'${arg}'`).join(' '),
);
mockParse.mockImplementation((cmd: string) => cmd.split(' '));
config = {
getCoreTools: () => [],
getPermissionsDeny: () => [],
Expand Down Expand Up @@ -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', () => {
Expand Down
45 changes: 25 additions & 20 deletions packages/core/src/utils/shell-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -289,34 +290,38 @@ 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();
if (!trimmedCommand) {
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[] {
Expand Down
Loading