Skip to content

Commit 2ed56eb

Browse files
committed
Drop operator approval when session identity changes
1 parent 0e93e44 commit 2ed56eb

16 files changed

Lines changed: 513 additions & 62 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
3939
can grant and retry, without hanging on operator approval.
4040
- Operator approval resume late-binds to the live agent and keeps the TUI
4141
busy across the overlay so a reload cannot drop the parked tool call.
42+
- Operator approval drops on session identity change. inFlight occupancy
43+
owns idle rebuild; delivery generation owns session identity, so interrupt,
44+
/clear, and /new abort the outstanding overlay, skip minting a grant, and
45+
notify the operator instead of delivering into a rebuilt agent.
4246

4347
### Changed
4448

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ tool call
382382
- **command** — Splits chained commands for security classification and derives command-shape approval scopes. Multi-segment chains only offer an exact-command persist pattern (a prefix like `npm *` must not cover `npm i && rm -rf /` later).
383383
- **auto-shell-policy** — Constrains `run_shell` even when auto mode would otherwise rubber-stamp it. Before matching, `expandShellSubjects` peels `bash`/`sh`/`zsh -c`, `xargs` utility tails, and transparent prefixes (`env`, `nice`, `timeout`, …) so rules see the real payload; an unparseable wrapper (variable expansion or command substitution) sets an opaque flag that forces `ask`. Effects: `deny` blocks outright (file mutations through ad-hoc tooling — output redirection, `tee`, `sed -i`/`perl -i`, interpreter inline programs or heredocs — which must instead go through `write_file`/`edit_file`); `ask` declines to auto-allow and falls through to the operator prompt (recursive `rm`, dependency installs and remote runners: npm/yarn/pnpm/bun, pip, cargo, go, brew, npx/bunx, …, force or uncontained `git worktree` ops, shell that references a sensitive path such as `.env` or a private key, and opaque wrappers). Contained non-force `git worktree add`/`remove`/`prune` and read-only `list` auto-allow (sibling destinations like `../corbits-dispatch-wts/…` included; absolute outside, `~`, globs, and credential basenames still ask). Deny beats ask when multiple subjects match. Quoted spans are stripped before pattern matching so a quoted `>` or install word in an argument is not flagged, and program names are matched only in command position. Adding a table category is a one-line rule append in `AUTO_SHELL_RULES`.
384384
- **gate** — Evaluates a call: `skipPermissions` allows everything; `allow`-tier passes; for `ask`-tier, checks persisted approvals, otherwise requests operator approval. Shell security classifies each chain segment (`||` / `&&` / `|` / `;` / newlines), but the operator is prompted once for the full command block — any unapproved segment fails the whole block, and execution always runs the unsplit original. Safe pipeline tails and pure shell no-ops (`true` / `false` / `:` and bare control-flow keywords stranded by chain-splitting) skip without a prompt. In a non-interactive run an unresolved `ask` becomes a denial. In auto mode: non-shell built-ins in `AUTO_ALLOWED_TOOLS` (writes/edits/deletes, `manage_tasks`, `spawn_agent`, `wait_agents`, …) auto-allow when not path-restricted; for `run_shell` the gate consults the auto-shell policy — a `deny` rule fails the call, an `ask` rule skips the auto-allow shortcut and proceeds to the normal approval flow, and anything unmatched is auto-allowed. Paths outside the workspace and writes under the session state root (`~/.corbits/projects/...` and legacy `.agent-state`) still ask under auto mode. Under `--dangerously-skip-permissions` (forces this process) or `/yolo` (persists as the user-global default via `setSkipPermissions`), the gate auto-allows those same cases, and pre-gate sandboxes (path-escape, shell session cwd retention, `list_dir` / `delete_file` workspace bounds) honor `getSkipPermissions()` live so outside-workspace access is not hard-denied after the gate already allowed it — without rebuilding the plugin stack. Secret-guard path denies and authorization hard blocks still apply. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted.
385-
- **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. The TUI stays busy across the overlay and late-binds deliver through the live agent / session queue. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict only when id, name, and arguments match, and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. A reused `codex-proxy` id cannot apply an outer `shell` allow to an inner `run_shell` deny. Inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy) never pass `env.authorize`, so `gateToolCall` decides on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`.
385+
- **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. `inFlight` occupancy owns idle rebuild: the TUI stays busy across the overlay and waits until the correlated resume is accepted (`message.received` / `message.correlated`) or a generation bump `settleAll`s the waiter. Delivery generation owns session identity: interrupt, `/clear`, and `/new` abort the outstanding overlay, skip minting a grant, drop the decision, and surface an operator notice rather than delivering into a rebuilt agent. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict only when id, name, and arguments match, and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. A reused `codex-proxy` id cannot apply an outer `shell` allow to an inner `run_shell` deny. Inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy) never pass `env.authorize`, so `gateToolCall` decides on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`.
386386
- **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix plugins and inherited MCP tools are bound to that view at worker start, so they take the reactor-gated `gateToolCall` path because the view reports `isReactorGated()` — they do not close over the parent's middleware-gated `isReactorGated()`. Deny still blocks; ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant.
387387

388388
- **matcher** — Approval pattern matching via `@intx/authz` `matchPattern` (`*` wildcards). Exact-command grants store a backslash before each metacharacter; those patterns match by equality after unescape (the package has no escape syntax).

src/permission/gate.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ function classifyOutcome(outcome: ApprovalOutcome | undefined): ApprovalOutcomeK
6767
if (!outcome.allow) {
6868
const message = outcome.message ?? "";
6969
if (message.includes("timed out")) return "timeout";
70-
if (message.includes("no longer running")) return "abort";
70+
if (message.includes("no longer running") || message.includes("identity changed")) {
71+
return "abort";
72+
}
7173
return "deny";
7274
}
7375
return outcome.persist !== undefined ? "allow-with-scope" : "allow-once";
@@ -325,8 +327,12 @@ export interface PermissionGate {
325327
// run_shell, colliding reused ids, and tests).
326328
executionVerdict: (call: ToolCall) => Promise<AuthorizeVerdict>;
327329
// Resolve a suspended reactor approval against the operator (and mint the
328-
// outcome's grant). Returns undefined when no outcome arrived.
329-
resolveSuspended: (request: PermissionRequest) => Promise<ApprovalOutcome | undefined>;
330+
// outcome's grant when the session identity is still current). Returns
331+
// undefined when no outcome arrived.
332+
resolveSuspended: (
333+
request: PermissionRequest,
334+
stillCurrent?: () => boolean,
335+
) => Promise<ApprovalOutcome | undefined>;
330336
// True when this gate's decisions go through env.authorize (authorizeCall)
331337
// rather than evaluate() in the tool-runner middleware. Under reactor gating,
332338
// gateToolCall is an execution backstop: it consumes a matching cached
@@ -724,7 +730,10 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
724730
// span, await the requestApproval seam, settle the log/span, and mint any
725731
// grant the outcome carries (never for secret-path shell). Returns undefined
726732
// when no outcome arrived (timeout/abort auto-deny paths).
727-
const resolveInteractiveAsk = async (decision: Extract<GateDecision, { kind: "ask" }>) => {
733+
const resolveInteractiveAsk = async (
734+
decision: Extract<GateDecision, { kind: "ask" }>,
735+
stillCurrent?: () => boolean,
736+
) => {
728737
const { request, anySecret, segmentCount } = decision;
729738
const askRule = anySecret ? "sensitive-path" : undefined;
730739
const ask = approvalLog.ask({
@@ -752,7 +761,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
752761
finishApprovalWait(telemetry, waitSpanId, request.tool, outcome);
753762
ask.settle(classifyOutcome(outcome));
754763
}
755-
if (outcome !== undefined && outcome.allow && !anySecret) {
764+
if (outcome !== undefined && outcome.allow && !anySecret && (stillCurrent?.() ?? true)) {
756765
mintGrant(request.tool, outcome);
757766
}
758767
return outcome;
@@ -823,18 +832,21 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
823832
// Resolve a suspended reactor approval once the operator answers. The
824833
// request is the one authorizeCall built at decision time, so the ask log,
825834
// wait span, and grant minting are identical to the middleware path.
826-
const resolveSuspended = (request: PermissionRequest) => {
835+
const resolveSuspended = (request: PermissionRequest, stillCurrent?: () => boolean) => {
827836
const anySecret =
828837
request.tool === "run_shell" && commandReferencesSensitivePath(request.subject) !== undefined;
829-
return resolveInteractiveAsk({
830-
kind: "ask",
831-
request,
832-
anySecret,
833-
segmentCount:
834-
request.tool === "run_shell"
835-
? splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s)).length
836-
: 0,
837-
});
838+
return resolveInteractiveAsk(
839+
{
840+
kind: "ask",
841+
request,
842+
anySecret,
843+
segmentCount:
844+
request.tool === "run_shell"
845+
? splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s)).length
846+
: 0,
847+
},
848+
stillCurrent,
849+
);
838850
};
839851

840852
const reset = (): void => {

src/session/approval-resume.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import type { PermissionRequest } from "../permission/types.js";
2626

2727
const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-resume"]);
2828

29+
export const APPROVAL_DROPPED_NOTICE = "Approval dropped because the session changed.";
30+
2931
const ApprovalSnapshotShape = type({
3032
name: "string",
3133
"arguments?": "Record<string, unknown>",
@@ -108,16 +110,19 @@ function decisionMessage(
108110
}
109111

110112
export function createApprovalResume(args: {
111-
// Late-bound: the live agent is read at history/deliver time so rebuilds
112-
// (/clear, model switch) deliver through the current instance, not a
113-
// snapshot taken at handle() start.
113+
// Live agent at history/deliver time. TUI occupancy holds this identity
114+
// until the correlated resume is accepted; a generation bump aborts the
115+
// gate rather than retargeting a rebuilt agent.
114116
getAgent: () => Pick<Agent, "deliver" | "history"> | undefined;
115117
// TUI session queue. When present, each decision is awaited through this
116118
// seam; exec omits it and uses getAgent().deliver.
117119
deliver?: (message: InboundMessage, stillCurrent: () => boolean) => void | Promise<void>;
118-
// TUI: capture at handle() start so /clear or interrupt during the overlay
119-
// drops the decision instead of delivering into the rebuilt agent. Exec omits this.
120+
// TUI: capture at handle() start so interrupt, /clear, or /new during the
121+
// overlay aborts the gate and drops the decision. Exec omits this.
120122
captureGeneration?: () => () => boolean;
123+
// TUI: operator-visible notice when an overlay decision is dropped after
124+
// a generation bump.
125+
onDropped?: (text: string) => void;
121126
gate: PermissionGate;
122127
}): ApprovalResume {
123128
const { getAgent, gate } = args;
@@ -149,6 +154,7 @@ export function createApprovalResume(args: {
149154
// out" tool result appended after this point means the reactor settled
150155
// this very correlation before our decision lands.
151156
const turnsAtSuspend = (await requireAgent().history()).length;
157+
if (!stillCurrent()) return true;
152158

153159
if (approvalSnapshot === undefined) {
154160
// A suspension without a snapshot cannot be surfaced; fail closed by
@@ -168,8 +174,11 @@ export function createApprovalResume(args: {
168174
return true;
169175
}
170176

171-
const outcome = await gate.resolveSuspended(request);
172-
if (!stillCurrent()) return true;
177+
const outcome = await gate.resolveSuspended(request, stillCurrent);
178+
if (!stillCurrent()) {
179+
args.onDropped?.(APPROVAL_DROPPED_NOTICE);
180+
return true;
181+
}
173182
if (settledAfterSuspend(await requireAgent().history(), turnsAtSuspend)) {
174183
// The reactor already answered the parked call (its approval timeout
175184
// fired while the surface was still up). Delivering now would append
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, test } from "bun:test";
2+
3+
import { createCorrelationAcceptance } from "./correlation-acceptance.js";
4+
5+
describe("createCorrelationAcceptance", () => {
6+
test("settle resolves the waiter for that correlation id", async () => {
7+
const acceptance = createCorrelationAcceptance();
8+
const pending = acceptance.wait("corr-1");
9+
acceptance.settle("corr-1");
10+
await pending;
11+
});
12+
13+
test("settleAll releases every outstanding waiter", async () => {
14+
const acceptance = createCorrelationAcceptance();
15+
const first = acceptance.wait("a");
16+
const second = acceptance.wait("b");
17+
acceptance.settleAll();
18+
await Promise.all([first, second]);
19+
});
20+
21+
test("settle of an unknown id is a no-op", () => {
22+
const acceptance = createCorrelationAcceptance();
23+
acceptance.settle("missing");
24+
});
25+
});

src/tui/correlation-acceptance.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Occupancy wait for a fire-and-forget Agent.deliver of a correlated
3+
* approval. The reactor accepts the resume asynchronously after deliver
4+
* returns; inFlight must not drop until that acceptance (or an uncorrelated
5+
* pass-through / identity bump) settles the waiter.
6+
*/
7+
8+
export function createCorrelationAcceptance() {
9+
const waiters = new Map<string, () => void>();
10+
11+
const settle = (correlationId: string): void => {
12+
const resolve = waiters.get(correlationId);
13+
if (resolve === undefined) return;
14+
waiters.delete(correlationId);
15+
resolve();
16+
};
17+
18+
return {
19+
wait(correlationId: string): Promise<void> {
20+
const pending = waiters.get(correlationId);
21+
return new Promise<void>((resolve) => {
22+
waiters.set(correlationId, () => {
23+
pending?.();
24+
resolve();
25+
});
26+
});
27+
},
28+
settle,
29+
settleAll(): void {
30+
for (const correlationId of [...waiters.keys()]) settle(correlationId);
31+
},
32+
};
33+
}

src/tui/gate-wire.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import { moveOverlaySelection, toggleOverlayExpand } from "./shell/overlay-list.js";
2020
import { streamRowGutter } from "./stream.js";
2121
import { APPROVAL_UNAVAILABLE_MESSAGE } from "./gate-events.js";
22+
import { SESSION_IDENTITY_ABORT_REASON } from "./queued-delivery.js";
2223
import {
2324
approvalOutcomeFromSelection,
2425
operatorCancelResult,
@@ -1204,6 +1205,40 @@ describe("permission.gate auto-deny", () => {
12041205
});
12051206
});
12061207

1208+
test("identity abort reason auto-denies and closes the overlay", async () => {
1209+
await withTestRenderer(async (h) => {
1210+
const shell = createAppShell(h.renderer, {
1211+
terminal: { columns: 80, rows: 24 },
1212+
run: "idle",
1213+
});
1214+
const emitter = new EventEmitter();
1215+
const controller = new AbortController();
1216+
let resolved: unknown;
1217+
try {
1218+
wireGates(emitter, shell);
1219+
emitter.emit("permission.gate", {
1220+
id: "req-1",
1221+
request: baseRequest(),
1222+
resolve: (outcome: unknown) => {
1223+
resolved = outcome;
1224+
},
1225+
signal: controller.signal,
1226+
});
1227+
expect(shell.overlayKind).toBe("permissions");
1228+
1229+
controller.abort(SESSION_IDENTITY_ABORT_REASON);
1230+
1231+
expect(resolved).toEqual({
1232+
allow: false,
1233+
message: SESSION_IDENTITY_ABORT_REASON,
1234+
});
1235+
expect(shell.overlayList).toBeNull();
1236+
} finally {
1237+
shell.dispose();
1238+
}
1239+
});
1240+
});
1241+
12071242
test("resolving normally clears the timer instead of firing it later", async () => {
12081243
await withTestRenderer(async (h) => {
12091244
const shell = createAppShell(h.renderer, {

src/tui/gate-wire.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,15 @@ export function wireGates(
412412
settle({ allow: false, message });
413413
};
414414
function onAbort(): void {
415-
autoDeny("tool no longer running; permission request denied");
415+
const reason = ev.signal?.reason;
416+
autoDeny(
417+
typeof reason === "string" && reason.length > 0
418+
? reason
419+
: "tool no longer running; permission request denied",
420+
);
416421
}
417422
if (ev.signal?.aborted === true) {
418-
autoDeny("tool no longer running; permission request denied");
423+
onAbort();
419424
return;
420425
}
421426
ev.signal?.addEventListener("abort", onAbort, { once: true });

0 commit comments

Comments
 (0)