Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/agent/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import {
createResumeAgentTool,
createInterruptAgentTool,
createSendInputTool,
createApproveWorkerCallTool,
} from "../subagent/lifecycle-tools.js";
import { createManageTasksRunner } from "./tasks.js";
import { createSpillingBackgroundShellExitNotifier } from "./background-shell-tool.js";
Expand Down Expand Up @@ -609,6 +610,11 @@ export async function createAgentToolset(
createResumeAgentTool({ sessions: fleetSessions, fleetRecords }),
createInterruptAgentTool({ sessions: fleetSessions, fleetRecords }),
createSendInputTool({ sessions: fleetSessions, fleetRecords }),
createApproveWorkerCallTool({
sessions: fleetSessions,
fleetRecords,
permissionGate: args.permissionGate,
}),
);
// Exec-primary opt-in only; TUI primary and nested orchestrators
// collect via mailbox mail, so wait_agents stays unmounted there.
Expand Down
27 changes: 27 additions & 0 deletions src/permission/reactor-authorize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,33 @@ test("nested orchestrator wait_agents allows with only a spawn_agent grant", asy
).toBe("allow");
});

test("nested parent approval tool passes the worker authorization seam", async () => {
const policy = gate({ interactive: true, auto: false });
const authorize = createWorkerAuthorize(policy);
expect(
(
await authorize(
"tool:approve_worker_call",
"invoke",
namedCall("approve_worker_call", {
target: "child-session",
question_id: "child-question",
request_id: "child-request",
}),
)
).effect,
).toBe("allow");
expect(
(
await authorize(
"tool:write_file",
"invoke",
namedCall("write_file", { path: "ordinary.txt", content: "data" }),
)
).effect,
).toBe("deny");
});

test("worker spawn_agent still needs a parent grant", async () => {
const policy = gate({ interactive: true, auto: false });
expect(
Expand Down
49 changes: 21 additions & 28 deletions src/permission/reactor-authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import type { AuthorizeVerdict, GateVerdict, PermissionGate } from "./gate.js";
import type { PermissionRequest } from "./types.js";
import {
createDeniedCallEnvelope,
fingerprintDeniedCall,
formatWorkerDenyWithGrantId,
getProcessWorkerGrantStore,
type WorkerGrantStore,
Expand Down Expand Up @@ -164,17 +163,6 @@ function denyWorkerCallWithEnvelope(
};
}

/** Mutex key covering the envelope match: concurrent identical worker calls
* serialize through precheck-to-consume so one envelope allows exactly once. */
function workerGrantKey(
sessionId: string,
call: ToolCallType,
cwd: string,
): string {
const identity = workerCallIdentity(sessionId, call, cwd);
return `${sessionId}\n${fingerprintDeniedCall(identity.canonicalTool, identity.args, identity.cwd)}`;
}

async function authorizeWorkerCall(
gate: PermissionGate,
call: ToolCallType,
Expand All @@ -187,11 +175,13 @@ async function authorizeWorkerCall(
if (sessionId === undefined)
return authorizeWorkerCallInner(gate, call, grantOptions, workerCwd);
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () =>
authorizeWorkerCallInner(gate, call, grantOptions, workerCwd, {
sessionId,
store,
}),
return store.runExclusiveIdentity(
workerCallIdentity(sessionId, call, workerCwd),
() =>
authorizeWorkerCallInner(gate, call, grantOptions, workerCwd, {
sessionId,
store,
}),
);
}

Expand All @@ -203,10 +193,10 @@ async function authorizeWorkerCallInner(
grant?: { sessionId: string; store: WorkerGrantStore },
): Promise<{ effect: "allow" } | { effect: "deny"; reason: string }> {
if (grant !== undefined) {
const precheck = grant.store.precheck(
workerCallIdentity(grant.sessionId, call, workerCwd),
);
const identity = workerCallIdentity(grant.sessionId, call, workerCwd);
const precheck = grant.store.precheck(identity);
if (!precheck.ok) return { effect: "deny", reason: precheck.blocker };
if (grant.store.hasAuthorizedRetry(identity)) return { effect: "allow" };
}
const verdict = await gate.authorizeCall(call);
// Authorize never consumes: it only permits the call to proceed to the
Expand Down Expand Up @@ -248,11 +238,13 @@ async function executionVerdictWorkerCall(
if (sessionId === undefined)
return executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd);
const store = grantOptions?.store ?? getProcessWorkerGrantStore();
return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () =>
executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd, {
sessionId,
store,
}),
return store.runExclusiveIdentity(
workerCallIdentity(sessionId, call, workerCwd),
() =>
executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd, {
sessionId,
store,
}),
);
}

Expand All @@ -264,10 +256,11 @@ async function executionVerdictWorkerCallInner(
grant?: { sessionId: string; store: WorkerGrantStore },
): Promise<AuthorizeVerdict> {
if (grant !== undefined) {
const precheck = grant.store.precheck(
workerCallIdentity(grant.sessionId, call, workerCwd),
);
const identity = workerCallIdentity(grant.sessionId, call, workerCwd);
const precheck = grant.store.precheck(identity);
if (!precheck.ok) return { effect: "deny", reason: precheck.blocker };
if (grant.store.consumeAuthorizedRetry(identity) !== undefined)
return { effect: "allow" };
}
const verdict = await gate.executionVerdict(call);
// Sole consumption point: the execution backstop spends the envelope when
Expand Down
7 changes: 3 additions & 4 deletions src/permission/worker-grant-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe("worker grant-request flow: deny → parent replay grant → one retry"
expect(store.peek(requestId)?.status).toBe("pending");
});

test("send_input text answers the ask but grants nothing", () => {
test("send_input text answers an unbound ask but grants nothing", () => {
const sessions = createSubAgentSessionStore();
const session = sessions.start({
id: "fleet-1",
Expand Down Expand Up @@ -221,9 +221,8 @@ describe("worker grant-request flow: deny → parent replay grant → one retry"
},
}),
).toBe(true);
expect(sessions.peekAsk(session.id)?.deniedCall?.requestId).toBe(
envelope.requestId,
);
// A request ID quoted in prose is not an authority-bearing binding.
expect(sessions.peekAsk(session.id)?.deniedCall).toBeUndefined();

const echo = JSON.stringify({
requestId: envelope.requestId,
Expand Down
61 changes: 48 additions & 13 deletions src/permission/worker-grant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,15 @@ describe("fingerprintDeniedCall", () => {
expect(a).toBe(b);
});

test("differs across args and tool (cwd binds at precheck, not in the hash)", () => {
test("differs across args, tool, and cwd", () => {
const base = fingerprintDeniedCall("run_shell", ARGS, CWD);
expect(
fingerprintDeniedCall("run_shell", { command: "npm run evil" }, CWD),
).not.toBe(base);
expect(fingerprintDeniedCall("read_file", ARGS, CWD)).not.toBe(base);
expect(fingerprintDeniedCall("run_shell", ARGS, "/elsewhere")).not.toBe(
base,
);
expect(
fingerprintDeniedCall("read_file", { path: "a.md" }, "/repo") ===
fingerprintDeniedCall("read_file", { path: "b.md" }, "/repo"),
Expand Down Expand Up @@ -283,19 +286,32 @@ describe("WorkerGrantStore lifecycle", () => {
expect(store.invalidateSession("worker-1", "again")).toBe(0);
});

test("attachToAsk stamps the questionId and keeps pending", () => {
test("attachToAsk binds only an explicit requestId and keeps it pending", () => {
const store = new WorkerGrantStore();
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
// Reads pin the clock: fixture now predates wall time, so an unpinned
// read would sweep the envelope as expired.
expect(store.attachToAsk("worker-1", "ask-7", undefined, 1_000_001)).toBe(
envelope,
);
expect(envelope.questionId).toBe("ask-7");
expect(
store.attachToAsk("worker-1", "ask-generic", undefined, 1_000_001),
).toBeUndefined();
expect(envelope.questionId).toBeUndefined();
expect(envelope.status).toBe("pending");
expect(store.peek(envelope.requestId)).toBe(envelope);
expect(
store.attachToAsk("worker-9", "ask-8", undefined, 1_000_001),
store.attachToAsk("worker-1", "ask-7", envelope.requestId, 1_000_001),
).toBe(envelope);
expect(envelope.questionId).toBe("ask-7");
expect(
store.attachToAsk(
"worker-1",
"ask-rebound",
envelope.requestId,
1_000_001,
),
).toBeUndefined();
expect(envelope.questionId).toBe("ask-7");
expect(
store.attachToAsk("worker-9", "ask-8", envelope.requestId, 1_000_001),
).toBeUndefined();
expect(envelope.audit.map((event) => event.event)).toEqual([
"denied",
Expand Down Expand Up @@ -329,12 +345,12 @@ describe("WorkerGrantStore lifecycle", () => {
expect(store.peek(envelopeB.requestId)?.questionId).toBe("ask-B");
});

test("unnamed ask keeps the legacy first-pending bind", () => {
test("unnamed ask does not bind a pending denial", () => {
const { store, envelopeA, envelopeB } = twoDenies();
expect(store.attachToAsk("worker-1", "ask-x", undefined, 1_000_001)).toBe(
envelopeA,
);
expect(envelopeA.questionId).toBe("ask-x");
expect(
store.attachToAsk("worker-1", "ask-x", undefined, 1_000_001),
).toBeUndefined();
expect(envelopeA.questionId).toBeUndefined();
expect(envelopeB.questionId).toBeUndefined();
});

Expand Down Expand Up @@ -370,7 +386,7 @@ describe("WorkerGrantStore lifecycle", () => {
test("audit trail orders deny → ask → consume", () => {
const store = new WorkerGrantStore();
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
store.attachToAsk("worker-1", "ask-1", undefined, 1_000_001);
store.attachToAsk("worker-1", "ask-1", envelope.requestId, 1_000_001);
store.consumeOnAllow({
sessionId: "worker-1",
canonicalTool: "run_shell",
Expand All @@ -385,6 +401,25 @@ describe("WorkerGrantStore lifecycle", () => {
]);
});

test("revokeAuthorizedRetry returns authorized to pending and audits revoked", () => {
const store = new WorkerGrantStore();
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
expect(store.authorizeRetry(envelope.requestId, 1_000_001)).toBe(true);
store.revokeAuthorizedRetry(envelope.requestId);
expect(envelope.status).toBe("pending");
expect(envelope.audit.map((event) => event.event)).toEqual([
"denied",
"authorized",
"revoked",
]);
store.revokeAuthorizedRetry(envelope.requestId);
expect(envelope.audit.map((event) => event.event)).toEqual([
"denied",
"authorized",
"revoked",
]);
});

test("no text API: state moves only on typed identities, never prose", () => {
const store = new WorkerGrantStore();
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
Expand Down
Loading
Loading