Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b39d2a0
feat(core): add Tool(param:value) permission syntax for parameter-lev…
DennisYu07 Jul 1, 2026
37a4b63
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 1, 2026
8e59413
fix(permissions): resolve PR #6106 review comments for tool param per…
DennisYu07 Jul 1, 2026
afe0a2f
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 1, 2026
c440099
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 1, 2026
378ff95
Merge branch 'main' into feat/tool-param-permission-syntax
yiliang114 Jul 1, 2026
287bb58
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 1, 2026
e6b39f3
fix(core): address critical review feedback on Tool(param:value) perm…
DennisYu07 Jul 2, 2026
52f0151
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 2, 2026
aade36e
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 2, 2026
75997d3
Merge branch 'main' into feat/tool-param-permission-syntax
wenshao Jul 2, 2026
b026ffa
Merge branch 'main' into feat/tool-param-permission-syntax
wenshao Jul 2, 2026
892e8be
Merge branch 'main' into feat/tool-param-permission-syntax
wenshao Jul 2, 2026
89715c7
fix(permissions): address remaining review feedback for Tool(param:va…
DennisYu07 Jul 3, 2026
0adb27c
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 3, 2026
1960cd8
Merge branch 'main' into feat/tool-param-permission-syntax
wenshao Jul 5, 2026
fdbb222
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 6, 2026
cda1c29
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 6, 2026
2fecd2c
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 6, 2026
3b78d72
fix(permissions): address PR #6106 review comments and fix useStatusL…
DennisYu07 Jul 6, 2026
d1c2645
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 6, 2026
ed40d65
Merge branch 'main' into feat/tool-param-permission-syntax
DennisYu07 Jul 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/core/permission-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}

// ─────────────────────────────────────────────────────────────────────────────
Expand Down
202 changes: 202 additions & 0 deletions packages/core/src/permissions/permission-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,208 @@ describe('matchesRule', () => {
expect(matchesRule(rule, 'mcp__chrome__navigate')).toBe(false);
expect(matchesRule(rule, 'mcp__other__use_browser')).toBe(false);
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Missing test coverage for the MCP + toolParamMatchers combination in matchesRule. The MCP branch (rule-parser.ts ~lines 1155-1183) was rewritten with three new sub-paths: specifier rejection, param matcher evaluation, and their combination — but no test exercises any of them through matchesRule.

Recommended additions:

  1. parseRule('mcp__server__tool(model:opus)')matchesRule with {model: 'opus'} should be true
  2. Same rule with {model: 'sonnet'} should be false
  3. Same rule with no toolParams should be false
  4. parseRule('mcp__server__tool(feature_x)') (bare specifier) → matchesRule should be false (specifier rejection)

— qwen3.7-max via Qwen Code /review

// ─── Tool(param:value) syntax ───────────────────────────────────────────────
Comment thread
DennisYu07 marked this conversation as resolved.

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 () => {
Comment thread
DennisYu07 marked this conversation as resolved.
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 ──────────────────────────────────────────────────────
Expand Down
16 changes: 12 additions & 4 deletions packages/core/src/permissions/permission-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -283,6 +284,7 @@ export class PermissionManager {
domain,
pathCtx,
specifier,
toolParams,
] as const;

// Compute the base decision from explicit Bash/file/domain rules.
Expand Down Expand Up @@ -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
Expand All @@ -635,6 +638,7 @@ export class PermissionManager {
domain,
pathCtx,
specifier,
toolParams,
] as const;

for (const rule of [
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -764,6 +769,7 @@ export class PermissionManager {
domain,
pathCtx,
specifier,
toolParams,
] as const;

return allRules.some((rule) => matchesRule(rule, ...matchArgs));
Expand All @@ -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
Expand Down Expand Up @@ -839,6 +846,7 @@ export class PermissionManager {
domain,
pathCtx,
specifier,
toolParams,
] as const;

return askRules.some((rule) => matchesRule(rule, ...matchArgs));
Expand Down
Loading
Loading