Fix 2s slowloris kill of confirmation-gated control requests; add global seamless agent messaging toggle

[austinschneider]

Two commits: a bug fix that makes agent write/close confirmations work at all, and a trust model on top of the working dialog.

Bug: the slowloris guard kills confirmation-gated control requests (commit 1)

hook-server.ts arms req.setTimeout(SLOWLORIS_MS /* 2s */, () => req.destroy()) before routing. For /control/write and /control/close, the main-process handler forwards to the renderer and legitimately waits up to 120s for the user's confirmation dialog — so the socket is destroyed 2 seconds into that wait. The calling shim sees curl code 000 and prints "Could not reach nodeterm (control endpoint unreachable)" while the dialog is still on screen; with the one-confirm-at-a-time guard, one stuck dialog then bounces every later write. In practice, agent-to-agent messaging only worked if the user clicked within 2 seconds.

Fix: disarm the guard (req.setTimeout(0)) once the request body is fully read, on the /control/ and /context-link/ routes (a linked-transcript read over SSH can also legitimately exceed 2s). The receive phase — what the guard exists for — is still covered.

Feature: seamless agent messaging, global + per-pair (commits 1+2)

With the dialog now functional, pair-programming flows (agent A drives agent B, reviews, sends the next piece) still pay one click per message. This adds pre-approval at two scopes:

• Global (Settings → Agents → "Seamless agent messaging", default off): every agent write delivers without the dialog. Documented in the setting's comment as the blunt instrument it is.
• Per node pair (the interesting one): with the global off, the write dialog gains an "Always allow between these two nodes (both directions)" checkbox. Confirming with it checked stores a canonical sorted idA|idB key on the project (persisted like the kanban board), and writes between that pair — either direction — deliver directly from then on.

Design decisions, so review can disagree with them specifically:

• Grants die with their endpoints: deleteNodes prunes every grant referencing a deleted node, so trust can never outlive — or transfer to a later reuse of — a node id.
• close always confirms, at both scopes. Only messaging is pre-approvable.
• The one-confirm-at-a-time invariant holds: seamless sends never mount a dialog, and the checkbox rebuilds the dialog state through the guarded setConfirm (never the raw setter), sharing the reply closures so a rebuild can't orphan a pending request.
• Management/visibility: Settings → Agents → "Trusted node pairs" shows a live grant count for the active project with Clear-all. The ConfirmDialog option prop already existed (worktree deletion opt-in) and is reused, not duplicated.

Validation

• npm run typecheck clean; npm run test green (4468 passed; one pre-existing flaky filesystem-watch test in board-log-handlers.test.ts fails occasionally under full-suite load and passes in isolation, unchanged tree included).
• Exercised live: pre-fix, writes died at exactly 2s; post-fix with everything off, the dialog flow works (~seconds, human click); with a pair grant, delivery is ~100ms silent, both directions; deleting a granted node prunes the grant.

Happy to split the bug fix into its own PR if you'd rather take it separately, or to adjust the trust model (e.g. per-direction grants, TTLs) to taste.

[Copilot]

Pull request overview

Fixes agent control requests being prematurely killed by the slowloris receive-timeout, and adds an opt-in trust model to reduce repeated confirmations for agent-to-node write operations (global toggle + per-node-pair grants).

Changes:

• Disarms the slowloris socket timeout after request bodies are fully read for /control/* and /context-link/* routes so confirmation-gated requests can legitimately wait.
• Introduces “Seamless agent messaging” global setting (default off) and per-project trusted node-pair grants for agent write requests.
• Adds UI in Settings → Agents to manage the global toggle and to clear per-project trusted-pair grants; prunes grants when nodes are deleted.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/shared/types.ts	Adds Project.seamlessPairs and Settings.agentSeamlessWrites plus default setting value.
src/renderer/state/projects.ts	Adds project state mutations for adding/pruning/clearing trusted node-pair grants.
src/renderer/components/settings/sections/AgentsSection.tsx	Adds Settings UI for the global seamless toggle and a live count + “Clear all” for trusted pairs.
src/renderer/canvas/Canvas.tsx	Implements per-pair trust checkbox in the write confirmation dialog, seamless-delivery fast path, and pruning on node deletion.
src/core/agents/hook-server.ts	Disarms the slowloris timeout post-body-read for control/context-link requests to allow long confirmation waits.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[austinschneider]

Good catch, and confirmed: projectToFile/fileToProject whitelist fields, so for folder-backed and SSH projects every save dropped seamlessPairs while the docstring and settings copy promised persistence. (Inline cwd-less projects happened to survive, since the whole Project is stored verbatim in the index — which is what made this easy to miss in manual testing.)

Fixed in dfdad20, but deliberately not by adding the field to ProjectFileV1. The project file is a shared document — cloned from git, or read off a remote host — and it carries the node definitions. A file that could also carry grants could ship pre-approved pairs between its own nodes, so opening someone's repo would let their agent nodes type into each other's terminals with no dialog, on a machine whose user approved nothing. That's the same threat the exec-field stripping already guards against (@shared/node-exec: "stripped from the shared project file precisely so a cloned/hostile one cannot run code").

So grants now ride the machine-local index entry, exactly like localExec and localApprovalId:

• IndexEntryV3.seamlessPairs — userData, never git-shared;
• written in splitWorkspace for both ref shapes, restored via fileToProject's base on all three load paths, and preserved across an unavailable placeholder (same reasoning as the localExec restore);
• absent from ProjectFileV1 by construction, so a hostile file claiming the field is ignored.

Docstring and the Settings copy now say "stays on this machine, never in the shared project file" instead of "persisted on the project".

Tests mirror the existing exec suite: omitted from the file, ignored when a hostile file claims it, restored through the machine-local round trip, and no key at all when nothing is granted (workspace-files.test.ts, 26 passing). Full suite: typecheck clean, 4470 passing; the two failures are the debounced filesystem-watch tests in board-log* under full-suite load — they pass in isolation on this branch and on the clean tree alike.

[eneskirca]

The hook token authenticates a session, not which node — and three open PRs now rest on it

Cross-cutting review note, posted on both #112 and #113 because neither can fix this alone and the fix is the same in both. #98 sits on the same ground.

The mechanism

hookServer mints one randomUUID() for its whole lifetime (src/core/agents/hook-server.ts:171) and buildPtyEnv injects that same value into every session it spawns (:453). Request handling checks only that token (:180), then reads the acting node straight out of the request body — parseControlBody's form.nodeId for /control/*, and form.nodeId again on the /codex-thread/* routes (:189, :220, :246).

So the token proves "some session nodeterm spawned is calling". It proves nothing about which node is calling. Any control-capable agent can put a different node's id in the body and the server has no way to tell.

That was tolerable while a forged nodeId only mis-attributed an edge or a cwd. These PRs change what it buys.

What it buys in each PR

#113 — per-pair trust. pairKey = [sourceNodeId, args.node].sort().join('|') makes the asserted source id one half of the grant key. Agent C, holding the same env token, can send nodeId=A and deliver into B under the A↔B grant it was never given. The dialog the grant is meant to replace never appears — that is the grant's whole purpose, so there is no second gate behind it. The global toggle is honest about being blunt ("Any control-capable agent can then type into any node's terminal unseen"); the per-pair mode reads as the precise alternative, and precision is exactly what the asserted id cannot deliver.

#112 — thread ownership, and through it, browser control. The browser router does the right thing: it demands guest.ownerNodeId === nodeIdForSession(session_id) (browser-use-backend-core.ts:281). But that mapping is writable over /codex-thread/bind, whose nodeId comes from the body. bindCodexThreadIdentity refuses only when the thread is already bound to another live node (codex-identity-proxy.ts:165-167); an unowned or dead-owner thread binds freely to whatever node id the caller names (:175). Chain: find a real unowned thread id → bind it to victim node B → connect to the browser socket claiming that session → requireTab now resolves to B, and you can inspect, click, type and navigate B's browser node, including whatever authenticated session B has open. The ownership check is sound; the thing it checks against is attacker-writable.

#98 — same ground, smaller blast radius. notify verifies the source is context-linked to the target, but the source is the asserted id. With list returning every node id on the project, an unlinked agent can iterate ids as claimed sources until one satisfies the link check. The payload is a fixed string, so this is noise-injection rather than takeover — but it is the same gate resting on the same assertion.

Scope, stated plainly

All of this lives inside one OS user. Nothing crosses the user boundary: the hook server is loopback, the browser socket is 0600 in a 0700 dir, and an attacker who already has that uid can read the files directly. The realistic threat is a prompt-injected or misbehaving agent moving laterally to a sibling node — which is precisely the boundary #112's browser scoping and #113's per-pair grants are advertised to enforce.

The fix

Bind the token to the node, and stop reading identity from the body:

1. buildPtyEnv mints a per-node token instead of handing out the server-wide one, and registers token → nodeId in a map alongside the existing session bookkeeping.
2. Request handling resolves the acting node from the token (tokenMatches becomes nodeForToken), and nodeId in the body is either dropped or required to equal the resolved node.
3. Sessions outlive the app, so the map must survive a relaunch — the endpoint file (hook-server.ts:435) already carries the restart handoff and is the natural place for the per-node entries. Unknown token ⇒ 403, which is the safe degrade.

That is one change in src/core/agents/hook-server.ts plus its buildPtyEnv callers. It closes #113's grant spoof and #112's bind spoof at once, and turns #98's link check into something that means what it says. Doing it per-PR instead would mean three partial defenses over the same hole.

Two smaller things worth folding in while that seam is open, both from #112: /codex-thread/observed lets a caller set another node's title (index.ts:1003), and the CDP passthrough in browser-use-backend-core.ts:366 forwards any method with no allowlist — so a controlling agent can Page.navigate to file:///… then Runtime.evaluate. Harmless against your own node; a file-read primitive against a victim's once the ownership above is forgeable.

What I verified, and what I did not

Read and confirmed in the code: the single token and its identical injection (:171, :453), the body-sourced nodeId on /control/* and /codex-thread/bind (:189, :220), bindCodexThreadIdentity's live-only refusal (:165-175), the browser router's ownership check (:281), and #113's pairKey construction.

Not verified by running: no crafted POST was actually sent, and neither the bind→browser chain nor the grant spoof was reproduced end to end. Both are read off the code, and a multi-account setup with a live browser node is what would settle them. If either of you sees a check I have missed on the path from a forged body to the acting node, I would rather be wrong about this.

Credit where due: #112's cross-account resolution genuinely fails closed (duplicate thread ids resolve ambiguous unless one inode proves otherwise), its rollout hardlinking is carefully defended, and #113 fixes a real bug — the 2 s slowloris guard was killing confirmation-gated /control/write and /control/close outright, so agent messaging only worked if the user clicked within two seconds. Neither PR created this weakness; they inherited it and lean on it harder than anything before them.

[austinschneider]

I reproduced the spoof end to end before acting on it, then split this PR per your note.

Reproduction (single OS user, this machine): created a scratch terminal T; using the shared token from the endpoint file, sent a /control/write claiming nodeId = an idle Codex node whose shim I have never run, into T. First send (no grant) raised the dialog; I approved it with "always allow", creating a grant for the claimed node. Second send, same forged claim: delivered in 104 ms, no dialog. A third send claiming a non-existent id was rejected with "source node is not a control-capable agent" — so the only check is that the asserted source exists and is control-capable, never that the caller is it. That's exactly the path you described, now confirmed by running it, not just reading it.

The split:

• This PR (Fix 2s slowloris kill of confirmation-gated control requests; add global seamless agent messaging toggle #113) now contains only the two changes that don't rest on caller identity: the slowloris fix, and the global "Seamless agent messaging" toggle (default off). The global toggle is a blanket grant — any control-capable agent may write to any node — so a forged nodeId buys nothing extra, and it's honest about that in the setting copy. I force-pushed to drop the per-pair commits; the diff is now hook-server.ts + the agentSeamlessWrites setting + a global-only fast path in the write handler.
• The per-pair grant work is pulled out into a separate branch and will come back as its own PR after the token is node-scoped, since its grant key is built from the asserted source id and is therefore spoofable exactly as above.

On the node-scoped token itself: I agree it should be one shared fix the affected PRs rebase onto rather than three partial ones. I'm happy to take that on as its own PR. Two implementation notes from a first read, if useful — the per-node token can't live in the current endpoint file as extra shell vars (node ids contain hyphens, which aren't valid shell identifiers), so a per-session endpoint file is cleaner and needs no shim change; and the SSH leg reads its token from a per-host endpoint file shared by every node on that host, so it needs the same per-node treatment and gets rewritten on each reconnect. Want me to open that PR against main for #112/#113/#98 to build on?

[eneskirca]

Thanks @austinschneider — the slowloris half of this is a real, reproduced bug and it has landed separately as #158, with credit to you in the commit trailer and the PR body.

Two things were added on top of your fix there:

• A regression test (src/core/agents/hook-server.slowloris.test.ts) that spins the real hook server and parks a handler past the guard — verified red without the fix (UND_ERR_SOCKET) and green with it.
• req.setTimeout(0) became a bounded CONTROL_CEILING_MS = 130_000 (above the desktop's 120s pendingControl bound) rather than removing the ceiling entirely. Without a ceiling the /context-link/ route had no bound at all — handleContextLinkRequest has no timeout and a remote read over a wedged ControlMaster can hang indefinitely, so the agent's curl would park forever where the 2s destroy at least unblocked it. That route also races its handler at 30s and answers with the existing "Could not read linked context." text.

Worth recording for anyone reading this later: the bug is slightly worse than the description says. A late confirmation still delivers — the renderer already holds the request when the socket dies — so the calling agent is told the endpoint was unreachable while the text lands anyway, and may retry. Silent duplicate delivery.

On the seamless agent writes half: we're not taking it as a global bypass, but the capability behind it is wanted. It's being redesigned into a per-project agent-messaging model (project.json switch, like defaultPermissionMode) with four gates that a blanket toggle skips: target must be an agent node (a plain shell node turns "messaging" into command execution, since sendText appends Enter), target must be idle (an Enter typed at a permission prompt answers it), the expected CLI must actually own the pane, and every delivery leaves a trace. #98 and part of #112 reach for the same capability, so all three are converging on one substrate rather than three toggles.

Leaving this PR open for now so the discussion has a home. Appreciate the report and the diagnosis — the reproduction was exact.

[eneskirca]

Thank you for this — and in particular for the diagnosis in the first half, which was correct and is now fixed on main.

The slowloris half: your bug report landed, your patch was refined

You were right that req.setTimeout(SLOWLORIS_MS, () => req.destroy()) was killing confirmation-gated control requests two seconds into a wait that legitimately runs to 120s, and that agent-to-agent messaging therefore only worked if the user clicked within two seconds. That is fixed on main now.

The one change from your patch is that the guard is replaced with a higher ceiling rather than disarmed:

const CONTROL_CEILING_MS = 130_000

The reasoning, from the comment that shipped with it: the 120s bound on a control request lives in pendingControl in src/main/index.ts, which is outside core — so a future core-side handler with no bound of its own would inherit an unbounded socket from setTimeout(0). 130s sits above the desktop's own bound, so the handler's timeout always wins and this only ever fires as a backstop.

Your /context-link/ observation was right too, and got its own treatment: CONTEXT_LINK_READ_MS = 30_000, because that handler has no timeout of its own and its remote leg reads over an SSH ControlMaster that can wedge (ConnectTimeout only covers the connect).

The messaging half: superseded

The seamless-writes toggle and the per-node-pair grants are the same ground as the agent-messaging design I am implementing now, so I am not going to merge this and have you rebase onto something that is about to move underneath you.

Two things from your design I want to call out because they are going in:

• Grants die with their endpoints — pruning every grant referencing a deleted node in deleteNodes, so trust can never outlive or transfer to a later reuse of a node id. That is exactly right and I had not thought about the id-reuse case.
• close always confirms, at every scope. Also right. Only messaging is pre-approvable.

Both with credit to you.

Closing this as superseded rather than leaving it to rot — but the slowloris find was a genuinely good catch, and if you want to be in the loop on the messaging design before it lands, say so.