MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-07-16 21:10:40 -07:00
Code Issues Packages Projects Releases Wiki Activity

chore: remove reverse engineering of arg pattern

Browse Source
This commit is contained in:
Jack Wotherspoon
2026-03-14 20:44:42 +01:00
parent 3de096b444
commit 4204813895
10 changed files with 235 additions and 313 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/ui/components/PoliciesDialog.test.tsx
+5 -4
View File
@@ -20,6 +20,7 @@ function makeRule(
priority: 0,
source: undefined,
argsPattern: undefined,
constraintDisplay: undefined,
...overrides,
} as PolicyRule;
}
@@ -32,21 +33,21 @@ const ALLOW_RULES: PolicyRule[] = [
toolName: 'run_shell_command',
priority: 4.1,
source: 'User: allowed-tools.toml',
argsPattern: new RegExp('"command":"git\\ show(?:[\\s"]|\\\\")'),
constraintDisplay: 'git show*',
}),
makeRule({
decision: PolicyDecision.ALLOW,
toolName: 'run_shell_command',
priority: 4.1,
source: 'User: allowed-tools.toml',
argsPattern: new RegExp('"command":"git\\ diff(?:[\\s"]|\\\\")'),
constraintDisplay: 'git diff*',
}),
makeRule({
decision: PolicyDecision.ALLOW,
toolName: 'run_shell_command',
priority: 4.0,
source: 'Workspace: .gemini/settings.json',
argsPattern: new RegExp('"command":"npm\\ test(?:[\\s"]|\\\\")'),
constraintDisplay: 'npm test*',
}),
];
@@ -71,7 +72,7 @@ const DENY_RULES: PolicyRule[] = [
toolName: 'run_shell_command',
priority: 10,
source: 'Admin: admin-policies.toml',
argsPattern: new RegExp('"command":"rm\\ -rf(?:[\\s"]|\\\\")'),
constraintDisplay: 'rm -rf*',
}),
];
packages/cli/src/ui/utils/policyUtils.test.ts
+5 -67
View File
@@ -5,71 +5,9 @@
*/
import { describe, it, expect } from 'vitest';
import { formatArgsPattern, buildPolicyListItems } from './policyUtils.js';
import { buildPolicyListItems } from './policyUtils.js';
import { PolicyDecision } from '@google/gemini-cli-core';
describe('formatArgsPattern', () => {
it('should return undefined for undefined argsPattern', () => {
expect(formatArgsPattern(undefined)).toBeUndefined();
});
it('should parse commandPrefix pattern (git diff)', () => {
// buildArgsPatterns produces this for commandPrefix: "git diff"
const pattern = new RegExp('"command":"git\\ diff(?:[\\s"]|\\\\")');
expect(formatArgsPattern(pattern)).toBe('git diff*');
});
it('should parse commandPrefix with escaped quotes (git show)', () => {
// buildArgsPatterns uses escapeRegex() which escapes quotes to \"
const pattern = new RegExp('\\"command\\":\\"git\\ show(?:[\\s"]|\\\\")');
expect(formatArgsPattern(pattern)).toBe('git show*');
});
it('should parse commandPrefix with special chars (npm run test:ci)', () => {
// escapeRegex escapes the colon
const pattern = new RegExp(
'"command":"npm\\ run\\ test\\:ci(?:[\\s"]|\\\\")',
);
expect(formatArgsPattern(pattern)).toBe('npm run test:ci*');
});
it('should parse commandPrefix with single word (ls)', () => {
const pattern = new RegExp('"command":"ls(?:[\\s"]|\\\\")');
expect(formatArgsPattern(pattern)).toBe('ls*');
});
it('should parse commandRegex pattern', () => {
const pattern = new RegExp('"command":"npm run .*');
expect(formatArgsPattern(pattern)).toBe('npm run .*');
});
it('should parse commandRegex with escaped quotes', () => {
const pattern = new RegExp('\\"command\\":\\"npm run .*');
expect(formatArgsPattern(pattern)).toBe('npm run .*');
});
it('should parse file_path pattern', () => {
const pattern = new RegExp('"file_path":".*\\/plans\\/.*"');
expect(formatArgsPattern(pattern)).toBe('path: .*\\/plans\\/.*');
});
it('should return file_path fallback for unrecognized file_path shape', () => {
const pattern = new RegExp('"file_path"');
expect(formatArgsPattern(pattern)).toBe('path: ...');
});
it('should return raw source for unknown short regex', () => {
const pattern = new RegExp('something_unknown');
expect(formatArgsPattern(pattern)).toBe('something_unknown');
});
it('should truncate long unknown patterns', () => {
const longPattern = new RegExp('a'.repeat(50));
const result = formatArgsPattern(longPattern);
expect(result).toBe('a'.repeat(40) + '...');
});
});
describe('buildPolicyListItems', () => {
const toolDisplayNames = new Map([
['run_shell_command', 'Shell'],
@@ -145,7 +83,7 @@ describe('buildPolicyListItems', () => {
{
decision: PolicyDecision.ALLOW,
toolName: 'run_shell_command',
argsPattern: new RegExp('"command":"git\\ diff(?:[\\s"]|\\\\")'),
constraintDisplay: 'git diff*',
},
];
@@ -175,12 +113,12 @@ describe('buildPolicyListItems', () => {
expect(items[0].searchText).toContain('run_shell_command');
});
it('should format constraint from argsPattern', () => {
it('should take constraint from constraintDisplay', () => {
const rules = [
{
decision: PolicyDecision.ALLOW,
toolName: 'run_shell_command',
argsPattern: new RegExp('"command":"git\\ show(?:[\\s"]|\\\\")'),
constraintDisplay: 'git show*',
},
];
@@ -192,7 +130,7 @@ describe('buildPolicyListItems', () => {
expect(items[0].constraint).toBe('git show*');
});
it('should have undefined constraint when no argsPattern', () => {
it('should have undefined constraint when no constraintDisplay', () => {
const rules = [
{ decision: PolicyDecision.ALLOW, toolName: 'glob', priority: 5 },
];
packages/cli/src/ui/utils/policyUtils.ts
+1 -75
View File
@@ -28,80 +28,6 @@ export interface PolicyListItem {
searchText: string;
}
/**
* Un-escapes a regex string that was escaped by `escapeRegex()` from
* packages/core/src/policy/utils.ts.
*
* escapeRegex escapes: [-[\]{}()*+?.,\\^$|#\s"]
* by prefixing each with a backslash.
*/
function unescapeRegex(escaped: string): string {
return escaped.replace(/\\(.)/g, '$1');
}
/**
* Formats an argsPattern regex into a human-readable constraint string.
*
* The policy engine compiles TOML rule fields (commandPrefix, commandRegex,
* argsPattern) into RegExp objects via buildArgsPatterns() in
* packages/core/src/policy/utils.ts. This function reverses that compilation
* back into readable text for display.
*
* Pattern formats (checked in order):
*
* 1. commandPrefix — `"command":"<escaped>(?:[\s"]|\\")` → `<prefix>*`
* 2. commandRegex — starts with `"command":"` → raw regex after the prefix
* 3. file_path — contains `"file_path":"<path>"` → `path: <path>`
* 4. Fallback — truncated raw regex source
*
* Returns undefined if there is no constraint (tool-only or wildcard rules).
*/
export function formatArgsPattern(
argsPattern: RegExp | undefined,
): string | undefined {
if (!argsPattern) {
return undefined;
}
const source = argsPattern.source;
// buildArgsPatterns uses escapeRegex() which escapes quotes to \", so the
// regex source contains escaped quotes. We support both escaped (\") and
// unescaped (") formats for robustness.
const cmdPrefix = /^\\?"command\\?":\\?"(.+?)\(\?:\[\\s"]\|\\\\"?\)$/;
// 1. commandPrefix: \"command\":\"<escaped-prefix>(?:[\s"]|\\")"
// The lookahead ensures the prefix is word-bounded in JSON.
const prefixMatch = source.match(cmdPrefix);
if (prefixMatch) {
return unescapeRegex(prefixMatch[1]) + '*';
}
// 2. commandRegex: starts with "command":" or \"command\":\"
const cmdRegexPrefix = /^\\?"command\\?":\\?"/;
const cmdRegexMatch = source.match(cmdRegexPrefix);
if (cmdRegexMatch) {
const regex = source.slice(cmdRegexMatch[0].length);
return regex;
}
// 3. file_path pattern
if (source.includes('"file_path"')) {
const pathMatch = source.match(/"file_path":"(.+?)"/);
if (pathMatch) {
return `path: ${pathMatch[1]}`;
}
return 'path: ...';
}
// 4. Fallback: truncated raw regex
const maxLen = 40;
if (source.length > maxLen) {
return source.substring(0, maxLen) + '...';
}
return source;
}
/**
* Builds a list of PolicyListItems from rules, filtered by decision and sorted
* by priority descending.
@@ -119,7 +45,7 @@ export function buildPolicyListItems(
const toolDisplayName = rule.toolName
? (toolDisplayNames.get(rule.toolName) ?? rule.toolName)
: 'all tools';
const constraint = formatArgsPattern(rule.argsPattern);
const constraint = rule.constraintDisplay;
const priority = String(rule.priority ?? 0);
const source = rule.source ?? '';
packages/core/src/policy/config.ts
+4 -2
View File
@@ -445,13 +445,14 @@ export async function createPolicyEngineConfig(
// Treat args as a command prefix for shell tool
if (toolName === SHELL_TOOL_NAME) {
const patterns = buildArgsPatterns(undefined, args);
for (const pattern of patterns) {
for (const { pattern, display: constraintDisplay } of patterns) {
if (pattern) {
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
argsPattern: new RegExp(pattern),
constraintDisplay,
source: 'Settings (Tools Allowed)',
});
}
@@ -567,7 +568,7 @@ export function createPolicyUpdater(
return;
}
for (const pattern of patterns) {
for (const { pattern, display: constraintDisplay } of patterns) {
if (pattern) {
// Note: patterns from buildArgsPatterns are derived from escapeRegex,
// which is safe and won't contain ReDoS patterns.
@@ -576,6 +577,7 @@ export function createPolicyUpdater(
decision: PolicyDecision.ALLOW,
priority,
argsPattern: new RegExp(pattern),
constraintDisplay,
source: 'Dynamic (Confirmed)',
});
}
packages/core/src/policy/policy-engine.test.ts
+1 -1
View File
@@ -730,7 +730,7 @@ describe('PolicyEngine', () => {
const rules: PolicyRule[] = [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(patterns[0]!),
argsPattern: new RegExp(patterns[0].pattern!),
decision: PolicyDecision.ALLOW,
},
];
packages/core/src/policy/shell-safety.test.ts
+11 -11
View File
@@ -87,7 +87,7 @@ describe('Shell Safety Policy', () => {
const argsPatterns = buildArgsPatterns(undefined, prefix, undefined);
// Since buildArgsPatterns returns array of patterns (strings), we pick the first one
// and compile it.
const argsPattern = new RegExp(argsPatterns[0]!);
const argsPattern = new RegExp(argsPatterns[0].pattern!);
return new PolicyEngine({
rules: [
@@ -201,13 +201,13 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsEcho[0]!),
argsPattern: new RegExp(argsPatternsEcho[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
},
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGit[0]!),
argsPattern: new RegExp(argsPatternsGit[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
},
@@ -287,14 +287,14 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsEcho[0]!),
argsPattern: new RegExp(argsPatternsEcho[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
},
{
toolName: 'run_shell_command',
// Matches "git" at start of *subcommand*
argsPattern: new RegExp(argsPatternsGit[0]!),
argsPattern: new RegExp(argsPatternsGit[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
},
@@ -332,7 +332,7 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGitLog[0]!),
argsPattern: new RegExp(argsPatternsGitLog[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
allowRedirection: true,
@@ -375,7 +375,7 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsPush[0]!),
argsPattern: new RegExp(argsPatternsPush[0].pattern!),
decision: PolicyDecision.DENY,
priority: 2,
},
@@ -406,7 +406,7 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGitStatus[0]!),
argsPattern: new RegExp(argsPatternsGitStatus[0].pattern!),
decision: PolicyDecision.ALLOW,
priority: 2,
name: 'allow_git_status_rule', // Give a name to easily identify
@@ -443,7 +443,7 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAnotherUnknown[0]!),
argsPattern: new RegExp(argsPatternsAnotherUnknown[0].pattern!),
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_another_unknown_command_rule',
@@ -486,14 +486,14 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAsk1[0]!),
argsPattern: new RegExp(argsPatternsAsk1[0].pattern!),
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_rule_1',
},
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAsk2[0]!),
argsPattern: new RegExp(argsPatternsAsk2[0].pattern!),
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_rule_2',
packages/core/src/policy/toml-loader.ts
+136 -130
View File
@@ -444,83 +444,86 @@ export async function loadPoliciesFromToml(
);
// For each argsPattern, expand toolName arrays
return argsPatterns.flatMap((argsPattern) => {
const toolNames: Array<string | undefined> = rule.toolName
? Array.isArray(rule.toolName)
? rule.toolName
: [rule.toolName]
: [undefined];
return argsPatterns.flatMap(
({ pattern: argsPattern, display: constraintDisplay }) => {
const toolNames: Array<string | undefined> = rule.toolName
? Array.isArray(rule.toolName)
? rule.toolName
: [rule.toolName]
: [undefined];
// Create a policy rule for each tool name
return toolNames.map((toolName) => {
let effectiveToolName: string | undefined = toolName;
const mcpName = rule.mcpName;
// Create a policy rule for each tool name
return toolNames.map((toolName) => {
let effectiveToolName: string | undefined = toolName;
const mcpName = rule.mcpName;
if (mcpName) {
// TODO(mcp): Decouple mcpName rules from FQN string parsing
// to support underscores in server aliases natively. Leaving
// mcpName and toolName separate here and relying on metadata
// during policy evaluation will avoid underscore splitting bugs.
// See: https://github.com/google-gemini/gemini-cli/issues/21727
effectiveToolName = formatMcpToolName(
mcpName,
effectiveToolName,
);
}
const policyRule: PolicyRule = {
toolName: effectiveToolName,
subagent: rule.subagent,
mcpName: rule.mcpName,
decision: rule.decision,
priority: transformPriority(rule.priority, tier),
modes: rule.modes,
toolAnnotations: rule.toolAnnotations,
allowRedirection: rule.allow_redirection,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
denyMessage: rule.deny_message,
};
// Compile regex pattern
if (argsPattern) {
try {
new RegExp(argsPattern);
} catch (e) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const error = e as Error;
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Invalid regex pattern',
details: `Pattern: ${argsPattern}\nError: ${error.message}`,
suggestion:
'Check regex syntax for errors like unmatched brackets or invalid escape sequences',
});
return null;
if (mcpName) {
// TODO(mcp): Decouple mcpName rules from FQN string parsing
// to support underscores in server aliases natively. Leaving
// mcpName and toolName separate here and relying on metadata
// during policy evaluation will avoid underscore splitting bugs.
// See: https://github.com/google-gemini/gemini-cli/issues/21727
effectiveToolName = formatMcpToolName(
mcpName,
effectiveToolName,
);
}
if (!isSafeRegExp(argsPattern)) {
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Unsafe regex pattern (potential ReDoS)',
details: `Pattern: ${argsPattern}`,
suggestion:
'Avoid nested quantifiers or extremely long patterns',
});
return null;
const policyRule: PolicyRule = {
toolName: effectiveToolName,
subagent: rule.subagent,
mcpName: rule.mcpName,
decision: rule.decision,
priority: transformPriority(rule.priority, tier),
modes: rule.modes,
toolAnnotations: rule.toolAnnotations,
allowRedirection: rule.allow_redirection,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
denyMessage: rule.deny_message,
constraintDisplay,
};
// Compile regex pattern
if (argsPattern) {
try {
new RegExp(argsPattern);
} catch (e) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const error = e as Error;
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Invalid regex pattern',
details: `Pattern: ${argsPattern}\nError: ${error.message}`,
suggestion:
'Check regex syntax for errors like unmatched brackets or invalid escape sequences',
});
return null;
}
if (!isSafeRegExp(argsPattern)) {
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Unsafe regex pattern (potential ReDoS)',
details: `Pattern: ${argsPattern}`,
suggestion:
'Avoid nested quantifiers or extremely long patterns',
});
return null;
}
policyRule.argsPattern = new RegExp(argsPattern);
}
policyRule.argsPattern = new RegExp(argsPattern);
}
return policyRule;
});
});
return policyRule;
});
},
);
})
.filter((rule): rule is PolicyRule => rule !== null);
@@ -566,70 +569,73 @@ export async function loadPoliciesFromToml(
checker.commandRegex,
);
return argsPatterns.flatMap((argsPattern) => {
const toolNames: Array<string | undefined> = checker.toolName
? Array.isArray(checker.toolName)
? checker.toolName
: [checker.toolName]
: [undefined];
return argsPatterns.flatMap(
({ pattern: argsPattern, display: constraintDisplay }) => {
const toolNames: Array<string | undefined> = checker.toolName
? Array.isArray(checker.toolName)
? checker.toolName
: [checker.toolName]
: [undefined];
return toolNames.map((toolName) => {
let effectiveToolName: string | undefined;
if (checker.mcpName && toolName) {
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_${toolName}`;
} else if (checker.mcpName) {
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_*`;
} else {
effectiveToolName = toolName;
}
return toolNames.map((toolName) => {
let effectiveToolName: string | undefined;
if (checker.mcpName && toolName) {
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_${toolName}`;
} else if (checker.mcpName) {
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_*`;
} else {
effectiveToolName = toolName;
}
const safetyCheckerRule: SafetyCheckerRule = {
toolName: effectiveToolName,
mcpName: checker.mcpName,
priority: transformPriority(checker.priority, tier),
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
checker: checker.checker as SafetyCheckerConfig,
modes: checker.modes,
toolAnnotations: checker.toolAnnotations,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
};
if (argsPattern) {
try {
new RegExp(argsPattern);
} catch (e) {
const safetyCheckerRule: SafetyCheckerRule = {
toolName: effectiveToolName,
mcpName: checker.mcpName,
priority: transformPriority(checker.priority, tier),
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const error = e as Error;
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Invalid regex pattern in safety checker',
details: `Pattern: ${argsPattern}\nError: ${error.message}`,
});
return null;
checker: checker.checker as SafetyCheckerConfig,
modes: checker.modes,
toolAnnotations: checker.toolAnnotations,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
constraintDisplay,
};
if (argsPattern) {
try {
new RegExp(argsPattern);
} catch (e) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const error = e as Error;
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message: 'Invalid regex pattern in safety checker',
details: `Pattern: ${argsPattern}\nError: ${error.message}`,
});
return null;
}
if (!isSafeRegExp(argsPattern)) {
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message:
'Unsafe regex pattern in safety checker (potential ReDoS)',
details: `Pattern: ${argsPattern}`,
});
return null;
}
safetyCheckerRule.argsPattern = new RegExp(argsPattern);
}
if (!isSafeRegExp(argsPattern)) {
errors.push({
filePath,
fileName: file,
tier: tierName,
errorType: 'regex_compilation',
message:
'Unsafe regex pattern in safety checker (potential ReDoS)',
details: `Pattern: ${argsPattern}`,
});
return null;
}
safetyCheckerRule.argsPattern = new RegExp(argsPattern);
}
return safetyCheckerRule;
});
});
return safetyCheckerRule;
});
},
);
})
.filter((checker): checker is SafetyCheckerRule => checker !== null);
packages/core/src/policy/types.ts
+11
View File
@@ -129,6 +129,12 @@ export interface PolicyRule {
*/
argsPattern?: RegExp;
/**
* A human-readable display string representing the constraint (e.g., 'git diff*').
* Used for UI presentation to avoid reverse-engineering the compiled `argsPattern` regex.
*/
constraintDisplay?: string;
/**
* Metadata annotations provided by the tool (e.g. readOnlyHint).
* All keys and values in this record must match the tool's annotations.
@@ -190,6 +196,11 @@ export interface SafetyCheckerRule {
*/
argsPattern?: RegExp;
/**
* A human-readable display string representing the constraint.
*/
constraintDisplay?: string;
/**
* Metadata annotations provided by the tool (e.g. readOnlyHint).
* All keys and values in this record must match the tool's annotations.
packages/core/src/policy/utils.test.ts
+29 -14
View File
@@ -64,62 +64,77 @@ describe('policy/utils', () => {
describe('buildArgsPatterns', () => {
it('should return argsPattern if provided and no commandPrefix/regex', () => {
const result = buildArgsPatterns('my-pattern', undefined, undefined);
expect(result).toEqual(['my-pattern']);
expect(result).toEqual([
{ pattern: 'my-pattern', display: 'my-pattern' },
]);
});
it('should build pattern from a single commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'ls', undefined);
expect(result).toEqual(['\\"command\\":\\"ls(?:[\\s"]|\\\\")']);
expect(result).toEqual([
{ pattern: '"command":"ls(?:[\\s"]|\\\\")', display: 'ls*' },
]);
});
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"]|\\\\")',
{ pattern: '"command":"echo(?:[\\s"]|\\\\")', display: 'echo*' },
{ pattern: '"command":"ls(?:[\\s"]|\\\\")', display: 'ls*' },
]);
});
it('should build pattern from commandRegex', () => {
const result = buildArgsPatterns(undefined, undefined, 'rm -rf .*');
expect(result).toEqual(['"command":"rm -rf .*']);
expect(result).toEqual([
{ pattern: '"command":"rm -rf .*', display: 'rm -rf .*' },
]);
});
it('should prioritize commandPrefix over commandRegex and argsPattern', () => {
const result = buildArgsPatterns('raw', 'prefix', 'regex');
expect(result).toEqual(['\\"command\\":\\"prefix(?:[\\s"]|\\\\")']);
expect(result).toEqual([
{ pattern: '"command":"prefix(?:[\\s"]|\\\\")', display: 'prefix*' },
]);
});
it('should prioritize commandRegex over argsPattern if no commandPrefix', () => {
const result = buildArgsPatterns('raw', undefined, 'regex');
expect(result).toEqual(['"command":"regex']);
expect(result).toEqual([
{ pattern: '"command":"regex', display: 'regex' },
]);
});
it('should escape characters in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git checkout -b', undefined);
expect(result).toEqual([
'\\"command\\":\\"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
{
pattern: '"command":"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
display: 'git checkout -b*',
},
]);
});
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\"]|\\\\\")',
{
pattern: '"command":"git\\ \\\\\\"fix\\\\\\"(?:[\\s"]|\\\\")',
display: 'git "fix"*',
},
]);
});
it('should handle undefined correctly when no inputs are provided', () => {
const result = buildArgsPatterns(undefined, undefined, undefined);
expect(result).toEqual([undefined]);
expect(result).toEqual([{ pattern: undefined, display: undefined }]);
});
it('should match prefixes followed by JSON escaped quotes', () => {
// Testing the security fix logic: allowing "echo \"foo\""
const prefix = 'echo ';
const patterns = buildArgsPatterns(undefined, prefix, undefined);
const regex = new RegExp(patterns[0]!);
const regex = new RegExp(patterns[0].pattern!);
// Mimic JSON stringified args
// echo "foo" -> {"command":"echo \"foo\""}
@@ -131,7 +146,7 @@ describe('policy/utils', () => {
// Testing that we blocked the hole: "echo\foo"
const prefix = 'echo ';
const patterns = buildArgsPatterns(undefined, prefix, undefined);
const regex = new RegExp(patterns[0]!);
const regex = new RegExp(patterns[0].pattern!);
// echo\foo -> {"command":"echo\\foo"}
// In regex matching: "echo " is followed by "\" which is NOT in [\s"] and is not \"
@@ -140,7 +155,7 @@ describe('policy/utils', () => {
// Also validation for "git " matching "git\status"
const gitPatterns = buildArgsPatterns(undefined, 'git ', undefined);
const gitRegex = new RegExp(gitPatterns[0]!);
const gitRegex = new RegExp(gitPatterns[0].pattern!);
// git\status -> {"command":"git\\status"}
const gitAttack = '{"command":"git\\\\status"}';
expect(gitAttack).not.toMatch(gitRegex);
packages/core/src/policy/utils.ts
+32 -9
View File
@@ -42,22 +42,25 @@ export function isSafeRegExp(pattern: string): boolean {
return true;
}
export interface ArgsPatternResult {
pattern: string | undefined;
display?: string;
}
/**
* Builds a list of args patterns for policy matching.
*
* This function handles the transformation of command prefixes and regexes into
* the internal argsPattern representation used by the PolicyEngine.
* Normalizes tool arguments (command prefix or raw regex) into strict Regular Expressions
* for the policy engine. Returns both the compiled string pattern and a human-readable display string.
*
* @param argsPattern An optional raw regex string for arguments.
* @param commandPrefix An optional command prefix (or list of prefixes) to allow.
* @param commandRegex An optional command regex string to allow.
* @returns An array of string patterns (or undefined) for the PolicyEngine.
* @returns An array of pattern results for the PolicyEngine.
*/
export function buildArgsPatterns(
argsPattern?: string,
commandPrefix?: string | string[],
commandRegex?: string,
): Array<string | undefined> {
): ArgsPatternResult[] {
if (commandPrefix) {
const prefixes = Array.isArray(commandPrefix)
? commandPrefix
@@ -78,15 +81,35 @@ export function buildArgsPatterns(
// 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"]|\\\\")`;
const pattern = `${matchSegment}(?:[\\s"]|\\\\")`;
return { pattern, display: `${prefix}*` };
});
}
if (commandRegex) {
return [`"command":"${commandRegex}`];
return [{ pattern: `"command":"${commandRegex}`, display: commandRegex }];
}
return [argsPattern];
// Raw argsPattern fallback logic for display
let display: string | undefined = undefined;
if (argsPattern) {
if (argsPattern.includes('"file_path"')) {
const pathMatch = argsPattern.match(/"file_path":"(.+?)"/);
if (pathMatch) {
display = `path: ${pathMatch[1]}`;
} else {
display = 'path: ...';
}
} else {
const maxLen = 40;
display =
argsPattern.length > maxLen
? argsPattern.substring(0, maxLen) + '...'
: argsPattern;
}
}
return [{ pattern: argsPattern, display }];
}
/**