plugins: implement unauthenticatedPaths — the half that was never built
`unauthenticatedPaths` has been in PluginManifest since it was designed, with
validation (`publicPathProblem`), a long docstring, and a promise that
`assertPluginPublicPaths` re-checks it at boot. Nothing read the field. No
function by that name existed. Kiosk, the plugin it was written for, declares
none — so it validated, documented itself, and did nothing, and no plugin has
ever actually been served above the gate.
Vault is the first real user: a stock Bitwarden client speaks Vaultwarden's
protocol and has nowhere to put a platform JWT, so it cannot use the
authenticated /api/vault router at all.
── What was missing, and where it had to go ──
Two places, not one, and the second is the one that matters:
• the permission gate skips declared paths — but that gate only runs when a
token WAS presented (`payload && !isOwner && …`), so it is what stops a
valid non-owner reaching something ungranted.
• `protectedRouter.use(userMiddleware)` is what stops an ANONYMOUS request.
A path meant to answer without a session has to skip that too, so it is now
conditional on the same predicate. Fixing only the gate would have left
every declared path returning 401 — passing its own tests and failing in
the one case it exists for.
── Containment ──
Paths resolve against the plugin's own mount (`/api` + mountPrefix + path) and
containment is re-checked there, so a plugin cannot name `/auth` or another
plugin's routes however it edits its manifest. Refusal is per-path: one bad
entry does not take the good ones with it, and each is logged by name.
Matching is on segments rather than characters — `/api/vault/bwx` must not
match `/api/vault/bw/*`, the same class of bug `permissionForApiPath` guards
against with `/user` vs `/username`.
Every plugin with public paths is logged at boot: a route that answers without
a session should not be something you have to read a manifest to discover.
17 tests, pinning the containment rules rather than the happy path. Suite
1061 pass / 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+13
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user