diff --git a/packages/cli/src/ui/components/PoliciesDialog.test.tsx b/packages/cli/src/ui/components/PoliciesDialog.test.tsx index 1d38747d90..2a02321484 100644 --- a/packages/cli/src/ui/components/PoliciesDialog.test.tsx +++ b/packages/cli/src/ui/components/PoliciesDialog.test.tsx @@ -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*', }), ]; diff --git a/packages/cli/src/ui/utils/policyUtils.test.ts b/packages/cli/src/ui/utils/policyUtils.test.ts index 816fa4fcd6..9ec927c734 100644 --- a/packages/cli/src/ui/utils/policyUtils.test.ts +++ b/packages/cli/src/ui/utils/policyUtils.test.ts @@ -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 }, ]; diff --git a/packages/cli/src/ui/utils/policyUtils.ts b/packages/cli/src/ui/utils/policyUtils.ts index 33da5c108f..85319dbaeb 100644 --- a/packages/cli/src/ui/utils/policyUtils.ts +++ b/packages/cli/src/ui/utils/policyUtils.ts @@ -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":"(?:[\s"]|\\")` → `*` - * 2. commandRegex — starts with `"command":"` → raw regex after the prefix - * 3. file_path — contains `"file_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\":\"(?:[\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 ?? ''; diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 392ab15c0c..83e9af909f 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -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)', }); } diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 376e465604..d3c3e764a8 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -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, }, ]; diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 340264485e..2f05101152 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -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', diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index f5c176dc25..c78104c205 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -444,83 +444,86 @@ export async function loadPoliciesFromToml( ); // For each argsPattern, expand toolName arrays - return argsPatterns.flatMap((argsPattern) => { - const toolNames: Array = rule.toolName - ? Array.isArray(rule.toolName) - ? rule.toolName - : [rule.toolName] - : [undefined]; + return argsPatterns.flatMap( + ({ pattern: argsPattern, display: constraintDisplay }) => { + const toolNames: Array = 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 = checker.toolName - ? Array.isArray(checker.toolName) - ? checker.toolName - : [checker.toolName] - : [undefined]; + return argsPatterns.flatMap( + ({ pattern: argsPattern, display: constraintDisplay }) => { + const toolNames: Array = 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); diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 6e14e1fac9..441bfc0af9 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -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. diff --git a/packages/core/src/policy/utils.test.ts b/packages/core/src/policy/utils.test.ts index db6225827a..ec765724d9 100644 --- a/packages/core/src/policy/utils.test.ts +++ b/packages/core/src/policy/utils.test.ts @@ -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); diff --git a/packages/core/src/policy/utils.ts b/packages/core/src/policy/utils.ts index 3c7bd4d16b..9885a3f171 100644 --- a/packages/core/src/policy/utils.ts +++ b/packages/core/src/policy/utils.ts @@ -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 { +): 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 }]; } /**