diff --git a/TODO.md b/TODO.md index 6d111040..dddf046b 100644 --- a/TODO.md +++ b/TODO.md @@ -20,39 +20,39 @@ the owner's OS user and can never be granted. Indirection there really is accide ## Themes -**87 open, 22 closed**, across 29 themes. +**86 open, 23 closed**, across 29 themes. -| Theme | Open | Closed | -| ------------------------------------------------------------------------------------------ | ---- | ------ | -| [Affordances](todo/affordances/) | 1 | 0 | -| [Speaking into a session from the backend](todo/agent-delivery/) | 2 | 0 | -| [Backburner](todo/backburner/) | 2 | 0 | -| [Background jobs](todo/background-jobs/) | 2 | 0 | -| [Chat interface](todo/chat/) | 1 | 0 | -| [Code editor](todo/code-editor/) | 1 | 0 | -| [Components — the shared UI layer](todo/components/) | 1 | 0 | -| [File watchers](todo/file-watchers/) | 1 | 0 | -| [Fleet — one command across every server](todo/fleet/) | 5 | 2 | -| [Headscale](todo/headscale/) | 0 | 5 | -| [Infra (alpha)](todo/infra/) | 6 | 0 | -| [Known bugs](todo/known-bugs/) | 4 | 1 | -| [Mobile apps](todo/mobile-apps/) | 3 | 0 | -| [Multi-user](todo/multi-user/) | 18 | 3 | -| [Notifications](todo/notifications/) | 1 | 0 | -| [Plugin ecosystem — before it opens to third parties (2026-08-18)](todo/plugin-ecosystem/) | 3 | 0 | -| [Plugin infrastructure](todo/plugin-infrastructure/) | 2 | 0 | -| [Plugin repos — what writing their READMEs turned up](todo/plugin-repos/) | 10 | 9 | -| [Plugin system — gaps the email extraction found (2026-08-18)](todo/plugin-system/) | 3 | 0 | -| [Prompt library](todo/prompt-library/) | 1 | 0 | -| [Settings — portable, and readable by an agent](todo/settings/) | 2 | 0 | -| [Shell — spotlight and the dock](todo/shell/) | 1 | 1 | -| [Sub-agents — agents spawning agents](todo/sub-agents/) | 1 | 1 | -| [Sudo terminal modal](todo/sudo-terminal-modal/) | 1 | 0 | -| [Tooling — the list about the list](todo/tooling/) | 1 | 0 | -| [Transmission](todo/transmission/) | 6 | 0 | -| [URL state — routing, query strings and selection](todo/url-state/) | 4 | 0 | -| [Waiting on the owner to run it](todo/waiting-on-owner/) | 2 | 0 | -| [Workspaces & Panels](todo/workspaces-panels/) | 2 | 0 | +| Theme | Open | Closed | +| --- | --- | --- | +| [Affordances](todo/affordances/) | 1 | 0 | +| [Speaking into a session from the backend](todo/agent-delivery/) | 2 | 0 | +| [Backburner](todo/backburner/) | 2 | 0 | +| [Background jobs](todo/background-jobs/) | 2 | 0 | +| [Chat interface](todo/chat/) | 1 | 0 | +| [Code editor](todo/code-editor/) | 1 | 0 | +| [Components — the shared UI layer](todo/components/) | 1 | 0 | +| [File watchers](todo/file-watchers/) | 1 | 0 | +| [Fleet — one command across every server](todo/fleet/) | 5 | 2 | +| [Headscale](todo/headscale/) | 0 | 5 | +| [Infra (alpha)](todo/infra/) | 6 | 0 | +| [Known bugs](todo/known-bugs/) | 4 | 1 | +| [Mobile apps](todo/mobile-apps/) | 3 | 0 | +| [Multi-user](todo/multi-user/) | 17 | 4 | +| [Notifications](todo/notifications/) | 1 | 0 | +| [Plugin ecosystem — before it opens to third parties (2026-08-18)](todo/plugin-ecosystem/) | 3 | 0 | +| [Plugin infrastructure](todo/plugin-infrastructure/) | 2 | 0 | +| [Plugin repos — what writing their READMEs turned up](todo/plugin-repos/) | 10 | 9 | +| [Plugin system — gaps the email extraction found (2026-08-18)](todo/plugin-system/) | 3 | 0 | +| [Prompt library](todo/prompt-library/) | 1 | 0 | +| [Settings — portable, and readable by an agent](todo/settings/) | 2 | 0 | +| [Shell — spotlight and the dock](todo/shell/) | 1 | 1 | +| [Sub-agents — agents spawning agents](todo/sub-agents/) | 1 | 1 | +| [Sudo terminal modal](todo/sudo-terminal-modal/) | 1 | 0 | +| [Tooling — the list about the list](todo/tooling/) | 1 | 0 | +| [Transmission](todo/transmission/) | 6 | 0 | +| [URL state — routing, query strings and selection](todo/url-state/) | 4 | 0 | +| [Waiting on the owner to run it](todo/waiting-on-owner/) | 2 | 0 | +| [Workspaces & Panels](todo/workspaces-panels/) | 2 | 0 | ## Records — no items, kept for the reasoning diff --git a/src/server.tsx b/src/server.tsx index 76fa9bda..5ef6cfce 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -162,16 +162,60 @@ const handlers: Record = { sidecar: sidecarWebsocket, }; -// Both doors into the platform, checked against the permission registry before either opens. -// -// This throws rather than warns, and it throws HERE — before serve() — so a surface nobody has gated -// cannot be reached even once. The HTTP half comes from hono.ts's own mount table and the socket half -// from the map directly above, so neither list can be a stale copy of the thing it describes. +/** + * Every websocket path Bun serves, and the provider each one upgrades to. + * + * ── Why this exists rather than a hand-written route table ── + * + * The boot check below used to be fed `Object.keys(handlers)` — the map of provider implementations — while + * Bun served a SEPARATE, hand-listed set of routes. Two lists describing one thing, and the comment beside + * the check said neither could be a stale copy of the other, which was simply not true: a socket route with + * no handler, or with no registry claim, was invisible to the very check that exists to refuse it. + * + * It had a live instance. `/api/cliamp/ws` and `/api/cliamp/audio/ws` stayed in the route table for two + * days after their handlers and registry entries were commented out — not exploitable, since + * `isWsProviderAllowed` finds no permission and 403s a member while the owner upgrades onto a dead socket, + * but exactly the drift the guard is for. Deleting cliamp on 2026-08-15 removed the symptom and left the + * shape. + * + * Now the routes are GENERATED from this, and this is what totality reads. A path added here without a + * handler or without a registry claim fails the boot rather than serving quietly. + */ +const WS_ROUTES: Record = { + '/api/tasks/run/ws': 'task-runner', + '/api/tasks/pipeline/ws': 'pipeline', + '/api/terminal/ws': 'terminal', + '/api/chat/ws': 'chat', + '/api/chat/notify/ws': 'chat-notify', +}; + +// Every provider named by a served route, plus the two that upgrade through their own handlers rather than +// `upgradeWs`: `sidecar` (loopback process-to-process, its own route below) and `dev-proxy` (upgraded from +// inside the proxy router). Named explicitly so their absence from WS_ROUTES cannot read as an oversight. +const SERVED_WS_PROVIDERS = [...new Set([...Object.values(WS_ROUTES), 'sidecar', 'dev-proxy'])]; + +/* + Both doors into the platform, checked against the permission registry before either opens. + + This throws rather than warns, and it throws HERE — before serve() — so a surface nobody has gated cannot + be reached even once. The HTTP half comes from hono.ts's own mount table. The socket half now comes from + WS_ROUTES, which the route table is BUILT from, so the check reads what is actually served rather than a + parallel list that agrees with it by hand. + + A provider with a handler and no route is fine and is not checked: unreachable is the safe direction. The + failure this catches is the other one — a route reachable with nothing claiming it. +*/ assertPermissionTotality({ apiPrefixes: [...PROTECTED_API_PREFIXES, ...UNPROTECTED_API_PREFIXES], - wsProviders: Object.keys(handlers), + wsProviders: SERVED_WS_PROVIDERS, }); +// A route cannot name a provider that has no implementation. Cheap, and it turns a 500 on first connect +// into a refusal to boot with the name in it. +for (const [path, provider] of Object.entries(WS_ROUTES)) { + if (!handlers[provider]) throw new Error(`${path} upgrades to '${provider}', which has no handler`); +} + // Chat's phone channel. Subscribes to the same signal the browser bell uses and asks the same question; // it just answers it into APNs/FCM instead of a websocket. Registered here, and explicitly, because a // module whose only job is a side effect is one an import sweep quietly deletes. @@ -312,11 +356,16 @@ console.log( : '[web] serving via the HTML import (dev) — HMR is live, and `build/` is not used', ); -async function upgradeWs( - req: Request, - server: any, - provider: 'terminal' | 'chat' | 'chat-notify' | 'task-runner' | 'pipeline', -) { +/** + * A provider reachable by upgrading a URL. + * + * Named rather than inline so `WS_ROUTES` is checked against it: a route naming a provider this function + * cannot serve is now a typecheck failure instead of a runtime one. `sidecar` and `dev-proxy` are + * deliberately absent — they upgrade through their own handlers, not through here. + */ +type UpgradableProvider = 'terminal' | 'chat' | 'chat-notify' | 'task-runner' | 'pipeline'; + +async function upgradeWs(req: Request, server: any, provider: UpgradableProvider) { const token = new URL(req.url).searchParams.get('token'); if (!token) return new Response('Unauthorized', { status: 401 }); @@ -441,11 +490,13 @@ const server = serve({ }); if (!ok) return new Response('Upgrade failed', { status: 500 }); }, - '/api/tasks/run/ws': (req, server) => upgradeWs(req, server, 'task-runner'), - '/api/tasks/pipeline/ws': (req, server) => upgradeWs(req, server, 'pipeline'), - '/api/terminal/ws': (req, server) => upgradeWs(req, server, 'terminal'), - '/api/chat/ws': (req, server) => upgradeWs(req, server, 'chat'), - '/api/chat/notify/ws': (req, server) => upgradeWs(req, server, 'chat-notify'), + // Generated from WS_ROUTES, so this table and the boot check cannot describe different sets. + ...Object.fromEntries( + Object.entries(WS_ROUTES).map(([path, provider]) => [ + path, + (req: Request, server: any) => upgradeWs(req, server, provider), + ]), + ), // CalDAV/CardDAV. These live OUTSIDE /api because DAV clients are given a bare domain and probe // fixed, spec-defined paths — `/.well-known/caldav` unauthenticated, before they hold any // credential at all. They need naming explicitly here or the `/*` SPA fallback below swallows them diff --git a/todo/multi-user/assertpermissiontotality-checks-the-wrong.md b/todo/multi-user/assertpermissiontotality-checks-the-wrong.md index 809a50f2..0a67ff76 100644 --- a/todo/multi-user/assertpermissiontotality-checks-the-wrong.md +++ b/todo/multi-user/assertpermissiontotality-checks-the-wrong.md @@ -1,8 +1,8 @@ --- id: multi-user/assertpermissiontotality-checks-the-wrong -title: "`assertPermissionTotality` checks the wrong list, and `registry.test.ts` has been red since 2026-08-13" +title: '`assertPermissionTotality` checks the wrong list, and `registry.test.ts` has been red since 2026-08-13' theme: multi-user -status: open +status: done created: 2026-08-13 date_source: stated order: 12 @@ -17,3 +17,27 @@ a dead socket. But the boot check that exists to stop exactly this cannot see it totality at the route table, and either delete the dead routes or restore their claims. The 8 failing tests in `registry.test.ts` are the same drift — `REAL_WS` still lists all nine providers as served, which is why nobody noticed. Found 2026-08-14. + +--- + +**Fixed 2026-08-22.** Half of this item was already stale and half was still exactly true, which is worth +separating because the stale half is what would have made someone close it. + +Stale: the eight red tests in `registry.test.ts` are green, and have been since cliamp was deleted on +2026-08-15 — that removed the live instance of the drift along with the routes. + +Still true: the two lists were still two lists. `assertPermissionTotality` read `Object.keys(handlers)`, the +map of implementations, while Bun served a hand-written route table twenty lines away. The comment beside +the check claimed neither could be a stale copy of the other, and that was simply false. + +The fix is one declaration. `WS_ROUTES` maps path → provider, the Bun route table is GENERATED from it, and +the boot check reads it — so a socket route cannot exist without being checked. Two smaller guards came with +it: a route naming a provider with no handler now refuses to boot with the path in the message, and the +provider union that was inline in `upgradeWs` is named (`UpgradableProvider`) so the route table typechecks +against what that function can actually serve. + +`sidecar` and `dev-proxy` are listed explicitly as served-but-not-in-WS_ROUTES, because they upgrade through +their own handlers. Named rather than omitted, so their absence cannot read as an oversight. + +Verified by running the check against the real prefix lists: the served set passes, and adding one unclaimed +provider is refused with `websocket provider 'cliamp-audio' is served but no permission claims it`.