officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

the boot check now reads the sockets that are served, not a list beside them

Browse Source 
assertPermissionTotality exists to refuse a surface nobody has gated, and it was reading the wrong list:
Object.keys(handlers) — the map of implementations — while Bun served a hand-written route table twenty
lines away. The comment beside it said neither could be a stale copy of the other. That was false, and 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 claims were commented out. Not exploitable — isWsProviderAllowed finds no
permission and 403s a member, while the owner upgrades onto a dead socket — but precisely the drift the
guard is for, invisible to the guard.

Now there is one declaration. WS_ROUTES maps path → provider, the route table is generated from it, and the
check reads it. A socket route cannot exist without being checked.

Two smaller guards arrived with it. A route naming a provider with no handler refuses to boot, with the path
in the message, instead of 500-ing on first connect. And the provider union that was inline in upgradeWs is
named, so the route table typechecks against what that function can actually serve — which caught my first
attempt, where WS_ROUTES was Record<string, string>.

sidecar and dev-proxy are listed explicitly as served-but-absent-from-WS_ROUTES, because they upgrade
through their own handlers rather than upgradeWs. Named so the absence cannot read as an oversight.

Half the TODO was stale and it is recorded as such rather than quietly closed: the eight red tests it cites
have been green since cliamp was deleted on 2026-08-15. Deleting cliamp removed the symptom and left the
shape, which is the more interesting half.

Verified against the real prefix lists rather than by reasoning: the served set passes, one unclaimed
provider is refused by name, all five generated routes answer 401, and the server boots.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
pastilhas
2026-08-22 04:25:48 +00:00
co-authored by Claude Opus 5
parent 78db9c8c93
commit c982353381
3 changed files with 125 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
TODO.md
+32 -32
View File
@@ -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
src/server.tsx
+67 -16
View File
@@ -162,16 +162,60 @@ const handlers: Record<string, any> = {
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<string, UpgradableProvider> = {
'/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
todo/multi-user/assertpermissiontotality-checks-the-wrong.md
+26 -2
View File
@@ -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`.