diff --git a/src/server.tsx b/src/server.tsx index 13dabf7c..51de4844 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -4,8 +4,7 @@ import { serve } from 'bun'; import { honoServer, PROTECTED_API_PREFIXES, UNPROTECTED_API_PREFIXES } from './servers/hono'; import { assertCapabilityTotality } from './servers/capabilities/totality'; import { verify } from './servers/jwt'; -import { isSuperAdmin } from './servers/super-admin'; -import { isWsProviderAllowedForNonOwner } from './servers/_middlewares'; +import { isWsProviderAllowed } from './servers/capabilities/authorize'; import { isTokenBlacklisted } from 'officerdb'; import { terminalWebsocket } from './servers/api/terminal/websocket'; import { chatWebsocket } from './servers/api/chat/websocket'; @@ -166,11 +165,15 @@ async function upgradeWs( 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)) { + // The capability 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. + // + // This resolves against the same registry as the HTTP door rather than a parallel list, which is the + // whole point: the two doors cannot disagree about what a role holds, because there is only one + // declaration to read. `terminal`, `chat`, `task-runner`, `pipeline` and `desktop` are refused here + // by being `execution` capabilities, not by being absent from an array someone has to maintain. + if (!(await isWsProviderAllowed(user.id, provider))) { return new Response('Forbidden', { status: 403 }); } diff --git a/src/servers/_middlewares/origin-validation.ts b/src/servers/_middlewares/origin-validation.ts index 4e1ecc97..837624a7 100644 --- a/src/servers/_middlewares/origin-validation.ts +++ b/src/servers/_middlewares/origin-validation.ts @@ -3,6 +3,8 @@ import * as errors from '../custom-errors'; import { IS_DEV_BUILD } from '../build-env'; import { verify } from '../jwt'; import { isSuperAdmin } from '../super-admin'; +import { isApiRequestAllowed } from '../capabilities/authorize'; +import { isExemptApiPath } from '../capabilities/totality'; const { PUBLIC_URL } = process.env; @@ -53,32 +55,15 @@ const APP_ORIGIN_LIST: AppOrigin[] = Object.entries(process.env).flatMap(([key, const APP_ORIGINS: string[] = APP_ORIGIN_LIST.map((a) => a.origin); -// The only path prefixes a non-owner account (and the music app) may reach. Still hand-written because -// 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']; - -// The SAME rule, for the other door into the platform. +// The account backstop used to live here as two hand-written lists: NON_OWNER_PATHS, confining every +// non-owner to '/api/auth' + '/api/music', and NON_OWNER_WS_PROVIDERS doing the same for sockets. Both +// are gone, replaced by the capability registry (src/servers/capabilities/). // -// 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); -} +// They were not wrong, they were unscalable in one specific way: a hardcoded allow-list answers "which +// paths" but never "why", so onboarding anyone who needed anything other than music meant editing an +// array in a middleware file and hoping the socket half got edited too. The registry makes the two doors +// read the same declaration, and the boot-time totality check makes a THIRD door impossible to add +// without noticing. See capabilities/totality.ts for the incident that motivated it. function pathAllowed(path: string, prefixes: string[]): boolean { return prefixes.some((prefix) => path === prefix || path.startsWith(`${prefix}/`)); @@ -212,9 +197,16 @@ export const originScopeMiddleware: MiddlewareHandler = async function (ctx, nex } const isOwner = payload ? await isSuperAdmin(payload) : false; - // 1. Account backstop — non-owner accounts are confined to /api/auth + /api/music everywhere. - if (payload && !isOwner && !pathAllowed(path, NON_OWNER_PATHS)) { - throw errors.FORBIDDEN('This account is limited to the music app'); + // 1. Capability backstop — origin-INDEPENDENT, and the airtight half of this middleware. A valid + // non-owner token may reach only what its ROLE has been granted, whatever Origin it claims and whether + // or not it sends one. Exempt paths (signin, the public pages, the Bitwarden door) are skipped because + // they are served above the account gate — the same list the boot check uses, deliberately. + // + // Non-/api paths are not ours: the DAV sync door authenticates with its own app password and carries no + // platform account, so there is nothing here to resolve. + if (payload && !isOwner && path.startsWith('/api') && !isExemptApiPath(path)) { + const { allowed, reason } = await isApiRequestAllowed(payload.id, ctx.req.method, path); + if (!allowed) throw errors.FORBIDDEN(reason ?? 'Not permitted for this account'); } // 2. Per-origin rules. Skipped entirely while origin checking is off (the account backstop above is diff --git a/src/servers/capabilities/authorize.ts b/src/servers/capabilities/authorize.ts new file mode 100644 index 00000000..aece6343 --- /dev/null +++ b/src/servers/capabilities/authorize.ts @@ -0,0 +1,145 @@ +import { getUserById, getRoleGrants } from 'officerdb'; +import type { UserRole } from 'officerdb'; +import { + CAPABILITY_BY_KEY, + CORE_CAPABILITIES, + capabilityForApiPath, + capabilityForWsProvider, + isRequestAllowedAtLevel, + type CapabilityLevel, +} from './registry'; + +// Resolving "may this account do this". Every deny path in the platform ends up here. +// +// Two rules run the whole file: +// +// 1. The owner bypasses everything. isSuperAdmin is the single question asked first, and a Super Admin +// never consults the grants table — which is why the schema refuses to store a row for that role. +// 2. Everyone else gets core capabilities plus whatever their ROLE has been granted, and nothing else. +// An unrecognised capability, a missing row, a database error, a user who no longer exists: all deny. +// +// Fail-closed is not decoration here. This function is what stands between a Member and a shell, and the +// failure mode of a permissive default is not a bug report — it is someone else's session. Every catch in +// this file returns "no", and none of them log-and-continue. + +export type EffectiveCapabilities = { + isOwner: boolean; + /** Capability key → level. Empty for an account with nothing granted; the owner's is never consulted. */ + grants: Map; +}; + +// ── Grant cache ─────────────────────────────────────────────────────────────────────────────────── +// +// Keyed on ROLE, not user, so it holds at most one entry per role and a new account needs no warm-up. +// +// super-admin.ts deliberately does NOT cache, and says why: a cache with no invalidation contract is a +// staleness bug waiting for whoever builds the role UI. This one has a contract — the only writer is the +// grants API in api/users, which calls invalidateRoleGrants on every mutation, in this same process. That +// is the entire set of writers; if a second one ever appears it has to call this too, which is why the +// cache and its invalidator live in the same file as the reader that depends on them. +const grantCache = new Map>(); + +/** Called by every path that writes a grant. Clears one role, or all of them. */ +export function invalidateRoleGrants(role?: UserRole): void { + if (role) grantCache.delete(role); + else grantCache.clear(); +} + +async function grantsForRole(role: UserRole): Promise> { + const cached = grantCache.get(role); + if (cached) return cached; + const grants = (await getRoleGrants(role)) as Map; + grantCache.set(role, grants); + return grants; +} + +/** + * What this account may reach, resolved from its role. + * + * Core capabilities come in at `write` unconditionally: they are the caller's own profile, dock and bug + * reports, and a read-only version of "change your own password" is not a coherent thing to offer. + * + * `execution` and `admin` capabilities are dropped even if a row somehow grants them. The API refuses to + * write such a row, but this is the layer that has to hold if one ever exists — a constraint the database + * does not enforce is a constraint the reader must. + */ +export async function getEffectiveCapabilities(userId: number | undefined): Promise { + const empty: EffectiveCapabilities = { isOwner: false, grants: new Map() }; + if (!userId) return empty; + + try { + const user = await getUserById(userId); + if (!user) return empty; + if (user.role === 'Super Admin') return { isOwner: true, grants: new Map() }; + + const grants = new Map(); + for (const capability of CORE_CAPABILITIES) grants.set(capability.key, 'write'); + + for (const [key, level] of await grantsForRole(user.role)) { + const capability = CAPABILITY_BY_KEY.get(key); + // Unknown key: a capability that was renamed or removed while a grant survived. Ignore it — the + // alternative is honouring a name nothing defines. + if (!capability) continue; + if (capability.kind !== 'app') continue; + grants.set(key, level); + } + + return { isOwner: false, grants }; + } catch { + // A transient database error must never grant anything. Deny, and let the next request retry. + return empty; + } +} + +/** + * May this account make this HTTP request? + * + * `path` is the full request path. Paths outside `/api` are not this function's business — the DAV sync + * door authenticates with its own app password and never carries a platform account. + */ +export async function isApiRequestAllowed( + userId: number | undefined, + method: string, + path: string, +): Promise<{ allowed: boolean; reason?: string }> { + const { isOwner, grants } = await getEffectiveCapabilities(userId); + if (isOwner) return { allowed: true }; + + const capability = capabilityForApiPath(path); + // Totality guarantees every mounted, non-exempt prefix maps to a capability, so reaching this branch + // means either an exempt prefix (which the caller checks before us) or a path nothing serves. Deny: + // a 403 on a route that does not exist is not a leak, and a permissive default here would be. + if (!capability) return { allowed: false, reason: 'no capability covers this path' }; + + if (capability.kind === 'execution') { + return { allowed: false, reason: `${capability.label} runs as the server owner and cannot be shared` }; + } + if (capability.kind === 'admin') { + return { allowed: false, reason: `${capability.label} is restricted to the server owner` }; + } + + const level = grants.get(capability.key); + if (!level) return { allowed: false, reason: `your role does not have access to ${capability.label}` }; + + if (!isRequestAllowedAtLevel(capability, level, method, path)) { + return { allowed: false, reason: `you have read-only access to ${capability.label}` }; + } + return { allowed: true }; +} + +/** + * May this account open this WebSocket provider? + * + * There is no method to reason about, so a socket needs the capability at any level. That is deliberate + * for the two that reach here — cliamp and cliamp-audio are the music app's playback transport, which is + * the whole point of a music account. Every other provider belongs to an `execution` capability and is + * refused above, structurally, rather than by being left off a list. + */ +export async function isWsProviderAllowed(userId: number | undefined, provider: string): Promise { + const { isOwner, grants } = await getEffectiveCapabilities(userId); + if (isOwner) return true; + + const capability = capabilityForWsProvider(provider); + if (!capability || capability.kind !== 'app') return false; + return grants.has(capability.key); +} diff --git a/src/servers/capabilities/registry.test.ts b/src/servers/capabilities/registry.test.ts new file mode 100644 index 00000000..c7158a9a --- /dev/null +++ b/src/servers/capabilities/registry.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, test } from 'bun:test'; +import { + CAPABILITIES, + CAPABILITY_BY_KEY, + GRANTABLE_CAPABILITIES, + capabilityForApiPath, + capabilityForWsProvider, + isRequestAllowedAtLevel, +} from './registry'; +import { assertCapabilityTotality, isExemptApiPath } from './totality'; + +// The database-backed half (authorize.ts) is exercised against the live schema; this file covers the pure +// half, which is where the rules actually live. Everything here runs without a database. + +const REAL_WS = [ + 'terminal', + 'chat', + 'task-runner', + 'pipeline', + 'cliamp', + 'cliamp-audio', + 'desktop', + 'vault', + 'sidecar', +]; +const realApi = () => [...new Set(CAPABILITIES.flatMap((c) => c.api))]; +const surface = () => ({ + apiPrefixes: [...realApi(), '/auth', '/landing-page-data', '/waitlist', '/vault', '/sidecar'], + wsProviders: REAL_WS, +}); + +describe('totality', () => { + test('the registry covers its own declared surface', () => { + expect(() => assertCapabilityTotality(surface())).not.toThrow(); + }); + + // The four ways this is allowed to fail. Each one is a real bug it exists to catch, and a check that + // only ever passes is worth nothing — so assert that it refuses, not merely that it runs. + test('refuses a mounted router no capability claims', () => { + const s = surface(); + s.apiPrefixes.push('/newthing'); + expect(() => assertCapabilityTotality(s)).toThrow(/\/api\/newthing is mounted but no capability claims it/); + }); + + test('refuses a served socket no capability claims — the 2026-08-06 hole', () => { + const s = surface(); + s.wsProviders = [...REAL_WS, 'newsocket']; + expect(() => assertCapabilityTotality(s)).toThrow(/websocket provider 'newsocket' is served/); + }); + + test('refuses a claim on a router that no longer exists', () => { + const s = surface(); + s.apiPrefixes = s.apiPrefixes.filter((p) => p !== '/gitea'); + expect(() => assertCapabilityTotality(s)).toThrow(/claims \/api\/gitea, which nothing mounts/); + }); + + test('refuses a claim on a socket that is not served', () => { + const s = surface(); + s.wsProviders = REAL_WS.filter((p) => p !== 'cliamp'); + expect(() => assertCapabilityTotality(s)).toThrow(/claims websocket 'cliamp', which is not served/); + }); + + test('no two capabilities claim the same prefix', () => { + const seen = new Map(); + for (const capability of CAPABILITIES) { + for (const prefix of capability.api) { + expect(seen.has(prefix)).toBe(false); + seen.set(prefix, capability.key); + } + } + }); + + test('signin is exempt and gitea is not', () => { + expect(isExemptApiPath('/api/auth/signin')).toBe(true); + expect(isExemptApiPath('/api/gitea')).toBe(false); + expect(isExemptApiPath('/api/music/albums')).toBe(false); + }); +}); + +describe('path → capability', () => { + test('resolves a prefix and its descendants', () => { + expect(capabilityForApiPath('/api/gitea')?.key).toBe('gitea'); + expect(capabilityForApiPath('/api/gitea/repos/a/b')?.key).toBe('gitea'); + }); + + test('does not match a prefix that is merely a string prefix', () => { + // '/api/musicbrainz' must not resolve to the 'music' capability. A naive startsWith would. + expect(capabilityForApiPath('/api/musicbrainz')).toBeNull(); + }); + + test('longest prefix wins, so /dav and /caldav do not fight', () => { + expect(capabilityForApiPath('/api/caldav/x')?.key).toBe('calendar'); + expect(capabilityForApiPath('/api/dav/x')?.key).toBe('calendar'); + }); + + test('an unclaimed path resolves to nothing rather than to something permissive', () => { + expect(capabilityForApiPath('/api/not-a-thing')).toBeNull(); + }); + + test('sockets resolve to their capability', () => { + expect(capabilityForWsProvider('cliamp')?.key).toBe('music'); + expect(capabilityForWsProvider('terminal')?.key).toBe('terminal'); + expect(capabilityForWsProvider('nope')).toBeNull(); + }); +}); + +describe('levels', () => { + const music = CAPABILITY_BY_KEY.get('music')!; + + test('write permits anything within the capability', () => { + expect(isRequestAllowedAtLevel(music, 'write', 'DELETE', '/api/music/track/9')).toBe(true); + }); + + test('read permits safe methods', () => { + expect(isRequestAllowedAtLevel(music, 'read', 'GET', '/api/music/albums')).toBe(true); + expect(isRequestAllowedAtLevel(music, 'read', 'HEAD', '/api/music/albums')).toBe(true); + }); + + test('read permits mutations only under personal sub-paths', () => { + expect(isRequestAllowedAtLevel(music, 'read', 'POST', '/api/music/favorites/7')).toBe(true); + expect(isRequestAllowedAtLevel(music, 'read', 'PUT', '/api/music/now-playing')).toBe(true); + expect(isRequestAllowedAtLevel(music, 'read', 'POST', '/api/music/scan')).toBe(false); + expect(isRequestAllowedAtLevel(music, 'read', 'DELETE', '/api/music/track/9')).toBe(false); + }); + + test('a personal entry does not leak across a name boundary', () => { + // '/favorites-export' must not be covered by the '/favorites' personal entry. + expect(isRequestAllowedAtLevel(music, 'read', 'POST', '/api/music/favorites-export')).toBe(false); + }); +}); + +describe('kinds', () => { + test('execution capabilities are never grantable', () => { + const grantable = new Set(GRANTABLE_CAPABILITIES.map((c) => c.key)); + for (const key of ['terminal', 'chat', 'files', 'tasks', 'desktop', 'browser', 'items']) { + expect(CAPABILITY_BY_KEY.get(key)?.kind).toBe('execution'); + expect(grantable.has(key)).toBe(false); + } + }); + + test('admin capabilities are never grantable', () => { + const grantable = new Set(GRANTABLE_CAPABILITIES.map((c) => c.key)); + for (const key of ['user-admin', 'server-admin', 'wallet', 'headscale']) { + expect(CAPABILITY_BY_KEY.get(key)?.kind).toBe('admin'); + expect(grantable.has(key)).toBe(false); + } + }); + + test('gitea is grantable — the case this was built for', () => { + expect(GRANTABLE_CAPABILITIES.some((c) => c.key === 'gitea')).toBe(true); + }); + + test('every capability declares at least one api prefix', () => { + for (const capability of CAPABILITIES) expect(capability.api.length).toBeGreaterThan(0); + }); +}); diff --git a/src/servers/capabilities/totality.ts b/src/servers/capabilities/totality.ts index 8ca37f9b..b38fc281 100644 --- a/src/servers/capabilities/totality.ts +++ b/src/servers/capabilities/totality.ts @@ -43,6 +43,19 @@ const EXEMPT_WS_PROVIDERS: Record = { vault: 'Vaultwarden notifications hub; upgraded by upgradeVaultWs with a Bitwarden token', }; +/** + * Is this path served above the account gate? + * + * Read by the capability backstop, so that signin — which by definition has no account to check — is not + * asked to prove a capability. Deliberately shares EXEMPT_API_PREFIXES with the boot check: an exemption + * granted at boot and an exemption honoured at request time must be the same list, or one of them is a + * hole. + */ +export function isExemptApiPath(path: string): boolean { + const rest = path.startsWith('/api') ? path.slice('/api'.length) : path; + return Object.keys(EXEMPT_API_PREFIXES).some((prefix) => rest === prefix || rest.startsWith(`${prefix}/`)); +} + export type CapabilitySurface = { /** Every prefix mounted on protectedRouter, as written in hono.ts. */ apiPrefixes: string[];