From e0efb3f066e0b717ffecf7bffd73796417aa0a37 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:00:13 -0700 Subject: [PATCH 1/4] 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/4] 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); + }); +}); From 1f16d506445a7cd970e462f9717c7af7d7a24a3b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:00:56 -0700 Subject: [PATCH 3/4] Share one predicate between isSingleShellCommand and shellApprovalScopes Both answered "is this a single shell command, or a chain" by independently combining splitChainedCommand/tokenize, so a segmenting-rule change to one silently didn't reach the other. They disagreed on a comment-trailing command like "echo hi && # why": shellApprovalScopes filtered the comment-only segment before counting (treating it as one command), while gate.ts's isSingleShellCommand did not (treating it as a two-segment chain). isSingleShellCommand now lives in classify.ts, built on the same comment-filtered segment list shellApprovalScopes derives its ladder from, and gate.ts imports it instead of maintaining its own copy. --- src/permission/classify.ts | 19 ++++++++++++++++++- src/permission/gate.ts | 12 +----------- src/permission/permission.test.ts | 24 ++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/permission/classify.ts b/src/permission/classify.ts index f7ab32fac..0b1b8bc89 100644 --- a/src/permission/classify.ts +++ b/src/permission/classify.ts @@ -384,12 +384,29 @@ export const MEGA_CHAIN_SEGMENT_THRESHOLD = 5; export const MEGA_CHAIN_NOTICE = `Chains of ${MEGA_CHAIN_SEGMENT_THRESHOLD}+ steps are approved once only — split into shorter commands for reusable approvals.`; +// The real (non-comment-only) chain segments of a shell command — the basis +// both shellApprovalScopes and isSingleShellCommand use to answer "is this +// one command or a chain." +function realShellSegments(command: string): string[] { + return splitChainedCommand(command).filter((segment) => !isShellCommentOnly(segment)); +} + +// Whether `command` is exactly one real command — not a chain (`a && b`), not +// a pipeline (`a | b`), not empty/comment-only. Shared by preApprove's gate +// (src/permission/gate.ts) and the interactive scope ladder below, so a +// segmenting-rule change here reaches both. +export function isSingleShellCommand(command: string): boolean { + const segments = realShellSegments(command); + if (segments.length !== 1) return false; + return tokenize(segments[0]!).length > 0; +} + // Approval scopes for a shell command the operator may persist. Multi-segment // chains only offer the exact full string — a prefix like `npm *` would also // match `npm i && rm -rf /` on a later call (fail-closed). At or above // MEGA_CHAIN_SEGMENT_THRESHOLD, no scope is offered at all — see the constant. function shellApprovalScopes(command: string): ApprovalScope[] { - const segments = splitChainedCommand(command).filter((segment) => !isShellCommentOnly(segment)); + const segments = realShellSegments(command); if (segments.length === 0) return []; if (segments.length >= MEGA_CHAIN_SEGMENT_THRESHOLD) return []; if (segments.length === 1) { diff --git a/src/permission/gate.ts b/src/permission/gate.ts index ad154e30e..8aa4a7f1c 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -6,6 +6,7 @@ import { buildRequests, isAutoAllowedShellCall, isAutoAllowedShellSegment, + isSingleShellCommand, callTargetsRestricted, commandTargetsRestricted, MEGA_CHAIN_SEGMENT_THRESHOLD, @@ -30,17 +31,6 @@ import { currentTurnId } from "../perf/reactor-spans.js"; export type GateVerdict = { allowed: true } | { allowed: false; reason: string }; -// A run_shell pre-approval must name exactly one real command — not a chain -// (`a && b`), not a pipeline (`a | b`), not an empty or whitespace-only string. -// Rejecting anything else here keeps ask_operator's `command` argument from -// minting a grant broader than the single command the operator actually saw. -function isSingleShellCommand(command: string): boolean { - const trimmed = command.trim(); - if (trimmed.length === 0) return false; - if (splitChainedCommand(trimmed).length !== 1) return false; - return tokenize(trimmed).length > 0; -} - // Multi-segment shell may only short-circuit on an exact full-command grant. // Prefix globs like `npm *` must not match `npm i && curl x` — the unapproved // tail still needs a full-block operator decision. String equality (not glob) diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index e943eaf3d..2451302a2 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -2001,6 +2001,30 @@ describe("preApprove", () => { expect((await gate.evaluate(shellCall("npm test | curl evil.com"))).allowed).toBe(true); expect(asked).toBe(1); }); + + test("agrees with the interactive scope ladder on whether a comment-trailing command is single", async () => { + // "echo hi && # why" has one real segment once the trailing comment is + // filtered out. The interactive scope ladder (buildRequests/shellApprovalScopes) + // already filters comment-only segments before counting, so it offers the + // full per-command ladder (prefix + exact) as if this were one command. + // preApprove's gate must reach the same verdict, since both answer the + // same underlying "is this a single shell command" question. + const command = "echo hi && # why"; + + const gate = createPermissionGate({ + approvals: [], + requestApproval: async () => ({ allow: true }), + interactive: true, + skipPermissions: false, + }); + gate.preApprove("run_shell", command); + const preApproveTreatsAsSingle = gate.getSessionApprovals().length === 1; + + const requests = buildRequests(shellCall(command)); + const scopeLadderTreatsAsSingle = requests[0]!.scopes.length > 1; + + expect(preApproveTreatsAsSingle).toBe(scopeLadderTreatsAsSingle); + }); }); describe("isAutoAllowedShellCall", () => { From b7e7388722a4f79d6a76e64c09a2e3717bd4b30f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:24:53 -0700 Subject: [PATCH 4/4] Add direct coverage for isSingleShellCommand's comment-handling edges grantScopeMatches consolidation's sibling PR fixed a live disagreement on comment-trailing chains, but the shared realShellSegments predicate also silently narrows pure-comment input from single-command to not-a-command, and treats a leading-comment-then-chain by its trailing real segment only. Neither was covered by a test, so a future segmenting change could flip either back with nothing catching it. --- src/permission/permission.test.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 2451302a2..87d65a7a0 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -14,7 +14,7 @@ import { } from "./command.js"; import { matchesPattern, escapeGlobLiteral } from "./matcher.js"; import { evaluateApprovals } from "./authz-grants.js"; -import { classifyTool, buildRequests, isAutoAllowedShellCall } from "./classify.js"; +import { classifyTool, buildRequests, isAutoAllowedShellCall, isSingleShellCommand } from "./classify.js"; import { createPermissionGate } from "./gate.js"; import { createMcpToolPermissionRegistry, registerMcpClientTools } from "../mcp/tool-permissions.js"; import { listWorktreeRoots, createWorktreeRootsProvider } from "./worktree-roots.js"; @@ -2025,6 +2025,26 @@ describe("preApprove", () => { expect(preApproveTreatsAsSingle).toBe(scopeLadderTreatsAsSingle); }); + + test("isSingleShellCommand narrows a pure-comment command to false", () => { + // Before the shared realShellSegments predicate, gate.ts's own + // isSingleShellCommand did not filter comment-only segments, so a + // pure-comment "command" like "# just a comment" counted as one real + // segment and was treated as single. The shared predicate filters it + // out, leaving zero segments, so this must now be false. + expect(isSingleShellCommand("# just a comment")).toBe(false); + }); + + test("isSingleShellCommand treats a leading-comment-then-chain as its trailing real segment", () => { + // splitChainedCommand splits on "&&" before recognizing that "#" extends + // a comment to end of line, so "# a && b" splits into ["# a", "b"] even + // though a real shell treats the whole line as one comment (nothing + // after "#" ever runs). Filtering the comment-only "# a" segment leaves + // exactly one real segment, "b", so this is scored as a single command — + // matching shellApprovalScopes' existing behavior, not a regression + // introduced here. + expect(isSingleShellCommand("# a && b")).toBe(true); + }); }); describe("isAutoAllowedShellCall", () => {