diff --git a/src/server.tsx b/src/server.tsx index 2faaf7ac..cd4b5768 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -5,6 +5,7 @@ import { join } from 'node:path'; import { honoServer, PROTECTED_API_PREFIXES, UNPROTECTED_API_PREFIXES } from './servers/hono'; import { assertPermissionTotality } from './servers/permissions/totality'; import { refreshPluginMounts, snapshotPlugins } from './servers/plugins/mount'; +import { listPluginRootClaims } from './servers/plugins/public-paths'; import { generatePluginsModule, rebuildFrontend, BUILD_DIR, SHELL_FILE } from './servers/plugins/generate'; import { assertInstallLayout } from './servers/data-path'; import { PORT } from './servers/officer-url.mjs'; @@ -429,6 +430,25 @@ const server = serve({ // '/.well-known/carddav': honoServer.fetch, // '/dav': honoServer.fetch, // '/dav/*': honoServer.fetch, + /* + Top-level paths a plugin claims (`/identity/*`, …), handed to hono BEFORE the SPA fallback. + + They need naming here for the same reason `/dav` did: Bun matches this table first, and only the + paths listed reach hono — anything else falls through to `/*`, which answers 200 with the React + shell. A missing entry therefore looks like a working endpoint returning nonsense rather than a 404, + which is among the least debuggable failures available. + + Built from the plugin set because `refreshPluginMounts` has already run by here (line ~204, well + before `serve()`), so the claims are known. The cost is that adding or removing one needs a restart — + this table is fixed once `serve()` is called. Stated rather than worked around: installing a plugin + that claims a root path is the one install that is not complete until `pm2 restart officer`. + */ + ...Object.fromEntries( + listPluginRootClaims().flatMap(({ segment }) => [ + [`/${segment}`, (req: Request, server: unknown) => honoServer.fetch(req, server)], + [`/${segment}/*`, (req: Request, server: unknown) => honoServer.fetch(req, server)], + ]), + ), ...spaRoutes, '/api': honoServer.fetch, // A CLOSURE, deliberately, and not the bound `honoServer.fetch`. diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 3da18cc4..47f99b57 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -22,7 +22,7 @@ import { dashboardsRouter } from './api/dashboards'; import { router as fileBrowserRouter } from './api/file-browser/router'; import { devProxyRouter } from './api/dev-proxy/router'; import { pluginsRouter } from './api/plugins/router'; -import { isPluginPublicPath } from './plugins/public-paths'; +import { isPluginPublicPath, pluginForProtocolHeaders } from './plugins/public-paths'; import { agentHandoffRouter } from './api/agent-handoff/router'; // import { slskdRouter } from './api/slskd/router'; // import { transmissionRouter } from './api/transmission/router'; @@ -64,7 +64,20 @@ export type { HonoVariables }; * A plugin's router and where it mounts — a plain pair, so this file needs no plugin knowledge at all. * Built by `plugins/mount.ts`, which is the side that knows what a manifest is. */ -export type MountedPlugin = { prefix: string; router: ReturnType }; +export type MountedPlugin = { + prefix: string; + router: ReturnType; + /** + * An OPTIONAL second router served above the account gate — at declared top-level paths, and for + * `/api/*` requests carrying a declared protocol header. Absent for every plugin that does not have a + * foreign protocol to host; see `unauthenticatedRootPaths` in plugins/manifest.ts. + */ + publicRouter?: ReturnType; + /** Top-level segments this plugin claims, already de-duplicated against other plugins. */ + rootSegments: string[]; + /** Headers that hand an `/api/*` request to `publicRouter` instead of to Officer. */ + protocolHeaders: string[]; +}; // ── The app is BUILT, not assembled once ── // @@ -188,6 +201,36 @@ export function buildHonoApp(plugins: MountedPlugin[] = []): Hono<{ Variables: H app.use((ctx, next) => (isDavPath(ctx.req.path) ? next() : corsMiddleware(ctx, next))); + /* + A plugin's PUBLIC router, mounted above everything — the gate, `userMiddleware`, the whole platform API. + + Two shapes, both for the same reason: a client speaking a protocol Officer does not define, which + derives its own URLs and cannot be told to look elsewhere. See `unauthenticatedRootPaths` in + plugins/manifest.ts for why this exists at all and how narrow it is kept. + + root paths `/identity/*` and friends — a single claimed segment each, refused if reserved or + already taken. + protocol header `/api/*` when the request carries a declared header. Registered BEFORE the platform's + own `/api` mount, because hono matches in registration order and this has to win. + ROUTING, not authentication — a forged header reaches the plugin, which demands its + own credential; it cannot reach anything of Officer's. + */ + for (const plugin of plugins) { + if (!plugin.publicRouter) continue; + for (const segment of plugin.rootSegments) app.route(`/${segment}`, plugin.publicRouter); + } + + const protocolPlugins = plugins.filter((p) => p.publicRouter && p.protocolHeaders.length); + if (protocolPlugins.length) { + app.use('/api/*', async (ctx, next) => { + const appName = pluginForProtocolHeaders(ctx.req.raw.headers); + if (!appName) return next(); + const claimant = protocolPlugins.find((p) => p.prefix === `/${appName}` || p.prefix.endsWith(`/${appName}`)); + if (!claimant?.publicRouter) return next(); + return claimant.publicRouter.fetch(ctx.req.raw, ctx.env); + }); + } + // The authorization gate: a valid non-owner token reaches only what its role grants. Ahead of every // router, and it re-verifies the token itself so it covers routes that never mount userMiddleware. app.use(permissionGateMiddleware); diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index f4c5e1a6..3d2a6ab7 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -166,6 +166,48 @@ export type PluginManifest = { */ unauthenticatedPaths?: string[]; + /** + * TOP-LEVEL paths this plugin serves, above the account gate and outside its own `/api/` mount. + * + * ── Why this exists, when `unauthenticatedPaths` already does ── + * + * Some clients are not ours and cannot be told where to look. A Bitwarden client asked for a + * self-hosted server takes ONE URL and derives `/identity`, `/icons`, `/notifications` and `/events` + * from it itself — at the root, always. Hosting the same thing under a plugin's mount is technically + * equivalent and practically useless: it turns "paste your Officer URL" into five fields copied by hand + * into an advanced settings pane, which is not a thing anyone will do and not a thing to ship. + * + * So this is the escape hatch for a protocol the platform does not define and cannot move. It is + * deliberately narrower than it looks: + * + * - one segment only (`/identity`, never `/a/b`), so a plugin claims a name rather than a namespace + * - refused if it collides with anything the platform serves, checked against a reserved list + * - refused if another installed plugin already claims it + * - every entry logged at boot, by plugin, because a root path answering without a session is not + * something anyone should have to read a manifest to discover + * + * Absent for every plugin but one, and it should stay that way — reach for `unauthenticatedPaths` + * unless a client genuinely cannot be pointed anywhere else. + */ + unauthenticatedRootPaths?: string[]; + + /** + * Request headers that hand `/api/*` to this plugin instead of to Officer. + * + * The other half of the same problem. Vaultwarden serves `/api/sync`, `/api/ciphers`; Officer serves + * `/api/chat`, `/api/settings`. A Bitwarden client pointed at the bare Officer URL calls the first set + * at the same prefix as the second, and the PATH cannot tell them apart — so the client says who it is, + * and every Bitwarden client stamps `Bitwarden-Client-Name` (or older ones `Device-Type`). + * + * ── This is ROUTING, not authentication ── + * + * Trusting a client-supplied header is fine here, and it is worth being precise about why: the worst a + * forged header achieves is reaching the plugin's own public router, which then demands whatever + * credential it demands. Nothing is authorised by this. An attacker who sets the header does not gain + * access to Officer's API — they lose it, for their own request. + */ + protocolHeaders?: string[]; + /** * Host binaries this plugin needs. Absent means it is self-sufficient, which offscale and example are. * @@ -254,6 +296,42 @@ export function publicPathProblem(path: unknown): string | null { return null; } +/** + * Top-level names the platform serves itself. A plugin claiming one would shadow it for every request, + * so each is refused by name rather than by ordering luck. + * + * `api` is the important one: it is where everything of Officer's lives, and `protocolHeaders` is the + * only sanctioned way to touch it. + */ +const RESERVED_ROOT = new Set([ + 'api', + 'auth', + 'plugins', + 'assets', + 'build', + 'public', + 'dav', + '.well-known', + 'index.html', +]); + +/** + * Check one declared ROOT path, returning a reason it is refused or null. + * + * Stricter than `publicPathProblem` because the surface is: this is the platform's own namespace, not the + * plugin's, so a plugin gets to claim a single name and nothing structural. + */ +export function rootPathProblem(path: unknown): string | null { + if (typeof path !== 'string' || !path) return 'must be a non-empty string'; + if (!path.startsWith('/')) return `"${path}" must start with "/"`; + const segment = path.slice(1); + if (!/^[a-z][a-z0-9-]{0,38}$/.test(segment)) { + return `"${path}" must be a single lowercase segment, e.g. "/identity"`; + } + if (RESERVED_ROOT.has(segment)) return `"${path}" is reserved by the platform`; + return null; +} + export function mountPrefix(plugin: { appName: string; manifest: { publisher: string } }): string { const { appName } = plugin; return plugin.manifest.publisher === FIRST_PARTY_PUBLISHER @@ -345,6 +423,27 @@ export function manifestProblems(appName: string, manifest: Partial { export async function loadPluginRouter(plugin: DiscoveredPlugin): Promise { if (!plugin.api) return null; - const module = (await import(plugin.api)) as { router?: unknown }; + const module = (await import(plugin.api)) as { router?: unknown; publicRouter?: unknown }; const router = module.router; if (!router || typeof (router as { fetch?: unknown }).fetch !== 'function') { throw new Error(`${plugin.appName}: api/router.ts must export \`router\` (a Hono router)`); } - return { prefix: mountPrefix(plugin), router: router as MountedPlugin['router'] }; + // Optional, and only meaningful alongside a manifest that declares where it is served. A plugin that + // exports one without declaring anything gets it loaded and mounted nowhere, which is the safe way + // round: the manifest is what the owner consented to, not the export. + const publicRouter = + module.publicRouter && typeof (module.publicRouter as { fetch?: unknown }).fetch === 'function' + ? (module.publicRouter as MountedPlugin['router']) + : undefined; + + return { + prefix: mountPrefix(plugin), + router: router as MountedPlugin['router'], + ...(publicRouter ? { publicRouter } : {}), + rootSegments: [], + protocolHeaders: plugin.manifest.protocolHeaders ?? [], + }; } /** The plugins that should be mounted right now: installed, enabled, and carrying an `api/router.ts`. */ @@ -161,7 +183,38 @@ export async function refreshPluginMounts(options: { skipFrontend?: boolean } = console.log(`[plugins] served WITHOUT a session: ${shown}`); } + // Top-level claims, resolved against ONE accumulating set so two plugins cannot both own `/identity`. + // First come wins and the loser is named, rather than the last mount silently shadowing the first. + const taken = new Set(); + const rootClaims: RootClaim[] = []; + const headerClaims: { header: string; appName: string }[] = []; + for (const { plugin } of enabled) { + const { segments, refused } = resolvePluginRootPaths(plugin, taken); + for (const reason of refused) console.error(`[plugins] root path refused — ${reason}`); + for (const segment of segments) rootClaims.push({ segment, appName: plugin.appName }); + for (const header of plugin.manifest.protocolHeaders ?? []) { + headerClaims.push({ header: header.toLowerCase(), appName: plugin.appName }); + } + } + setPluginRootClaims(rootClaims); + setPluginProtocolHeaders(headerClaims); + if (rootClaims.length) { + console.log(`[plugins] top-level paths: ${rootClaims.map((c) => `/${c.segment} (${c.appName})`).join(', ')}`); + } + if (headerClaims.length) { + console.log( + `[plugins] /api diverted by header: ${headerClaims.map((c) => `${c.header} -> ${c.appName}`).join(', ')}`, + ); + } + const mounted = await mountablePlugins(snapshot); + // The claims were resolved above from the manifests; attach them to the entries hono actually mounts, + // so the router set and the claim set can never describe different plugins. + for (const entry of mounted) { + const appName = entry.prefix.split('/').filter(Boolean).pop(); + entry.rootSegments = rootClaims.filter((c) => c.appName === appName).map((c) => c.segment); + entry.protocolHeaders = headerClaims.filter((c) => c.appName === appName).map((c) => c.header); + } rebuildHonoApp(mounted); // Republish every enabled plugin's `assets/` on the way past. diff --git a/src/servers/plugins/public-paths.test.ts b/src/servers/plugins/public-paths.test.ts index 3cc362e2..9ac0a57a 100644 --- a/src/servers/plugins/public-paths.test.ts +++ b/src/servers/plugins/public-paths.test.ts @@ -1,5 +1,14 @@ import { describe, expect, test, afterEach } from 'bun:test'; -import { isPluginPublicPath, resolvePluginPublicPaths, setPluginPublicPaths } from './public-paths'; +import { + isPluginPublicPath, + pluginForProtocolHeaders, + pluginForRootPath, + resolvePluginPublicPaths, + resolvePluginRootPaths, + setPluginProtocolHeaders, + setPluginPublicPaths, + setPluginRootClaims, +} from './public-paths'; import type { DiscoveredPlugin } from './manifest'; // This is an auth boundary: everything matched here skips `userMiddleware` AND the permission gate. The @@ -27,7 +36,11 @@ const plugin = (appName: string, unauthenticatedPaths?: string[], publisher = 'o web: null, }) as DiscoveredPlugin; -afterEach(() => setPluginPublicPaths([])); +afterEach(() => { + setPluginPublicPaths([]); + setPluginRootClaims([]); + setPluginProtocolHeaders([]); +}); describe('resolving a plugin’s declared paths', () => { test('resolves against the plugin’s own mount', () => { @@ -100,3 +113,58 @@ describe('matching a request path', () => { expect(isPluginPublicPath('/api/users')).toBe(false); }); }); + +describe('top-level claims', () => { + const withRoots = (appName: string, roots: string[]) => { + const p = plugin(appName); + (p.manifest as { unauthenticatedRootPaths?: string[] }).unauthenticatedRootPaths = roots; + return p; + }; + + test('a single lowercase segment is claimed', () => { + const { segments, refused } = resolvePluginRootPaths(withRoots('vault', ['/identity', '/icons']), new Set()); + expect(segments).toEqual(['identity', 'icons']); + expect(refused).toEqual([]); + }); + + // The whole point of the reserved list: a plugin claiming these would shadow the platform for every + // request, and `/api` in particular is where all of Officer lives. + test.each([['/api'], ['/auth'], ['/plugins'], ['/dav']])('refuses the reserved segment %s', (root) => { + const { segments, refused } = resolvePluginRootPaths(withRoots('evil', [root]), new Set()); + expect(segments).toEqual([]); + expect(refused).toHaveLength(1); + }); + + test.each([['/a/b'], ['identity'], ['/Identity'], ['/']])('refuses the malformed segment %s', (root) => { + expect(resolvePluginRootPaths(withRoots('evil', [root]), new Set()).segments).toEqual([]); + }); + + test('two plugins cannot both claim one segment — first come, loser named', () => { + const taken = new Set(); + expect(resolvePluginRootPaths(withRoots('vault', ['/identity']), taken).segments).toEqual(['identity']); + const second = resolvePluginRootPaths(withRoots('other', ['/identity']), taken); + expect(second.segments).toEqual([]); + expect(second.refused[0]).toContain('already claimed'); + }); + + test('matching a claimed root is by segment, not string prefix', () => { + setPluginRootClaims([{ segment: 'identity', appName: 'vault' }]); + expect(pluginForRootPath('/identity')).toBe('vault'); + expect(pluginForRootPath('/identity/connect/token')).toBe('vault'); + expect(pluginForRootPath('/identityx')).toBeNull(); + expect(pluginForRootPath('/other')).toBeNull(); + }); + + test('a protocol header hands /api over, and its absence does not', () => { + setPluginProtocolHeaders([{ header: 'bitwarden-client-name', appName: 'vault' }]); + expect(pluginForProtocolHeaders(new Headers({ 'bitwarden-client-name': 'browser' }))).toBe('vault'); + expect(pluginForProtocolHeaders(new Headers({ authorization: 'Bearer x' }))).toBeNull(); + }); + + test('nothing is claimed by default', () => { + setPluginRootClaims([]); + setPluginProtocolHeaders([]); + expect(pluginForRootPath('/identity')).toBeNull(); + expect(pluginForProtocolHeaders(new Headers({ 'bitwarden-client-name': 'browser' }))).toBeNull(); + }); +}); diff --git a/src/servers/plugins/public-paths.ts b/src/servers/plugins/public-paths.ts index f26c168d..fad5c258 100644 --- a/src/servers/plugins/public-paths.ts +++ b/src/servers/plugins/public-paths.ts @@ -1,4 +1,4 @@ -import { mountPrefix, publicPathProblem, type DiscoveredPlugin } from './manifest'; +import { mountPrefix, publicPathProblem, rootPathProblem, type DiscoveredPlugin } from './manifest'; // Which plugin paths are served ABOVE the account gate. // @@ -85,3 +85,83 @@ export function isPluginPublicPath(path: string): boolean { } return false; } + +// ── Top-level claims: root paths, and the header that hands `/api/*` to a plugin ── +// +// Everything above is about a plugin's own mount. This is about the platform's namespace, which a plugin +// may reach into only for a protocol it does not control and cannot move — see `unauthenticatedRootPaths` +// in manifest.ts for the full argument. Kept in the same file because it is the same question ("what does +// a plugin serve without a session") and splitting it would let the two drift. + +export type RootClaim = { segment: string; appName: string }; + +let rootClaims: RootClaim[] = []; +let protocolHeaders: { header: string; appName: string }[] = []; + +/** + * Resolve a plugin's root claims, refusing collisions against a set already taken. + * + * `taken` is threaded in rather than read from module state so the caller can resolve every plugin + * against one accumulating set — first come, first served, deterministic in install order, and the loser + * is told by name rather than silently overwritten. + */ +export function resolvePluginRootPaths( + plugin: DiscoveredPlugin, + taken: Set, +): { segments: string[]; refused: string[] } { + const segments: string[] = []; + const refused: string[] = []; + for (const path of plugin.manifest.unauthenticatedRootPaths ?? []) { + const problem = rootPathProblem(path); + if (problem) { + refused.push(`${plugin.appName}: ${problem}`); + continue; + } + const segment = path.slice(1); + if (taken.has(segment)) { + refused.push(`${plugin.appName}: "/${segment}" is already claimed by another plugin`); + continue; + } + taken.add(segment); + segments.push(segment); + } + return { segments, refused }; +} + +export function setPluginRootClaims(claims: RootClaim[]): void { + rootClaims = claims; +} + +export function listPluginRootClaims(): RootClaim[] { + return rootClaims; +} + +/** Does this top-level path belong to a plugin? Segment match, so `/identityx` never counts. */ +export function pluginForRootPath(path: string): string | null { + for (const { segment, appName } of rootClaims) { + if (path === `/${segment}` || path.startsWith(`/${segment}/`)) return appName; + } + return null; +} + +export function setPluginProtocolHeaders(entries: { header: string; appName: string }[]): void { + protocolHeaders = entries; +} + +export function listPluginProtocolHeaders(): { header: string; appName: string }[] { + return protocolHeaders; +} + +/** + * Which plugin, if any, claims this `/api/*` request by header. + * + * ROUTING, not authentication: a forged header reaches the plugin's own public router, which demands + * whatever credential it demands. The worst outcome for a caller who sets it is losing access to + * Officer's API for that request. + */ +export function pluginForProtocolHeaders(headers: Headers): string | null { + for (const { header, appName } of protocolHeaders) { + if (headers.has(header)) return appName; + } + return null; +}