Skip to content

Commit 918aed9

Browse files
committed
refactor(permission): drop dead params and duplicate helpers
1 parent 60cf8c1 commit 918aed9

14 files changed

Lines changed: 60 additions & 104 deletions

‎src/agent/tool-aliases.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
withAuthzParityDefinitions,
1414
WIRE_TO_ENGINE,
1515
} from "./tool-aliases.js";
16-
import { evaluateApprovals } from "../permission/authz-grants.js";
16+
import { approvalCoversSubject } from "../permission/authz-grants.js";
1717

1818
const noWorkspace = { resolvedCwd: "/repo", roots: ["/repo"] };
1919

@@ -55,7 +55,7 @@ describe("one advertised posix set", () => {
5555
describe("grant aliases", () => {
5656
test("a grant stored as run_shell covers bash", async () => {
5757
expect(
58-
await evaluateApprovals({
58+
await approvalCoversSubject({
5959
tool: "bash",
6060
subject: "npm test",
6161
approvals: [{ tool: "run_shell", pattern: "npm *" }],
@@ -66,7 +66,7 @@ describe("grant aliases", () => {
6666

6767
test("a grant stored as bash matches a run_shell request after canonicalize", async () => {
6868
expect(
69-
await evaluateApprovals({
69+
await approvalCoversSubject({
7070
tool: "run_shell",
7171
subject: "npm test",
7272
approvals: [{ tool: "bash", pattern: "npm *" }],
@@ -77,15 +77,15 @@ describe("grant aliases", () => {
7777

7878
test("pure renames stay bidirectional: read covers read_file and vice versa", async () => {
7979
expect(
80-
await evaluateApprovals({
80+
await approvalCoversSubject({
8181
tool: "read_file",
8282
subject: "src/a.ts",
8383
approvals: [{ tool: "read", pattern: "src/*" }],
8484
workspace: noWorkspace,
8585
}),
8686
).toBe(true);
8787
expect(
88-
await evaluateApprovals({
88+
await approvalCoversSubject({
8989
tool: "read",
9090
subject: "src/a.ts",
9191
approvals: [{ tool: "read_file", pattern: "src/*" }],
@@ -102,7 +102,7 @@ describe("grant aliases", () => {
102102
// update_plan keys, so that replay path is unreachable.
103103
test("a stored update_plan grant does not cover manage_tasks", async () => {
104104
expect(
105-
await evaluateApprovals({
105+
await approvalCoversSubject({
106106
tool: "manage_tasks",
107107
subject: "manage_tasks",
108108
approvals: [{ tool: "update_plan", pattern: "*" }],
@@ -113,7 +113,7 @@ describe("grant aliases", () => {
113113

114114
test("a stored manage_tasks grant covers manage_tasks", async () => {
115115
expect(
116-
await evaluateApprovals({
116+
await approvalCoversSubject({
117117
tool: "manage_tasks",
118118
subject: "manage_tasks",
119119
approvals: [{ tool: "manage_tasks", pattern: "*" }],

‎src/agent/tools.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1527,7 +1527,7 @@ export async function createAgentToolset(
15271527
const failures: unknown[] = [];
15281528
// Kill live background process groups before the posix teardown so
15291529
// reload/interrupt cannot leave orphans behind.
1530-
backgroundShells.disposeAll("session closed");
1530+
backgroundShells.disposeAll();
15311531
try {
15321532
await posixTools.dispose();
15331533
} catch (err: unknown) {

‎src/permission/authz-grants.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export function normalizeSeededApprovals(
8484
}
8585

8686
// Single owner for grant tool/providerModel/cwd scope coverage, independent
87-
// of pattern matching. Both live call sites (evaluateApprovals,
87+
// of pattern matching. Both live call sites (approvalCoversSubject,
8888
// isRequestCoveredByGrant) delegate here so a scoping-dimension change lands
8989
// in one place.
9090
export function grantScopeMatches(
@@ -113,7 +113,7 @@ export interface EvaluateApprovalsInput {
113113

114114
// Grant-evaluation owner for the live decide() path (shell per-segment checks
115115
// and the path-arg check). The queued-reconciliation path
116-
// (isRequestCoveredByApprovals in gate.ts) matches inline against the same
116+
// (isRequestCoveredByGrant in gate.ts) matches inline against the same
117117
// helpers instead of calling here — keep the two in sync. Fail-closed:
118118
// unknown tools, unknown runners, and empty grant lists all refuse.
119119
export async function approvalCoversSubject(
@@ -165,10 +165,3 @@ export async function approvalCoversSubject(
165165
const decision = await evaluateGrants(grants, subject, action);
166166
return decision.effect === "allow";
167167
}
168-
169-
// Grant-store evaluation entry point; delegates to approvalCoversSubject.
170-
export async function evaluateApprovals(
171-
input: EvaluateApprovalsInput,
172-
): Promise<boolean> {
173-
return approvalCoversSubject(input);
174-
}

‎src/permission/gate.ts‎

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -254,36 +254,17 @@ export function preGrantGuardReason(
254254
// is per-segment; whole-string chain patterns never cover.
255255
export function isRequestCoveredByGrant(
256256
request: PermissionRequest,
257-
approval: Approval,
258-
activeProviderModel: string | undefined,
259-
isRestricted: (path: string, isWrite: boolean) => boolean,
260-
workspace: GrantWorkspace,
261-
rootsProvider?: RootsProvider,
262-
isExtraDenied: (value: string) => boolean = () => false,
263-
): boolean {
264-
return isRequestCoveredByApprovals(
265-
request,
266-
[approval],
267-
activeProviderModel,
268-
isRestricted,
269-
workspace,
270-
rootsProvider,
271-
isExtraDenied,
272-
);
273-
}
274-
275-
// Coverage against the live approvals list; mintGrant hands this to onGrant
276-
// so a queued identical chain drains once every per-segment grant is in the
277-
// list.
278-
function isRequestCoveredByApprovals(
279-
request: PermissionRequest,
280-
approvals: readonly Approval[],
257+
approval: Approval | readonly Approval[],
281258
activeProviderModel: string | undefined,
282259
isRestricted: (path: string, isWrite: boolean) => boolean,
283260
workspace: GrantWorkspace,
284261
rootsProvider?: RootsProvider,
285262
isExtraDenied: (value: string) => boolean = () => false,
286263
): boolean {
264+
// Coverage against one approval or the live approvals list; mintGrant
265+
// hands the list to onGrant so a queued identical chain drains once every
266+
// per-segment grant is in the list.
267+
const approvals = Array.isArray(approval) ? approval : [approval];
287268
const scoped = approvals.filter((a) =>
288269
grantScopeMatches(
289270
a,
@@ -592,7 +573,7 @@ export function createPermissionGate(
592573
persist?.(approval, grant);
593574
}
594575
options.onGrant?.(approval, (request) =>
595-
isRequestCoveredByApprovals(
576+
isRequestCoveredByGrant(
596577
request,
597578
approvals,
598579
activeProviderModel,

‎src/permission/grant-scope.test.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { ToolCall } from "@intx/types/runtime";
33
import type { Approval, PermissionRequest } from "./types.js";
44
import {
55
approvalCoversSubject,
6-
evaluateApprovals,
76
grantScopeMatches,
87
normalizeSeededApprovals,
98
cwdMatchesGrant,
@@ -13,10 +12,10 @@ import { createPermissionGate, isRequestCoveredByGrant } from "./gate.js";
1312
import { createPermissionRequestQueue } from "./queue.js";
1413
import { buildRequests } from "./classify.js";
1514

16-
// approvalCoversSubject is the single grant-evaluation owner; evaluateApprovals
17-
// delegates to it and isRequestCoveredByGrant must agree on scope. Driving
18-
// the same grant+request pairs through all three catches a call site that
19-
// reimplements the check with subtly different semantics.
15+
// approvalCoversSubject is the single grant-evaluation owner;
16+
// isRequestCoveredByGrant must agree on scope. Driving the same grant+request
17+
// pairs through both catches a call site that reimplements the check with
18+
// subtly different semantics.
2019
describe("grant tool/providerModel/cwd scoping agrees across call sites", () => {
2120
const workspace: GrantWorkspace = { resolvedCwd: "/proj", roots: ["/proj"] };
2221
const noopRestricted = () => false;
@@ -429,9 +428,9 @@ describe("foreign grant cwd matching request cwd under a different workspace is
429428
});
430429

431430
test("both live call sites refuse a foreign grant replaying into /proj's workspace", async () => {
432-
// evaluateApprovals
431+
// approvalCoversSubject
433432
expect(
434-
await evaluateApprovals({
433+
await approvalCoversSubject({
435434
tool: "run_shell",
436435
subject: "npm test",
437436
approvals: [foreignGrant],
@@ -461,7 +460,7 @@ describe("foreign grant cwd matching request cwd under a different workspace is
461460

462461
test("the same grant with its own workspace still replays the session root", async () => {
463462
expect(
464-
await evaluateApprovals({
463+
await approvalCoversSubject({
465464
tool: "run_shell",
466465
subject: "npm test",
467466
approvals: [ownGrant],

‎src/permission/path-restriction.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { lstatSync, readlinkSync, realpathSync } from "node:fs";
22
import { dirname, join, resolve, sep } from "node:path";
33
import { homedir } from "node:os";
4-
import type { RootsProvider } from "./worktree-roots.js";
4+
import { realpathOr, type RootsProvider } from "./worktree-roots.js";
55
import { projectSessionsRoot } from "../session/project-key.js";
66

77
// Paths the agent must not touch without explicit operator approval, even
@@ -26,14 +26,6 @@ export interface PathRestriction {
2626

2727
const LEGACY_STATE_DIR = ".agent-state";
2828

29-
function realpathOr(path: string): string {
30-
try {
31-
return realpathSync(path);
32-
} catch {
33-
return path;
34-
}
35-
}
36-
3729
// Sentinel from realpathNearestOr for a path that exists but cannot be
3830
// resolved (dangling symlink or loop). Contains a NUL byte, which can never
3931
// appear in a real path, so it cannot collide with a genuine result.

‎src/permission/permission.test.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
stripCommentLines,
2020
} from "./command.js";
2121
import { matchesPattern, escapeGlobLiteral } from "./matcher.js";
22-
import { evaluateApprovals } from "./authz-grants.js";
22+
import { approvalCoversSubject } from "./authz-grants.js";
2323
import {
2424
classifyTool,
2525
buildRequests,
@@ -360,7 +360,7 @@ describe("matchesPattern directory-grant traversal (CL-8989)", () => {
360360
});
361361
});
362362

363-
describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
363+
describe("approvalCoversSubject (@intx/authz evaluateGrants)", () => {
364364
const approvals: Approval[] = [
365365
{ tool: "run_shell", pattern: "npm *" },
366366
{ tool: "write_file", pattern: "src/*" },
@@ -374,23 +374,23 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
374374

375375
test("allows package-compatible wildcard grants", async () => {
376376
expect(
377-
await evaluateApprovals({
377+
await approvalCoversSubject({
378378
tool: "run_shell",
379379
subject: "npm test",
380380
approvals,
381381
workspace: noWorkspace,
382382
}),
383383
).toBe(true);
384384
expect(
385-
await evaluateApprovals({
385+
await approvalCoversSubject({
386386
tool: "run_shell",
387387
subject: "curl x",
388388
approvals,
389389
workspace: noWorkspace,
390390
}),
391391
).toBe(false);
392392
expect(
393-
await evaluateApprovals({
393+
await approvalCoversSubject({
394394
tool: "write_file",
395395
subject: "src/a.ts",
396396
approvals,
@@ -407,7 +407,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
407407
];
408408
const workspace = { resolvedCwd: "/proj", roots: [] as string[] };
409409
expect(
410-
await evaluateApprovals({
410+
await approvalCoversSubject({
411411
tool: "write_file",
412412
subject: "/proj/sub/nested/file.txt",
413413
approvals: dirApprovals,
@@ -420,7 +420,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
420420
"/proj/sub//../evil",
421421
]) {
422422
expect(
423-
await evaluateApprovals({
423+
await approvalCoversSubject({
424424
tool: "write_file",
425425
subject,
426426
approvals: dirApprovals,
@@ -432,7 +432,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
432432

433433
test("a grant for read_file covers default.read_file", async () => {
434434
expect(
435-
await evaluateApprovals({
435+
await approvalCoversSubject({
436436
tool: "default.read_file",
437437
subject: "src/a.ts",
438438
approvals: [{ tool: "read_file", pattern: "src/*" }],
@@ -443,7 +443,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
443443

444444
test("a grant for an MCP tool covers the default. prefixed name", async () => {
445445
expect(
446-
await evaluateApprovals({
446+
await approvalCoversSubject({
447447
tool: "default.mcp__linear__save_issue",
448448
subject: "mcp__linear__save_issue",
449449
approvals: [
@@ -459,15 +459,15 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => {
459459

460460
test("allows exact-escaped grants without treating * as a wildcard", async () => {
461461
expect(
462-
await evaluateApprovals({
462+
await approvalCoversSubject({
463463
tool: "run_shell",
464464
subject: "rm -rf build/*",
465465
approvals,
466466
workspace: noWorkspace,
467467
}),
468468
).toBe(true);
469469
expect(
470-
await evaluateApprovals({
470+
await approvalCoversSubject({
471471
tool: "run_shell",
472472
subject: "rm -rf build/../../etc",
473473
approvals,

‎src/permission/worktree-roots.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export function listWorktreeRootsSync(cwd: string): string[] {
8383
// re-lists when the debounce window has elapsed since the last one — so a
8484
// burst of refresh requests (e.g. several foreign-path checks in a row)
8585
// shells out to git at most once per window.
86-
export type RootsProvider = (forceRefresh?: boolean) => readonly string[];
86+
export type RootsProvider = (_forceRefresh?: boolean) => readonly string[];
8787

8888
const DEFAULT_DEBOUNCE_MS = 3000;
8989

@@ -99,20 +99,18 @@ export function createWorktreeRootsProvider(
9999
let roots: string[] | undefined;
100100
let lastRefreshAt = -Infinity;
101101

102-
return (forceRefresh = false): readonly string[] => {
102+
return (_forceRefresh = false): readonly string[] => {
103103
if (roots === undefined) {
104104
roots = lister(cwd);
105105
lastRefreshAt = Date.now();
106106
return roots;
107107
}
108108
const now = Date.now();
109109
const due = now - lastRefreshAt >= debounceMs;
110-
if (due || forceRefresh) {
111-
if (due) {
112-
lastRefreshAt = now;
113-
// Replace the cache wholesale so roots git no longer lists are evicted.
114-
roots = lister(cwd);
115-
}
110+
if (due) {
111+
lastRefreshAt = now;
112+
// Replace the cache wholesale so roots git no longer lists are evicted.
113+
roots = lister(cwd);
116114
}
117115
return roots;
118116
};

‎src/plugins/shell-guard-plugin.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ describe("background run_shell (shellGuardPlugin)", () => {
288288
// The START call resolved while the child was still sleeping.
289289
expect(Date.now() - start).toBeLessThan(400);
290290
await recorder.waitFor(parsed.shell_id);
291-
registry.disposeAll("test done");
291+
registry.disposeAll();
292292
});
293293

294294
test("without a registry wired, background fails closed and spawns nothing", async () => {
@@ -320,7 +320,7 @@ describe("background run_shell (shellGuardPlugin)", () => {
320320
neverAbort(),
321321
);
322322
expect(String(after.content).trim()).toBe(process.cwd());
323-
registry.disposeAll("test done");
323+
registry.disposeAll();
324324
});
325325

326326
test("foreground run_shell is unchanged when background is unset", async () => {
@@ -333,7 +333,7 @@ describe("background run_shell (shellGuardPlugin)", () => {
333333
expect(result.isError).toBeUndefined();
334334
expect(String(result.content)).toContain("direct");
335335
expect(registry.runningCount()).toBe(0);
336-
registry.disposeAll("test done");
336+
registry.disposeAll();
337337
});
338338

339339
test("background omitted timeout skips the foreground default", async () => {
@@ -362,7 +362,7 @@ describe("background run_shell (shellGuardPlugin)", () => {
362362
const exit = await recorder.waitFor(parsed.shell_id);
363363
expect(exit.timedOut).toBe(false);
364364
expect(exit.output).toContain("survived");
365-
registry.disposeAll("test done");
365+
registry.disposeAll();
366366
});
367367

368368
test("an unwired shell-output feed spawns fine and paints no tail", async () => {

0 commit comments

Comments
 (0)