close the websocket hole: authenticate was never authorise
A Member account could open a terminal. Demonstrated, not inferred: on 2026-08-06 a token for dingoshf@gmail.com (role Member) was refused GET /api/tasks with 403 and, in the same minute, opened /api/tasks/pipeline/ws with 101 Switching Protocols. The account backstop lives in originScopeMiddleware, which is a Hono middleware. Websocket upgrades never reach Hono: Bun's route table in server.tsx matches '/api/terminal/ws' and its siblings before the '/api/*' entry that hands off, so NON_OWNER_PATHS was enforced on HTTP and nowhere else. upgradeWs verified the token and the blacklist, then upgraded — it proved WHO was calling and never asked what they could reach. None of the handlers behind it checked either; terminal, chat and desktop have no authorisation code at all. What was reachable with any valid token: a shell as the owner in the owner's home, the agent with --dangerously-skip-permissions, arbitrary script execution through task-runner and pipeline, and the owner's physical screen and keyboard over the VNC mirror. upgradeWs now refuses any provider a non-owner has no business on. cliamp and cliamp-audio stay open to them — those are the socket half of /api/music, which is what a music account exists for. An unrecognised provider denies. NON_OWNER_WS_PROVIDERS is declared beside NON_OWNER_PATHS on purpose. They are one rule at two doors, and the whole failure was that only one door had it; splitting them across files is how that happens again. The vault socket was already gated — it verifies isSuperAdmin in `open` and closes 4001 — so upgradeWs was the only gap. Not yet verified against the running server: platform TS does not hot-reload, so the 101 above still reproduces until `pm2 restart officer`. The provider table was checked in isolation: the five execution providers and an unknown name all deny, cliamp and cliamp-audio allow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,8 @@ import type { ServerWebSocket } from 'bun';
|
|||||||
import { serve } from 'bun';
|
import { serve } from 'bun';
|
||||||
import { honoServer } from './servers/hono';
|
import { honoServer } from './servers/hono';
|
||||||
import { verify } from './servers/jwt';
|
import { verify } from './servers/jwt';
|
||||||
|
import { isSuperAdmin } from './servers/super-admin';
|
||||||
|
import { isWsProviderAllowedForNonOwner } from './servers/_middlewares';
|
||||||
import { isTokenBlacklisted } from 'officerdb';
|
import { isTokenBlacklisted } from 'officerdb';
|
||||||
import { terminalWebsocket } from './servers/api/terminal/websocket';
|
import { terminalWebsocket } from './servers/api/terminal/websocket';
|
||||||
import { chatWebsocket } from './servers/api/chat/websocket';
|
import { chatWebsocket } from './servers/api/chat/websocket';
|
||||||
@@ -153,6 +155,14 @@ async function upgradeWs(
|
|||||||
if (await isTokenBlacklisted(user.jti)) return new Response('Unauthorized', { status: 401 });
|
if (await isTokenBlacklisted(user.jti)) return new Response('Unauthorized', { status: 401 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The account backstop, applied to sockets. Everything above this line AUTHENTICATES — it proves who
|
||||||
|
// is calling and never asks what they may reach. That is why a Member with a valid token could open
|
||||||
|
// a terminal here in the same minute it was 403'd on GET /api/tasks. Same rule as
|
||||||
|
// originScopeMiddleware, deliberately declared in that same file so the two cannot drift apart.
|
||||||
|
if (!(await isSuperAdmin(user)) && !isWsProviderAllowedForNonOwner(provider)) {
|
||||||
|
return new Response('Forbidden', { status: 403 });
|
||||||
|
}
|
||||||
|
|
||||||
const url = new URL(req.url);
|
const url = new URL(req.url);
|
||||||
const sessionId = url.searchParams.get('sessionId') ?? undefined;
|
const sessionId = url.searchParams.get('sessionId') ?? undefined;
|
||||||
const cwd = url.searchParams.get('cwd') ?? undefined;
|
const cwd = url.searchParams.get('cwd') ?? undefined;
|
||||||
|
|||||||
@@ -57,6 +57,29 @@ const APP_ORIGINS: string[] = APP_ORIGIN_LIST.map((a) => a.origin);
|
|||||||
// this is the ACCOUNT backstop below, not an origin rule — it holds whatever Origin a caller claims.
|
// this is the ACCOUNT backstop below, not an origin rule — it holds whatever Origin a caller claims.
|
||||||
const NON_OWNER_PATHS = ['/api/auth', '/api/music'];
|
const NON_OWNER_PATHS = ['/api/auth', '/api/music'];
|
||||||
|
|
||||||
|
// The SAME rule, for the other door into the platform.
|
||||||
|
//
|
||||||
|
// WebSocket upgrades never reach this file's middleware. Bun's route table in server.tsx matches
|
||||||
|
// '/api/terminal/ws' and friends before the '/api/*' catch-all that hands off to Hono, so the account
|
||||||
|
// backstop below — and every other Hono middleware — is simply not on that code path. It was written
|
||||||
|
// when routes were the only surface anyone was thinking about.
|
||||||
|
//
|
||||||
|
// The consequence was demonstrated on 2026-08-06: a Member token 403'd on `GET /api/tasks` opened
|
||||||
|
// `/api/tasks/pipeline/ws` with a 101 in the same minute. Terminal, chat, task-runner, pipeline and
|
||||||
|
// desktop were all reachable — a shell, the agent with --dangerously-skip-permissions, arbitrary script
|
||||||
|
// execution, and the owner's physical screen.
|
||||||
|
//
|
||||||
|
// Kept here, beside NON_OWNER_PATHS, because these two are one rule expressed at two doors. Split them
|
||||||
|
// across files and they drift; the drift is invisible until someone tries it. `cliamp`/`cliamp-audio`
|
||||||
|
// are the socket half of `/api/music` — the music app's playback transport, which is exactly what a
|
||||||
|
// music account is for.
|
||||||
|
const NON_OWNER_WS_PROVIDERS = ['cliamp', 'cliamp-audio'];
|
||||||
|
|
||||||
|
/** Whether a non-owner account may open this websocket provider. Owners bypass this entirely. */
|
||||||
|
export function isWsProviderAllowedForNonOwner(provider: string): boolean {
|
||||||
|
return NON_OWNER_WS_PROVIDERS.includes(provider);
|
||||||
|
}
|
||||||
|
|
||||||
function pathAllowed(path: string, prefixes: string[]): boolean {
|
function pathAllowed(path: string, prefixes: string[]): boolean {
|
||||||
return prefixes.some((prefix) => path === prefix || path.startsWith(`${prefix}/`));
|
return prefixes.some((prefix) => path === prefix || path.startsWith(`${prefix}/`));
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user