plugins: let a plugin claim top-level paths and divert /api by header
Restores the setup the vault extraction regressed: paste the bare Officer URL
into a Bitwarden client and log in. Nothing else.
The extraction moved the Bitwarden door to /api/vault/bw, which is correct and
unusable — a client derives /api, /identity, /icons and /notifications from ONE
url it is given, so a prefixed base turns setup into five fields copied by hand
into an advanced pane. That is not a feature anyone completes, and the owner
said so. My regression, my wrong call: I treated "a plugin cannot serve
top-level paths" as a fact about the platform when it was a fact about what had
been built.
Two additions, both manifest-declared, both narrow:
unauthenticatedRootPaths single lowercase segments (`/identity`), served
above the gate. Refused if reserved (`/api`,
`/auth`, `/plugins`, `/dav`, …), refused if another
plugin already claims it, refused if it is anything
more structural than one name.
protocolHeaders hands `/api/*` to the plugin when the request
carries one. Needed because /api is the one
namespace both servers claim — Vaultwarden has
/api/sync, Officer has /api/chat — and the path
cannot tell them apart, so the client says who it
is. ROUTING, not authentication: a forged header
reaches the plugin, which demands its own
credential, and cannot reach anything of Officer's.
The worst it does is cost that caller Officer's API.
The Bun route table gets an entry per claimed segment, built from the plugin
set — `refreshPluginMounts` runs at line ~204, long before serve() at ~378, so
the claims are known. This is the constraint I earlier asserted made the whole
thing impossible, without checking it. The real cost is smaller and stated in
the code: a plugin claiming a root path is not fully installed until
`pm2 restart officer`, because the table is fixed once serve() is called.
Every claim is logged at boot by plugin — a root path answering without a
session should not be something you read a manifest to discover.
13 more tests (30 in the file): reserved segments, malformed segments, two
plugins colliding, and segment-not-substring matching so `/identityx` never
matches `/identity`. Suite 1074 pass / 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -166,6 +166,48 @@ export type PluginManifest = {
|
||||
*/
|
||||
unauthenticatedPaths?: string[];
|
||||
|
||||
/**
|
||||
* TOP-LEVEL paths this plugin serves, above the account gate and outside its own `/api/<name>` 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<PluginManife
|
||||
for (const field of ['version', 'platform', 'label', 'summary', 'color'] as const) {
|
||||
if (typeof manifest[field] !== 'string' || !manifest[field]) problems.push(`${field} is required`);
|
||||
}
|
||||
if (manifest.unauthenticatedRootPaths !== undefined) {
|
||||
if (!Array.isArray(manifest.unauthenticatedRootPaths)) {
|
||||
problems.push('unauthenticatedRootPaths must be an array of paths');
|
||||
} else {
|
||||
for (const [i, path] of manifest.unauthenticatedRootPaths.entries()) {
|
||||
const problem = rootPathProblem(path);
|
||||
if (problem) problems.push(`unauthenticatedRootPaths[${i}] ${problem}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (manifest.protocolHeaders !== undefined) {
|
||||
if (!Array.isArray(manifest.protocolHeaders)) {
|
||||
problems.push('protocolHeaders must be an array of header names');
|
||||
} else {
|
||||
for (const [i, header] of manifest.protocolHeaders.entries()) {
|
||||
if (typeof header !== 'string' || !/^[a-z][a-z0-9-]{0,63}$/.test(header)) {
|
||||
problems.push(`protocolHeaders[${i}] must be a lowercase header name`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (manifest.unauthenticatedPaths !== undefined) {
|
||||
if (!Array.isArray(manifest.unauthenticatedPaths)) {
|
||||
problems.push('unauthenticatedPaths must be an array of paths');
|
||||
|
||||
Reference in New Issue
Block a user