diff --git a/src/servers/_middlewares/permission-gate.ts b/src/servers/_middlewares/permission-gate.ts index e40c635a..74db20fa 100644 --- a/src/servers/_middlewares/permission-gate.ts +++ b/src/servers/_middlewares/permission-gate.ts @@ -4,6 +4,7 @@ import { resolveAuthToken } from '../auth-token'; import { isSuperAdmin } from '../super-admin'; import { isApiRequestAllowed } from '../permissions/authorize'; import { isExemptApiPath } from '../permissions/totality'; +import { isPluginPublicPath } from '../plugins/public-paths'; // The global authorization gate: a valid NON-owner token may reach only what its ROLE has been granted. // @@ -37,12 +38,13 @@ export const permissionGateMiddleware: MiddlewareHandler = async function (ctx, const payload = token ? await resolveAuthToken(token) : null; const isOwner = payload ? await isSuperAdmin(payload) : false; - // 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. + // Exempt paths (signin, the public pages) are skipped because they are served above the account gate — + // the same list the boot check uses, deliberately. A plugin's declared `unauthenticatedPaths` are + // skipped for the same reason, resolved against its own mount: see plugins/public-paths.ts. // // 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)) { + if (payload && !isOwner && path.startsWith('/api') && !isExemptApiPath(path) && !isPluginPublicPath(path)) { const { allowed, reason } = await isApiRequestAllowed(payload.id, ctx.req.method, path); if (!allowed) throw errors.FORBIDDEN(reason ?? 'Not permitted for this account'); } diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 5d55bcb0..3da18cc4 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -22,6 +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 { agentHandoffRouter } from './api/agent-handoff/router'; // import { slskdRouter } from './api/slskd/router'; // import { transmissionRouter } from './api/transmission/router'; @@ -254,7 +255,18 @@ export function buildHonoApp(plugins: MountedPlugin[] = []): Hono<{ Variables: H const protectedRouter = createRouter(); protectedRouter.use(bodyParser()); - protectedRouter.use(userMiddleware); + /* + A session is required for everything here EXCEPT a plugin's declared `unauthenticatedPaths`. + + The permission gate above is not enough on its own, and the reason is worth stating: that gate only + runs when a token was presented (`payload && !isOwner && …`), so it is what stops a valid non-owner + reaching something they were not granted. What stops an ANONYMOUS request is this line — so a path + that is supposed to answer without a session has to skip it here, not only there. + + `ctx.req.path` is the full request path even inside a mounted router, so the patterns can be absolute + and resolved once at mount rather than re-derived per request. + */ + protectedRouter.use((ctx, next) => (isPluginPublicPath(ctx.req.path) ? next() : userMiddleware(ctx, next))); for (const [prefix, router] of PROTECTED_MOUNTS) protectedRouter.route(prefix, router); // Plugin routes, mounted behind the same account gate as everything else — a plugin is part of the diff --git a/src/servers/plugins/mount.ts b/src/servers/plugins/mount.ts index 1ffff8aa..aa8aa7e7 100644 --- a/src/servers/plugins/mount.ts +++ b/src/servers/plugins/mount.ts @@ -2,6 +2,7 @@ import { listPluginInstalls, type PluginInstall } from 'officerdb'; import type { MountedPlugin } from '../hono'; import { rebuildHonoApp } from '../hono'; import { setPluginPermissions, type Permission } from '../permissions/registry'; +import { resolvePluginPublicPaths, setPluginPublicPaths, type PublicPath } from './public-paths'; import { discoverPlugins } from './discover'; import { mountPrefix, type DiscoveredPlugin } from './manifest'; import { generatePluginsModule, rebuildFrontend } from './generate'; @@ -144,6 +145,22 @@ export async function refreshPluginMounts(options: { skipFrontend?: boolean } = console.error(`[plugins] permission keys refused (already taken by the platform): ${rejected.join(', ')}`); } + // Unauthenticated paths, recomputed on the same cycle and for the same reason as permissions above: + // install, uninstall, enable and disable are one recomputation rather than a diff. Logged at INFO + // whenever there are any — a route that answers without a session should never be something you have to + // read a manifest to discover. + const publicPaths: PublicPath[] = []; + for (const { plugin } of enabled) { + const { paths, refused } = resolvePluginPublicPaths(plugin); + for (const reason of refused) console.error(`[plugins] unauthenticated path refused — ${reason}`); + for (const pattern of paths) publicPaths.push({ pattern, appName: plugin.appName }); + } + setPluginPublicPaths(publicPaths); + if (publicPaths.length) { + const shown = publicPaths.map((p) => `${p.pattern} (${p.appName})`).join(', '); + console.log(`[plugins] served WITHOUT a session: ${shown}`); + } + const mounted = await mountablePlugins(snapshot); rebuildHonoApp(mounted); diff --git a/src/servers/plugins/public-paths.test.ts b/src/servers/plugins/public-paths.test.ts new file mode 100644 index 00000000..3cc362e2 --- /dev/null +++ b/src/servers/plugins/public-paths.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, test, afterEach } from 'bun:test'; +import { isPluginPublicPath, resolvePluginPublicPaths, setPluginPublicPaths } from './public-paths'; +import type { DiscoveredPlugin } from './manifest'; + +// This is an auth boundary: everything matched here skips `userMiddleware` AND the permission gate. The +// property worth pinning is not "the happy path works" but "a plugin cannot name a path outside its own +// mount", because that is the only failure that would matter. + +const plugin = (appName: string, unauthenticatedPaths?: string[], publisher = 'officerdev'): DiscoveredPlugin => + ({ + appName, + dir: `/tmp/${appName}`, + manifest: { + publisher, + version: '1.0.0', + platform: '>=1.0.0', + label: appName, + summary: '', + color: '#000', + permissions: [], + ...(unauthenticatedPaths ? { unauthenticatedPaths } : {}), + }, + icon: null, + api: null, + schema: null, + sidecar: null, + web: null, + }) as DiscoveredPlugin; + +afterEach(() => setPluginPublicPaths([])); + +describe('resolving a plugin’s declared paths', () => { + test('resolves against the plugin’s own mount', () => { + expect(resolvePluginPublicPaths(plugin('vault', ['/bw/*'])).paths).toEqual(['/api/vault/bw/*']); + }); + + test('a third-party plugin resolves under its publisher, never at the root', () => { + const { paths } = resolvePluginPublicPaths(plugin('vault', ['/bw/*'], 'alice')); + expect(paths).toEqual(['/api/p/alice/vault/bw/*']); + }); + + test('declaring nothing is the ordinary case and yields nothing', () => { + expect(resolvePluginPublicPaths(plugin('kiosk')).paths).toEqual([]); + }); + + // The rules `publicPathProblem` enforces, checked through this function because that is how they are + // actually reached. Each is refused rather than corrected. + test.each([ + ['..', ['/../../auth']], + ['a double slash', ['//auth']], + ['a mid-path wildcard', ['/a/*/b']], + ['a bare wildcard suffix', ['/a*']], + ['the whole plugin', ['/*']], + ['the plugin root', ['/']], + ['a relative path', ['bw/*']], + ['a query string', ['/bw?x=1']], + ])('refuses %s', (_label, declared) => { + const { paths, refused } = resolvePluginPublicPaths(plugin('vault', declared)); + expect(paths).toEqual([]); + expect(refused).toHaveLength(1); + }); + + test('one bad entry does not take the good ones with it', () => { + const { paths, refused } = resolvePluginPublicPaths(plugin('vault', ['/bw/*', '/../nope'])); + expect(paths).toEqual(['/api/vault/bw/*']); + expect(refused).toHaveLength(1); + }); +}); + +describe('matching a request path', () => { + test('a trailing /* matches the prefix and everything under it', () => { + setPluginPublicPaths([{ pattern: '/api/vault/bw/*', appName: 'vault' }]); + expect(isPluginPublicPath('/api/vault/bw')).toBe(true); + expect(isPluginPublicPath('/api/vault/bw/identity/connect/token')).toBe(true); + }); + + test('it does not leak to the plugin’s own gated routes', () => { + setPluginPublicPaths([{ pattern: '/api/vault/bw/*', appName: 'vault' }]); + // The whole point: /config stays behind the session, one segment away from a public prefix. + expect(isPluginPublicPath('/api/vault/config')).toBe(false); + expect(isPluginPublicPath('/api/vault')).toBe(false); + }); + + test('a prefix match is on segments, not characters', () => { + setPluginPublicPaths([{ pattern: '/api/vault/bw/*', appName: 'vault' }]); + // `/bwx` shares the string prefix and must not match — the same class of bug `permissionForApiPath` + // guards against with `/user` vs `/username`. + expect(isPluginPublicPath('/api/vault/bwx')).toBe(false); + expect(isPluginPublicPath('/api/vault/bwevil/x')).toBe(false); + }); + + test('an exact pattern matches only itself', () => { + setPluginPublicPaths([{ pattern: '/api/kiosk/manifest', appName: 'kiosk' }]); + expect(isPluginPublicPath('/api/kiosk/manifest')).toBe(true); + expect(isPluginPublicPath('/api/kiosk/manifest/extra')).toBe(false); + }); + + test('nothing is public when nothing is declared — the default for every plugin today', () => { + expect(isPluginPublicPath('/api/vault/bw/identity')).toBe(false); + expect(isPluginPublicPath('/api/users')).toBe(false); + }); +}); diff --git a/src/servers/plugins/public-paths.ts b/src/servers/plugins/public-paths.ts new file mode 100644 index 00000000..f26c168d --- /dev/null +++ b/src/servers/plugins/public-paths.ts @@ -0,0 +1,87 @@ +import { mountPrefix, publicPathProblem, type DiscoveredPlugin } from './manifest'; + +// Which plugin paths are served ABOVE the account gate. +// +// The declaration side of this — `unauthenticatedPaths` in a manifest, `publicPathProblem` validating it — +// existed from the day the field was designed. Nothing read it: no plugin had needed one yet (kiosk +// declares none), so the field validated, documented itself, and did nothing. Vault is the first real +// user, and this is the half that was missing. +// +// ── What a plugin is trusted with, and what it is not ── +// +// NOT trusted: which paths. A declared path is resolved against the plugin's OWN mount and re-checked +// here at every mount, so a plugin cannot name `/auth`, `/users`, or another plugin's routes however it +// edits its manifest. `publicPathProblem` already refuses `..`, `//`, mid-path wildcards and the whole +// plugin; this adds the containment check that makes the promise structural rather than syntactic. +// +// Trusted: what those paths DO. Authorising them is the plugin's job — it asked for them to be public, so +// whatever credential they check is the plugin's to define. The platform's job is to be exact about which +// ones, and to say so out loud at boot. + +/** A resolved entry: the absolute path pattern, and who asked for it. */ +export type PublicPath = { pattern: string; appName: string }; + +let publicPaths: PublicPath[] = []; + +/** + * Resolve a plugin's declared paths against its own mount. + * + * Returns the absolute patterns and the reasons any were refused. Refusal is per-path rather than + * per-plugin: one bad entry must not quietly take the good ones with it, and the caller logs what it lost. + */ +export function resolvePluginPublicPaths(plugin: DiscoveredPlugin): { paths: string[]; refused: string[] } { + const declared = plugin.manifest.unauthenticatedPaths ?? []; + const base = `/api${mountPrefix(plugin)}`; + const paths: string[] = []; + const refused: string[] = []; + + for (const path of declared) { + const problem = publicPathProblem(path); + if (problem) { + refused.push(`${plugin.appName}: ${problem}`); + continue; + } + const absolute = `${base}${path}`; + // Containment, re-checked rather than assumed. `publicPathProblem` already makes escaping hard; this + // makes it impossible, and it is cheap. A path that does not sit under the plugin's own mount is the + // one failure that would matter, so it is checked where the mount is actually known. + if (absolute !== base && !absolute.startsWith(`${base}/`)) { + refused.push(`${plugin.appName}: "${path}" resolves outside the plugin's own mount`); + continue; + } + paths.push(absolute); + } + return { paths, refused }; +} + +/** + * Replace the whole set. Called from `refreshPluginMounts` alongside `setPluginPermissions`, and for the + * same reason: install, uninstall, enable and disable are all one recomputation rather than a diff. + */ +export function setPluginPublicPaths(entries: PublicPath[]): void { + publicPaths = entries; +} + +/** Everything currently public, for logging and for the plugins API to show. */ +export function listPluginPublicPaths(): PublicPath[] { + return publicPaths; +} + +/** + * Is this request path served above the account gate? + * + * A trailing `/*` matches the prefix and everything under it; anything else is exact. Deliberately not a + * regex: the patterns come from a manifest, and the set of shapes `publicPathProblem` allows is small + * enough to match literally — which is one fewer place for a pattern to mean more than it looks like. + */ +export function isPluginPublicPath(path: string): boolean { + for (const { pattern } of publicPaths) { + if (pattern.endsWith('/*')) { + const prefix = pattern.slice(0, -2); + if (path === prefix || path.startsWith(`${prefix}/`)) return true; + } else if (path === pattern) { + return true; + } + } + return false; +}