From b39d2a05477c6110c0e8339be71439d94a37807d Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Wed, 1 Jul 2026 14:07:59 +0800 Subject: [PATCH 1/5] feat(core): add Tool(param:value) permission syntax for parameter-level access control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces key:value parameter matching in permission rules, allowing users to grant or deny tool access based on specific input parameters. - Parse key:value pairs from specifiers for literal-kind rules - Support wildcard patterns (*), multiple params, and mixed syntax - Thread toolParams through PermissionCheckContext and matchesRule - Add 11 unit tests covering parsing, matching, wildcards, and edge cases - Maintain backward compatibility with existing specifier kinds Example rules: Agent(model:opus) # deny agents using Opus model Agent(coder,model:*) # deny coder-type agents with any model Bash(git:*) # still works (legacy :* → git *) Closes #6100 --- packages/core/src/core/permission-helpers.ts | 2 +- .../permissions/permission-manager.test.ts | 202 ++++++++++++++++++ .../src/permissions/permission-manager.ts | 16 +- packages/core/src/permissions/rule-parser.ts | 152 +++++++++---- packages/core/src/permissions/types.ts | 16 ++ 5 files changed, 340 insertions(+), 48 deletions(-) diff --git a/packages/core/src/core/permission-helpers.ts b/packages/core/src/core/permission-helpers.ts index d546455d155..f143fef89af 100644 --- a/packages/core/src/core/permission-helpers.ts +++ b/packages/core/src/core/permission-helpers.ts @@ -88,7 +88,7 @@ export function buildPermissionCheckContext( ? toolParams['server_name'] : undefined; - return { toolName, command, cwd, filePath, domain, specifier }; + return { toolName, command, cwd, filePath, domain, specifier, toolParams }; } // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 1260ccfa137..a0cae8d29c4 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -877,6 +877,208 @@ describe('matchesRule', () => { expect(matchesRule(rule, 'mcp__chrome__navigate')).toBe(false); expect(matchesRule(rule, 'mcp__other__use_browser')).toBe(false); }); + + // ─── Tool(param:value) syntax ─────────────────────────────────────────────── + + it('parseRule extracts key:value param matchers', async () => { + const r = parseRule('Agent(model:opus)'); + expect(r.toolName).toBe('agent'); + expect(r.specifier).toBeUndefined(); + expect(r.toolParamMatchers).toEqual([ + { key: 'model', valuePattern: 'opus' }, + ]); + }); + + it('parseRule extracts multiple key:value pairs', async () => { + const r = parseRule('Agent(model:opus,type:code)'); + expect(r.toolParamMatchers).toEqual([ + { key: 'model', valuePattern: 'opus' }, + { key: 'type', valuePattern: 'code' }, + ]); + }); + + it('parseRule handles mixed specifier and param matchers', async () => { + const r = parseRule('Agent(coder,model:opus)'); + expect(r.specifier).toBe('coder'); + expect(r.toolParamMatchers).toEqual([ + { key: 'model', valuePattern: 'opus' }, + ]); + }); + + it('parseRule supports wildcard in value pattern', async () => { + const r = parseRule('Agent(model:*)'); + expect(r.toolParamMatchers).toEqual([{ key: 'model', valuePattern: '*' }]); + }); + + it('parseRule does not treat WebFetch domain: as key:value', async () => { + const r = parseRule('WebFetch(domain:example.com)'); + expect(r.specifierKind).toBe('domain'); + expect(r.toolParamMatchers).toBeUndefined(); + }); + + it('parseRule preserves legacy :* for command specifiers', async () => { + const r = parseRule('Bash(git:*)'); + expect(r.specifier).toBe('git *'); + expect(r.toolParamMatchers).toBeUndefined(); + }); + + it('matchesRule matches tool with param matcher', async () => { + const rule = parseRule('Agent(model:opus)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'sonnet', + }, + ), + ).toBe(false); + }); + + it('matchesRule fails when toolParams missing for param matcher rule', async () => { + const rule = parseRule('Agent(model:opus)'); + expect(matchesRule(rule, 'agent')).toBe(false); + }); + + it('matchesRule supports wildcard value pattern', async () => { + const rule = parseRule('Agent(model:*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'sonnet', + }, + ), + ).toBe(true); + expect(matchesRule(rule, 'agent')).toBe(false); // no toolParams + }); + + it('matchesRule requires all param matchers to match', async () => { + const rule = parseRule('Agent(model:opus,type:code)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + type: 'code', + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + type: 'chat', + }, + ), + ).toBe(false); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + }, + ), + ).toBe(false); // missing 'type' param + }); + + it('matchesRule handles mixed specifier and param matchers', async () => { + const rule = parseRule('Agent(coder,model:opus)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + 'coder', + { model: 'opus' }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + 'coder', + { model: 'sonnet' }, + ), + ).toBe(false); // param mismatch + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + 'explore', + { model: 'opus' }, + ), + ).toBe(false); // specifier mismatch + }); }); // ─── PermissionManager ────────────────────────────────────────────────────── diff --git a/packages/core/src/permissions/permission-manager.ts b/packages/core/src/permissions/permission-manager.ts index 2fb63a661c3..c4f07e9b75d 100644 --- a/packages/core/src/permissions/permission-manager.ts +++ b/packages/core/src/permissions/permission-manager.ts @@ -265,7 +265,8 @@ export class PermissionManager { * to match equivalent shell commands (e.g. `cat` → Read, `curl` → WebFetch). */ private evaluateSingle(ctx: PermissionCheckContext): PermissionDecision { - const { toolName, command, cwd, filePath, domain, specifier } = ctx; + const { toolName, command, cwd, filePath, domain, specifier, toolParams } = + ctx; // Build path context for resolving relative path patterns const pathCtx: PathMatchContext | undefined = @@ -283,6 +284,7 @@ export class PermissionManager { domain, pathCtx, specifier, + toolParams, ] as const; // Compute the base decision from explicit Bash/file/domain rules. @@ -618,7 +620,8 @@ export class PermissionManager { */ findMatchingDenyRule(ctx: PermissionCheckContext): string | undefined { ctx = this.normalizePermissionContext(ctx); - const { toolName, command, cwd, filePath, domain, specifier } = ctx; + const { toolName, command, cwd, filePath, domain, specifier, toolParams } = + ctx; const pathCtx: PathMatchContext | undefined = this.config.getProjectRoot && this.config.getCwd @@ -635,6 +638,7 @@ export class PermissionManager { domain, pathCtx, specifier, + toolParams, ] as const; for (const rule of [ @@ -696,7 +700,8 @@ export class PermissionManager { */ hasRelevantRules(ctx: PermissionCheckContext): boolean { ctx = this.normalizePermissionContext(ctx); - const { toolName, command, cwd, filePath, domain, specifier } = ctx; + const { toolName, command, cwd, filePath, domain, specifier, toolParams } = + ctx; const pathCtx: PathMatchContext | undefined = this.config.getProjectRoot && this.config.getCwd @@ -764,6 +769,7 @@ export class PermissionManager { domain, pathCtx, specifier, + toolParams, ] as const; return allRules.some((rule) => matchesRule(rule, ...matchArgs)); @@ -780,7 +786,8 @@ export class PermissionManager { */ hasMatchingAskRule(ctx: PermissionCheckContext): boolean { ctx = this.normalizePermissionContext(ctx); - const { toolName, command, cwd, filePath, domain, specifier } = ctx; + const { toolName, command, cwd, filePath, domain, specifier, toolParams } = + ctx; const pathCtx: PathMatchContext | undefined = this.config.getProjectRoot && this.config.getCwd @@ -839,6 +846,7 @@ export class PermissionManager { domain, pathCtx, specifier, + toolParams, ] as const; return askRules.some((rule) => matchesRule(rule, ...matchArgs)); diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 6fec5e1d671..adecb64931d 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -262,37 +262,75 @@ export function toolMatchesRuleToolName( export function parseRule(raw: string): PermissionRule { const trimmed = raw.trim(); - // Handle legacy `:*` suffix (deprecated, equivalent to ` *`) - // e.g. "Bash(git:*)" → "Bash(git *)" - const normalized = trimmed.replace(/:(\*)/, ' $1'); - - const openParen = normalized.indexOf('('); + const openParen = trimmed.indexOf('('); if (openParen === -1) { // Simple tool name rule (no specifier) - const canonicalName = resolveToolName(normalized); + const canonicalName = resolveToolName(trimmed); return { raw: trimmed, toolName: canonicalName, }; } - const toolPart = normalized.substring(0, openParen).trim(); + const toolPart = trimmed.substring(0, openParen).trim(); - if (!normalized.endsWith(')')) { + if (!trimmed.endsWith(')')) { // Malformed: unbalanced parentheses — mark as invalid so it never matches. return { raw: trimmed, toolName: resolveToolName(toolPart), invalid: true }; } - const specifier = normalized.substring(openParen + 1, normalized.length - 1); + let rawSpecifier = trimmed.substring(openParen + 1, trimmed.length - 1); const canonicalName = resolveToolName(toolPart); - const specifierKind = specifier ? getSpecifierKind(canonicalName) : undefined; + + // Handle legacy `:*` suffix for command specifiers (deprecated, equivalent to ` *`) + // e.g. "Bash(git:*)" → specifier becomes "git *" + // Only applies to command-type specifiers to avoid interfering with key:value syntax + const specifierKind = rawSpecifier + ? getSpecifierKind(canonicalName) + : undefined; + if (specifierKind === 'command' && rawSpecifier.endsWith(':*')) { + rawSpecifier = rawSpecifier.slice(0, -2) + ' *'; + } + + // For literal specifier kind, extract `key:value` param matchers. + // Comma-separated: `Agent(coder,model:opus,type:*)` → + // specifier = "coder", toolParamMatchers = [{model,opus},{type,*}] + let specifier: string | undefined = rawSpecifier; + let toolParamMatchers: + | Array<{ key: string; valuePattern: string }> + | undefined; + + if (specifierKind === 'literal' && rawSpecifier.includes(':')) { + const parts = rawSpecifier.split(',').map((p) => p.trim()); + const plainParts: string[] = []; + const matchers: Array<{ key: string; valuePattern: string }> = []; + + for (const part of parts) { + const colonIdx = part.indexOf(':'); + if (colonIdx > 0) { + const key = part.substring(0, colonIdx).trim(); + const valuePattern = part.substring(colonIdx + 1).trim(); + if (key && /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + matchers.push({ key, valuePattern }); + continue; + } + } + plainParts.push(part); + } + + if (matchers.length > 0) { + toolParamMatchers = matchers; + specifier = plainParts.join(',').trim() || undefined; + } + } return { raw: trimmed, toolName: canonicalName, specifier, specifierKind, + toolParamMatchers, }; } @@ -994,6 +1032,7 @@ export function matchesRule( domain?: string, pathContext?: PathMatchContext, specifier?: string, + toolParams?: Record, ): boolean { const canonicalCtxToolName = resolveToolName(toolName); @@ -1015,53 +1054,80 @@ export function matchesRule( return false; } - // ── No specifier → match any invocation of the tool ────────────────── - if (!rule.specifier) { + // ── No specifier and no param matchers → match any invocation ──────── + if (!rule.specifier && !rule.toolParamMatchers?.length) { return true; } // ── Specifier matching (kind-dependent) ────────────────────────────── const kind = rule.specifierKind ?? getSpecifierKind(rule.toolName); - switch (kind) { - case 'command': { - if (command === undefined) { - return false; + let specifierMatched = true; + + if (rule.specifier) { + switch (kind) { + case 'command': { + if (command === undefined) { + return false; + } + specifierMatched = matchesCommandPattern(rule.specifier, command); + break; } - return matchesCommandPattern(rule.specifier, command); - } - case 'path': { - if (filePath === undefined) { - return false; + case 'path': { + if (filePath === undefined) { + return false; + } + const ctx = pathContext ?? { + projectRoot: process.cwd(), + cwd: process.cwd(), + }; + specifierMatched = matchesPathPattern( + rule.specifier, + filePath, + ctx.projectRoot, + ctx.cwd, + ); + break; } - const ctx = pathContext ?? { - projectRoot: process.cwd(), - cwd: process.cwd(), - }; - return matchesPathPattern( - rule.specifier, - filePath, - ctx.projectRoot, - ctx.cwd, - ); - } - case 'domain': { - if (domain === undefined) { - return false; + case 'domain': { + if (domain === undefined) { + return false; + } + specifierMatched = matchesDomainPattern(rule.specifier, domain); + break; } - return matchesDomainPattern(rule.specifier, domain); - } - case 'literal': - default: { - // Literal/exact matching (for Skill names, Agent subagent types, etc.) - const value = command ?? specifier; - if (value !== undefined) { - return value === rule.specifier; + case 'literal': + default: { + const value = command ?? specifier; + specifierMatched = value !== undefined && value === rule.specifier; + break; } + } + } + + if (!specifierMatched) { + return false; + } + + // ── Tool param matching (key:value syntax) ─────────────────────────── + if (rule.toolParamMatchers?.length) { + if (!toolParams) { return false; } + for (const matcher of rule.toolParamMatchers) { + const actualValue = toolParams[matcher.key]; + if (actualValue === undefined || actualValue === null) { + return false; + } + const actualStr = String(actualValue); + if (!matchesCommandPattern(matcher.valuePattern, actualStr)) { + return false; + } + } } + + return true; } diff --git a/packages/core/src/permissions/types.ts b/packages/core/src/permissions/types.ts index 3106c8e4d04..70f0d36924e 100644 --- a/packages/core/src/permissions/types.ts +++ b/packages/core/src/permissions/types.ts @@ -40,6 +40,7 @@ export type SpecifierKind = 'command' | 'path' | 'domain' | 'literal'; * "Read(./secrets/**)" → file reads matching path pattern * "Edit(/src/**\/*.ts)" → file edits matching path pattern * "WebFetch(domain:x.com)" → web fetch matching domain + * "Agent(model:opus)" → subagent with model param matching * "mcp__server__tool" → specific MCP tool */ export interface PermissionRule { @@ -52,6 +53,7 @@ export interface PermissionRule { * For shell tools: a command pattern (e.g. "git *"). * For file tools: a path pattern (e.g. "./secrets/**"). * For WebFetch: a domain pattern (e.g. "domain:example.com"). + * For tool param matching: the plain (non-key:value) part of the specifier. */ specifier?: string; /** @@ -59,6 +61,15 @@ export interface PermissionRule { * Set automatically during parsing based on the tool name/category. */ specifierKind?: SpecifierKind; + /** + * Parsed `key:value` parameter matchers extracted from the specifier. + * When set, `matchesRule` also requires every listed key to be present + * in the tool's actual input params with a value matching the pattern + * (supports `*` glob wildcards). + * + * Example: rule `Agent(model:opus)` → `[{ key: 'model', valuePattern: 'opus' }]` + */ + toolParamMatchers?: Array<{ key: string; valuePattern: string }>; /** True if the raw rule was malformed (e.g. unbalanced parens) and should never match. */ invalid?: boolean; } @@ -106,6 +117,11 @@ export interface PermissionCheckContext { * specifier that doesn't fall into command/path/domain categories. */ specifier?: string; + /** + * Raw tool input parameters, used for `Tool(param:value)` rule matching. + * Populated by `buildPermissionCheckContext` from the tool invocation args. + */ + toolParams?: Record; } /** A rule with its type and source scope, used for listing rules. */ From 8e59413237256f213ae591f878ab6e351f5afe2d Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Wed, 1 Jul 2026 17:24:58 +0800 Subject: [PATCH 2/5] fix(permissions): resolve PR #6106 review comments for tool param permission syntax - buildPermissionRules now propagates toolParamMatchers for 'Always Allow' flow - MCP tool rules now check param matchers after name matching - Added Object.hasOwn check to prevent prototype chain lookup vulnerability - Replaced matchesCommandPattern with matchesParamValuePattern for param value matching - Added diagnostic logging for param matching failures - Added warnings for empty valuePattern, invalid keys, and non-literal key:value syntax - Added type checking for non-primitive param values --- packages/core/src/permissions/rule-parser.ts | 124 ++++++++++++++++++- 1 file changed, 119 insertions(+), 5 deletions(-) diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index adecb64931d..26d79b83efd 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -312,8 +312,17 @@ export function parseRule(raw: string): PermissionRule { const key = part.substring(0, colonIdx).trim(); const valuePattern = part.substring(colonIdx + 1).trim(); if (key && /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + if (valuePattern === '') { + debugLogger.warn( + `Empty valuePattern in rule "${trimmed}": key="${key}" will only match empty strings. Use "*" to match any value.`, + ); + } matchers.push({ key, valuePattern }); continue; + } else if (key && !/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + debugLogger.warn( + `Invalid key "${key}" in rule "${trimmed}": keys must match /^[a-zA-Z_][a-zA-Z0-9_]*$/. Hyphens and dots are not supported.`, + ); } } plainParts.push(part); @@ -323,6 +332,14 @@ export function parseRule(raw: string): PermissionRule { toolParamMatchers = matchers; specifier = plainParts.join(',').trim() || undefined; } + } else if ( + specifierKind !== 'literal' && + rawSpecifier.includes(':') && + !rawSpecifier.startsWith('domain:') + ) { + debugLogger.warn( + `key:value syntax is only supported for literal-specifier tools (got ${specifierKind} for "${canonicalName}")`, + ); } return { @@ -484,11 +501,19 @@ export function buildPermissionRules(ctx: PermissionCheckContext): string[] { return [displayName]; case 'literal': - default: - if (ctx.specifier) { - return [`${displayName}(${ctx.specifier})`]; + default: { + const parts: string[] = []; + if (ctx.specifier) parts.push(ctx.specifier); + if (ctx.toolParams) { + for (const [k, v] of Object.entries(ctx.toolParams)) { + if (typeof v === 'string' || typeof v === 'number') { + parts.push(`${k}:${v}`); + } + } } + if (parts.length > 0) return [`${displayName}(${parts.join(',')})`]; return [displayName]; + } } } @@ -767,6 +792,34 @@ export function matchesCommandPattern( } } +/** + * Match a parameter value against a pattern. + * Unlike matchesCommandPattern, this uses simple glob matching without + * shell-specific semantics (no prefix+space matching, no variable stripping). + * Supports `*` as wildcard for any substring. + */ +function matchesParamValuePattern(pattern: string, value: string): boolean { + if (pattern === '*') { + return true; + } + if (!pattern.includes('*')) { + return value === pattern; + } + // Convert glob pattern to regex: escape special chars, replace * with .* + const regexStr = + '^' + + pattern + .split('*') + .map((part) => part.replace(/[.+?^${}()|[\]\\]/g, '\\$&')) + .join('.*') + + '$'; + try { + return new RegExp(regexStr).test(value); + } catch { + return value === pattern; + } +} + /** * Escape special regex characters. */ @@ -1046,7 +1099,47 @@ export function matchesRule( rule.toolName.startsWith('mcp__') || canonicalCtxToolName.startsWith('mcp__') ) { - return matchesMcpPattern(rule.toolName, canonicalCtxToolName); + if (!matchesMcpPattern(rule.toolName, canonicalCtxToolName)) { + return false; + } + // MCP tools matched by name; now check param matchers if present + if (rule.toolParamMatchers?.length) { + if (!toolParams) { + debugLogger.debug( + `Param matcher rule "${rule.raw}" skipped: no toolParams`, + ); + return false; + } + for (const matcher of rule.toolParamMatchers) { + if (!Object.hasOwn(toolParams, matcher.key)) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=missing`, + ); + return false; + } + const actualValue = toolParams[matcher.key]; + if (actualValue === undefined || actualValue === null) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=null/undefined`, + ); + return false; + } + if (typeof actualValue !== 'string' && typeof actualValue !== 'number') { + debugLogger.debug( + `Param matcher skipped: rule="${rule.raw}" key=${matcher.key} value is ${typeof actualValue}`, + ); + return false; + } + const actualStr = String(actualValue); + if (!matchesParamValuePattern(matcher.valuePattern, actualStr)) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual="${actualStr}"`, + ); + return false; + } + } + } + return true; } // ── Standard tool name matching (with meta-category support) ───────── @@ -1115,15 +1208,36 @@ export function matchesRule( // ── Tool param matching (key:value syntax) ─────────────────────────── if (rule.toolParamMatchers?.length) { if (!toolParams) { + debugLogger.debug( + `Param matcher rule "${rule.raw}" skipped: no toolParams`, + ); return false; } for (const matcher of rule.toolParamMatchers) { + if (!Object.hasOwn(toolParams, matcher.key)) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=missing`, + ); + return false; + } const actualValue = toolParams[matcher.key]; if (actualValue === undefined || actualValue === null) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=null/undefined`, + ); + return false; + } + if (typeof actualValue !== 'string' && typeof actualValue !== 'number') { + debugLogger.debug( + `Param matcher skipped: rule="${rule.raw}" key=${matcher.key} value is ${typeof actualValue}`, + ); return false; } const actualStr = String(actualValue); - if (!matchesCommandPattern(matcher.valuePattern, actualStr)) { + if (!matchesParamValuePattern(matcher.valuePattern, actualStr)) { + debugLogger.debug( + `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual="${actualStr}"`, + ); return false; } } From e6b39f3840f09e1dc92831ddd9ce999e57af00ca Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Thu, 2 Jul 2026 17:16:29 +0800 Subject: [PATCH 3/5] fix(core): address critical review feedback on Tool(param:value) permission syntax - Add 's' flag to RegExp in matchesParamValuePattern for multiline support - Filter buildPermissionRules to stable param keys only (model, subagent_type, skill, server_name) to prevent sensitive data leakage - Reject MCP rules with unsupported specifiers instead of silently ignoring - Fix :* wildcard conversion to use global replace for backward compatibility - Extract shared evaluateParamMatchers helper to deduplicate MCP and standard branches --- packages/core/src/permissions/rule-parser.ts | 147 ++++++++++-------- .../core/src/services/gitWorktreeService.ts | 2 + 2 files changed, 80 insertions(+), 69 deletions(-) diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 26d79b83efd..ea305dcf714 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -289,8 +289,8 @@ export function parseRule(raw: string): PermissionRule { const specifierKind = rawSpecifier ? getSpecifierKind(canonicalName) : undefined; - if (specifierKind === 'command' && rawSpecifier.endsWith(':*')) { - rawSpecifier = rawSpecifier.slice(0, -2) + ' *'; + if (specifierKind === 'command') { + rawSpecifier = rawSpecifier.replace(/:(\*)/g, ' $1'); } // For literal specifier kind, extract `key:value` param matchers. @@ -504,10 +504,22 @@ export function buildPermissionRules(ctx: PermissionCheckContext): string[] { default: { const parts: string[] = []; if (ctx.specifier) parts.push(ctx.specifier); + // Only serialize stable, identity-bearing params — not volatile content + // like `prompt` or `query`, which would make rules invocation-specific + // and could leak sensitive data into settings.json. + const stableParamKeys = new Set([ + 'model', + 'subagent_type', + 'skill', + 'server_name', + ]); if (ctx.toolParams) { - for (const [k, v] of Object.entries(ctx.toolParams)) { + for (const key of stableParamKeys) { + const v = ctx.toolParams[key]; if (typeof v === 'string' || typeof v === 'number') { - parts.push(`${k}:${v}`); + // Skip values already represented by ctx.specifier + if (ctx.specifier && String(v) === ctx.specifier) continue; + parts.push(`${key}:${v}`); } } } @@ -814,12 +826,57 @@ function matchesParamValuePattern(pattern: string, value: string): boolean { .join('.*') + '$'; try { - return new RegExp(regexStr).test(value); + return new RegExp(regexStr, 's').test(value); } catch { return value === pattern; } } +/** + * Evaluate all param matchers against the given toolParams. + * Returns true if all matchers pass, false otherwise. + * Shared between MCP and standard matching branches. + */ +function evaluateParamMatchers( + matchers: Array<{ key: string; valuePattern: string }>, + toolParams: Record | undefined, + ruleRaw: string, +): boolean { + if (!toolParams) { + debugLogger.debug(`Param matcher rule "${ruleRaw}" skipped: no toolParams`); + return false; + } + for (const matcher of matchers) { + if (!Object.hasOwn(toolParams, matcher.key)) { + debugLogger.debug( + `Param matcher failed: rule="${ruleRaw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=missing`, + ); + return false; + } + const actualValue = toolParams[matcher.key]; + if (actualValue === undefined || actualValue === null) { + debugLogger.debug( + `Param matcher failed: rule="${ruleRaw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=null/undefined`, + ); + return false; + } + if (typeof actualValue !== 'string' && typeof actualValue !== 'number') { + debugLogger.debug( + `Param matcher skipped: rule="${ruleRaw}" key=${matcher.key} value is ${typeof actualValue}`, + ); + return false; + } + const actualStr = String(actualValue); + if (!matchesParamValuePattern(matcher.valuePattern, actualStr)) { + debugLogger.debug( + `Param matcher failed: rule="${ruleRaw}" key=${matcher.key} expected="${matcher.valuePattern}" actual="${actualStr}"`, + ); + return false; + } + } + return true; +} + /** * Escape special regex characters. */ @@ -1102,42 +1159,25 @@ export function matchesRule( if (!matchesMcpPattern(rule.toolName, canonicalCtxToolName)) { return false; } + + // MCP rules should not carry an unexpected specifier — the tool name + // already encodes server + tool identity. If a specifier was somehow + // parsed (e.g. user wrote `mcp__srv__tool(XXX)`), reject the match + // rather than silently ignoring the constraint. + if (rule.specifier) { + debugLogger.debug( + `MCP rule "${rule.raw}" has specifier "${rule.specifier}" — MCP tool names already encode server identity; specifier not supported`, + ); + return false; + } + // MCP tools matched by name; now check param matchers if present if (rule.toolParamMatchers?.length) { - if (!toolParams) { - debugLogger.debug( - `Param matcher rule "${rule.raw}" skipped: no toolParams`, - ); + if ( + !evaluateParamMatchers(rule.toolParamMatchers, toolParams, rule.raw) + ) { return false; } - for (const matcher of rule.toolParamMatchers) { - if (!Object.hasOwn(toolParams, matcher.key)) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=missing`, - ); - return false; - } - const actualValue = toolParams[matcher.key]; - if (actualValue === undefined || actualValue === null) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=null/undefined`, - ); - return false; - } - if (typeof actualValue !== 'string' && typeof actualValue !== 'number') { - debugLogger.debug( - `Param matcher skipped: rule="${rule.raw}" key=${matcher.key} value is ${typeof actualValue}`, - ); - return false; - } - const actualStr = String(actualValue); - if (!matchesParamValuePattern(matcher.valuePattern, actualStr)) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual="${actualStr}"`, - ); - return false; - } - } } return true; } @@ -1207,40 +1247,9 @@ export function matchesRule( // ── Tool param matching (key:value syntax) ─────────────────────────── if (rule.toolParamMatchers?.length) { - if (!toolParams) { - debugLogger.debug( - `Param matcher rule "${rule.raw}" skipped: no toolParams`, - ); + if (!evaluateParamMatchers(rule.toolParamMatchers, toolParams, rule.raw)) { return false; } - for (const matcher of rule.toolParamMatchers) { - if (!Object.hasOwn(toolParams, matcher.key)) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=missing`, - ); - return false; - } - const actualValue = toolParams[matcher.key]; - if (actualValue === undefined || actualValue === null) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual=null/undefined`, - ); - return false; - } - if (typeof actualValue !== 'string' && typeof actualValue !== 'number') { - debugLogger.debug( - `Param matcher skipped: rule="${rule.raw}" key=${matcher.key} value is ${typeof actualValue}`, - ); - return false; - } - const actualStr = String(actualValue); - if (!matchesParamValuePattern(matcher.valuePattern, actualStr)) { - debugLogger.debug( - `Param matcher failed: rule="${rule.raw}" key=${matcher.key} expected="${matcher.valuePattern}" actual="${actualStr}"`, - ); - return false; - } - } } return true; diff --git a/packages/core/src/services/gitWorktreeService.ts b/packages/core/src/services/gitWorktreeService.ts index 68aa2b8bd24..1d495fb0fe2 100644 --- a/packages/core/src/services/gitWorktreeService.ts +++ b/packages/core/src/services/gitWorktreeService.ts @@ -1558,6 +1558,8 @@ export class GitWorktreeService { if (!hooksPath) return; const worktreeGit = simpleGit(worktreePath, { + // @ts-expect-error — allowUnsafeHooksPath is supported at runtime but + // not yet declared in simple-git@3.28.0 type definitions. unsafe: { allowUnsafeHooksPath: true }, }); let existing = ''; From 89715c74c17846d5e70f62eabc2723761593071f Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Fri, 3 Jul 2026 10:16:27 +0800 Subject: [PATCH 4/5] fix(permissions): address remaining review feedback for Tool(param:value) syntax - Remove unused @ts-expect-error in gitWorktreeService.ts (CI blocker) - Fix ReDoS in matchesParamValuePattern: replace regex with linear-time glob matcher using indexOf, avoiding catastrophic backtracking on multi-wildcard patterns like *a*a*a*a*b - Fix MCP backward compatibility: exclude MCP tools from key:value parsing in parseRule and buildPermissionRules to preserve existing MCP deny rule semantics - Add tests for MCP + param matcher, partial wildcards, ReDoS prevention, number coercion, and buildPermissionRules with toolParams (stable params, volatile params, sensitive data, round-trip) --- .../permissions/permission-manager.test.ts | 203 ++++++++++++++++++ packages/core/src/permissions/rule-parser.ts | 63 ++++-- .../core/src/services/gitWorktreeService.ts | 2 - 3 files changed, 253 insertions(+), 15 deletions(-) diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index a0cae8d29c4..28fef163ed2 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -1079,6 +1079,125 @@ describe('matchesRule', () => { ), ).toBe(false); // specifier mismatch }); + + it('parseRule does not extract key:value for MCP tools (backward compat)', async () => { + const r = parseRule('mcp__server__tool(server_name:myserver)'); + expect(r.toolName).toBe('mcp__server__tool'); + expect(r.specifier).toBe('server_name:myserver'); + expect(r.toolParamMatchers).toBeUndefined(); + }); + + it('matchesRule supports partial wildcard patterns', async () => { + const rule = parseRule('Agent(model:op*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opus', + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'opera', + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + model: 'sonnet', + }, + ), + ).toBe(false); + }); + + it('matchesRule handles multi-wildcard patterns without ReDoS', async () => { + const rule = parseRule('Agent(prompt:*x*x*x*x*x*y)'); + // This should not hang (ReDoS) and should return false for non-matching input + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + prompt: 'a'.repeat(1000), + }, + ), + ).toBe(false); + // Should match when pattern is present + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + prompt: 'xaxaxaxaxay', + }, + ), + ).toBe(true); + }); + + it('matchesRule coerces number values to string for matching', async () => { + const rule = parseRule('Agent(count:42)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + count: 42, + }, + ), + ).toBe(true); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { + count: 43, + }, + ), + ).toBe(false); + }); }); // ─── PermissionManager ────────────────────────────────────────────────────── @@ -2367,6 +2486,90 @@ describe('buildPermissionRules', () => { expect(rules).toEqual(['mcp__puppeteer__navigate']); }); }); + + describe('with toolParams (stable param serialization)', () => { + it('serializes stable params (model, subagent_type) for Agent', async () => { + const rules = buildPermissionRules({ + toolName: 'agent', + specifier: 'coder', + toolParams: { + subagent_type: 'coder', + model: 'opus', + prompt: 'Fix the bug', + }, + }); + // prompt is not serialized (not in stableParamKeys) + // subagent_type is skipped because it matches specifier + expect(rules).toEqual(['Agent(coder,model:opus)']); + }); + + it('does not serialize volatile params like prompt or query', async () => { + const rules = buildPermissionRules({ + toolName: 'agent', + toolParams: { + model: 'sonnet', + prompt: 'Some long prompt that should not be persisted', + }, + }); + expect(rules).toEqual(['Agent(model:sonnet)']); + // Verify prompt is NOT in the rule string + expect(rules[0]).not.toContain('prompt'); + }); + + it('does not serialize sensitive params (no secret leakage)', async () => { + const rules = buildPermissionRules({ + toolName: 'agent', + toolParams: { + model: 'opus', + api_key: 'sk-secret-123', + token: 'bearer-xyz', + }, + }); + // api_key and token are not in stableParamKeys, so not serialized + expect(rules).toEqual(['Agent(model:opus)']); + expect(rules[0]).not.toContain('secret'); + expect(rules[0]).not.toContain('bearer'); + }); + + it('generates bare MCP tool name without specifier or params', async () => { + const rules = buildPermissionRules({ + toolName: 'mcp__chrome__navigate', + toolParams: { + server_name: 'chrome', + url: 'https://example.com', + }, + }); + // MCP tools get bare name — specifier rejection in matchesRule + // would make any specifier-carrying rule a dead entry + expect(rules).toEqual(['mcp__chrome__navigate']); + }); + + it('round-trips Agent rule with stable params through parseRule', async () => { + const rules = buildPermissionRules({ + toolName: 'agent', + specifier: 'coder', + toolParams: { subagent_type: 'coder', model: 'opus' }, + }); + expect(rules).toEqual(['Agent(coder,model:opus)']); + + // Parse the generated rule back + const parsed = parseRule(rules[0]!); + expect(parsed.toolName).toBe('agent'); + expect(parsed.specifier).toBe('coder'); + expect(parsed.toolParamMatchers).toEqual([ + { key: 'model', valuePattern: 'opus' }, + ]); + }); + + it('handles number values in toolParams', async () => { + const rules = buildPermissionRules({ + toolName: 'agent', + toolParams: { model: 'opus', count: 42 }, + }); + // count is not in stableParamKeys, so not serialized + expect(rules).toEqual(['Agent(model:opus)']); + }); + }); }); // ─── buildHumanReadableRuleLabel ───────────────────────────────────────────── diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index ea305dcf714..9c5b7b9fc6f 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -301,7 +301,11 @@ export function parseRule(raw: string): PermissionRule { | Array<{ key: string; valuePattern: string }> | undefined; - if (specifierKind === 'literal' && rawSpecifier.includes(':')) { + if ( + specifierKind === 'literal' && + !canonicalName.startsWith('mcp__') && + rawSpecifier.includes(':') + ) { const parts = rawSpecifier.split(',').map((p) => p.trim()); const plainParts: string[] = []; const matchers: Array<{ key: string; valuePattern: string }> = []; @@ -502,6 +506,14 @@ export function buildPermissionRules(ctx: PermissionCheckContext): string[] { case 'literal': default: { + // MCP tool names already encode server + tool identity. + // Don't add specifiers or params — matchesRule rejects MCP rules + // with specifiers, and existing MCP rules in user configs should + // remain backward compatible. + if (canonicalName.startsWith('mcp__')) { + return [displayName]; + } + const parts: string[] = []; if (ctx.specifier) parts.push(ctx.specifier); // Only serialize stable, identity-bearing params — not volatile content @@ -810,6 +822,11 @@ export function matchesCommandPattern( * shell-specific semantics (no prefix+space matching, no variable stripping). * Supports `*` as wildcard for any substring. */ +/** + * Match a glob pattern against a value using linear-time greedy matching. + * `*` matches any substring (including empty). No regex involved — avoids + * ReDoS risk from catastrophic backtracking on multi-wildcard patterns. + */ function matchesParamValuePattern(pattern: string, value: string): boolean { if (pattern === '*') { return true; @@ -817,19 +834,39 @@ function matchesParamValuePattern(pattern: string, value: string): boolean { if (!pattern.includes('*')) { return value === pattern; } - // Convert glob pattern to regex: escape special chars, replace * with .* - const regexStr = - '^' + - pattern - .split('*') - .map((part) => part.replace(/[.+?^${}()|[\]\\]/g, '\\$&')) - .join('.*') + - '$'; - try { - return new RegExp(regexStr, 's').test(value); - } catch { - return value === pattern; + + const segments = pattern.split('*'); + let pos = 0; + + // First segment must match at the start + if (segments[0]!.length > 0) { + if (!value.startsWith(segments[0]!)) { + return false; + } + pos = segments[0]!.length; + } + + // Middle segments: find next occurrence greedily + for (let i = 1; i < segments.length - 1; i++) { + const seg = segments[i]!; + if (seg.length === 0) continue; + const idx = value.indexOf(seg, pos); + if (idx === -1) { + return false; + } + pos = idx + seg.length; } + + // Last segment must match at the end + const last = segments[segments.length - 1]!; + if (last.length > 0) { + if (value.length - pos < last.length) { + return false; + } + return value.endsWith(last); + } + + return true; } /** diff --git a/packages/core/src/services/gitWorktreeService.ts b/packages/core/src/services/gitWorktreeService.ts index 1d495fb0fe2..68aa2b8bd24 100644 --- a/packages/core/src/services/gitWorktreeService.ts +++ b/packages/core/src/services/gitWorktreeService.ts @@ -1558,8 +1558,6 @@ export class GitWorktreeService { if (!hooksPath) return; const worktreeGit = simpleGit(worktreePath, { - // @ts-expect-error — allowUnsafeHooksPath is supported at runtime but - // not yet declared in simple-git@3.28.0 type definitions. unsafe: { allowUnsafeHooksPath: true }, }); let existing = ''; From 3b78d72f7f3f58181001173d30cfcef606d257c4 Mon Sep 17 00:00:00 2001 From: DennisYu07 <617072224@qq.com> Date: Mon, 6 Jul 2026 14:15:30 +0800 Subject: [PATCH 5/5] fix(permissions): address PR #6106 review comments and fix useStatusLine test timeout - Make matchesParamValuePattern case-insensitive to match matchesDomainPattern convention - Remove duplicate JSDoc block before matchesParamValuePattern - Remove dead server_name from stableParamKeys in buildPermissionRules - Add PermissionManager integration tests with toolParams (evaluate, findMatchingDenyRule, hasRelevantRules, hasMatchingAskRule) - Add type guard tests for evaluateParamMatchers (null, undefined, boolean, object) - Fix useStatusLine.test.ts timeout by stubbing cron-task exports in core mock --- .../cli/src/ui/hooks/useStatusLine.test.ts | 6 + .../permissions/permission-manager.test.ts | 202 ++++++++++++++++++ packages/core/src/permissions/rule-parser.ts | 29 ++- 3 files changed, 221 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/ui/hooks/useStatusLine.test.ts b/packages/cli/src/ui/hooks/useStatusLine.test.ts index 1bbd7fd86e7..c2e3713dbc3 100644 --- a/packages/cli/src/ui/hooks/useStatusLine.test.ts +++ b/packages/cli/src/ui/hooks/useStatusLine.test.ts @@ -95,6 +95,12 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { return { ...original, createDebugLogger: () => debugLogMock, + // Stub cron-task exports to avoid loading async-mutex under fake timers + readCronTasks: vi.fn(async () => []), + updateCronTasks: vi.fn(async () => undefined), + removeCronTasks: vi.fn(async () => undefined), + getCronFilePath: vi.fn(() => '/tmp/cron-tasks.json'), + generateCronTaskId: vi.fn(() => 'test-id'), }; }); diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index 28fef163ed2..9474737f056 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -3084,3 +3084,205 @@ describe('PermissionManager — compound shell write attribution', () => { ).toBe('deny'); }); }); + +// ─── PermissionManager integration tests with toolParams ───────────────────── + +describe('PermissionManager — toolParams end-to-end', () => { + it('evaluate respects allow rule with param matcher', async () => { + const pm = new PermissionManager( + makeConfig({ + permissionsAllow: ['Agent(coder,model:opus)'], + }), + ); + pm.initialize(); + + expect( + await pm.evaluate({ + toolName: 'agent', + specifier: 'coder', + toolParams: { subagent_type: 'coder', model: 'opus' }, + }), + ).toBe('allow'); + }); + + it('evaluate denies when param matcher does not match', async () => { + const pm = new PermissionManager( + makeConfig({ + permissionsAllow: ['Agent(coder,model:opus)'], + }), + ); + pm.initialize(); + + expect( + await pm.evaluate({ + toolName: 'agent', + specifier: 'coder', + toolParams: { subagent_type: 'coder', model: 'sonnet' }, + }), + ).not.toBe('allow'); + }); + + it('findMatchingDenyRule matches deny rule with param matcher', () => { + const pm = new PermissionManager( + makeConfig({ + permissionsDeny: ['Agent(model:restricted)'], + }), + ); + pm.initialize(); + + expect( + pm.findMatchingDenyRule({ + toolName: 'agent', + toolParams: { model: 'restricted' }, + }), + ).toBe('Agent(model:restricted)'); + }); + + it('findMatchingDenyRule returns undefined when param does not match', () => { + const pm = new PermissionManager( + makeConfig({ + permissionsDeny: ['Agent(model:restricted)'], + }), + ); + pm.initialize(); + + expect( + pm.findMatchingDenyRule({ + toolName: 'agent', + toolParams: { model: 'opus' }, + }), + ).toBeUndefined(); + }); + + it('hasRelevantRules returns true when param matcher rule exists', () => { + const pm = new PermissionManager( + makeConfig({ + permissionsAsk: ['Agent(model:opus)'], + }), + ); + pm.initialize(); + + expect( + pm.hasRelevantRules({ + toolName: 'agent', + toolParams: { model: 'opus' }, + }), + ).toBe(true); + }); + + it('hasMatchingAskRule returns true when param matcher ask rule matches', () => { + const pm = new PermissionManager( + makeConfig({ + permissionsAsk: ['Agent(model:opus)'], + }), + ); + pm.initialize(); + + expect( + pm.hasMatchingAskRule({ + toolName: 'agent', + toolParams: { model: 'opus' }, + }), + ).toBe(true); + }); + + it('case-insensitive param matching: deny rule blocks different casing', async () => { + const pm = new PermissionManager( + makeConfig({ + permissionsDeny: ['Agent(model:Sonnet)'], + }), + ); + pm.initialize(); + + expect( + await pm.evaluate({ + toolName: 'agent', + toolParams: { model: 'sonnet' }, + }), + ).toBe('deny'); + }); +}); + +// ─── evaluateParamMatchers type guard tests ────────────────────────────────── + +describe('matchesRule — param matcher type guards', () => { + it('rejects boolean param values', () => { + const rule = parseRule('Agent(model:*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { model: true }, + ), + ).toBe(false); + }); + + it('rejects null param values', () => { + const rule = parseRule('Agent(model:*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { model: null }, + ), + ).toBe(false); + }); + + it('rejects undefined param values', () => { + const rule = parseRule('Agent(model:*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { model: undefined }, + ), + ).toBe(false); + }); + + it('rejects object param values', () => { + const rule = parseRule('Agent(model:*)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { model: { nested: 'opus' } }, + ), + ).toBe(false); + }); + + it('accepts number param values via coercion', () => { + const rule = parseRule('Agent(count:42)'); + expect( + matchesRule( + rule, + 'agent', + undefined, + undefined, + undefined, + undefined, + undefined, + { count: 42 }, + ), + ).toBe(true); + }); +}); diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 9c5b7b9fc6f..532a0deb065 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -523,7 +523,6 @@ export function buildPermissionRules(ctx: PermissionCheckContext): string[] { 'model', 'subagent_type', 'skill', - 'server_name', ]); if (ctx.toolParams) { for (const key of stableParamKeys) { @@ -816,31 +815,29 @@ export function matchesCommandPattern( } } -/** - * Match a parameter value against a pattern. - * Unlike matchesCommandPattern, this uses simple glob matching without - * shell-specific semantics (no prefix+space matching, no variable stripping). - * Supports `*` as wildcard for any substring. - */ /** * Match a glob pattern against a value using linear-time greedy matching. - * `*` matches any substring (including empty). No regex involved — avoids + * `*` matches any substring (including empty). Case-insensitive to match + * the convention used by matchesDomainPattern. No regex involved — avoids * ReDoS risk from catastrophic backtracking on multi-wildcard patterns. */ function matchesParamValuePattern(pattern: string, value: string): boolean { - if (pattern === '*') { + const normalizedPattern = pattern.toLowerCase(); + const normalizedValue = value.toLowerCase(); + + if (normalizedPattern === '*') { return true; } - if (!pattern.includes('*')) { - return value === pattern; + if (!normalizedPattern.includes('*')) { + return normalizedValue === normalizedPattern; } - const segments = pattern.split('*'); + const segments = normalizedPattern.split('*'); let pos = 0; // First segment must match at the start if (segments[0]!.length > 0) { - if (!value.startsWith(segments[0]!)) { + if (!normalizedValue.startsWith(segments[0]!)) { return false; } pos = segments[0]!.length; @@ -850,7 +847,7 @@ function matchesParamValuePattern(pattern: string, value: string): boolean { for (let i = 1; i < segments.length - 1; i++) { const seg = segments[i]!; if (seg.length === 0) continue; - const idx = value.indexOf(seg, pos); + const idx = normalizedValue.indexOf(seg, pos); if (idx === -1) { return false; } @@ -860,10 +857,10 @@ function matchesParamValuePattern(pattern: string, value: string): boolean { // Last segment must match at the end const last = segments[segments.length - 1]!; if (last.length > 0) { - if (value.length - pos < last.length) { + if (normalizedValue.length - pos < last.length) { return false; } - return value.endsWith(last); + return normalizedValue.endsWith(last); } return true;