Skip to content

Commit c896243

Browse files
committed
Apply the same secret and restricted path guards during reconciliation
A queued shell request could auto-drain from a newly minted grant even when it referenced a secret path or a restricted target, because reconciliation only matched the grant pattern and skipped the guards evaluate() enforces first. Requests like a queued .env read now stay queued and prompt normally when a broader grant is minted.
1 parent eb805d7 commit c896243

2 files changed

Lines changed: 76 additions & 1 deletion

File tree

src/permission/gate.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ function hasExactFullCommandGrant(
6464
// Pure reconciliation check used to re-evaluate the TUI's pending approval
6565
// queue against a single newly-minted grant (see PermissionGateOptions.onGrant).
6666
// A queued request is covered only when this one grant, by itself, would have
67-
// let it skip the prompt — mirrors the matching evaluate() itself applies, so
67+
// let it skip the prompt AND the request clears the same secret-path and
68+
// restricted-path guards evaluate() enforces ahead of grant matching — so
6869
// reconciliation never auto-approves something evaluate() would still ask for.
6970
export function isRequestCoveredByGrant(
7071
request: PermissionRequest,
@@ -84,6 +85,10 @@ export function isRequestCoveredByGrant(
8485
}
8586
const segments = splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s));
8687
if (segments.length === 0) return false;
88+
const resolvedCwd = request.cwd ?? process.cwd();
89+
const isRestricted = createPathRestriction(resolvedCwd, createWorktreeRootsProvider(resolvedCwd)).isRestricted;
90+
if (segments.some((segment) => commandReferencesSensitivePath(segment) !== undefined)) return false;
91+
if (segments.some((segment) => commandTargetsRestricted(segment, isRestricted))) return false;
8792
if (segments.length > 1) {
8893
return approval.pattern === stripCommentLines(request.subject).trim();
8994
}

src/tui/hooks/use-gates.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,74 @@ describe("useGates queue reconciliation", () => {
187187
await new Promise((r) => setTimeout(r, 0));
188188
unmount();
189189
});
190+
191+
test("a secret-path request stays queued after a covering grant mints", async () => {
192+
const emitter = new EventEmitter();
193+
const controllerRef: { current: GateController | null } = { current: null };
194+
const element = () => createElement(Harness, { emitter, controllerRef });
195+
const { rerender, unmount } = render(element());
196+
197+
const outcomes: ApprovalOutcome[] = [];
198+
enqueuePermission(emitter, request({ subject: "cat .env" })).then((o) => outcomes.push(o));
199+
rerender(element());
200+
201+
grant(emitter, { tool: "run_shell", pattern: "cat *" });
202+
rerender(element());
203+
await new Promise((r) => setTimeout(r, 0));
204+
rerender(element());
205+
206+
expect(controllerRef.current!.permissionQueueDepth).toBe(1);
207+
expect(outcomes).toHaveLength(0);
208+
209+
controllerRef.current!.resetGates();
210+
await new Promise((r) => setTimeout(r, 0));
211+
unmount();
212+
});
213+
214+
test("a restricted-path request stays queued after a covering grant mints", async () => {
215+
const emitter = new EventEmitter();
216+
const controllerRef: { current: GateController | null } = { current: null };
217+
const element = () => createElement(Harness, { emitter, controllerRef });
218+
const { rerender, unmount } = render(element());
219+
220+
const outcomes: ApprovalOutcome[] = [];
221+
enqueuePermission(emitter, request({ subject: "cat ../../etc/passwd" })).then((o) =>
222+
outcomes.push(o),
223+
);
224+
rerender(element());
225+
226+
grant(emitter, { tool: "run_shell", pattern: "cat *" });
227+
rerender(element());
228+
await new Promise((r) => setTimeout(r, 0));
229+
rerender(element());
230+
231+
expect(controllerRef.current!.permissionQueueDepth).toBe(1);
232+
expect(outcomes).toHaveLength(0);
233+
234+
controllerRef.current!.resetGates();
235+
await new Promise((r) => setTimeout(r, 0));
236+
unmount();
237+
});
238+
239+
test("a plain covered request still auto-drains despite the new guards", async () => {
240+
const emitter = new EventEmitter();
241+
const controllerRef: { current: GateController | null } = { current: null };
242+
const element = () => createElement(Harness, { emitter, controllerRef });
243+
const { rerender, unmount } = render(element());
244+
245+
const outcomes: ApprovalOutcome[] = [];
246+
enqueuePermission(emitter, request({ subject: "cat README.md" })).then((o) => outcomes.push(o));
247+
rerender(element());
248+
249+
grant(emitter, { tool: "run_shell", pattern: "cat *" });
250+
rerender(element());
251+
await new Promise((r) => setTimeout(r, 0));
252+
rerender(element());
253+
254+
expect(controllerRef.current!.permissionQueueDepth).toBe(0);
255+
expect(outcomes).toHaveLength(1);
256+
expect(outcomes[0]!.allow).toBe(true);
257+
258+
unmount();
259+
});
190260
});

0 commit comments

Comments
 (0)