From 71081a62fd8b0e3fcd0529f09f1bdc89e8823a48 Mon Sep 17 00:00:00 2001 From: "Michal J. Gajda" Date: Mon, 20 Jul 2026 11:44:35 +0200 Subject: [PATCH 1/4] permission: use most-specific-pattern-wins instead of last-match-wins Replace findLast() in evaluate() with a specificity-based algorithm: most specific pattern (longest literal prefix before first wildcard) wins over less specific patterns, regardless of insertion order. Fixes the problem where a broad deny like /home/m/* at the end of a ruleset silently overrides more specific allows like /home/m/Projects/tools/paper-making-system/* that appear earlier. - Add specificity() helper: counts literal chars before first * or ? - V1 evaluate() and disabled(): use specificity-based selection - V2 evaluate(): same change for the core permission system - Tests: update names and expectations to match new semantics --- packages/core/src/permission.ts | 30 +++++++++---- packages/opencode/src/permission/index.ts | 44 ++++++++++++++----- .../opencode/test/permission-task.test.ts | 2 +- .../opencode/test/permission/next.test.ts | 19 ++++---- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index 3f28632a034d..80ddfc34831f 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -73,16 +73,30 @@ export class NotFoundError extends Schema.TaggedErrorClass()("Per export type Error = BlockedError | CorrectedError +function specificity(s: string): number { + const idx = s.search(/[*?]/) + return idx === -1 ? s.length : idx +} + export function evaluate(action: string, resource: string, ...rulesets: Permission.Ruleset[]): Permission.Rule { - return ( - rulesets - .flat() - .findLast((rule) => Wildcard.match(action, rule.action) && Wildcard.match(resource, rule.resource)) ?? { - action, - resource: "*", - effect: "ask", + const all = rulesets.flat() + let best: Permission.Rule | undefined + let bestActionSpec = -1 + let bestResSpec = -1 + + for (const rule of all) { + if (Wildcard.match(action, rule.action) && Wildcard.match(resource, rule.resource)) { + const actionSpec = specificity(rule.action) + const resSpec = specificity(rule.resource) + if (best === undefined || actionSpec > bestActionSpec || (actionSpec === bestActionSpec && resSpec > bestResSpec) || (actionSpec === bestActionSpec && resSpec === bestResSpec)) { + best = rule + bestActionSpec = actionSpec + bestResSpec = resSpec + } } - ) + } + + return best ?? { action, resource: "*", effect: "ask" } } export function merge(...rulesets: Permission.Ruleset[]): Permission.Ruleset { diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 2e27ff2424db..9f08b901d88d 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -25,16 +25,30 @@ interface State { approved: PermissionV1.Rule[] } +function specificity(s: string): number { + const idx = s.search(/[*?]/) + return idx === -1 ? s.length : idx +} + export function evaluate(permission: string, pattern: string, ...rulesets: PermissionV1.Ruleset[]): PermissionV1.Rule { - return ( - rulesets - .flat() - .findLast((rule) => Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern)) ?? { - action: "ask", - permission, - pattern: "*", + const all = rulesets.flat() + let best: PermissionV1.Rule | undefined + let bestPermSpec = -1 + let bestPatSpec = -1 + + for (const rule of all) { + if (Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern)) { + const permSpec = specificity(rule.permission) + const patSpec = specificity(rule.pattern) + if (best === undefined || permSpec > bestPermSpec || (permSpec === bestPermSpec && patSpec > bestPatSpec) || (permSpec === bestPermSpec && patSpec === bestPatSpec)) { + best = rule + bestPermSpec = permSpec + bestPatSpec = patSpec + } } - ) + } + + return best ?? { action: "ask", permission, pattern: "*" } } export class Service extends Context.Service()("@opencode/Permission") {} @@ -207,8 +221,18 @@ export function disabled(tools: string[], ruleset: PermissionV1.Ruleset): Set { const permission = edits.includes(tool) ? "edit" : reads.includes(tool) ? "read" : tool - const rule = ruleset.findLast((rule) => Wildcard.match(permission, rule.permission)) - return rule?.pattern === "*" && rule.action === "deny" + let best: PermissionV1.Rule | undefined + let bestSpec = -1 + for (const rule of ruleset) { + if (Wildcard.match(permission, rule.permission)) { + const spec = specificity(rule.permission) + if (best === undefined || spec > bestSpec || (spec === bestSpec)) { + best = rule + bestSpec = spec + } + } + } + return best?.pattern === "*" && best.action === "deny" }), ) } diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 09a71465d5c7..95f48daa5709 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -57,7 +57,7 @@ describe("Permission.evaluate for permission.task", () => { expect(Permission.evaluate("task", "code-reviewer", globalRuleset).action).toBe("ask") }) - test("later rules take precedence (last match wins)", () => { + test("more specific pattern beats broader deny", () => { const ruleset = createRuleset({ "orchestrator-*": "deny", "orchestrator-fast": "allow", diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 2f2ce98efb34..d79a97b9ddc8 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -132,15 +132,16 @@ test("fromConfig - does not expand tilde in middle of path", () => { // last matching rule, so later config entries intentionally override earlier // entries even when a wildcard appears after a specific permission. -test("fromConfig - preserves top-level config key order", () => { +test("fromConfig - specific permission beats wildcard regardless of order", () => { const wildcardFirst = Permission.fromConfig({ "*": "deny", bash: "allow" }) const specificFirst = Permission.fromConfig({ bash: "allow", "*": "deny" }) expect(wildcardFirst.map((r) => r.permission)).toEqual(["*", "bash"]) expect(specificFirst.map((r) => r.permission)).toEqual(["bash", "*"]) + // Specific permission "bash" (spec 4) beats wildcard "*" (spec 0) expect(Permission.evaluate("bash", "ls", wildcardFirst).action).toBe("allow") - expect(Permission.evaluate("bash", "ls", specificFirst).action).toBe("deny") + expect(Permission.evaluate("bash", "ls", specificFirst).action).toBe("allow") }) test("fromConfig - wildcard acts as fallback when it appears before specifics", () => { @@ -159,7 +160,7 @@ test("fromConfig - top-level ordering is not sorted by wildcard specificity", () expect(ruleset.map((r) => r.permission)).toEqual(["bash", "*", "edit", "mcp_*"]) }) -test("fromConfig - sub-pattern insertion order inside a tool key is preserved", () => { +test("fromConfig - more specific sub-pattern beats broader deny", () => { const ruleset = Permission.fromConfig({ bash: { "*": "deny", "git *": "allow" } }) expect(ruleset.map((r) => r.pattern)).toEqual(["*", "git *"]) expect(Permission.evaluate("bash", "rm foo", ruleset).action).toBe("deny") @@ -295,12 +296,12 @@ test("evaluate - last matching rule wins", () => { expect(result.action).toBe("deny") }) -test("evaluate - last matching rule wins (wildcard after specific)", () => { +test("evaluate - more specific pattern wins over less specific", () => { const result = Permission.evaluate("bash", "rm", [ { permission: "bash", pattern: "rm", action: "deny" }, { permission: "bash", pattern: "*", action: "allow" }, ]) - expect(result.action).toBe("allow") + expect(result.action).toBe("deny") }) test("evaluate - glob pattern match", () => { @@ -316,12 +317,12 @@ test("evaluate - last matching glob wins", () => { expect(result.action).toBe("allow") }) -test("evaluate - order matters for specificity", () => { +test("evaluate - more specific path wins over broader deny", () => { const result = Permission.evaluate("edit", "src/components/Button.tsx", [ { permission: "edit", pattern: "src/components/*", action: "allow" }, { permission: "edit", pattern: "src/*", action: "deny" }, ]) - expect(result.action).toBe("deny") + expect(result.action).toBe("allow") }) test("evaluate - unknown permission returns ask", () => { @@ -372,12 +373,12 @@ test("evaluate - exact match at end wins over earlier wildcard", () => { expect(result.action).toBe("deny") }) -test("evaluate - wildcard at end overrides earlier exact match", () => { +test("evaluate - exact match beats wildcard regardless of order", () => { const result = Permission.evaluate("bash", "/bin/rm", [ { permission: "bash", pattern: "/bin/rm", action: "deny" }, { permission: "bash", pattern: "*", action: "allow" }, ]) - expect(result.action).toBe("allow") + expect(result.action).toBe("deny") }) // wildcard permission tests From a54a353d9f13a8f1778fbbacdd87ca0c5cb55590 Mon Sep 17 00:00:00 2001 From: "Michal J. Gajda" Date: Tue, 21 Jul 2026 17:53:08 +0200 Subject: [PATCH 2/4] fix(permission): add pattern specificity to disabled(), add V2 evaluate specificity tests - disabled() now considers pattern specificity (not just permission specificity), so specific patterns keep a tool visible even when a wildcard deny exists for the same permission - Fix 2 tests that weren't updated to specificity semantics: - next.test.ts: wildcard permission no longer overrides specific - permission-task.test.ts: disabled() with specific patterns is false - Add 10 direct evaluate() specificity tests for V2 in core --- packages/core/test/permission.test.ts | 67 ++++++++++++++++++- packages/opencode/src/permission/index.ts | 11 +-- .../opencode/test/permission-task.test.ts | 48 ++++++------- .../opencode/test/permission/next.test.ts | 4 +- 4 files changed, 95 insertions(+), 35 deletions(-) diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index df87c1d0efd6..b97888b278bd 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -1,4 +1,4 @@ -import { describe, expect } from "bun:test" +import { describe, expect, test } from "bun:test" import { Cause, Deferred, Effect, Fiber, Layer } from "effect" import { AgentV2 } from "@opencode-ai/core/agent" import { Database } from "@opencode-ai/core/database/database" @@ -102,6 +102,71 @@ function waitForRequest() { }) } +describe("evaluate specificity", () => { + test("exact match", () => { + expect(PermissionV2.evaluate("bash", "rm", [{ action: "bash", resource: "rm", effect: "deny" }]).effect).toBe("deny") + }) + + test("wildcard match", () => { + expect(PermissionV2.evaluate("bash", "rm", [{ action: "bash", resource: "*", effect: "allow" }]).effect).toBe("allow") + }) + + test("no match returns ask", () => { + expect(PermissionV2.evaluate("bash", "rm", [{ action: "edit", resource: "*", effect: "allow" }]).effect).toBe("ask") + }) + + test("empty ruleset returns ask", () => { + expect(PermissionV2.evaluate("bash", "rm", []).effect).toBe("ask") + }) + + test("more specific action beats less specific", () => { + const result = PermissionV2.evaluate("bash", "rm", [ + { action: "bash", resource: "*", effect: "allow" }, + { action: "*", resource: "*", effect: "deny" }, + ]) + expect(result.effect).toBe("allow") + }) + + test("more specific resource beats less specific", () => { + const result = PermissionV2.evaluate("bash", "rm", [ + { action: "bash", resource: "rm", effect: "deny" }, + { action: "bash", resource: "*", effect: "allow" }, + ]) + expect(result.effect).toBe("deny") + }) + + test("exact resource beats wildcard regardless of order", () => { + const result = PermissionV2.evaluate("bash", "/bin/rm", [ + { action: "bash", resource: "/bin/rm", effect: "deny" }, + { action: "bash", resource: "*", effect: "allow" }, + ]) + expect(result.effect).toBe("deny") + }) + + test("same specificity picks last (tiebreaker)", () => { + const result = PermissionV2.evaluate("bash", "rm", [ + { action: "bash", resource: "rm", effect: "deny" }, + { action: "bash", resource: "rm", effect: "allow" }, + ]) + expect(result.effect).toBe("allow") + }) + + test("glob resource specificity", () => { + const result = PermissionV2.evaluate("edit", "src/components/Button.tsx", [ + { action: "edit", resource: "src/components/*", effect: "allow" }, + { action: "edit", resource: "src/*", effect: "deny" }, + ]) + expect(result.effect).toBe("allow") + }) + + test("action wildcard with specific resource", () => { + const result = PermissionV2.evaluate("bash", "rm", [ + { action: "*", resource: "rm", effect: "deny" }, + ]) + expect(result.effect).toBe("deny") + }) +}) + describe("PermissionV2", () => { it.effect("returns the evaluated effect and only queues prompts", () => Effect.gen(function* () { diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 9f08b901d88d..3458c220f1c8 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -222,13 +222,16 @@ export function disabled(tools: string[], ruleset: PermissionV1.Ruleset): Set { const permission = edits.includes(tool) ? "edit" : reads.includes(tool) ? "read" : tool let best: PermissionV1.Rule | undefined - let bestSpec = -1 + let bestPermSpec = -1 + let bestPatSpec = -1 for (const rule of ruleset) { if (Wildcard.match(permission, rule.permission)) { - const spec = specificity(rule.permission) - if (best === undefined || spec > bestSpec || (spec === bestSpec)) { + const permSpec = specificity(rule.permission) + const patSpec = specificity(rule.pattern) + if (best === undefined || permSpec > bestPermSpec || (permSpec === bestPermSpec && patSpec > bestPatSpec) || (permSpec === bestPermSpec && patSpec === bestPatSpec)) { best = rule - bestSpec = spec + bestPermSpec = permSpec + bestPatSpec = patSpec } } } diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 95f48daa5709..40751ef50b04 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -84,26 +84,25 @@ describe("Permission.disabled for task tool", () => { action, })) - test("task tool is disabled when global deny pattern exists (even with specific allows)", () => { - // When "*": "deny" exists, the task tool is disabled because the disabled() function - // only checks for wildcard deny patterns - it doesn't consider that specific subagents might be allowed + test("task tool is NOT disabled when specific allow overrides global deny", () => { + // Even though "*": "deny" exists, the specific "orchestrator-*": "allow" + // has higher pattern specificity, so the tool is not disabled const ruleset = createRuleset({ "orchestrator-*": "allow", "*": "deny", }) const disabled = Permission.disabled(["task", "bash", "read"], ruleset) - // The task tool IS disabled because there's a pattern: "*" with action: "deny" - expect(disabled.has("task")).toBe(true) + expect(disabled.has("task")).toBe(false) }) - test("task tool is disabled when global deny pattern exists (even with ask overrides)", () => { + test("task tool is NOT disabled when specific ask overrides global deny", () => { const ruleset = createRuleset({ "orchestrator-*": "ask", "*": "deny", }) const disabled = Permission.disabled(["task"], ruleset) - // The task tool IS disabled because there's a pattern: "*" with action: "deny" - expect(disabled.has("task")).toBe(true) + // Specific pattern "orchestrator-*" beats "*" due to pattern specificity + expect(disabled.has("task")).toBe(false) }) test("task tool is disabled when global deny pattern exists", () => { @@ -129,16 +128,13 @@ describe("Permission.disabled for task tool", () => { expect(disabled.has("task")).toBe(false) }) - test("task tool is NOT disabled when last wildcard pattern is allow", () => { - // Last matching rule wins - if wildcard allow comes after wildcard deny, tool is enabled + test("task tool is NOT disabled when specific allow overrides wildcard deny", () => { const ruleset = createRuleset({ "*": "deny", "orchestrator-coder": "allow", }) const disabled = Permission.disabled(["task"], ruleset) - // The disabled() function uses findLast and checks if the last matching rule - // has pattern: "*" and action: "deny". In this case, the last rule matching - // "task" permission has pattern "orchestrator-coder", not "*", so not disabled + // "orchestrator-coder" has higher pattern specificity than "*", so it wins expect(disabled.has("task")).toBe(false) }) }) @@ -236,8 +232,7 @@ describe("permission.task with real config files", () => { const disabled = Permission.disabled(["bash", "edit", "task"], ruleset) expect(disabled.has("bash")).toBe(false) expect(disabled.has("edit")).toBe(false) - // task is NOT disabled because disabled() uses findLast, and the last rule - // matching "task" permission is {pattern: "general", action: "allow"}, not pattern: "*" + // task is NOT disabled because "general" (spec 7) has higher pattern specificity than "*" (spec 0) expect(disabled.has("task")).toBe(false) }), { @@ -256,21 +251,20 @@ describe("permission.task with real config files", () => { ) it.instance( - "task tool disabled when global deny comes last in config", + "task tool NOT disabled when specific patterns are allowed despite wildcard deny", () => Effect.gen(function* () { const config = yield* load const ruleset = Permission.fromConfig(config.permission ?? {}) - // Last matching rule wins - "*" deny is last, so all agents are denied - expect(Permission.evaluate("task", "general", ruleset).action).toBe("deny") - expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny") + // Specific patterns "general" (spec 7) and "code-reviewer" (spec 13) beat "*" (spec 0) + expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") + expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "unknown", ruleset).action).toBe("deny") - // Since "*": "deny" is the last rule, disabled() finds it with findLast - // and sees pattern: "*" with action: "deny", so task is disabled + // The most specific pattern wins — "general" or "code-reviewer" beats "*" const disabled = Permission.disabled(["task"], ruleset) - expect(disabled.has("task")).toBe(true) + expect(disabled.has("task")).toBe(false) }), { git: true, @@ -287,20 +281,18 @@ describe("permission.task with real config files", () => { ) it.instance( - "task tool NOT disabled when specific allow comes last in config", + "task tool NOT disabled when specific allow has higher pattern specificity", () => Effect.gen(function* () { const config = yield* load const ruleset = Permission.fromConfig(config.permission ?? {}) - // Evaluate uses findLast - "general" allow comes after "*" deny + // "general" (spec 7) beats "*" (spec 0) due to higher pattern specificity expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") - // Other agents still denied by the earlier "*" deny + // Other agents without specific allow are denied expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny") - // disabled() uses findLast and checks if the last rule has pattern: "*" with action: "deny" - // In this case, the last rule is {pattern: "general", action: "allow"}, not pattern: "*" - // So the task tool is NOT disabled (even though most subagents are denied) + // "general" (spec 7) beats "*" (spec 0) in pattern specificity too const disabled = Permission.disabled(["task"], ruleset) expect(disabled.has("task")).toBe(false) }), diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index d79a97b9ddc8..d9ec46aef0f5 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -433,12 +433,12 @@ test("evaluate - wildcard permission fallback for unknown tool", () => { expect(result.action).toBe("ask") }) -test("evaluate - later wildcard permission can override earlier specific permission", () => { +test("evaluate - specific permission beats wildcard permission regardless of order", () => { const result = Permission.evaluate("bash", "rm", [ { permission: "bash", pattern: "*", action: "allow" }, { permission: "*", pattern: "*", action: "deny" }, ]) - expect(result.action).toBe("deny") + expect(result.action).toBe("allow") }) test("evaluate - merges multiple rulesets", () => { From f0d92c90a3054f1a212d16158c1e5b7c3282e126 Mon Sep 17 00:00:00 2001 From: "Michal J. Gajda" Date: Tue, 21 Jul 2026 18:05:25 +0200 Subject: [PATCH 3/4] fix(permission): fix whollyDisabled() in V2 registry to use specificity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit whollyDisabled() in tool/registry.ts had the same findLast bug — a specific allow like question.private was ignored when *: deny appeared later. Now uses the same action+resource specificity algorithm as evaluate(). Also rename remaining tests/comments that still referenced 'last matching rule' semantics to reflect specificity-based behavior. --- packages/core/src/tool/registry.ts | 22 +++++++++++++++++-- .../test/session-runner-tool-registry.test.ts | 2 +- .../opencode/test/permission/next.test.ts | 12 +++++----- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index 1c2dfe7ab459..ae83169d2140 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -129,9 +129,27 @@ const layer = Layer.effect( Service.use((registry) => Effect.succeed(Tools.Service.of({ register: registry.register }))), ).pipe(Layer.provideMerge(registryLayer)) +function specificity(s: string): number { + const idx = s.search(/[*?]/) + return idx === -1 ? s.length : idx +} + function whollyDisabled(action: string, rules: PermissionV2.Ruleset) { - const rule = rules.findLast((rule) => Wildcard.match(action, rule.action)) - return rule?.resource === "*" && rule.effect === "deny" + let best: PermissionV2.Rule | undefined + let bestActionSpec = -1 + let bestResSpec = -1 + for (const rule of rules) { + if (Wildcard.match(action, rule.action)) { + const actionSpec = specificity(rule.action) + const resSpec = specificity(rule.resource) + if (best === undefined || actionSpec > bestActionSpec || (actionSpec === bestActionSpec && resSpec > bestResSpec) || (actionSpec === bestActionSpec && resSpec === bestResSpec)) { + best = rule + bestActionSpec = actionSpec + bestResSpec = resSpec + } + } + } + return best?.resource === "*" && best.effect === "deny" } export const node = makeLocationNode({ diff --git a/packages/core/test/session-runner-tool-registry.test.ts b/packages/core/test/session-runner-tool-registry.test.ts index 82cd015aaf68..28fe191ebe06 100644 --- a/packages/core/test/session-runner-tool-registry.test.ts +++ b/packages/core/test/session-runner-tool-registry.test.ts @@ -89,7 +89,7 @@ describe("ToolRegistry", () => { { action: "question", resource: "private", effect: "allow" }, { action: "*", resource: "*", effect: "deny" }, ]), - ).toEqual([]) + ).toEqual(["question"]) expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual(["question", "bash"]) }), ) diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index d9ec46aef0f5..eb07c963b720 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -128,9 +128,9 @@ test("fromConfig - does not expand tilde in middle of path", () => { expect(result).toEqual([{ permission: "external_directory", pattern: "/some/~/path", action: "allow" }]) }) -// Permission precedence follows config insertion order. `evaluate()` uses the -// last matching rule, so later config entries intentionally override earlier -// entries even when a wildcard appears after a specific permission. +// Permission precedence follows specificity: more specific patterns win over +// less specific ones regardless of insertion order. Ties broken by position +// (later wins). test("fromConfig - specific permission beats wildcard regardless of order", () => { const wildcardFirst = Permission.fromConfig({ "*": "deny", bash: "allow" }) @@ -288,7 +288,7 @@ test("evaluate - wildcard pattern match", () => { expect(result.action).toBe("allow") }) -test("evaluate - last matching rule wins", () => { +test("evaluate - specific deny beats broad allow at end", () => { const result = Permission.evaluate("bash", "rm", [ { permission: "bash", pattern: "*", action: "allow" }, { permission: "bash", pattern: "rm", action: "deny" }, @@ -296,7 +296,7 @@ test("evaluate - last matching rule wins", () => { expect(result.action).toBe("deny") }) -test("evaluate - more specific pattern wins over less specific", () => { +test("evaluate - specific deny beats broad allow at start", () => { const result = Permission.evaluate("bash", "rm", [ { permission: "bash", pattern: "rm", action: "deny" }, { permission: "bash", pattern: "*", action: "allow" }, @@ -309,7 +309,7 @@ test("evaluate - glob pattern match", () => { expect(result.action).toBe("allow") }) -test("evaluate - last matching glob wins", () => { +test("evaluate - more specific glob beats broader", () => { const result = Permission.evaluate("edit", "src/components/Button.tsx", [ { permission: "edit", pattern: "src/*", action: "deny" }, { permission: "edit", pattern: "src/components/*", action: "allow" }, From fbe219b5741c540cb2e52823d8bceca7438c15cd Mon Sep 17 00:00:00 2001 From: "Michal J. Gajda" Date: Tue, 21 Jul 2026 22:22:18 +0200 Subject: [PATCH 4/4] fix(policy): use specificity instead of findLast in evaluate() Policy.evaluate() had the same specificity-blind bug as permission evaluate(): findLast picked the last matching statement regardless of specificity. Changed to the same action+resource specificity algorithm used in permission evaluate(). Add test for specific-before-wildcard ordering that would have failed with findLast. --- packages/core/src/policy.ts | 25 ++++++++++++++++++++----- packages/core/test/policy.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/core/src/policy.ts b/packages/core/src/policy.ts index a2adebb54e4e..f86bc5828e63 100644 --- a/packages/core/src/policy.ts +++ b/packages/core/src/policy.ts @@ -28,17 +28,32 @@ const layer = Layer.effect( let statements: Info[] = [] yield* Location.Service + function specificity(s: string): number { + const idx = s.search(/[*?]/) + return idx === -1 ? s.length : idx + } + return Service.of({ load: EffectRuntime.fn("Policy.load")(function* (input) { statements = input }), hasStatements: () => statements.length > 0, evaluate: EffectRuntime.fn("Policy.evaluate")(function* (action, resource, fallback) { - return ( - statements.findLast( - (statement) => Wildcard.match(action, statement.action) && Wildcard.match(resource, statement.resource), - )?.effect ?? fallback - ) + let best: Info | undefined + let bestActionSpec = -1 + let bestResSpec = -1 + for (const statement of statements) { + if (Wildcard.match(action, statement.action) && Wildcard.match(resource, statement.resource)) { + const actionSpec = specificity(statement.action) + const resSpec = specificity(statement.resource) + if (best === undefined || actionSpec > bestActionSpec || (actionSpec === bestActionSpec && resSpec > bestResSpec) || (actionSpec === bestActionSpec && resSpec === bestResSpec)) { + best = statement + bestActionSpec = actionSpec + bestResSpec = resSpec + } + } + } + return best?.effect ?? fallback }), }) }), diff --git a/packages/core/test/policy.test.ts b/packages/core/test/policy.test.ts index 1428c1b830fa..062a8ae80a0c 100644 --- a/packages/core/test/policy.test.ts +++ b/packages/core/test/policy.test.ts @@ -47,6 +47,28 @@ describe("Policy", () => { }), ) + it.effect("more specific action beats less specific regardless of order", () => + Effect.gen(function* () { + const policy = yield* Policy.Service + yield* policy.load([ + new Policy.Info({ + effect: "allow", + action: "provider.use", + resource: "anthropic", + }), + new Policy.Info({ + effect: "deny", + action: "provider.*", + resource: "*", + }), + ]) + + // "provider.use" (spec 12) beats "provider.*" (spec 9), so allow wins + expect(yield* policy.evaluate("provider.use", "anthropic", "deny")).toBe("allow") + expect(yield* policy.evaluate("provider.use", "openai", "deny")).toBe("deny") + }), + ) + it.effect("matches action and resource independently", () => Effect.gen(function* () { const policy = yield* Policy.Service