Skip to content

Commit 093408a

Browse files
Merge pull request #402 from corbitsdev/cl-5676-grant-scoping-filter-is-duplicated-across-three-matchers-one
Consolidate grant tool/providerModel/cwd scoping into one predicate
2 parents dbfb4e7 + 028c438 commit 093408a

5 files changed

Lines changed: 144 additions & 67 deletions

File tree

src/permission/authz-grants.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,26 @@ export function cwdMatchesGrant(
6161
return workspace.roots.includes(realpathOr(requestCwd));
6262
}
6363

64+
// The single place that decides whether a grant's tool/providerModel/cwd
65+
// scope covers a request, independent of whether the grant's pattern matches
66+
// the request's subject. Every live call site that needs to know "does this
67+
// grant cover this request's scope" — evaluateApprovals, isRequestCoveredByGrant,
68+
// hasExactFullCommandGrant — delegates here so a scoping-dimension change
69+
// never has to be made in more than one place.
70+
export function grantScopeMatches(
71+
approval: Approval,
72+
tool: string,
73+
activeProviderModel: string | undefined,
74+
requestCwd: string | undefined,
75+
workspace: GrantWorkspace,
76+
): boolean {
77+
return (
78+
approval.tool === tool &&
79+
(approval.providerModel === undefined || approval.providerModel === activeProviderModel) &&
80+
cwdMatchesGrant(approval.cwd, requestCwd, workspace)
81+
);
82+
}
83+
6484
export type EvaluateApprovalsInput = {
6585
tool: string;
6686
subject: string;
@@ -70,19 +90,14 @@ export type EvaluateApprovalsInput = {
7090
workspace: GrantWorkspace;
7191
};
7292

73-
// Grant-store evaluation via @intx/authz. Filters provider-model and cwd the
74-
// same way isApproved does, then asks evaluateGrants for the highest-specificity
93+
// Grant-store evaluation via @intx/authz. Filters provider-model and cwd via
94+
// grantScopeMatches, then asks evaluateGrants for the highest-specificity
7595
// allow among package-compatible grants. Exact-escaped grants are checked with
7696
// matchesPattern (equality after unescape) first so a stored exact command is
7797
// never lost.
7898
export async function evaluateApprovals(input: EvaluateApprovalsInput): Promise<boolean> {
7999
const { tool, subject, approvals, activeProviderModel, requestCwd, workspace } = input;
80-
const scoped = approvals.filter(
81-
(a) =>
82-
a.tool === tool &&
83-
(a.providerModel === undefined || a.providerModel === activeProviderModel) &&
84-
cwdMatchesGrant(a.cwd, requestCwd, workspace),
85-
);
100+
const scoped = approvals.filter((a) => grantScopeMatches(a, tool, activeProviderModel, requestCwd, workspace));
86101
if (scoped.length === 0) return false;
87102

88103
for (const a of scoped) {

src/permission/gate.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { autoShellRuleForCall } from "./auto-shell-policy.js";
1414
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
1515
import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js";
1616
import { matchesPattern, escapeGlobLiteral } from "./matcher.js";
17-
import { evaluateApprovals, cwdMatchesGrant, type GrantWorkspace } from "./authz-grants.js";
17+
import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js";
1818
import { splitChainedCommand, tokenize, isShellCommentOnly, stripCommentLines } from "./command.js";
1919
import { createPathRestriction } from "./path-restriction.js";
2020
import { createWorktreeRootsProvider, type RootsProvider } from "./worktree-roots.js";
@@ -59,11 +59,7 @@ function hasExactFullCommandGrant(
5959
// storing a run_shell pattern).
6060
const normalized = stripCommentLines(fullCommand).trim();
6161
return approvals.some(
62-
(a) =>
63-
a.tool === tool &&
64-
a.pattern === normalized &&
65-
(a.providerModel === undefined || a.providerModel === activeProviderModel) &&
66-
cwdMatchesGrant(a.cwd, requestCwd, workspace),
62+
(a) => a.pattern === normalized && grantScopeMatches(a, tool, activeProviderModel, requestCwd, workspace),
6763
);
6864
}
6965

@@ -145,14 +141,7 @@ export function isRequestCoveredByGrant(
145141
isRestricted: (path: string, isWrite: boolean) => boolean,
146142
workspace: GrantWorkspace,
147143
): boolean {
148-
if (request.tool !== approval.tool) return false;
149-
if (!cwdMatchesGrant(approval.cwd, request.cwd, workspace)) return false;
150-
if (
151-
approval.providerModel !== undefined &&
152-
approval.providerModel !== activeProviderModel
153-
) {
154-
return false;
155-
}
144+
if (!grantScopeMatches(approval, request.tool, activeProviderModel, request.cwd, workspace)) return false;
156145
if (request.tool !== "run_shell") {
157146
return matchesPattern(request.subject, approval.pattern);
158147
}

src/permission/grant-scope.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { describe, test, expect } from "bun:test";
2+
import type { ToolCall } from "@intx/types/runtime";
3+
import type { Approval, PermissionRequest } from "./types.js";
4+
import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js";
5+
import { createPermissionGate, isRequestCoveredByGrant } from "./gate.js";
6+
7+
// evaluateApprovals and isRequestCoveredByGrant each decide, independently,
8+
// whether a grant's tool/providerModel/cwd scope covers a request. Both are
9+
// expected to delegate to the same shared predicate (grantScopeMatches)
10+
// rather than reimplementing the condition. This test drives the same
11+
// grant+request pairs through all three and asserts they agree — a
12+
// regression where one call site reimplements the check with subtly
13+
// different semantics would fail here even if each function still "looks
14+
// right" in isolation.
15+
describe("grant tool/providerModel/cwd scoping agrees across call sites", () => {
16+
const workspace: GrantWorkspace = { resolvedCwd: "/proj", roots: ["/proj"] };
17+
const noopRestricted = () => false;
18+
19+
const grants: Approval[] = [
20+
{ tool: "run_shell", pattern: "npm test" },
21+
{ tool: "run_shell", pattern: "npm test", providerModel: "openai:gpt-5" },
22+
{ tool: "run_shell", pattern: "npm test", cwd: "/proj" },
23+
{ tool: "write_file", pattern: "npm test" },
24+
];
25+
26+
const requests: Array<{ tool: string; cwd?: string | undefined; activeProviderModel?: string | undefined }> = [
27+
{ tool: "run_shell", cwd: "/proj", activeProviderModel: "openai:gpt-5" },
28+
{ tool: "run_shell", cwd: "/proj", activeProviderModel: "anthropic:opus" },
29+
{ tool: "run_shell", cwd: "/other", activeProviderModel: "openai:gpt-5" },
30+
{ tool: "run_shell", cwd: undefined, activeProviderModel: undefined },
31+
{ tool: "write_file", cwd: "/proj", activeProviderModel: undefined },
32+
];
33+
34+
for (const grant of grants) {
35+
for (const req of requests) {
36+
test(`grant ${JSON.stringify(grant)} vs request ${JSON.stringify(req)}`, async () => {
37+
const expected = grantScopeMatches(grant, req.tool, req.activeProviderModel, req.cwd, workspace);
38+
39+
const viaEvaluateApprovals = await evaluateApprovals({
40+
tool: req.tool,
41+
subject: "npm test",
42+
approvals: [grant],
43+
activeProviderModel: req.activeProviderModel,
44+
requestCwd: req.cwd,
45+
workspace,
46+
});
47+
48+
const request: PermissionRequest = {
49+
tool: req.tool,
50+
action: req.tool,
51+
subject: "npm test",
52+
scopes: [],
53+
...(req.cwd !== undefined ? { cwd: req.cwd } : {}),
54+
};
55+
const viaGate = isRequestCoveredByGrant(request, grant, req.activeProviderModel, noopRestricted, workspace);
56+
57+
// Both live call sites additionally require the pattern to match the
58+
// subject, which is true for every case here ("npm test" grants an
59+
// exact "npm test" subject), so a scope mismatch is the only thing
60+
// that can make either disagree with the shared predicate.
61+
expect(viaEvaluateApprovals).toBe(expected);
62+
expect(viaGate).toBe(expected);
63+
});
64+
}
65+
}
66+
});
67+
68+
// hasExactFullCommandGrant (gate.ts) is the third live call site grantScopeMatches
69+
// unifies, but it is not exported — it only surfaces through the exact-full-command
70+
// replay path inside evaluate(). This drives that path directly with grants that
71+
// grantScopeMatches would refuse (wrong cwd, wrong providerModel) to confirm the
72+
// replay never fires when the shared predicate says no, matching the coverage the
73+
// other two call sites get above.
74+
describe("hasExactFullCommandGrant agrees with grantScopeMatches", () => {
75+
const full = "npm i && curl x";
76+
const shellCall = (command: string): ToolCall => ({ id: "c", name: "run_shell", arguments: { command } });
77+
78+
test("does not replay a grant scoped to a different cwd", async () => {
79+
let asked = 0;
80+
const gate = createPermissionGate({
81+
approvals: [{ tool: "run_shell", pattern: full, cwd: "/other-project" }],
82+
requestApproval: async () => { asked++; return { allow: true }; },
83+
interactive: true,
84+
skipPermissions: false,
85+
});
86+
expect((await gate.evaluate(shellCall(full))).allowed).toBe(true);
87+
// grantScopeMatches would refuse this grant (cwd mismatch), so the
88+
// exact-full-command shortcut must not fire — the operator is still asked.
89+
expect(asked).toBeGreaterThan(0);
90+
});
91+
92+
test("does not replay a grant scoped to a different provider model", async () => {
93+
let asked = 0;
94+
const gate = createPermissionGate({
95+
approvals: [{ tool: "run_shell", pattern: full, providerModel: "openai:gpt-5" }],
96+
providerName: "anthropic",
97+
model: "opus",
98+
requestApproval: async () => { asked++; return { allow: true }; },
99+
interactive: true,
100+
skipPermissions: false,
101+
});
102+
expect((await gate.evaluate(shellCall(full))).allowed).toBe(true);
103+
expect(asked).toBeGreaterThan(0);
104+
});
105+
106+
test("replays a grant whose scope grantScopeMatches accepts", async () => {
107+
let asked = 0;
108+
const gate = createPermissionGate({
109+
approvals: [{ tool: "run_shell", pattern: full }],
110+
requestApproval: async () => { asked++; return { allow: true }; },
111+
interactive: true,
112+
skipPermissions: false,
113+
});
114+
expect((await gate.evaluate(shellCall(full))).allowed).toBe(true);
115+
expect(asked).toBe(0);
116+
});
117+
});

src/permission/matcher.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { matchPattern } from "@intx/authz";
22

3-
import type { Approval } from "./types.js";
4-
53
// Exact-command grants (see escapeGlobLiteral) store a backslash before every
64
// glob metacharacter so a command like `rm -rf build/*` never becomes the
75
// wildcard `rm -rf build/*`. @intx/authz's matchPattern has no escape syntax —
@@ -38,25 +36,3 @@ export function matchesPattern(subject: string, pattern: string): boolean {
3836
}
3937
return matchPattern(pattern, subject);
4038
}
41-
42-
// True when any stored approval for this tool matches the subject. The subject
43-
// is the shell command segment (run_shell) or the file path (write/edit). An
44-
// approval bound to a `providerModel` only matches when `activeProviderModel`
45-
// equals it, so a grant scoped to one model never leaks to another. An
46-
// approval bound to a `cwd` (project-scoped) only matches when `requestCwd`
47-
// equals it, so a project grant from one repo never leaks into another.
48-
export function isApproved(
49-
tool: string,
50-
subject: string,
51-
approvals: readonly Approval[],
52-
activeProviderModel?: string,
53-
requestCwd?: string,
54-
): boolean {
55-
return approvals.some(
56-
(a) =>
57-
a.tool === tool &&
58-
matchesPattern(subject, a.pattern) &&
59-
(a.providerModel === undefined || a.providerModel === activeProviderModel) &&
60-
(a.cwd === undefined || a.cwd === requestCwd),
61-
);
62-
}

src/permission/permission.test.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
isShellNoOp,
1313
stripCommentLines,
1414
} from "./command.js";
15-
import { matchesPattern, isApproved, escapeGlobLiteral } from "./matcher.js";
15+
import { matchesPattern, escapeGlobLiteral } from "./matcher.js";
1616
import { evaluateApprovals } from "./authz-grants.js";
1717
import { classifyTool, buildRequests, isAutoAllowedShellCall } from "./classify.js";
1818
import { createPermissionGate } from "./gate.js";
@@ -241,19 +241,6 @@ describe("matchesPattern (@intx/authz + exact escapes)", () => {
241241
});
242242
});
243243

244-
describe("isApproved", () => {
245-
const approvals: Approval[] = [
246-
{ tool: "run_shell", pattern: "npm *" },
247-
{ tool: "write_file", pattern: "src/*" },
248-
];
249-
test("matches by tool and pattern", () => {
250-
expect(isApproved("run_shell", "npm test", approvals)).toBe(true);
251-
expect(isApproved("run_shell", "curl x", approvals)).toBe(false);
252-
expect(isApproved("write_file", "src/a.ts", approvals)).toBe(true);
253-
expect(isApproved("write_file", "lib/a.ts", approvals)).toBe(false);
254-
});
255-
});
256-
257244
describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
258245
const approvals: Approval[] = [
259246
{ tool: "run_shell", pattern: "npm *" },
@@ -1884,13 +1871,6 @@ describe("scoped grants", () => {
18841871
expect(routed[0]).toEqual({ tool: "run_shell", pattern: "npm *", providerModel: "openai:gpt-5" });
18851872
});
18861873

1887-
test("a provider-model approval does not auto-allow under a different model", () => {
1888-
const approvals: Approval[] = [{ tool: "run_shell", pattern: "npm *", providerModel: "openai:gpt-5" }];
1889-
expect(isApproved("run_shell", "npm test", approvals, "openai:gpt-5")).toBe(true);
1890-
expect(isApproved("run_shell", "npm test", approvals, "anthropic:opus")).toBe(false);
1891-
expect(isApproved("run_shell", "npm test", approvals, undefined)).toBe(false);
1892-
});
1893-
18941874
test("a seeded provider-model approval auto-allows when the gate's model matches", async () => {
18951875
let asked = 0;
18961876
const gate = createPermissionGate({

0 commit comments

Comments
 (0)