Skip to content

Commit 051e527

Browse files
Merge pull request #386 from corbitsdev/cl-5662-project-grants-never-match-subagent-worktree
Fix project-scoped grants never matching sub-agent worktree requests
2 parents 47beb46 + bbf43b8 commit 051e527

5 files changed

Lines changed: 277 additions & 20 deletions

File tree

src/permission/authz-grants.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { evaluateGrants, type GrantRule } from "@intx/authz";
22

33
import type { Approval } from "./types.js";
44
import { matchesPattern } from "./matcher.js";
5+
import { realpathOr } from "./worktree-roots.js";
56

67
// Exact-escaped patterns (backslash before metacharacters) cannot round-trip
78
// through @intx/authz matchPattern, so those grants are filtered out of the
@@ -25,12 +26,48 @@ export function approvalToGrantRule(approval: Approval, index: number): GrantRul
2526
};
2627
}
2728

29+
// The gate's own project boundary: the session root it was constructed with,
30+
// plus every git worktree registered against that root (which, per CL-4929,
31+
// may live outside the root entirely — a sibling directory, not a
32+
// subdirectory). Built once per gate from its closed-over resolvedCwd and
33+
// rootsProvider and threaded through — never accept one built anywhere else,
34+
// or "same project" quietly stops meaning "same gate's project."
35+
export type GrantWorkspace = { resolvedCwd: string; roots: readonly string[] };
36+
37+
// A project-scoped grant (Approval.cwd set) is confined to the session that
38+
// minted it: it may replay only for a request whose cwd is that same session
39+
// root, or one of the root's registered worktrees. A worktree cwd never
40+
// equals the session root by string identity (that's the bug this closes),
41+
// so membership is resolved through `workspace` instead of a bare `===`.
42+
//
43+
// `grantCwd !== workspace.resolvedCwd` is the boundary: a grant stamped with
44+
// some OTHER project's root is rejected before roots are ever consulted, so
45+
// a request cwd that happens to coincide with a different project's worktree
46+
// can never match. Membership within a matching project is exact equality
47+
// against the resolved roots, never a path-prefix — a prefix check would let
48+
// a maliciously named sibling directory (`/repo/wt-1-evil`) match a
49+
// legitimate root (`/repo/wt-1`). `workspace.roots` already comes back
50+
// realpath-resolved (see worktree-roots.ts); `requestCwd` is realpath'd here
51+
// so a symlinked checkout (macOS /tmp vs /private/tmp) still compares equal.
52+
export function cwdMatchesGrant(
53+
grantCwd: string | undefined,
54+
requestCwd: string | undefined,
55+
workspace: GrantWorkspace,
56+
): boolean {
57+
if (grantCwd === undefined) return true;
58+
if (requestCwd === undefined) return false;
59+
if (grantCwd === requestCwd) return true;
60+
if (grantCwd !== workspace.resolvedCwd) return false;
61+
return workspace.roots.includes(realpathOr(requestCwd));
62+
}
63+
2864
export type EvaluateApprovalsInput = {
2965
tool: string;
3066
subject: string;
3167
approvals: readonly Approval[];
3268
activeProviderModel?: string | undefined;
3369
requestCwd?: string | undefined;
70+
workspace: GrantWorkspace;
3471
};
3572

3673
// Grant-store evaluation via @intx/authz. Filters provider-model and cwd the
@@ -39,12 +76,12 @@ export type EvaluateApprovalsInput = {
3976
// matchesPattern (equality after unescape) first so a stored exact command is
4077
// never lost.
4178
export async function evaluateApprovals(input: EvaluateApprovalsInput): Promise<boolean> {
42-
const { tool, subject, approvals, activeProviderModel, requestCwd } = input;
79+
const { tool, subject, approvals, activeProviderModel, requestCwd, workspace } = input;
4380
const scoped = approvals.filter(
4481
(a) =>
4582
a.tool === tool &&
4683
(a.providerModel === undefined || a.providerModel === activeProviderModel) &&
47-
(a.cwd === undefined || a.cwd === requestCwd),
84+
cwdMatchesGrant(a.cwd, requestCwd, workspace),
4885
);
4986
if (scoped.length === 0) return false;
5087

src/permission/gate.test.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
6060
cwd,
6161
};
6262
const grant: Approval = { tool: "run_shell", pattern: command };
63-
expect(isRequestCoveredByGrant(request, grant, undefined, isRestricted)).toBe(false);
63+
expect(
64+
isRequestCoveredByGrant(request, grant, undefined, isRestricted, { resolvedCwd: cwd, roots: [] }),
65+
).toBe(false);
6466
});
6567

6668
test(`${name}: evaluate() never allows outright`, async () => {
@@ -85,7 +87,9 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
8587
};
8688
expect(preGrantGuardReason(request, isRestricted)).toBeUndefined();
8789
const grant: Approval = { tool: "run_shell", pattern: "npm test" };
88-
expect(isRequestCoveredByGrant(request, grant, undefined, isRestricted)).toBe(true);
90+
expect(
91+
isRequestCoveredByGrant(request, grant, undefined, isRestricted, { resolvedCwd: cwd, roots: [] }),
92+
).toBe(true);
8993
});
9094
});
9195

@@ -122,7 +126,12 @@ describe("grant coverage rebinds relative paths to the request process cwd", ()
122126
cwd: agentCwd,
123127
};
124128
const grant: Approval = { tool: "run_shell", pattern: "cat *" };
125-
expect(isRequestCoveredByGrant(request, grant, undefined, sessionRestricted)).toBe(false);
129+
expect(
130+
isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, {
131+
resolvedCwd: sessionCwd,
132+
roots: [],
133+
}),
134+
).toBe(false);
126135
});
127136

128137
test("a relative path inside the registered worktree is not forced-restricted", () => {
@@ -135,6 +144,11 @@ describe("grant coverage rebinds relative paths to the request process cwd", ()
135144
cwd: agentCwd,
136145
};
137146
const grant: Approval = { tool: "run_shell", pattern: "cat *" };
138-
expect(isRequestCoveredByGrant(request, grant, undefined, sessionRestricted)).toBe(true);
147+
expect(
148+
isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, {
149+
resolvedCwd: sessionCwd,
150+
roots: [],
151+
}),
152+
).toBe(true);
139153
});
140154
});

src/permission/gate.ts

Lines changed: 24 additions & 9 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 } from "./authz-grants.js";
17+
import { evaluateApprovals, cwdMatchesGrant, 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";
@@ -51,6 +51,7 @@ function hasExactFullCommandGrant(
5151
approvals: readonly Approval[],
5252
activeProviderModel: string | undefined,
5353
requestCwd: string | undefined,
54+
workspace: GrantWorkspace,
5455
): boolean {
5556
// Comment-insensitive: a model-authored "# why" line prepended to an
5657
// otherwise-identical command must still replay against a grant minted
@@ -62,7 +63,7 @@ function hasExactFullCommandGrant(
6263
a.tool === tool &&
6364
a.pattern === normalized &&
6465
(a.providerModel === undefined || a.providerModel === activeProviderModel) &&
65-
(a.cwd === undefined || a.cwd === requestCwd),
66+
cwdMatchesGrant(a.cwd, requestCwd, workspace),
6667
);
6768
}
6869

@@ -142,9 +143,10 @@ export function isRequestCoveredByGrant(
142143
approval: Approval,
143144
activeProviderModel: string | undefined,
144145
isRestricted: (path: string, isWrite: boolean) => boolean,
146+
workspace: GrantWorkspace,
145147
): boolean {
146148
if (request.tool !== approval.tool) return false;
147-
if (approval.cwd !== undefined && approval.cwd !== request.cwd) return false;
149+
if (!cwdMatchesGrant(approval.cwd, request.cwd, workspace)) return false;
148150
if (
149151
approval.providerModel !== undefined &&
150152
approval.providerModel !== activeProviderModel
@@ -268,11 +270,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
268270
const { requestApproval, persist, interactive, skipPermissions, providerName, model, cwd } = options;
269271
const mcpTiers = options.mcpTiers ?? createMcpToolPermissionRegistry();
270272
const resolvedCwd = cwd ?? process.cwd();
271-
const pathRestriction = createPathRestriction(
272-
resolvedCwd,
273-
options.rootsProvider ?? createWorktreeRootsProvider(resolvedCwd),
274-
);
273+
const rootsProvider = options.rootsProvider ?? createWorktreeRootsProvider(resolvedCwd);
274+
const pathRestriction = createPathRestriction(resolvedCwd, rootsProvider);
275275
const isRestricted = pathRestriction.isRestricted;
276+
// This gate's project boundary for grant matching (see cwdMatchesGrant):
277+
// this session's root plus its currently-known registered worktrees. Built
278+
// fresh per read from the same rootsProvider the gate already uses for
279+
// path containment, so "same project" for a grant and "inside the
280+
// workspace" for a path share one authority.
281+
const grantWorkspace = (): GrantWorkspace => ({ resolvedCwd, roots: rootsProvider() });
276282
let auto = options.auto;
277283
// Own a private copy so evaluating a grant never mutates the caller's array.
278284
const approvals: Approval[] = [...options.approvals];
@@ -309,7 +315,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
309315
persist?.(approval, grant);
310316
}
311317
options.onGrant?.(approval, (request) =>
312-
isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted),
318+
isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted, grantWorkspace()),
313319
);
314320
};
315321

@@ -405,7 +411,14 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
405411
!fullReferencesSecret &&
406412
!commandTargetsRestricted(fullCommand, isRestrictedHere) &&
407413
segments.length > 1 &&
408-
hasExactFullCommandGrant(request.tool, fullCommand, approvals, activeProviderModel, effectiveCwd)
414+
hasExactFullCommandGrant(
415+
request.tool,
416+
fullCommand,
417+
approvals,
418+
activeProviderModel,
419+
effectiveCwd,
420+
grantWorkspace(),
421+
)
409422
) {
410423
continue;
411424
}
@@ -432,6 +445,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
432445
approvals,
433446
activeProviderModel,
434447
requestCwd: effectiveCwd,
448+
workspace: grantWorkspace(),
435449
})
436450
) {
437451
continue;
@@ -502,6 +516,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
502516
approvals,
503517
activeProviderModel,
504518
requestCwd: effectiveCwd,
519+
workspace: grantWorkspace(),
505520
});
506521
if (alreadyApproved) {
507522
continue;

0 commit comments

Comments
 (0)