mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-12 20:37:08 -07:00
Auto allow should be for more specific command patterns
This commit is contained in:
Keith Schaab
@@ -147,7 +147,7 @@ export interface UpdatePolicy {
|
||||
persist?: boolean;
|
||||
persistScope?: 'workspace' | 'user';
|
||||
argsPattern?: string;
|
||||
commandPrefix?: string | string[];
|
||||
commandPrefix?: string | string[] | string[][];
|
||||
mcpName?: string;
|
||||
allowRedirection?: boolean;
|
||||
}
|
||||
|
||||
@@ -539,7 +539,7 @@ interface TomlRule {
|
||||
mcpName?: string;
|
||||
decision?: string;
|
||||
priority?: number;
|
||||
commandPrefix?: string | string[];
|
||||
commandPrefix?: string | string[] | string[][];
|
||||
argsPattern?: string;
|
||||
allowRedirection?: boolean;
|
||||
// Index signature to satisfy Record type if needed for toml.stringify
|
||||
@@ -561,7 +561,12 @@ export function createPolicyUpdater(
|
||||
|
||||
if (message.commandPrefix) {
|
||||
// Convert commandPrefix(es) to argsPatterns for in-memory rules
|
||||
const patterns = buildArgsPatterns(undefined, message.commandPrefix);
|
||||
const patterns = buildArgsPatterns(
|
||||
undefined,
|
||||
message.commandPrefix,
|
||||
undefined,
|
||||
message.allowRedirection,
|
||||
);
|
||||
const tier =
|
||||
message.persistScope === 'user'
|
||||
? USER_POLICY_TIER
|
||||
|
||||
@@ -25,8 +25,10 @@ vi.mock('node:fs/promises');
|
||||
vi.mock('../config/storage.js');
|
||||
vi.mock('../utils/shell-utils.js', () => ({
|
||||
getCommandRoots: vi.fn(),
|
||||
getCommandSegments: vi.fn(),
|
||||
stripShellWrapper: vi.fn(),
|
||||
hasRedirection: vi.fn(),
|
||||
isNakedSensitiveCommand: vi.fn(),
|
||||
}));
|
||||
interface ParsedPolicy {
|
||||
rule?: Array<{
|
||||
@@ -82,7 +84,7 @@ describe('createPolicyUpdater', () => {
|
||||
priority: ALWAYS_ALLOW_PRIORITY,
|
||||
mcpName: 'test-mcp',
|
||||
argsPattern: new RegExp(
|
||||
escapeRegex('"command":"echo') + '(?:[\\s"]|\\\\")',
|
||||
`\\x00${escapeRegex('"command":"')}echo\\b(?:(?:[^"&|;\\n\\r<>]|\\\\"))*${escapeRegex('"')}\\x00`,
|
||||
),
|
||||
}),
|
||||
);
|
||||
@@ -93,7 +95,7 @@ describe('createPolicyUpdater', () => {
|
||||
priority: ALWAYS_ALLOW_PRIORITY,
|
||||
mcpName: 'test-mcp',
|
||||
argsPattern: new RegExp(
|
||||
escapeRegex('"command":"ls') + '(?:[\\s"]|\\\\")',
|
||||
`\\x00${escapeRegex('"command":"')}ls\\b(?:(?:[^"&|;\\n\\r<>]|\\\\"))*${escapeRegex('"')}\\x00`,
|
||||
),
|
||||
}),
|
||||
);
|
||||
@@ -172,7 +174,7 @@ describe('createPolicyUpdater', () => {
|
||||
toolName: 'run_shell_command',
|
||||
priority: ALWAYS_ALLOW_PRIORITY,
|
||||
argsPattern: new RegExp(
|
||||
escapeRegex('"command":"git') + '(?:[\\s"]|\\\\")',
|
||||
`\\x00${escapeRegex('"command":"git')}\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*${escapeRegex('"')}\\x00`,
|
||||
),
|
||||
}),
|
||||
);
|
||||
@@ -259,10 +261,14 @@ describe('ShellToolInvocation Policy Update', () => {
|
||||
(c: string) => c,
|
||||
);
|
||||
vi.mocked(shellUtils.hasRedirection).mockReturnValue(false);
|
||||
vi.mocked(shellUtils.isNakedSensitiveCommand).mockReturnValue(false);
|
||||
});
|
||||
|
||||
it('should extract multiple root commands for chained commands', () => {
|
||||
vi.mocked(shellUtils.getCommandRoots).mockReturnValue(['git', 'npm']);
|
||||
it('should extract multiple command segments for chained commands', () => {
|
||||
vi.mocked(shellUtils.getCommandSegments).mockReturnValue([
|
||||
['git'],
|
||||
['npm'],
|
||||
]);
|
||||
|
||||
const invocation = new ShellToolInvocation(
|
||||
mockConfig,
|
||||
@@ -276,14 +282,14 @@ describe('ShellToolInvocation Policy Update', () => {
|
||||
const options = (
|
||||
invocation as unknown as TestableShellToolInvocation
|
||||
).getPolicyUpdateOptions(ToolConfirmationOutcome.ProceedAlways);
|
||||
expect(options!.commandPrefix).toEqual(['git', 'npm']);
|
||||
expect(shellUtils.getCommandRoots).toHaveBeenCalledWith(
|
||||
expect(options!.commandPrefix).toEqual([['git'], ['npm']]);
|
||||
expect(shellUtils.getCommandSegments).toHaveBeenCalledWith(
|
||||
'git status && npm test',
|
||||
);
|
||||
});
|
||||
|
||||
it('should extract a single root command', () => {
|
||||
vi.mocked(shellUtils.getCommandRoots).mockReturnValue(['ls']);
|
||||
it('should extract a single command segment', () => {
|
||||
vi.mocked(shellUtils.getCommandSegments).mockReturnValue([['ls']]);
|
||||
|
||||
const invocation = new ShellToolInvocation(
|
||||
mockConfig,
|
||||
@@ -297,12 +303,12 @@ describe('ShellToolInvocation Policy Update', () => {
|
||||
const options = (
|
||||
invocation as unknown as TestableShellToolInvocation
|
||||
).getPolicyUpdateOptions(ToolConfirmationOutcome.ProceedAlways);
|
||||
expect(options!.commandPrefix).toEqual(['ls']);
|
||||
expect(shellUtils.getCommandRoots).toHaveBeenCalledWith('ls -la /tmp');
|
||||
expect(options!.commandPrefix).toEqual([['ls']]);
|
||||
expect(shellUtils.getCommandSegments).toHaveBeenCalledWith('ls -la /tmp');
|
||||
});
|
||||
|
||||
it('should include allowRedirection if command has redirection', () => {
|
||||
vi.mocked(shellUtils.getCommandRoots).mockReturnValue(['echo']);
|
||||
vi.mocked(shellUtils.getCommandSegments).mockReturnValue([['echo']]);
|
||||
vi.mocked(shellUtils.hasRedirection).mockReturnValue(true);
|
||||
|
||||
const invocation = new ShellToolInvocation(
|
||||
@@ -316,7 +322,7 @@ describe('ShellToolInvocation Policy Update', () => {
|
||||
const options = (
|
||||
invocation as unknown as TestableShellToolInvocation
|
||||
).getPolicyUpdateOptions(ToolConfirmationOutcome.ProceedAlways);
|
||||
expect(options!.commandPrefix).toEqual(['echo']);
|
||||
expect(options!.commandPrefix).toEqual([['echo']]);
|
||||
expect(options!.allowRedirection).toBe(true);
|
||||
expect(shellUtils.hasRedirection).toHaveBeenCalledWith(
|
||||
'echo "hello" > file.txt',
|
||||
|
||||
@@ -150,6 +150,64 @@ describe('Shell Safety Policy', () => {
|
||||
expect(result.decision).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
|
||||
describe('Sequence-based matching (e.g. ["git", "log"])', () => {
|
||||
let sequencePolicyEngine: PolicyEngine;
|
||||
|
||||
beforeEach(() => {
|
||||
const argsPatterns = buildArgsPatterns(
|
||||
undefined,
|
||||
[['git', 'log']],
|
||||
undefined,
|
||||
);
|
||||
sequencePolicyEngine = new PolicyEngine({
|
||||
rules: [
|
||||
{
|
||||
toolName: 'run_shell_command',
|
||||
argsPattern: new RegExp(argsPatterns[0]!),
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
defaultDecision: PolicyDecision.ASK_USER,
|
||||
});
|
||||
});
|
||||
|
||||
it('SHOULD allow "git log" and variations with flags', async () => {
|
||||
const commands = [
|
||||
'git log',
|
||||
'git log --oneline',
|
||||
'git --no-pager log',
|
||||
'git -c color.ui=always log',
|
||||
];
|
||||
|
||||
for (const command of commands) {
|
||||
const result = await sequencePolicyEngine.check(
|
||||
{ name: 'run_shell_command', args: { command } },
|
||||
undefined,
|
||||
);
|
||||
expect(result.decision).toBe(PolicyDecision.ALLOW);
|
||||
}
|
||||
});
|
||||
|
||||
it('SHOULD NOT allow "git push" or unrelated git commands', async () => {
|
||||
const toolCall: FunctionCall = {
|
||||
name: 'run_shell_command',
|
||||
args: { command: 'git push' },
|
||||
};
|
||||
const result = await sequencePolicyEngine.check(toolCall, undefined);
|
||||
expect(result.decision).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
|
||||
it('SHOULD NOT allow compound commands if any part is disallowed', async () => {
|
||||
const toolCall: FunctionCall = {
|
||||
name: 'run_shell_command',
|
||||
args: { command: 'git log && rm -rf /' },
|
||||
};
|
||||
const result = await sequencePolicyEngine.check(toolCall, undefined);
|
||||
expect(result.decision).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
});
|
||||
|
||||
it('SHOULD NOT allow "git log; rm -rf /" (semicolon separator)', async () => {
|
||||
const toolCall: FunctionCall = {
|
||||
name: 'run_shell_command',
|
||||
@@ -327,6 +385,7 @@ describe('Shell Safety Policy', () => {
|
||||
undefined,
|
||||
'git log',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
const policyWithRedirection = new PolicyEngine({
|
||||
rules: [
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
ApprovalMode,
|
||||
PRIORITY_SUBAGENT_TOOL,
|
||||
} from './types.js';
|
||||
import { stableStringify } from './stable-stringify.js';
|
||||
import * as fs from 'node:fs/promises';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
@@ -94,11 +95,15 @@ priority = 100
|
||||
expect(result.rules[0].toolName).toBe('run_shell_command');
|
||||
expect(result.rules[1].toolName).toBe('run_shell_command');
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git status"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git status' }),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
result.rules[1].argsPattern?.test(
|
||||
stableStringify({ command: 'git log' }),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(result.rules[1].argsPattern?.test('{"command":"git log"}')).toBe(
|
||||
true,
|
||||
);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
@@ -160,13 +165,19 @@ priority = 100
|
||||
|
||||
expect(result.rules).toHaveLength(1);
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git status"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git status' }),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git log --all"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git log --all' }),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git branch"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git branch' }),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
@@ -184,7 +195,9 @@ priority = 100
|
||||
// The generated pattern is "command":"^git status
|
||||
// This will NOT match '{"command":"git status"}' because of the '{"' at the start.
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git status"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git status' }),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
@@ -421,10 +434,14 @@ priority = 100
|
||||
expect(result.rules).toHaveLength(1);
|
||||
// The regex should have escaped the * and .
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git log file.txt"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git log file.txt' }),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git log *.txt"}'),
|
||||
result.rules[0].argsPattern?.test(
|
||||
stableStringify({ command: 'git log *.txt' }),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
@@ -41,7 +41,9 @@ const PolicyRuleSchema = z.object({
|
||||
subagent: z.string().optional(),
|
||||
mcpName: z.string().optional(),
|
||||
argsPattern: z.string().optional(),
|
||||
commandPrefix: z.union([z.string(), z.array(z.string())]).optional(),
|
||||
commandPrefix: z
|
||||
.union([z.string(), z.array(z.union([z.string(), z.array(z.string())]))])
|
||||
.optional(),
|
||||
commandRegex: z.string().optional(),
|
||||
decision: z.nativeEnum(PolicyDecision),
|
||||
// Priority must be in range [0, 999] to prevent tier overflow.
|
||||
@@ -76,7 +78,9 @@ const SafetyCheckerRuleSchema = z.object({
|
||||
toolName: z.union([z.string(), z.array(z.string())]),
|
||||
mcpName: z.string().optional(),
|
||||
argsPattern: z.string().optional(),
|
||||
commandPrefix: z.union([z.string(), z.array(z.string())]).optional(),
|
||||
commandPrefix: z
|
||||
.union([z.string(), z.array(z.union([z.string(), z.array(z.string())]))])
|
||||
.optional(),
|
||||
commandRegex: z.string().optional(),
|
||||
priority: z.number().int().default(0),
|
||||
modes: z.array(z.nativeEnum(ApprovalMode)).optional(),
|
||||
@@ -458,6 +462,7 @@ export async function loadPoliciesFromToml(
|
||||
rule.argsPattern,
|
||||
rule.commandPrefix,
|
||||
rule.commandRegex,
|
||||
rule.allowRedirection ?? rule.allow_redirection,
|
||||
);
|
||||
|
||||
// For each argsPattern, expand toolName arrays
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
import { expect, describe, it } from 'vitest';
|
||||
import { escapeRegex, buildArgsPatterns, isSafeRegExp } from './utils.js';
|
||||
import { stableStringify } from './stable-stringify.js';
|
||||
|
||||
describe('policy/utils', () => {
|
||||
describe('escapeRegex', () => {
|
||||
@@ -69,44 +70,51 @@ describe('policy/utils', () => {
|
||||
|
||||
it('should build pattern from a single commandPrefix', () => {
|
||||
const result = buildArgsPatterns(undefined, 'ls', undefined);
|
||||
expect(result).toEqual(['\\"command\\":\\"ls(?:[\\s"]|\\\\")']);
|
||||
expect(result).toEqual([
|
||||
'\\x00\\"command\\":\\"ls\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should build patterns from an array of commandPrefixes', () => {
|
||||
const result = buildArgsPatterns(undefined, ['echo', 'ls'], undefined);
|
||||
expect(result).toEqual([
|
||||
'\\"command\\":\\"echo(?:[\\s"]|\\\\")',
|
||||
'\\"command\\":\\"ls(?:[\\s"]|\\\\")',
|
||||
'\\x00\\"command\\":\\"echo\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
'\\x00\\"command\\":\\"ls\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should build pattern from commandRegex', () => {
|
||||
const result = buildArgsPatterns(undefined, undefined, 'rm -rf .*');
|
||||
expect(result).toEqual(['"command":"rm -rf .*']);
|
||||
expect(result).toEqual([
|
||||
'\\x00\\"command\\":\\"rm -rf .*(?:(?!&|;\n\r<>)(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should prioritize commandPrefix over commandRegex and argsPattern', () => {
|
||||
const result = buildArgsPatterns('raw', 'prefix', 'regex');
|
||||
expect(result).toEqual(['\\"command\\":\\"prefix(?:[\\s"]|\\\\")']);
|
||||
expect(result).toEqual([
|
||||
'\\x00\\"command\\":\\"prefix\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should prioritize commandRegex over argsPattern if no commandPrefix', () => {
|
||||
const result = buildArgsPatterns('raw', undefined, 'regex');
|
||||
expect(result).toEqual(['"command":"regex']);
|
||||
expect(result).toEqual([
|
||||
'\\x00\\"command\\":\\"regex(?:(?!&|;\n\r<>)(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should escape characters in commandPrefix', () => {
|
||||
const result = buildArgsPatterns(undefined, 'git checkout -b', undefined);
|
||||
expect(result).toEqual([
|
||||
'\\"command\\":\\"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
|
||||
'\\x00\\"command\\":\\"git\\ checkout\\ \\-b\\b(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should correctly escape quotes in commandPrefix', () => {
|
||||
const result = buildArgsPatterns(undefined, 'git "fix"', undefined);
|
||||
expect(result).toEqual([
|
||||
// eslint-disable-next-line no-useless-escape
|
||||
'\\\"command\\\":\\\"git\\ \\\\\\\"fix\\\\\\\"(?:[\\s\"]|\\\\\")',
|
||||
'\\x00\\"command\\":\\"git\\ \\\\\\"fix\\\\\\"(?:(?:[^"&|;\n\r<>]|\\\\"))*\\"\\x00',
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -121,9 +129,8 @@ describe('policy/utils', () => {
|
||||
const patterns = buildArgsPatterns(undefined, prefix, undefined);
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
|
||||
// Mimic JSON stringified args
|
||||
// echo "foo" -> {"command":"echo \"foo\""}
|
||||
const validJsonArgs = '{"command":"echo \\"foo\\""}';
|
||||
// Mimic JSON stringified args with null byte boundaries
|
||||
const validJsonArgs = stableStringify({ command: 'echo "foo"' });
|
||||
expect(regex.test(validJsonArgs)).toBe(true);
|
||||
});
|
||||
|
||||
@@ -134,16 +141,76 @@ describe('policy/utils', () => {
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
|
||||
// echo\foo -> {"command":"echo\\foo"}
|
||||
// In regex matching: "echo " is followed by "\" which is NOT in [\s"] and is not \"
|
||||
const attackJsonArgs = '{"command":"echo\\\\foo"}';
|
||||
const attackJsonArgs = stableStringify({ command: 'echo\\foo' });
|
||||
expect(regex.test(attackJsonArgs)).toBe(false);
|
||||
|
||||
// Also validation for "git " matching "git\status"
|
||||
const gitPatterns = buildArgsPatterns(undefined, 'git ', undefined);
|
||||
const gitRegex = new RegExp(gitPatterns[0]!);
|
||||
// git\status -> {"command":"git\\status"}
|
||||
const gitAttack = '{"command":"git\\\\status"}';
|
||||
const gitAttack = stableStringify({ command: 'git\\status' });
|
||||
expect(gitAttack).not.toMatch(gitRegex);
|
||||
});
|
||||
|
||||
it('should NOT match chained commands using shell operators (security check)', () => {
|
||||
// Testing that we block "git log && rm -rf /" when the prefix is "git log"
|
||||
const prefix = 'git log';
|
||||
const patterns = buildArgsPatterns(undefined, prefix, undefined);
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
|
||||
const attackJsonArgs = stableStringify({
|
||||
command: 'git log && rm -rf /',
|
||||
});
|
||||
expect(regex.test(attackJsonArgs)).toBe(false);
|
||||
|
||||
const semicolonAttack = stableStringify({
|
||||
command: 'git log; rm -rf /',
|
||||
});
|
||||
expect(regex.test(semicolonAttack)).toBe(false);
|
||||
});
|
||||
|
||||
describe('sequence matching', () => {
|
||||
it('should build pattern from a sequence of tokens', () => {
|
||||
const result = buildArgsPatterns(
|
||||
undefined,
|
||||
[['git', 'log']],
|
||||
undefined,
|
||||
);
|
||||
const regex = new RegExp(result[0]!);
|
||||
|
||||
expect(regex.test(stableStringify({ command: 'git log' }))).toBe(true);
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'git log --oneline' })),
|
||||
).toBe(true);
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'git --no-pager log' })),
|
||||
).toBe(true);
|
||||
expect(regex.test(stableStringify({ command: 'git push' }))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow flags and options between tokens', () => {
|
||||
const result = buildArgsPatterns(
|
||||
undefined,
|
||||
[['python3', 'main.py']],
|
||||
undefined,
|
||||
);
|
||||
const regex = new RegExp(result[0]!);
|
||||
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'python3 main.py' })),
|
||||
).toBe(true);
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'python3 main.py --help' })),
|
||||
).toBe(true);
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'python3 -u main.py' })),
|
||||
).toBe(true);
|
||||
expect(
|
||||
regex.test(stableStringify({ command: 'python3 other.py' })),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -49,21 +49,57 @@ export function isSafeRegExp(pattern: string): boolean {
|
||||
* the internal argsPattern representation used by the PolicyEngine.
|
||||
*
|
||||
* @param argsPattern An optional raw regex string for arguments.
|
||||
* @param commandPrefix An optional command prefix (or list of prefixes) to allow.
|
||||
* @param commandPrefix An optional command prefix (or list of prefixes/sequences) to allow.
|
||||
* @param commandRegex An optional command regex string to allow.
|
||||
* @param allowRedirection Whether to allow redirection operators in the command.
|
||||
* @returns An array of string patterns (or undefined) for the PolicyEngine.
|
||||
*/
|
||||
export function buildArgsPatterns(
|
||||
argsPattern?: string,
|
||||
commandPrefix?: string | string[],
|
||||
commandPrefix?: string | Array<string | string[]>,
|
||||
commandRegex?: string,
|
||||
allowRedirection?: boolean,
|
||||
): Array<string | undefined> {
|
||||
const shellSeparators = '&|;\n\r';
|
||||
const forbiddenChars = allowRedirection
|
||||
? shellSeparators
|
||||
: `${shellSeparators}<>`;
|
||||
|
||||
const forbiddenCharClass = `(?:[^"${forbiddenChars}]|\\\\")`;
|
||||
|
||||
if (commandPrefix) {
|
||||
const prefixes = Array.isArray(commandPrefix)
|
||||
? commandPrefix
|
||||
: [commandPrefix];
|
||||
|
||||
return prefixes.map((prefix) => {
|
||||
if (Array.isArray(prefix)) {
|
||||
// Handle sequence matching (e.g., ['git', 'log'])
|
||||
// We want to match these tokens in order, allowing flags in between.
|
||||
if (prefix.length === 0) return undefined;
|
||||
|
||||
// Escape each token and ensure word boundaries
|
||||
const escapedTokens = prefix.map((token) => escapeRegex(token));
|
||||
|
||||
// Start with matching \0"command":"<first_token>
|
||||
let pattern = `\\x00${escapeRegex('"command":"')}${escapedTokens[0]}`;
|
||||
|
||||
// For subsequent tokens, allow flags in between
|
||||
for (let i = 1; i < escapedTokens.length; i++) {
|
||||
// Allow whitespace or flags (starting with -) in between tokens.
|
||||
// [^"]*? is replaced by forbiddenCharClass to stay within the command string value
|
||||
// and avoid separators.
|
||||
const gap = `(?:(?:\\s+|-(?:${forbiddenCharClass})*?)\\s*)`;
|
||||
pattern += `${gap}*${escapedTokens[i]}`;
|
||||
}
|
||||
|
||||
// After the last token, we also don't allow forbidden characters until the end of the command string.
|
||||
// We match until the closing quote of the command value followed by \0.
|
||||
// We add a word boundary if the last token ends in a word character.
|
||||
const suffix = /\w$/.test(prefix[prefix.length - 1]) ? '\\b' : '';
|
||||
return `${pattern}${suffix}(?:${forbiddenCharClass})*${escapeRegex('"')}\\x00`;
|
||||
}
|
||||
|
||||
// JSON.stringify safely encodes the prefix in quotes.
|
||||
// We remove ONLY the trailing quote to match it as an open prefix string.
|
||||
const encodedPrefix = JSON.stringify(prefix);
|
||||
@@ -72,18 +108,22 @@ export function buildArgsPatterns(
|
||||
encodedPrefix.length - 1,
|
||||
);
|
||||
|
||||
// Escape the exact JSON literal segment we expect to see
|
||||
const matchSegment = escapeRegex(`"command":${openQuotePrefix}`);
|
||||
// Escape the exact JSON literal segment we expect to see, including \0 boundary.
|
||||
const matchSegment = `\\x00${escapeRegex(
|
||||
`"command":${openQuotePrefix}`,
|
||||
)}`;
|
||||
|
||||
// We allow [\s], ["], or the specific sequence [\"] (for escaped quotes
|
||||
// in JSON). We do NOT allow generic [\\], which would match "git\status"
|
||||
// -> "gitstatus".
|
||||
return `${matchSegment}(?:[\\s"]|\\\\")`;
|
||||
// We allow any characters except forbidden shell separators until the closing quote and \0.
|
||||
// We add a word boundary if the prefix ends in a word character.
|
||||
const suffix = /\w$/.test(prefix) ? '\\b' : '';
|
||||
return `${matchSegment}${suffix}(?:${forbiddenCharClass})*${escapeRegex('"')}\\x00`;
|
||||
});
|
||||
}
|
||||
|
||||
if (commandRegex) {
|
||||
return [`"command":"${commandRegex}`];
|
||||
return [
|
||||
`\\x00${escapeRegex('"command":"')}${commandRegex}(?:(?!${forbiddenChars})${forbiddenCharClass})*${escapeRegex('"')}\\x00`,
|
||||
];
|
||||
}
|
||||
|
||||
return [argsPattern];
|
||||
@@ -102,10 +142,10 @@ export function buildParamArgsPattern(
|
||||
value: unknown,
|
||||
): string {
|
||||
const encodedValue = JSON.stringify(value);
|
||||
// We wrap the JSON string in escapeRegex and prepend/append \\0 to explicitly
|
||||
// We wrap the JSON string in escapeRegex and prepend/append \\x00 to explicitly
|
||||
// match top-level JSON properties generated by stableStringify, preventing
|
||||
// argument injection bypass attacks.
|
||||
return `\\\\0${escapeRegex(`"${paramName}":${encodedValue}`)}\\\\0`;
|
||||
return `\\x00${escapeRegex(`"${paramName}":${encodedValue}`)}\\x00`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -77,8 +77,28 @@ export function isKnownSafeCommand(args: string[]): boolean {
|
||||
// Normalize zsh to bash
|
||||
const normalizedArgs = args.map((a) => (a === 'zsh' ? 'bash' : a));
|
||||
|
||||
const shellMetaCharacters = [
|
||||
'&',
|
||||
'|',
|
||||
';',
|
||||
'<',
|
||||
'>',
|
||||
'(',
|
||||
')',
|
||||
'$',
|
||||
'`',
|
||||
'\\\\',
|
||||
'\\n',
|
||||
'\\r',
|
||||
];
|
||||
const hasShellMeta = (str: string) =>
|
||||
shellMetaCharacters.some((meta) => str.includes(meta));
|
||||
|
||||
if (isSafeToCallWithExec(normalizedArgs)) {
|
||||
return true;
|
||||
// Ensure no arguments contain shell metacharacters that could lead to injection
|
||||
// when this is called from a shell context (like PolicyEngine).
|
||||
// We allow the root command itself (normalizedArgs[0]) but check all arguments.
|
||||
return !normalizedArgs.slice(1).some(hasShellMeta);
|
||||
}
|
||||
|
||||
// Support `bash -lc "..."`
|
||||
|
||||
@@ -696,7 +696,7 @@ describe('policy.ts', () => {
|
||||
expect.objectContaining({
|
||||
toolName: 'write_file',
|
||||
argsPattern:
|
||||
'\\\\0' + escapeRegex('"file_path":"src/foo.ts"') + '\\\\0',
|
||||
'\\x00' + escapeRegex('"file_path":"src/foo.ts"') + '\\x00',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -37,11 +37,14 @@ import { formatBytes } from '../utils/formatters.js';
|
||||
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
||||
import {
|
||||
getCommandRoots,
|
||||
getCommandSegments,
|
||||
initializeShellParsers,
|
||||
stripShellWrapper,
|
||||
parseCommandDetails,
|
||||
hasRedirection,
|
||||
isNakedSensitiveCommand,
|
||||
} from '../utils/shell-utils.js';
|
||||
import { buildParamArgsPattern } from '../policy/utils.js';
|
||||
import { SHELL_TOOL_NAME } from './tool-names.js';
|
||||
import { PARAM_ADDITIONAL_PERMISSIONS } from './definitions/base-declarations.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
@@ -142,13 +145,23 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
outcome === ToolConfirmationOutcome.ProceedAlways
|
||||
) {
|
||||
const command = stripShellWrapper(this.params.command);
|
||||
const rootCommands = [...new Set(getCommandRoots(command))];
|
||||
const segments = getCommandSegments(command);
|
||||
const allowRedirection = hasRedirection(command) ? true : undefined;
|
||||
|
||||
if (rootCommands.length > 0) {
|
||||
return { commandPrefix: rootCommands, allowRedirection };
|
||||
// Filter out "naked" sensitive commands to prevent over-broad matching
|
||||
const safeSegments = segments.filter(
|
||||
(seg) => !isNakedSensitiveCommand(seg),
|
||||
);
|
||||
|
||||
if (safeSegments.length > 0) {
|
||||
return { commandPrefix: safeSegments, allowRedirection };
|
||||
}
|
||||
return { commandPrefix: this.params.command, allowRedirection };
|
||||
// If the command is naked and sensitive, we fall back to the raw command as an exact argsPattern
|
||||
// instead of a broad prefix rule.
|
||||
return {
|
||||
argsPattern: buildParamArgsPattern('command', this.params.command),
|
||||
allowRedirection,
|
||||
};
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -149,7 +149,7 @@ export function isBackgroundExecutionData(
|
||||
*/
|
||||
export interface PolicyUpdateOptions {
|
||||
argsPattern?: string;
|
||||
commandPrefix?: string | string[];
|
||||
commandPrefix?: string | string[] | string[][];
|
||||
mcpName?: string;
|
||||
toolName?: string;
|
||||
allowRedirection?: boolean;
|
||||
|
||||
@@ -751,6 +751,131 @@ export function getCommandRoots(command: string): string[] {
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
/**
|
||||
* Binaries that are considered sensitive and should not be allowed to be saved as "naked" rules.
|
||||
*/
|
||||
export const SENSITIVE_BINARIES = new Set([
|
||||
'git',
|
||||
'node',
|
||||
'npm',
|
||||
'npx',
|
||||
'yarn',
|
||||
'pnpm',
|
||||
'python',
|
||||
'python3',
|
||||
'pip',
|
||||
'pip3',
|
||||
'bash',
|
||||
'sh',
|
||||
'zsh',
|
||||
'sudo',
|
||||
'docker',
|
||||
'kubectl',
|
||||
'aws',
|
||||
'gcloud',
|
||||
'az',
|
||||
'chmod',
|
||||
'chown',
|
||||
'rm',
|
||||
'mv',
|
||||
'cp',
|
||||
'find',
|
||||
'grep',
|
||||
'sed',
|
||||
'awk',
|
||||
'curl',
|
||||
'wget',
|
||||
'ssh',
|
||||
'scp',
|
||||
'rsync',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Checks if a command sequence represents a "naked" sensitive command.
|
||||
* A command is naked if it only contains the binary name without any subcommands or scripts.
|
||||
*
|
||||
* @param sequence - The command sequence (e.g., ['git', 'log']).
|
||||
* @returns true if the command is a naked sensitive command.
|
||||
*/
|
||||
export function isNakedSensitiveCommand(sequence: string[]): boolean {
|
||||
if (sequence.length === 0) return false;
|
||||
const binary = sequence[0];
|
||||
// A command is "naked" if it has only one token (the binary itself)
|
||||
// and that binary is in the sensitive list.
|
||||
return sequence.length === 1 && SENSITIVE_BINARIES.has(binary);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts sequences of positional arguments for each command in a potentially chained shell command.
|
||||
* Flags (starting with -) are ignored, and subcommands/scripts are captured.
|
||||
*
|
||||
* @param command - The shell command string to parse.
|
||||
* @returns An array of sequences, where each sequence is string[].
|
||||
* @example "git --no-pager log" -> [["git", "log"]]
|
||||
* @example "git log && ls -la" -> [["git", "log"], ["ls"]]
|
||||
*/
|
||||
export function getCommandSegments(command: string): string[][] {
|
||||
if (!command) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const bashResult = parseBashCommandDetails(command);
|
||||
if (!bashResult || bashResult.hasError) {
|
||||
// Fallback to simple root extraction if tree-sitter fails or is not bash
|
||||
const roots = getCommandRoots(command);
|
||||
return roots.map((r) => [r]);
|
||||
}
|
||||
|
||||
const tree = parseCommandTree(command);
|
||||
if (!tree) {
|
||||
const roots = getCommandRoots(command);
|
||||
return roots.map((r) => [r]);
|
||||
}
|
||||
|
||||
const segments: string[][] = [];
|
||||
|
||||
function walk(node: Node) {
|
||||
if (node.type === 'command') {
|
||||
const sequence: string[] = [];
|
||||
const nameNode = node.childForFieldName('name');
|
||||
if (nameNode) {
|
||||
sequence.push(normalizeCommandName(nameNode.text));
|
||||
}
|
||||
|
||||
for (let i = 0; i < node.childCount; i++) {
|
||||
const child = node.child(i);
|
||||
if (!child || child.id === nameNode?.id) continue;
|
||||
|
||||
// We only care about words/strings that are arguments
|
||||
if (
|
||||
child.type === 'word' ||
|
||||
child.type === 'string' ||
|
||||
child.type === 'raw_string'
|
||||
) {
|
||||
const text = child.text;
|
||||
if (!text.startsWith('-')) {
|
||||
sequence.push(normalizeCommandName(text));
|
||||
}
|
||||
}
|
||||
// Limit sequence length to command + first positional argument
|
||||
// to avoid over-specifying (e.g., git commit "message" -> git commit)
|
||||
if (sequence.length >= 2) break;
|
||||
}
|
||||
if (sequence.length > 0) {
|
||||
segments.push(sequence);
|
||||
}
|
||||
}
|
||||
|
||||
for (let i = 0; i < node.childCount; i++) {
|
||||
const child = node.child(i);
|
||||
if (child) walk(child);
|
||||
}
|
||||
}
|
||||
|
||||
walk(tree.rootNode);
|
||||
return segments;
|
||||
}
|
||||
|
||||
export function stripShellWrapper(command: string): string {
|
||||
const pattern =
|
||||
/^\s*(?:(?:(?:\S+\/)?(?:sh|bash|zsh))\s+-c|cmd\.exe\s+\/c|powershell(?:\.exe)?\s+(?:-NoProfile\s+)?-Command|pwsh(?:\.exe)?\s+(?:-NoProfile\s+)?-Command)\s+/i;
|
||||
|
||||
Reference in New Issue
Block a user