Skip to content

Commit cdc80d4

Browse files
committed
fix(permission): bind ask to its denial, expire open, consume once
Ask names its denial via grant_request_id; unnamed keeps first-pending. Expiry yields a fresh gate round that mints a fresh envelope. Precheck-to-consume serializes per fingerprint.
1 parent 40c8bfd commit cdc80d4

9 files changed

Lines changed: 368 additions & 47 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ The interactive TUI may offer an explicit alternate-provider/model selector only
103103

104104
### Worker permission grants
105105

106-
A worker deny-on-ask registers a harness-owned denied-call envelope (`src/permission/worker-grant.ts`): the exact denied ToolCall plus a stable path-aware fingerprint, keyed by worker session with a single-parent-turn expiry. The deny reason names only the envelope's requestId; the worker's `ask_director` prose carries no authority and plain `send_input` text can never mint, consume, or extend an envelope. The parent observes the envelope on the ask record, replays the exact call through its own gate operator path, and retries the retained worker via the existing `resume_agent` with exact args plus the questionId ref. The first covering-grant retry consumes the envelope; replays, tampered args/cwd/tool/session, expiry, decline, or interrupt fail closed with a truthful blocker. There is no dedicated grant verb — single-use and exactness are enforced by the envelope sidecar, not by tool plumbing.
106+
A worker deny-on-ask registers a harness-owned denied-call envelope (`src/permission/worker-grant.ts`): the exact denied ToolCall plus a stable path-aware fingerprint, keyed by worker session with a ten-minute wall-clock TTL. The `turnId` on the envelope is metadata only — no turn enforcement exists, so expiry is purely `expiresAt`-driven. The deny reason names only the envelope's requestId; the worker's `ask_director` prose carries no authority and plain `send_input` text can never mint, consume, or extend an envelope. The worker quotes that requestId back as `grant_request_id`, binding the ask to its own denial at register time; an unnamed ask keeps the legacy first-pending bind. The parent observes the envelope on the ask record via the session-store subscription, replays the exact call through its own gate operator path, and retries the retained worker via the existing `resume_agent` with exact args plus the questionId ref. The first covering-grant retry consumes the envelope; replays, tampered args/cwd/tool/session, decline, or interrupt fail closed with a truthful blocker. Expiry deliberately does not fail closed: the lapsed envelope is marked, skipped, and the retry falls through to a fresh gate round that denies fresh with a new grant id, so a lapsed window can never strand the worker. Expiry is lazy — every pendency read sweeps overdue envelopes, and no periodic scheduler exists by decision, not omission. Concurrent identical retries serialize on a per-fingerprint mutex across the precheck-to-consume gap, so one envelope allows exactly once. There is no dedicated grant verb — single-use and exactness are enforced by the envelope sidecar, not by tool plumbing. Phase 2 (by design, not here): a broad parent session grant can cover more than the exact denied subject; binding the grant to the envelope args needs the atomic grant-and-retry verb.
107107

108108
### TUI Runner (`src/tui/runner/`)
109109

‎src/permission/reactor-authorize.ts‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type { AuthorizeVerdict, GateVerdict, PermissionGate } from "./gate.js";
2323
import type { PermissionRequest } from "./types.js";
2424
import {
2525
createDeniedCallEnvelope,
26+
fingerprintDeniedCall,
2627
formatWorkerDenyWithGrantId,
2728
getProcessWorkerGrantStore,
2829
type WorkerDeniedCallEnvelope,
@@ -171,6 +172,17 @@ function denyWorkerCallWithEnvelope(
171172
};
172173
}
173174

175+
/** Mutex key covering the envelope match: concurrent identical worker calls
176+
* serialize through precheck-to-consume so one envelope allows exactly once. */
177+
function workerGrantKey(
178+
sessionId: string,
179+
call: ToolCallType,
180+
cwd: string,
181+
): string {
182+
const identity = workerCallIdentity(sessionId, call, cwd);
183+
return `${sessionId}\n${fingerprintDeniedCall(identity.canonicalTool, identity.args, identity.cwd)}`;
184+
}
185+
174186
async function authorizeWorkerCall(
175187
gate: PermissionGate,
176188
call: ToolCallType,
@@ -180,18 +192,36 @@ async function authorizeWorkerCall(
180192
if (isWorkerControlPlaneTool(call.name)) return { effect: "allow" };
181193
const workerCwd = resolveWorkerCwd(cwd);
182194
const sessionId = resolveWorkerSessionId(grantOptions?.sessionId);
183-
if (sessionId !== undefined) {
184-
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
185-
const precheck = store.precheck(
186-
workerCallIdentity(sessionId, call, workerCwd),
195+
if (sessionId === undefined)
196+
return authorizeWorkerCallInner(gate, call, grantOptions, workerCwd);
197+
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
198+
return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () =>
199+
authorizeWorkerCallInner(gate, call, grantOptions, workerCwd, {
200+
sessionId,
201+
store,
202+
}),
203+
);
204+
}
205+
206+
async function authorizeWorkerCallInner(
207+
gate: PermissionGate,
208+
call: ToolCallType,
209+
grantOptions: WorkerGrantOptions | undefined,
210+
workerCwd: string,
211+
grant?: { sessionId: string; store: WorkerGrantStore },
212+
): Promise<{ effect: "allow" } | { effect: "deny"; reason: string }> {
213+
if (grant !== undefined) {
214+
const precheck = grant.store.precheck(
215+
workerCallIdentity(grant.sessionId, call, workerCwd),
187216
);
188217
if (!precheck.ok) return { effect: "deny", reason: precheck.blocker };
189218
}
190219
const verdict = await gate.authorizeCall(call);
191220
if (verdict.effect === "allow") {
192-
if (sessionId !== undefined) {
193-
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
194-
store.consumeOnAllow(workerCallIdentity(sessionId, call, workerCwd));
221+
if (grant !== undefined) {
222+
grant.store.consumeOnAllow(
223+
workerCallIdentity(grant.sessionId, call, workerCwd),
224+
);
195225
}
196226
return verdict;
197227
}
@@ -224,18 +254,36 @@ async function executionVerdictWorkerCall(
224254
if (isWorkerControlPlaneTool(call.name)) return { effect: "allow" };
225255
const workerCwd = resolveWorkerCwd(cwd);
226256
const sessionId = resolveWorkerSessionId(grantOptions?.sessionId);
227-
if (sessionId !== undefined) {
228-
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
229-
const precheck = store.precheck(
230-
workerCallIdentity(sessionId, call, workerCwd),
257+
if (sessionId === undefined)
258+
return executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd);
259+
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
260+
return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () =>
261+
executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd, {
262+
sessionId,
263+
store,
264+
}),
265+
);
266+
}
267+
268+
async function executionVerdictWorkerCallInner(
269+
gate: PermissionGate,
270+
call: ToolCallType,
271+
grantOptions: WorkerGrantOptions | undefined,
272+
workerCwd: string,
273+
grant?: { sessionId: string; store: WorkerGrantStore },
274+
): Promise<AuthorizeVerdict> {
275+
if (grant !== undefined) {
276+
const precheck = grant.store.precheck(
277+
workerCallIdentity(grant.sessionId, call, workerCwd),
231278
);
232279
if (!precheck.ok) return { effect: "deny", reason: precheck.blocker };
233280
}
234281
const verdict = await gate.executionVerdict(call);
235282
if (verdict.effect === "allow") {
236-
if (sessionId !== undefined) {
237-
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
238-
store.consumeOnAllow(workerCallIdentity(sessionId, call, workerCwd));
283+
if (grant !== undefined) {
284+
grant.store.consumeOnAllow(
285+
workerCallIdentity(grant.sessionId, call, workerCwd),
286+
);
239287
}
240288
return verdict;
241289
}

‎src/permission/worker-grant-flow.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { ToolCall } from "@intx/types/runtime";
66
import { createPermissionGate } from "./gate.js";
77
import { workerPermissionGate } from "./reactor-authorize.js";
88
import {
9+
WORKER_GRANT_TTL_MS,
910
WorkerGrantStore,
1011
createDeniedCallEnvelope,
1112
getProcessWorkerGrantStore,
@@ -206,4 +207,70 @@ describe("worker grant-request flow: deny → parent replay grant → one retry"
206207
expect(store.peek(envelope.requestId)?.status).toBe("pending");
207208
store.expireSession(session.id, "test teardown");
208209
});
210+
211+
test("concurrent identical retries allow exactly once", async () => {
212+
const cwd = mkdtempSync(join(tmpdir(), "worker-grant-race-"));
213+
const parentGate = makeParentGate(cwd, true);
214+
const store = new WorkerGrantStore();
215+
const workerGate = workerPermissionGate(parentGate, {
216+
sessionId: "worker-race",
217+
store,
218+
});
219+
220+
const denied = await workerGate.authorizeCall(shellCall("c1", COMMAND));
221+
if (denied.effect !== "deny") throw new Error("expected worker deny");
222+
const requestId = extractRequestId(denied.reason);
223+
expect(await parentGate.evaluate(shellCall("parent-1", COMMAND))).toEqual({
224+
allowed: true,
225+
});
226+
227+
// Two in-flight copies of the exact call (fresh call ids, same args):
228+
// without serialization both pass precheck before either consumes and
229+
// the envelope allows twice.
230+
const [retryA, retryB] = await Promise.all([
231+
workerGate.authorizeCall(shellCall("c2", COMMAND)),
232+
workerGate.authorizeCall(shellCall("c3", COMMAND)),
233+
]);
234+
const effects = [retryA.effect, retryB.effect].sort();
235+
expect(effects).toEqual(["allow", "deny"]);
236+
const loser = retryA.effect === "deny" ? retryA : retryB;
237+
if (loser.effect !== "deny") throw new Error("expected replay deny");
238+
expect(loser.reason).toContain("already consumed");
239+
expect(loser.reason).toContain(requestId);
240+
expect(store.peek(requestId)?.status).toBe("consumed");
241+
});
242+
243+
test("post-expiry retry round mints a fresh envelope, then succeeds", async () => {
244+
const cwd = mkdtempSync(join(tmpdir(), "worker-grant-reissue-"));
245+
const parentGate = makeParentGate(cwd, true);
246+
const store = new WorkerGrantStore();
247+
const workerGate = workerPermissionGate(parentGate, {
248+
sessionId: "worker-reissue",
249+
store,
250+
});
251+
252+
const denied = await workerGate.authorizeCall(shellCall("c1", COMMAND));
253+
if (denied.effect !== "deny") throw new Error("expected worker deny");
254+
const lapsedId = extractRequestId(denied.reason);
255+
expect(store.sweepExpired(Date.now() + WORKER_GRANT_TTL_MS + 1)).toBe(1);
256+
expect(store.peek(lapsedId)?.status).toBe("expired");
257+
258+
// The lapsed window is not a blackhole: the retry falls through to the
259+
// gate, which denies fresh with a NEW grant id the worker can ask out of.
260+
const reissue = await workerGate.authorizeCall(shellCall("c2", COMMAND));
261+
expect(reissue.effect).toBe("deny");
262+
if (reissue.effect !== "deny") throw new Error("expected re-issue deny");
263+
const freshId = extractRequestId(reissue.reason);
264+
expect(freshId).not.toBe(lapsedId);
265+
expect(store.peek(freshId)?.status).toBe("pending");
266+
expect(store.peek(lapsedId)?.status).toBe("expired");
267+
268+
expect(await parentGate.evaluate(shellCall("parent-1", COMMAND))).toEqual({
269+
allowed: true,
270+
});
271+
expect(await workerGate.authorizeCall(shellCall("c3", COMMAND))).toEqual({
272+
effect: "allow",
273+
});
274+
expect(store.peek(freshId)?.status).toBe("consumed");
275+
});
209276
});

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

Lines changed: 132 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,27 @@ describe("WorkerGrantStore lifecycle", () => {
196196
expect(result.blocker).toContain("worker-1");
197197
});
198198

199-
test("expiry fails closed, lazily and via sweep", () => {
199+
test("expiry yields a fresh gate round, never a blackhole", () => {
200200
const store = new WorkerGrantStore();
201201
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
202202
const after = envelope.expiresAt + 1;
203-
const result = store.precheck({
204-
sessionId: "worker-1",
205-
canonicalTool: "run_shell",
206-
args: { ...ARGS },
207-
cwd: CWD,
208-
now: after,
209-
});
210-
expect(result.ok).toBe(false);
211-
if (result.ok) throw new Error("expected blocker");
212-
expect(result.blocker).toContain("expired");
203+
// The lapsed window passes through instead of denying: the retry falls to
204+
// the gate, which denies fresh and mints a fresh envelope the worker can
205+
// ask out of.
206+
expect(
207+
store.precheck({
208+
sessionId: "worker-1",
209+
canonicalTool: "run_shell",
210+
args: { ...ARGS },
211+
cwd: CWD,
212+
now: after,
213+
}),
214+
).toEqual({ ok: true });
213215
expect(envelope.status).toBe("expired");
216+
expect(envelope.audit.map((event) => event.event)).toEqual([
217+
"denied",
218+
"expired",
219+
]);
214220

215221
const second = store.register(
216222
createDeniedCallEnvelope(descriptor({ callId: "call-2" })),
@@ -219,6 +225,53 @@ describe("WorkerGrantStore lifecycle", () => {
219225
expect(second.status).toBe("expired");
220226
});
221227

228+
test("post-expiry re-issue mints a fresh envelope for the same exact call", () => {
229+
const store = new WorkerGrantStore();
230+
const first = store.register(createDeniedCallEnvelope(descriptor()));
231+
const after = first.expiresAt + 1;
232+
expect(store.sweepExpired(after)).toBe(1);
233+
expect(first.status).toBe("expired");
234+
// The retry round denies fresh: a new envelope with a new grant id.
235+
const reissue = store.register(
236+
createDeniedCallEnvelope(
237+
descriptor({ callId: "call-2", now: after + 1_000 }),
238+
),
239+
);
240+
expect(reissue.requestId).not.toBe(first.requestId);
241+
expect(reissue.status).toBe("pending");
242+
const identity = {
243+
sessionId: "worker-1",
244+
canonicalTool: "run_shell",
245+
args: { ...ARGS },
246+
cwd: CWD,
247+
now: after + 1_001,
248+
};
249+
expect(store.precheck(identity)).toEqual({ ok: true });
250+
expect(store.consumeOnAllow(identity)?.requestId).toBe(reissue.requestId);
251+
expect(reissue.status).toBe("consumed");
252+
expect(first.status).toBe("expired");
253+
});
254+
255+
test("read sites never surface an expired denial without an explicit sweep", () => {
256+
const store = new WorkerGrantStore();
257+
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
258+
const after = envelope.expiresAt + 1;
259+
// No sweepExpired call here: each read enforces the TTL itself.
260+
expect(
261+
store.pendingMatch("worker-1", "run_shell", { ...ARGS }, CWD, after),
262+
).toBeUndefined();
263+
expect(
264+
store.attachToAsk("worker-1", "ask-late", undefined, after),
265+
).toBeUndefined();
266+
expect(store.pendingForSession("worker-1", after)).toBeUndefined();
267+
expect(envelope.status).toBe("expired");
268+
expect(envelope.audit.map((event) => event.event)).toEqual([
269+
"denied",
270+
"expired",
271+
]);
272+
expect(envelope.questionId).toBeUndefined();
273+
});
274+
222275
test("decline fails closed; headless declineAllForSession covers the session", () => {
223276
const store = new WorkerGrantStore();
224277
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
@@ -258,11 +311,17 @@ describe("WorkerGrantStore lifecycle", () => {
258311
test("attachToAsk stamps the questionId and keeps pending", () => {
259312
const store = new WorkerGrantStore();
260313
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
261-
expect(store.attachToAsk("worker-1", "ask-7")).toBe(envelope);
314+
// Reads pin the envelope's clock: the fixture now (1_000_000) predates
315+
// wall time, so an unpinned read would sweep it as expired.
316+
expect(store.attachToAsk("worker-1", "ask-7", undefined, 1_000_001)).toBe(
317+
envelope,
318+
);
262319
expect(envelope.questionId).toBe("ask-7");
263320
expect(envelope.status).toBe("pending");
264321
expect(store.byQuestion("ask-7")).toBe(envelope);
265-
expect(store.attachToAsk("worker-9", "ask-8")).toBeUndefined();
322+
expect(
323+
store.attachToAsk("worker-9", "ask-8", undefined, 1_000_001),
324+
).toBeUndefined();
266325
expect(envelope.audit.map((event) => event.event)).toEqual([
267326
"denied",
268327
"asked",
@@ -272,7 +331,7 @@ describe("WorkerGrantStore lifecycle", () => {
272331
test("audit trail orders deny → ask → consume", () => {
273332
const store = new WorkerGrantStore();
274333
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
275-
store.attachToAsk("worker-1", "ask-1");
334+
store.attachToAsk("worker-1", "ask-1", undefined, 1_000_001);
276335
store.consumeOnAllow({
277336
sessionId: "worker-1",
278337
canonicalTool: "run_shell",
@@ -315,10 +374,68 @@ describe("buildRetryMessage", () => {
315374
test("carries exact args + questionId ref from the envelope", () => {
316375
const store = new WorkerGrantStore();
317376
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
318-
store.attachToAsk("worker-1", "ask-3");
377+
store.attachToAsk("worker-1", "ask-3", undefined, 1_000_001);
319378
const message = buildRetryMessage(envelope);
320379
expect(message).toContain(envelope.requestId);
321380
expect(message).toContain("ask-3");
322381
expect(message).toContain(JSON.stringify(ARGS));
323382
});
324383
});
384+
385+
describe("WorkerGrantStore.runExclusive", () => {
386+
test("serializes same-key holders: no overlap, FIFO, error still releases", async () => {
387+
const store = new WorkerGrantStore();
388+
const events: string[] = [];
389+
let releaseFirst!: () => void;
390+
const firstGate = new Promise<void>((resolve) => {
391+
releaseFirst = resolve;
392+
});
393+
const first = store.runExclusive("k", async () => {
394+
events.push("first-start");
395+
await firstGate;
396+
events.push("first-end");
397+
return "first";
398+
});
399+
const second = store.runExclusive("k", async () => {
400+
events.push("second-start");
401+
events.push("second-end");
402+
return "second";
403+
});
404+
// Let both register; the second holder must not start while the first
405+
// holds the turn across the awaited gate.
406+
await new Promise((resolve) => setTimeout(resolve, 5));
407+
expect(events).toEqual(["first-start"]);
408+
releaseFirst();
409+
await expect(first).resolves.toBe("first");
410+
await expect(second).resolves.toBe("second");
411+
expect(events).toEqual([
412+
"first-start",
413+
"first-end",
414+
"second-start",
415+
"second-end",
416+
]);
417+
418+
// A throwing holder still releases: the next waiter proceeds.
419+
const failing = store.runExclusive("k", async () => {
420+
throw new Error("boom");
421+
});
422+
const after = store.runExclusive("k", async () => "after");
423+
await expect(failing).rejects.toThrow("boom");
424+
await expect(after).resolves.toBe("after");
425+
});
426+
427+
test("different keys do not block each other", async () => {
428+
const store = new WorkerGrantStore();
429+
const order: string[] = [];
430+
await Promise.all([
431+
store.runExclusive("a", async () => {
432+
await new Promise((resolve) => setTimeout(resolve, 5));
433+
order.push("a");
434+
}),
435+
store.runExclusive("b", async () => {
436+
order.push("b");
437+
}),
438+
]);
439+
expect(order).toEqual(["b", "a"]);
440+
});
441+
});

0 commit comments

Comments
 (0)