From e0efb3f066e0b717ffecf7bffd73796417aa0a37 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:00:13 -0700 Subject: [PATCH 1/2] Consolidate grant tool/providerModel/cwd scoping into one predicate evaluateApprovals, isRequestCoveredByGrant, and hasExactFullCommandGrant each reimplemented the same tool/providerModel/cwd scoping condition independently (src/permission/authz-grants.ts, src/permission/gate.ts), so a scoping-dimension change required editing three sites in lockstep. A fourth copy, matcher.ts's isApproved, was dead in production and only exercised by its own test, and lacked the specificity-ranking behavior evaluateApprovals gets from @intx/authz's evaluateGrants. All three live call sites now delegate to a single exported grantScopeMatches predicate. The dead isApproved matcher and its tests are removed. --- src/permission/authz-grants.ts | 31 ++++++++++---- src/permission/gate.ts | 17 ++------ src/permission/grant-scope.test.ts | 65 ++++++++++++++++++++++++++++++ src/permission/matcher.ts | 24 ----------- src/permission/permission.test.ts | 22 +--------- 5 files changed, 92 insertions(+), 67 deletions(-) create mode 100644 src/permission/grant-scope.test.ts diff --git a/src/permission/authz-grants.ts b/src/permission/authz-grants.ts index 985df5e03..6ebc4b05b 100644 --- a/src/permission/authz-grants.ts +++ b/src/permission/authz-grants.ts @@ -61,6 +61,26 @@ export function cwdMatchesGrant( return workspace.roots.includes(realpathOr(requestCwd)); } +// The single place that decides whether a grant's tool/providerModel/cwd +// scope covers a request, independent of whether the grant's pattern matches +// the request's subject. Every live call site that needs to know "does this +// grant cover this request's scope" — evaluateApprovals, isRequestCoveredByGrant, +// hasExactFullCommandGrant — delegates here so a scoping-dimension change +// never has to be made in more than one place. +export function grantScopeMatches( + approval: Approval, + tool: string, + activeProviderModel: string | undefined, + requestCwd: string | undefined, + workspace: GrantWorkspace, +): boolean { + return ( + approval.tool === tool && + (approval.providerModel === undefined || approval.providerModel === activeProviderModel) && + cwdMatchesGrant(approval.cwd, requestCwd, workspace) + ); +} + export type EvaluateApprovalsInput = { tool: string; subject: string; @@ -70,19 +90,14 @@ export type EvaluateApprovalsInput = { workspace: GrantWorkspace; }; -// Grant-store evaluation via @intx/authz. Filters provider-model and cwd the -// same way isApproved does, then asks evaluateGrants for the highest-specificity +// Grant-store evaluation via @intx/authz. Filters provider-model and cwd via +// grantScopeMatches, then asks evaluateGrants for the highest-specificity // allow among package-compatible grants. Exact-escaped grants are checked with // matchesPattern (equality after unescape) first so a stored exact command is // never lost. export async function evaluateApprovals(input: EvaluateApprovalsInput): Promise { const { tool, subject, approvals, activeProviderModel, requestCwd, workspace } = input; - const scoped = approvals.filter( - (a) => - a.tool === tool && - (a.providerModel === undefined || a.providerModel === activeProviderModel) && - cwdMatchesGrant(a.cwd, requestCwd, workspace), - ); + const scoped = approvals.filter((a) => grantScopeMatches(a, tool, activeProviderModel, requestCwd, workspace)); if (scoped.length === 0) return false; for (const a of scoped) { diff --git a/src/permission/gate.ts b/src/permission/gate.ts index ad8c1227e..ad154e30e 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -14,7 +14,7 @@ import { autoShellRuleForCall } from "./auto-shell-policy.js"; import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js"; import { matchesPattern, escapeGlobLiteral } from "./matcher.js"; -import { evaluateApprovals, cwdMatchesGrant, type GrantWorkspace } from "./authz-grants.js"; +import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js"; import { splitChainedCommand, tokenize, isShellCommentOnly, stripCommentLines } from "./command.js"; import { createPathRestriction } from "./path-restriction.js"; import { createWorktreeRootsProvider, type RootsProvider } from "./worktree-roots.js"; @@ -59,11 +59,7 @@ function hasExactFullCommandGrant( // storing a run_shell pattern). const normalized = stripCommentLines(fullCommand).trim(); return approvals.some( - (a) => - a.tool === tool && - a.pattern === normalized && - (a.providerModel === undefined || a.providerModel === activeProviderModel) && - cwdMatchesGrant(a.cwd, requestCwd, workspace), + (a) => a.pattern === normalized && grantScopeMatches(a, tool, activeProviderModel, requestCwd, workspace), ); } @@ -145,14 +141,7 @@ export function isRequestCoveredByGrant( isRestricted: (path: string, isWrite: boolean) => boolean, workspace: GrantWorkspace, ): boolean { - if (request.tool !== approval.tool) return false; - if (!cwdMatchesGrant(approval.cwd, request.cwd, workspace)) return false; - if ( - approval.providerModel !== undefined && - approval.providerModel !== activeProviderModel - ) { - return false; - } + if (!grantScopeMatches(approval, request.tool, activeProviderModel, request.cwd, workspace)) return false; if (request.tool !== "run_shell") { return matchesPattern(request.subject, approval.pattern); } diff --git a/src/permission/grant-scope.test.ts b/src/permission/grant-scope.test.ts new file mode 100644 index 000000000..1840f4e6f --- /dev/null +++ b/src/permission/grant-scope.test.ts @@ -0,0 +1,65 @@ +import { describe, test, expect } from "bun:test"; +import type { Approval, PermissionRequest } from "./types.js"; +import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js"; +import { isRequestCoveredByGrant } from "./gate.js"; + +// evaluateApprovals and isRequestCoveredByGrant each decide, independently, +// whether a grant's tool/providerModel/cwd scope covers a request. Both are +// expected to delegate to the same shared predicate (grantScopeMatches) +// rather than reimplementing the condition. This test drives the same +// grant+request pairs through all three and asserts they agree — a +// regression where one call site reimplements the check with subtly +// different semantics would fail here even if each function still "looks +// right" in isolation. +describe("grant tool/providerModel/cwd scoping agrees across call sites", () => { + const workspace: GrantWorkspace = { resolvedCwd: "/proj", roots: ["/proj"] }; + const noopRestricted = () => false; + + const grants: Approval[] = [ + { tool: "run_shell", pattern: "npm test" }, + { tool: "run_shell", pattern: "npm test", providerModel: "openai:gpt-5" }, + { tool: "run_shell", pattern: "npm test", cwd: "/proj" }, + { tool: "write_file", pattern: "npm test" }, + ]; + + const requests: Array<{ tool: string; cwd?: string | undefined; activeProviderModel?: string | undefined }> = [ + { tool: "run_shell", cwd: "/proj", activeProviderModel: "openai:gpt-5" }, + { tool: "run_shell", cwd: "/proj", activeProviderModel: "anthropic:opus" }, + { tool: "run_shell", cwd: "/other", activeProviderModel: "openai:gpt-5" }, + { tool: "run_shell", cwd: undefined, activeProviderModel: undefined }, + { tool: "write_file", cwd: "/proj", activeProviderModel: undefined }, + ]; + + for (const grant of grants) { + for (const req of requests) { + test(`grant ${JSON.stringify(grant)} vs request ${JSON.stringify(req)}`, async () => { + const expected = grantScopeMatches(grant, req.tool, req.activeProviderModel, req.cwd, workspace); + + const viaEvaluateApprovals = await evaluateApprovals({ + tool: req.tool, + subject: "npm test", + approvals: [grant], + activeProviderModel: req.activeProviderModel, + requestCwd: req.cwd, + workspace, + }); + + const request: PermissionRequest = { + tool: req.tool, + action: req.tool, + subject: "npm test", + scopes: [], + ...(req.cwd !== undefined ? { cwd: req.cwd } : {}), + }; + const viaGate = isRequestCoveredByGrant(request, grant, req.activeProviderModel, noopRestricted, workspace); + + // Both live call sites additionally require the pattern to match the + // subject, which is true for every case here ("npm test" grants an + // exact "npm test" subject), so a scope mismatch is the only thing + // that can make either disagree with the shared predicate. + expect(viaEvaluateApprovals).toBe(expected); + expect(viaGate).toBe(expected); + }); + } + } +}); diff --git a/src/permission/matcher.ts b/src/permission/matcher.ts index 56db797cd..9fc5b2fed 100644 --- a/src/permission/matcher.ts +++ b/src/permission/matcher.ts @@ -1,7 +1,5 @@ import { matchPattern } from "@intx/authz"; -import type { Approval } from "./types.js"; - // Exact-command grants (see escapeGlobLiteral) store a backslash before every // glob metacharacter so a command like `rm -rf build/*` never becomes the // wildcard `rm -rf build/*`. @intx/authz's matchPattern has no escape syntax — @@ -38,25 +36,3 @@ export function matchesPattern(subject: string, pattern: string): boolean { } return matchPattern(pattern, subject); } - -// True when any stored approval for this tool matches the subject. The subject -// is the shell command segment (run_shell) or the file path (write/edit). An -// approval bound to a `providerModel` only matches when `activeProviderModel` -// equals it, so a grant scoped to one model never leaks to another. An -// approval bound to a `cwd` (project-scoped) only matches when `requestCwd` -// equals it, so a project grant from one repo never leaks into another. -export function isApproved( - tool: string, - subject: string, - approvals: readonly Approval[], - activeProviderModel?: string, - requestCwd?: string, -): boolean { - return approvals.some( - (a) => - a.tool === tool && - matchesPattern(subject, a.pattern) && - (a.providerModel === undefined || a.providerModel === activeProviderModel) && - (a.cwd === undefined || a.cwd === requestCwd), - ); -} diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 4a546738a..e943eaf3d 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -12,7 +12,7 @@ import { isShellNoOp, stripCommentLines, } from "./command.js"; -import { matchesPattern, isApproved, escapeGlobLiteral } from "./matcher.js"; +import { matchesPattern, escapeGlobLiteral } from "./matcher.js"; import { evaluateApprovals } from "./authz-grants.js"; import { classifyTool, buildRequests, isAutoAllowedShellCall } from "./classify.js"; import { createPermissionGate } from "./gate.js"; @@ -241,19 +241,6 @@ describe("matchesPattern (@intx/authz + exact escapes)", () => { }); }); -describe("isApproved", () => { - const approvals: Approval[] = [ - { tool: "run_shell", pattern: "npm *" }, - { tool: "write_file", pattern: "src/*" }, - ]; - test("matches by tool and pattern", () => { - expect(isApproved("run_shell", "npm test", approvals)).toBe(true); - expect(isApproved("run_shell", "curl x", approvals)).toBe(false); - expect(isApproved("write_file", "src/a.ts", approvals)).toBe(true); - expect(isApproved("write_file", "lib/a.ts", approvals)).toBe(false); - }); -}); - describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { const approvals: Approval[] = [ { tool: "run_shell", pattern: "npm *" }, @@ -1884,13 +1871,6 @@ describe("scoped grants", () => { expect(routed[0]).toEqual({ tool: "run_shell", pattern: "npm *", providerModel: "openai:gpt-5" }); }); - test("a provider-model approval does not auto-allow under a different model", () => { - const approvals: Approval[] = [{ tool: "run_shell", pattern: "npm *", providerModel: "openai:gpt-5" }]; - expect(isApproved("run_shell", "npm test", approvals, "openai:gpt-5")).toBe(true); - expect(isApproved("run_shell", "npm test", approvals, "anthropic:opus")).toBe(false); - expect(isApproved("run_shell", "npm test", approvals, undefined)).toBe(false); - }); - test("a seeded provider-model approval auto-allows when the gate's model matches", async () => { let asked = 0; const gate = createPermissionGate({ From 028c438fcf98775339f888a8a0c42fd2c7327958 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:25:22 -0700 Subject: [PATCH 2/2] Cover hasExactFullCommandGrant's agreement with grantScopeMatches directly The other two live call sites (evaluateApprovals, isRequestCoveredByGrant) get a direct cross-check against grantScopeMatches; hasExactFullCommandGrant isn't exported, so it only had indirect coverage through evaluate()'s multi-segment replay path elsewhere in the suite. Drive that path directly with grants grantScopeMatches would refuse (wrong cwd, wrong providerModel) to confirm the replay never fires when the shared predicate says no. --- src/permission/grant-scope.test.ts | 54 +++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/permission/grant-scope.test.ts b/src/permission/grant-scope.test.ts index 1840f4e6f..66f642ab6 100644 --- a/src/permission/grant-scope.test.ts +++ b/src/permission/grant-scope.test.ts @@ -1,7 +1,8 @@ import { describe, test, expect } from "bun:test"; +import type { ToolCall } from "@intx/types/runtime"; import type { Approval, PermissionRequest } from "./types.js"; import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js"; -import { isRequestCoveredByGrant } from "./gate.js"; +import { createPermissionGate, isRequestCoveredByGrant } from "./gate.js"; // evaluateApprovals and isRequestCoveredByGrant each decide, independently, // whether a grant's tool/providerModel/cwd scope covers a request. Both are @@ -63,3 +64,54 @@ describe("grant tool/providerModel/cwd scoping agrees across call sites", () => } } }); + +// hasExactFullCommandGrant (gate.ts) is the third live call site grantScopeMatches +// unifies, but it is not exported — it only surfaces through the exact-full-command +// replay path inside evaluate(). This drives that path directly with grants that +// grantScopeMatches would refuse (wrong cwd, wrong providerModel) to confirm the +// replay never fires when the shared predicate says no, matching the coverage the +// other two call sites get above. +describe("hasExactFullCommandGrant agrees with grantScopeMatches", () => { + const full = "npm i && curl x"; + const shellCall = (command: string): ToolCall => ({ id: "c", name: "run_shell", arguments: { command } }); + + test("does not replay a grant scoped to a different cwd", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [{ tool: "run_shell", pattern: full, cwd: "/other-project" }], + requestApproval: async () => { asked++; return { allow: true }; }, + interactive: true, + skipPermissions: false, + }); + expect((await gate.evaluate(shellCall(full))).allowed).toBe(true); + // grantScopeMatches would refuse this grant (cwd mismatch), so the + // exact-full-command shortcut must not fire — the operator is still asked. + expect(asked).toBeGreaterThan(0); + }); + + test("does not replay a grant scoped to a different provider model", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [{ tool: "run_shell", pattern: full, providerModel: "openai:gpt-5" }], + providerName: "anthropic", + model: "opus", + requestApproval: async () => { asked++; return { allow: true }; }, + interactive: true, + skipPermissions: false, + }); + expect((await gate.evaluate(shellCall(full))).allowed).toBe(true); + expect(asked).toBeGreaterThan(0); + }); + + test("replays a grant whose scope grantScopeMatches accepts", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [{ tool: "run_shell", pattern: full }], + requestApproval: async () => { asked++; return { allow: true }; }, + interactive: true, + skipPermissions: false, + }); + expect((await gate.evaluate(shellCall(full))).allowed).toBe(true); + expect(asked).toBe(0); + }); +});