Skip to content

Commit 8fdc917

Browse files
Require operator confirmation for project approvals file grants (#935)
* Require operator confirmation for project approvals file grants A repo-committed .corbits/permissions.json previously seeded the permission gate with zero operator involvement, so a planted file could silently auto-allow destructive calls such as force-pushes and file writes. Gate loadProjectApprovals behind per-entry grant fingerprints stored in the existing project-trust record: an untrusted directory now contributes zero approvals, trust stays keyed by realpath, and the first encounter with an unconfirmed file surfaces what it would grant instead of dropping it silently. Project trust never implies grant trust — each entry needs its own confirmation, recorded when the operator persists a project-scope grant. Covers CL-7782. * Prune stale project grant fingerprints on load Hand-removing an entry from the project approvals file left its trust fingerprint behind, so a byte-identical replant applied silently. Loaders now reconcile trust against the entries on disk. * Bind project grant confirmations to the confirmed cwd
1 parent 6b219fa commit 8fdc917

14 files changed

Lines changed: 688 additions & 36 deletions

src/agent/tools.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,7 @@ export async function createAgentToolset(
743743
let mcpTrustStore: ProjectTrustStore = projectTrust ?? {
744744
trustedPluginPaths: [],
745745
trustedMcpFingerprints: [],
746+
trustedGrantFingerprints: [],
746747
};
747748
const untrustedLocalError = `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`;
748749

src/config.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,11 @@ describe("loadConfig", () => {
363363
filterMcpServersForConnect(servers, {
364364
source: "local",
365365
cwd: "/repo/without-trust-grant",
366-
store: { trustedPluginPaths: [], trustedMcpFingerprints: [] },
366+
store: {
367+
trustedPluginPaths: [],
368+
trustedMcpFingerprints: [],
369+
trustedGrantFingerprints: [],
370+
},
367371
}),
368372
).resolves.toEqual([BUILTIN_EXA_MCP]);
369373
});

src/exec/runner.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,9 @@ export async function runExec(config: Config): Promise<ExecResult> {
568568
onPersistNotice: (text) => {
569569
stderr.write(`${text}\n`);
570570
},
571+
onPendingProjectGrants: (text) => {
572+
stderr.write(`${text}\n`);
573+
},
571574
interactive,
572575
skipPermissions: config.dangerouslySkipPermissions,
573576
auto: config.auto,
Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { mkdir, mkdtemp, realpath, symlink, writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import { generateSessionId } from "../session/index.js";
6+
import { loadSeededApprovals } from "../session/runtime-assembly.js";
7+
import { runWithSubAgentIdentity } from "../subagent/identity-context.js";
8+
import { trustProjectGrants } from "../trust/project-trust.js";
9+
import { createPermissionGate } from "./gate.js";
10+
import type { Approval, PermissionRequest } from "./types.js";
11+
import {
12+
formatPendingProjectApprovals,
13+
loadPendingProjectApprovals,
14+
loadProjectApprovals,
15+
saveProjectApproval,
16+
} from "./store.js";
17+
18+
const PLANTED = [
19+
{ tool: "run_shell", pattern: "git push --force origin main" },
20+
{ tool: "write_file", pattern: "*.ts" },
21+
] as const;
22+
23+
async function plantProjectApprovals(
24+
cwd: string,
25+
entries: readonly unknown[] = PLANTED,
26+
): Promise<void> {
27+
const dir = join(cwd, ".corbits");
28+
await mkdir(dir, { recursive: true });
29+
await writeFile(
30+
join(dir, "permissions.json"),
31+
JSON.stringify({ version: 1, approvals: entries }),
32+
);
33+
}
34+
35+
async function driveGate(cwd: string, sessionId: string, home: string) {
36+
const asked: string[] = [];
37+
const gate = createPermissionGate({
38+
cwd,
39+
interactive: true,
40+
skipPermissions: false,
41+
reactorGated: false,
42+
requestApproval: async (request: PermissionRequest) => {
43+
asked.push(`${request.tool}:${request.subject}`);
44+
return { allow: true };
45+
},
46+
approvals: await loadSeededApprovals(cwd, sessionId, home),
47+
});
48+
return { gate, asked };
49+
}
50+
51+
const PUSH = {
52+
id: "push",
53+
name: "run_shell",
54+
arguments: { command: "git push --force origin main" },
55+
} as const;
56+
57+
const WRITE = {
58+
id: "write",
59+
name: "write_file",
60+
arguments: { path: "notes.ts" },
61+
} as const;
62+
63+
describe("CL-7782: project approvals require grant trust", () => {
64+
test("untrusted directory contributes zero project approvals through the real gate", async () => {
65+
const base = await mkdtemp(join(tmpdir(), "cl-7782-untrusted-"));
66+
const home = join(base, "home");
67+
const cwd = join(base, "repo");
68+
await plantProjectApprovals(cwd);
69+
const sessionId = generateSessionId();
70+
71+
expect(await loadProjectApprovals(cwd, home)).toEqual([]);
72+
const seeded = await loadSeededApprovals(cwd, sessionId, home);
73+
for (const entry of PLANTED) {
74+
expect(
75+
seeded.some(
76+
(approval) =>
77+
approval.tool === entry.tool && approval.pattern === entry.pattern,
78+
),
79+
).toBe(false);
80+
}
81+
82+
const { gate, asked } = await driveGate(cwd, sessionId, home);
83+
expect((await gate.evaluate({ ...PUSH })).allowed).toBe(true);
84+
expect((await gate.evaluate({ ...WRITE })).allowed).toBe(true);
85+
expect(asked).toEqual([
86+
"run_shell:git push --force origin main",
87+
"write_file:notes.ts",
88+
]);
89+
});
90+
91+
test("trusted and confirmed directory grants apply without asking", async () => {
92+
const base = await mkdtemp(join(tmpdir(), "cl-7782-trusted-"));
93+
const home = join(base, "home");
94+
const cwd = join(base, "repo");
95+
await plantProjectApprovals(cwd);
96+
await trustProjectGrants(cwd, [...PLANTED], home);
97+
const sessionId = generateSessionId();
98+
99+
expect(await loadProjectApprovals(cwd, home)).toHaveLength(2);
100+
101+
const { gate, asked } = await driveGate(cwd, sessionId, home);
102+
expect((await gate.evaluate({ ...PUSH })).allowed).toBe(true);
103+
expect((await gate.evaluate({ ...WRITE })).allowed).toBe(true);
104+
expect(asked).toEqual([]);
105+
});
106+
107+
test("trust is keyed by realpath: a symlinked checkout cannot inherit or confer", async () => {
108+
const base = await mkdtemp(join(tmpdir(), "cl-7782-link-"));
109+
const home = join(base, "home");
110+
const target = join(base, "real-repo");
111+
await plantProjectApprovals(target);
112+
const link = join(base, "linked-repo");
113+
await symlink(target, link);
114+
expect(await realpath(link)).toBe(await realpath(target));
115+
116+
await trustProjectGrants(link, [...PLANTED], home);
117+
expect(await loadProjectApprovals(target, home)).toHaveLength(2);
118+
119+
const twin = join(base, "twin-repo");
120+
await plantProjectApprovals(twin);
121+
expect(await loadProjectApprovals(twin, home)).toEqual([]);
122+
});
123+
124+
test("first encounter surfaces the would-be grants instead of dropping them silently", async () => {
125+
const base = await mkdtemp(join(tmpdir(), "cl-7782-pending-"));
126+
const home = join(base, "home");
127+
const cwd = join(base, "repo");
128+
await plantProjectApprovals(cwd);
129+
130+
const pending = await loadPendingProjectApprovals(cwd, home);
131+
expect(pending).toHaveLength(2);
132+
const notice = formatPendingProjectApprovals(pending);
133+
for (const entry of PLANTED) {
134+
expect(notice).toContain(entry.pattern);
135+
}
136+
137+
await trustProjectGrants(cwd, [...PLANTED], home);
138+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]);
139+
expect(formatPendingProjectApprovals([])).toBe("");
140+
});
141+
142+
test("hand-removing an entry revokes its confirmation: a byte-identical replant re-surfaces", async () => {
143+
const base = await mkdtemp(join(tmpdir(), "cl-7782-replant-"));
144+
const home = join(base, "home");
145+
const cwd = join(base, "repo");
146+
await plantProjectApprovals(cwd);
147+
await trustProjectGrants(cwd, [...PLANTED], home);
148+
expect(await loadProjectApprovals(cwd, home)).toHaveLength(2);
149+
150+
// Hand-edit the first entry out of the file without removeProjectApproval.
151+
await plantProjectApprovals(cwd, [PLANTED[1]]);
152+
expect(await loadProjectApprovals(cwd, home)).toEqual([{ ...PLANTED[1] }]);
153+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]);
154+
155+
// Replant the identical bytes: the removed entry surfaces, not applies.
156+
await plantProjectApprovals(cwd);
157+
expect(await loadProjectApprovals(cwd, home)).toEqual([{ ...PLANTED[1] }]);
158+
expect(await loadPendingProjectApprovals(cwd, home)).toEqual([
159+
{ ...PLANTED[0] },
160+
]);
161+
});
162+
163+
test("a gate-minted project grant ({tool, pattern, cwd}) survives save → reload and still applies", async () => {
164+
const base = await mkdtemp(join(tmpdir(), "cl-7782-cwd-"));
165+
const home = join(base, "home");
166+
const cwd = join(base, "repo");
167+
168+
// Mint through the real gate so the entry has the production shape.
169+
const minted: Approval[] = [];
170+
const mintGate = createPermissionGate({
171+
cwd,
172+
interactive: true,
173+
skipPermissions: false,
174+
reactorGated: false,
175+
requestApproval: async () => ({
176+
allow: true,
177+
persist: {
178+
id: "exact",
179+
label: "Always allow",
180+
pattern: "npm test",
181+
grant: "project",
182+
},
183+
}),
184+
persist: (approval, scope) => {
185+
expect(scope).toBe("project");
186+
minted.push(approval);
187+
},
188+
approvals: await loadSeededApprovals(cwd, generateSessionId(), home),
189+
});
190+
const NPM_TEST = {
191+
id: "npm-test",
192+
name: "run_shell",
193+
arguments: { command: "npm test" },
194+
} as const;
195+
expect((await mintGate.evaluate({ ...NPM_TEST })).allowed).toBe(true);
196+
expect(minted).toEqual([{ tool: "run_shell", pattern: "npm test", cwd }]);
197+
198+
// Production persist path, then reload: the cwd must round-trip, not strip.
199+
const grant = minted[0];
200+
if (grant === undefined) throw new Error("gate minted no project grant");
201+
await saveProjectApproval(cwd, grant, home);
202+
const reloaded = await loadProjectApprovals(cwd, home);
203+
expect(reloaded).toEqual(minted);
204+
205+
// The reloaded entry still applies: a fresh seeded gate asks nothing.
206+
const { gate, asked } = await driveGate(cwd, generateSessionId(), home);
207+
expect((await gate.evaluate({ ...NPM_TEST })).allowed).toBe(true);
208+
expect(asked).toEqual([]);
209+
});
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+
});
308+
});

0 commit comments

Comments
 (0)