Skip to content

Commit 7c309e8

Browse files
committed
Bind project grant confirmations to the confirmed cwd
1 parent b99f1ca commit 7c309e8

3 files changed

Lines changed: 160 additions & 7 deletions

File tree

src/permission/project-approvals-trust.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { generateSessionId } from "../session/index.js";
66
import { loadSeededApprovals } from "../session/runtime-assembly.js";
7+
import { runWithSubAgentIdentity } from "../subagent/identity-context.js";
78
import { trustProjectGrants } from "../trust/project-trust.js";
89
import { createPermissionGate } from "./gate.js";
910
import type { Approval, PermissionRequest } from "./types.js";
@@ -206,4 +207,102 @@ describe("CL-7782: project approvals require grant trust", () => {
206207
expect((await gate.evaluate({ ...NPM_TEST })).allowed).toBe(true);
207208
expect(asked).toEqual([]);
208209
});
210+
211+
test("confirming a planted entry through the pending flow converges the file to the minted shape", async () => {
212+
const base = await mkdtemp(join(tmpdir(), "cl-7782-converge-"));
213+
const home = join(base, "home");
214+
const cwd = join(base, "repo");
215+
await plantProjectApprovals(cwd, [
216+
{ tool: "run_shell", pattern: "npm test" },
217+
]);
218+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([
219+
{ tool: "run_shell", pattern: "npm test" },
220+
]);
221+
222+
// What the gate persist does when the operator confirms the pending entry
223+
// with a project-scope persist: mint {tool, pattern, cwd} and write it.
224+
await saveProjectApproval(
225+
cwd,
226+
{ tool: "run_shell", pattern: "npm test", cwd },
227+
home,
228+
);
229+
230+
// The planted twin is displaced by the minted shape — nothing lingers as
231+
// pending, and the grant applies without asking.
232+
expect(await loadProjectApprovals(cwd, home)).toEqual([
233+
{ tool: "run_shell", pattern: "npm test", cwd },
234+
]);
235+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]);
236+
237+
const { gate, asked } = await driveGate(cwd, generateSessionId(), home);
238+
expect(
239+
(
240+
await gate.evaluate({
241+
id: "npm-test",
242+
name: "run_shell",
243+
arguments: { command: "npm test" },
244+
})
245+
).allowed,
246+
).toBe(true);
247+
expect(asked).toEqual([]);
248+
});
249+
250+
test("stripping cwd from a confirmed entry re-surfaces as pending and never cross-repo auto-allows", async () => {
251+
const base = await mkdtemp(join(tmpdir(), "cl-7782-cwd-strip-"));
252+
const home = join(base, "home");
253+
const cwd = join(base, "repo");
254+
const other = join(base, "other");
255+
await mkdir(other, { recursive: true });
256+
257+
// Operator confirms {tool, pattern, cwd} through the production path.
258+
await saveProjectApproval(
259+
cwd,
260+
{ tool: "run_shell", pattern: "npm test", cwd },
261+
home,
262+
);
263+
expect(await loadProjectApprovals(cwd, home)).toEqual([
264+
{ tool: "run_shell", pattern: "npm test", cwd },
265+
]);
266+
267+
// Hand-edit drops the cwd key: byte-identical to a planted entry, but the
268+
// confirmation was bound to the cwd-bearing shape, so trust must not
269+
// follow the stripped bytes.
270+
await plantProjectApprovals(cwd, [
271+
{ tool: "run_shell", pattern: "npm test" },
272+
]);
273+
expect(await loadProjectApprovals(cwd, home)).toEqual([]);
274+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([
275+
{ tool: "run_shell", pattern: "npm test" },
276+
]);
277+
278+
// The real gate, seeded after the strip: neither the same-repo request
279+
// nor a cross-repo request (different request cwd) auto-allows.
280+
const asked: string[] = [];
281+
const gate = createPermissionGate({
282+
cwd,
283+
interactive: true,
284+
skipPermissions: false,
285+
reactorGated: false,
286+
requestApproval: async (request: PermissionRequest) => {
287+
asked.push(`${request.tool}:${request.subject}`);
288+
return { allow: false };
289+
},
290+
approvals: await loadSeededApprovals(cwd, generateSessionId(), home),
291+
});
292+
const NPM_TEST = {
293+
id: "npm-test",
294+
name: "run_shell",
295+
arguments: { command: "npm test" },
296+
} as const;
297+
expect((await gate.evaluate({ ...NPM_TEST })).allowed).toBe(false);
298+
expect(
299+
(
300+
await runWithSubAgentIdentity(
301+
{ description: "other", cwd: other },
302+
() => gate.evaluate({ ...NPM_TEST }),
303+
)
304+
).allowed,
305+
).toBe(false);
306+
expect(asked).toEqual(["run_shell:npm test", "run_shell:npm test"]);
307+
});
209308
});

src/permission/store.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ function parseApprovalList(raw: unknown): Approval[] {
6868
}
6969

7070
function sameApproval(a: Approval, b: Approval): boolean {
71+
// Removal equality deliberately ignores cwd: revocation targets arrive
72+
// cwd-less (see admin.ts toApproval), so a strict comparison would silently
73+
// keep a confined twin live. Removing the file entry is the revocation;
74+
// the next load's reconcile prunes the cwd-bound fingerprint with it.
75+
return (
76+
a.tool === b.tool &&
77+
a.pattern === b.pattern &&
78+
a.providerModel === b.providerModel
79+
);
80+
}
81+
82+
// Equality on every confirmed dimension except cwd: a planted file entry and
83+
// the gate's minted confirmation of it differ only in cwd.
84+
function sameGrantModuloCwd(a: Approval, b: Approval): boolean {
7185
return (
7286
a.tool === b.tool &&
7387
a.pattern === b.pattern &&
@@ -192,7 +206,19 @@ export async function saveProjectApproval(
192206
): Promise<void> {
193207
await chainObjectWrite(projectStorePath(cwd), (current) => ({
194208
...current,
195-
approvals: [...parseApprovalList(current.approvals), approval],
209+
approvals: [
210+
// A planted entry carries no cwd; confirming it through the pending flow
211+
// mints {tool, pattern, cwd} and writes that shape back here. Displace
212+
// its twin instead of stacking a duplicate that would linger as pending
213+
// forever — the dropped twin never applied, so nothing confirmed is
214+
// lost. A save without cwd keeps the plain append path and never
215+
// displaces a confined entry.
216+
...parseApprovalList(current.approvals).filter(
217+
(entry) =>
218+
approval.cwd === undefined || !sameGrantModuloCwd(entry, approval),
219+
),
220+
approval,
221+
],
196222
}));
197223
// The only production writer is the interactive grant path (an operator
198224
// answering a prompt with a project-scope persist), so writing an entry is

src/trust/project-trust.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,25 +325,38 @@ export async function trustMcpServer(
325325
* Stable fingerprint for one project-approval entry: tool + pattern, with the
326326
* provider-model binding folded in when set, so switching models invalidates a
327327
* prior confirmation exactly the way the gate's providerModel check does.
328-
* The fingerprint deliberately excludes the file's cwd — the trust record is
329-
* already keyed per project (realpath-keyed filename plus the repo guard).
328+
* The entry's cwd is folded in too (absent → ""): Approval has four enforced
329+
* dimensions and a cwd-less grant matches any request cwd (see
330+
* cwdMatchesGrant), so a fingerprint that ignored cwd would let a hand-edit
331+
* dropping `cwd` from a confirmed entry keep its confirmation and silently
332+
* widen a repo-confined grant to cross-repo. A cwd-less planted entry still
333+
* fingerprints the same way at trust and load time, so confirming it through
334+
* the pending flow matches the minted shape (saveProjectApproval converges
335+
* the file to that shape on write).
330336
*/
331337
export function projectGrantFingerprint(approval: {
332338
tool: string;
333339
pattern: string;
334340
providerModel?: string;
341+
cwd?: string;
335342
}): string {
336343
const payload = JSON.stringify({
337344
tool: approval.tool,
338345
pattern: approval.pattern,
339346
providerModel: approval.providerModel ?? "",
347+
cwd: approval.cwd ?? "",
340348
});
341349
return createHash("sha256").update(payload).digest("hex");
342350
}
343351

344352
export function isProjectGrantTrusted(
345353
store: ProjectTrustStore,
346-
approval: { tool: string; pattern: string; providerModel?: string },
354+
approval: {
355+
tool: string;
356+
pattern: string;
357+
providerModel?: string;
358+
cwd?: string;
359+
},
347360
): boolean {
348361
return store.trustedGrantFingerprints.includes(
349362
projectGrantFingerprint(approval),
@@ -362,7 +375,12 @@ export function isProjectGrantTrusted(
362375
*/
363376
export async function trustProjectGrants(
364377
cwd: string,
365-
approvals: { tool: string; pattern: string; providerModel?: string }[],
378+
approvals: {
379+
tool: string;
380+
pattern: string;
381+
providerModel?: string;
382+
cwd?: string;
383+
}[],
366384
home: string = homedir(),
367385
): Promise<ProjectTrustStore> {
368386
const fps = approvals.map(projectGrantFingerprint);
@@ -385,7 +403,12 @@ export async function trustProjectGrants(
385403
/** Drop confirmations for removed entries so a replanted file re-surfaces. */
386404
export async function untrustProjectGrants(
387405
cwd: string,
388-
approvals: { tool: string; pattern: string; providerModel?: string }[],
406+
approvals: {
407+
tool: string;
408+
pattern: string;
409+
providerModel?: string;
410+
cwd?: string;
411+
}[],
389412
home: string = homedir(),
390413
): Promise<ProjectTrustStore> {
391414
const fps = new Set(approvals.map(projectGrantFingerprint));
@@ -411,7 +434,12 @@ export async function untrustProjectGrants(
411434
*/
412435
export async function reconcileProjectGrants(
413436
cwd: string,
414-
onDisk: { tool: string; pattern: string; providerModel?: string }[],
437+
onDisk: {
438+
tool: string;
439+
pattern: string;
440+
providerModel?: string;
441+
cwd?: string;
442+
}[],
415443
home: string = homedir(),
416444
): Promise<ProjectTrustStore> {
417445
const live = new Set(onDisk.map(projectGrantFingerprint));

0 commit comments

Comments
 (0)