Skip to content

Commit 0c805e4

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

18 files changed

Lines changed: 102 additions & 128 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: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
} from "../agent/canonical-tool-name.js";
88
import type { Approval } from "./types.js";
99
import { directoryGrantAllows, matchesPattern } from "./matcher.js";
10-
import { realpathOr } from "./worktree-roots.js";
10+
import { realpathOrOriginal } from "./worktree-roots.js";
1111

1212
// Exact-escaped patterns (backslash before metacharacters) cannot round-trip
1313
// through @intx/authz matchPattern; they skip the package call and fall to
@@ -65,7 +65,7 @@ export function cwdMatchesGrant(
6565
if (requestCwd === undefined) return false;
6666
if (grantCwd !== workspace.resolvedCwd) return false;
6767
if (grantCwd === requestCwd) return true;
68-
return workspace.roots.includes(realpathOr(requestCwd));
68+
return workspace.roots.includes(realpathOrOriginal(requestCwd));
6969
}
7070

7171
// Seeded grants enter in native key space: pure renames collapse onto the
@@ -84,8 +84,8 @@ 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,
88-
// isRequestCoveredByGrant) delegate here so a scoping-dimension change lands
87+
// of pattern matching. Both live call sites (approvalCoversSubject,
88+
// isRequestCoveredByApprovals) delegate here so a scoping-dimension change lands
8989
// in one place.
9090
export function grantScopeMatches(
9191
approval: Approval,
@@ -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.test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { join } from "node:path";
1212
import type { ToolCall } from "@intx/types/runtime";
1313
import {
1414
createPermissionGate,
15-
isRequestCoveredByGrant,
15+
isRequestCoveredByApproval,
1616
preGrantGuardReason,
1717
} from "./gate.js";
1818
import { createPathRestriction } from "./path-restriction.js";
@@ -27,7 +27,7 @@ const shellCall = (command: string): ToolCall => ({
2727
});
2828

2929
// Every pre-grant guard, keyed to a command that trips it.
30-
// isRequestCoveredByGrant must refuse to cover each of these even when a
30+
// isRequestCoveredByApproval must refuse to cover each of these even when a
3131
// grant would otherwise match verbatim. The secret/restricted-path cases
3232
// queue and could be auto-approved by reconciliation once a broad grant
3333
// lands; the shell-authz hard-denies are unreachable today (evaluate()
@@ -44,7 +44,7 @@ const GUARD_CASES: { name: string; command: string }[] = [
4444
{ name: "restricted path target", command: "cat /etc/passwd" },
4545
];
4646

47-
describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
47+
describe("preGrantGuardReason / isRequestCoveredByApproval guard parity", () => {
4848
const cwd = mkdtempSync(join(tmpdir(), "gate-guard-"));
4949
const isRestricted = createPathRestriction(
5050
cwd,
@@ -63,7 +63,7 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
6363
expect(preGrantGuardReason(request, isRestricted)).not.toBeUndefined();
6464
});
6565

66-
test(`${name}: isRequestCoveredByGrant refuses an otherwise-matching grant`, () => {
66+
test(`${name}: isRequestCoveredByApproval refuses an otherwise-matching grant`, () => {
6767
const request: PermissionRequest = {
6868
tool: "run_shell",
6969
action: "Run",
@@ -73,7 +73,7 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
7373
};
7474
const grant: Approval = { tool: "run_shell", pattern: command };
7575
expect(
76-
isRequestCoveredByGrant(request, grant, undefined, isRestricted, {
76+
isRequestCoveredByApproval(request, grant, undefined, isRestricted, {
7777
resolvedCwd: cwd,
7878
roots: [],
7979
}),
@@ -104,7 +104,7 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
104104
expect(preGrantGuardReason(request, isRestricted)).toBeUndefined();
105105
const grant: Approval = { tool: "run_shell", pattern: "npm test" };
106106
expect(
107-
isRequestCoveredByGrant(request, grant, undefined, isRestricted, {
107+
isRequestCoveredByApproval(request, grant, undefined, isRestricted, {
108108
resolvedCwd: cwd,
109109
roots: [],
110110
}),
@@ -265,7 +265,7 @@ describe("lone-& bypass at the gate (CL-7781)", () => {
265265
const workspace = { resolvedCwd: cwd, roots: [] as string[] };
266266
const grant: Approval = { tool: "run_shell", pattern: "bun test *" };
267267
const covered = (subject: string): boolean =>
268-
isRequestCoveredByGrant(
268+
isRequestCoveredByApproval(
269269
{ tool: "run_shell", action: "Run", subject, scopes: [], cwd },
270270
grant,
271271
undefined,
@@ -319,7 +319,7 @@ describe("grant coverage rebinds relative paths to the request process cwd", ()
319319
};
320320
const grant: Approval = { tool: "run_shell", pattern: "cat *" };
321321
expect(
322-
isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, {
322+
isRequestCoveredByApproval(request, grant, undefined, sessionRestricted, {
323323
resolvedCwd: sessionCwd,
324324
roots: [],
325325
}),
@@ -337,7 +337,7 @@ describe("grant coverage rebinds relative paths to the request process cwd", ()
337337
};
338338
const grant: Approval = { tool: "run_shell", pattern: "cat *" };
339339
expect(
340-
isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, {
340+
isRequestCoveredByApproval(request, grant, undefined, sessionRestricted, {
341341
resolvedCwd: sessionCwd,
342342
roots: [],
343343
}),

‎src/permission/gate.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ function bindRestrictedToProcessCwd(
206206

207207
// Hard-deny and forced-ask checks a run_shell request must clear before grant
208208
// matching; no grant bypasses them. Shared by evaluate() and
209-
// isRequestCoveredByGrant (via segmentGuard), so a guard added here applies to
209+
// isRequestCoveredByApprovals (via segmentGuard), so a guard added here applies to
210210
// fresh requests and reconciliation alike.
211211
export function preGrantGuardReason(
212212
request: PermissionRequest,
@@ -252,7 +252,7 @@ export function preGrantGuardReason(
252252
// preGrantGuardReason sequence — so reconciliation never auto-approves
253253
// something evaluate() would still ask for or hard-deny. run_shell coverage
254254
// is per-segment; whole-string chain patterns never cover.
255-
export function isRequestCoveredByGrant(
255+
export function isRequestCoveredByApproval(
256256
request: PermissionRequest,
257257
approval: Approval,
258258
activeProviderModel: string | undefined,

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

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,19 @@ 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,
109
type GrantWorkspace,
1110
} from "./authz-grants.js";
12-
import { createPermissionGate, isRequestCoveredByGrant } from "./gate.js";
11+
import { createPermissionGate, isRequestCoveredByApproval } 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+
// isRequestCoveredByApproval 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;
@@ -70,7 +69,7 @@ describe("grant tool/providerModel/cwd scoping agrees across call sites", () =>
7069
scopes: [],
7170
...(req.cwd !== undefined ? { cwd: req.cwd } : {}),
7271
};
73-
const viaGate = isRequestCoveredByGrant(
72+
const viaGate = isRequestCoveredByApproval(
7473
request,
7574
grant,
7675
req.activeProviderModel,
@@ -151,7 +150,7 @@ describe("normalizeSeededApprovals", () => {
151150

152151
// mintGrant decomposes a chain scope into one grant per real segment; a grant
153152
// patterned on the full chain is legacy shape and never replays, because
154-
// evaluate() and isRequestCoveredByGrant both match per segment only.
153+
// evaluate() and isRequestCoveredByApproval both match per segment only.
155154
describe("a scope-mismatched grant never replays a multi-segment chain", () => {
156155
const full = "npm i && curl x";
157156
const shellCall = (command: string): ToolCall => ({
@@ -247,7 +246,7 @@ describe("legacy whole-string chain grants are explicitly rejected", () => {
247246
expect(asked).toBe(1);
248247
});
249248

250-
test("isRequestCoveredByGrant refuses a legacy full-chain pattern", () => {
249+
test("isRequestCoveredByApproval refuses a legacy full-chain pattern", () => {
251250
const request: PermissionRequest = {
252251
tool: "run_shell",
253252
action: "Run",
@@ -256,7 +255,7 @@ describe("legacy whole-string chain grants are explicitly rejected", () => {
256255
cwd: "/proj",
257256
};
258257
expect(
259-
isRequestCoveredByGrant(
258+
isRequestCoveredByApproval(
260259
request,
261260
{ tool: "run_shell", pattern: full },
262261
undefined,
@@ -275,7 +274,7 @@ describe("legacy whole-string chain grants are explicitly rejected", () => {
275274
cwd: "/proj",
276275
};
277276
expect(
278-
isRequestCoveredByGrant(
277+
isRequestCoveredByApproval(
279278
request,
280279
{ tool: "run_shell", pattern: "npm i" },
281280
undefined,
@@ -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],
@@ -440,7 +439,7 @@ describe("foreign grant cwd matching request cwd under a different workspace is
440439
}),
441440
).toBe(false);
442441

443-
// isRequestCoveredByGrant (request cwd coincides with the foreign grant's cwd)
442+
// isRequestCoveredByApproval (request cwd coincides with the foreign grant's cwd)
444443
const request: PermissionRequest = {
445444
tool: "run_shell",
446445
action: "Run",
@@ -449,7 +448,7 @@ describe("foreign grant cwd matching request cwd under a different workspace is
449448
cwd: "/foreign",
450449
};
451450
expect(
452-
isRequestCoveredByGrant(
451+
isRequestCoveredByApproval(
453452
request,
454453
foreignGrant,
455454
undefined,
@@ -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: 9 additions & 18 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 { realpathOrOriginal, 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.
@@ -154,10 +146,9 @@ function isResolvedPathInWorkspace(
154146
rootsProvider: RootsProvider,
155147
): boolean {
156148
if (real === UNRESOLVABLE) return false;
157-
const realCwd = realpathOr(resolve(cwd));
149+
const realCwd = realpathOrOriginal(resolve(cwd));
158150
if (real === realCwd || real.startsWith(realCwd + sep)) return true;
159151
if (inKnownRoots(real, rootsProvider())) return true;
160-
if (inKnownRoots(real, rootsProvider(true))) return true;
161152
return false;
162153
}
163154

@@ -180,14 +171,14 @@ export function isPermittedSiblingWorktreePath(
180171
if (path.startsWith("/") || /^[A-Za-z]:[\\/]/.test(path)) return false;
181172

182173
const abs = resolve(cwd, path);
183-
const realParent = realpathOr(dirname(abs));
184-
const realHome = realpathOr(resolve(home));
174+
const realParent = realpathOrOriginal(dirname(abs));
175+
const realHome = realpathOrOriginal(resolve(home));
185176
if (realParent === realHome) return false;
186177

187-
const knownRoots = [...rootsProvider(), ...rootsProvider(true)];
178+
const knownRoots = rootsProvider();
188179
const trustedParents = new Set<string>([
189-
realpathOr(resolve(cwd, "..")),
190-
...knownRoots.map((root) => realpathOr(dirname(root))),
180+
realpathOrOriginal(resolve(cwd, "..")),
181+
...knownRoots.map((root) => realpathOrOriginal(dirname(root))),
191182
]);
192183
return trustedParents.has(realParent);
193184
}
@@ -226,8 +217,8 @@ export function createPathRestriction(
226217
isRestricted: (path: string, isWrite: boolean): boolean => {
227218
const abs = resolve(cwd, path);
228219
const cacheKey = `${isWrite ? "w" : "r"}:${abs}`;
229-
// Use realpathNearestOr rather than realpathOr: for a not-yet-created
230-
// target, realpathOr returns the raw path unchanged — the same cache key
220+
// Use realpathNearestOr rather than realpathOrOriginal: for a not-yet-created
221+
// target, realpathOrOriginal returns the raw path unchanged — the same cache key
231222
// before and after a symlink retarget. realpathNearestOr resolves the
232223
// nearest existing ancestor, so the key changes when a symlink flips.
233224
const currentRealpath = realpathNearestOr(abs);

0 commit comments

Comments
 (0)