diff --git a/docs/plugin-route-authorization.md b/docs/plugin-route-authorization.md index fc76f24c..48dd0001 100644 --- a/docs/plugin-route-authorization.md +++ b/docs/plugin-route-authorization.md @@ -1,6 +1,9 @@ # Plugin route authorization -**Design, not built.** Agreed 2026-08-21, deferred deliberately. Nothing here is implemented. +**Platform half built 2026-08-21.** The gate enforces `ownerOnly`, the manifest accepts it, and the proxy +forwards `X-Officer-Owner` / `X-Officer-Level`. **No plugin declares it yet** — jellyfin's `/_config` is +still reachable by anyone granted the plugin, and cannot be locked until the credential question at the +bottom of this file is answered. The question: a plugin's **settings** routes should be reachable only by the owner, even when the plugin itself is granted to other accounts. Jellyfin is the case that raised it — connecting a server, storing @@ -79,9 +82,13 @@ forgets one route, and nothing anywhere notices — no boot failure, no warning, So the recommendation is not "platform enforces instead". It is **declare in the manifest, enforce in the proxy, and pass the facts to the sidecar as well**: +Built as a field on the permission rather than at the manifest root, following `readOnlyWrites` — both +are sub-paths written relative to the plugin's prefix with a rule attached, and one precedent is better +than two shapes: + ```ts // manifest.ts -ownerOnlyPaths: ['/_config'], +permissions: [{ key: 'jellyfin', label: 'Jellyfin', description: '…', ownerOnly: ['/_config'] }], ``` - **Declared by the plugin**, because the plugin knows which of its paths are administrative. @@ -121,11 +128,13 @@ guard is a few lines; this is the work. ## Status -| | | -| ----------- | ---------------------------------------------------------------------------------- | -| Agreed | plugin decides what a level means inside its own surface | -| Agreed | pass `isOwner` and the effective level; never the role name | -| Agreed | fail closed when the headers are absent | -| Proposed | `ownerOnlyPaths` in the manifest, enforced by the proxy — pending the owner's call | -| Blocked on | credential resolution for non-owners, above | -| Not started | all of it | +| | | +| ----------- | ------------------------------------------------------------------------------------- | +| Agreed | plugin decides what a level means inside its own surface | +| Agreed | pass `isOwner` and the effective level; never the role name | +| Agreed | fail closed when the headers are absent | +| Built | `ownerOnly` on a plugin permission, enforced by the gate before self-service and kind | +| Built | `X-Officer-Owner` and `X-Officer-Level`, sent when a proxy names its permission key | +| Built | manifest validation — a sub-path without a leading slash is a manifest error | +| Blocked on | credential resolution for non-owners, above | +| Not started | any plugin actually declaring one | diff --git a/src/servers/permissions/authorize.ts b/src/servers/permissions/authorize.ts index 572bbbd7..0afd49d6 100644 --- a/src/servers/permissions/authorize.ts +++ b/src/servers/permissions/authorize.ts @@ -6,6 +6,7 @@ import { permissionForApiPath, permissionForWsProvider, isRequestAllowedAtLevel, + isOwnerOnlyRoute, isSelfServiceRoute, type PermissionLevel, } from './registry'; @@ -131,6 +132,13 @@ export async function isApiRequestAllowed( // a 403 on a route that does not exist is not a leak, and a permissive default here would be. if (!permission) return { allowed: false, reason: 'no permission covers this path' }; + // BEFORE self-service, and before kind. A permission can be fully granted and still have an + // administrative corner — a plugin's settings, holding the credential the rest of it runs on. The owner + // already returned above, so reaching here at all means this is not them. + if (isOwnerOnlyRoute(permission, path)) { + return { allowed: false, reason: `that part of ${permission.label} is restricted to the server owner` }; + } + // Checked before kind, because a self-service route acts on the caller and is therefore not the thing // the permission around it restricts. Exact method and path only — see the field's comment. if (isSelfServiceRoute(permission, method, path)) return { allowed: true }; diff --git a/src/servers/permissions/registry.test.ts b/src/servers/permissions/registry.test.ts index 50085650..47326fcf 100644 --- a/src/servers/permissions/registry.test.ts +++ b/src/servers/permissions/registry.test.ts @@ -8,6 +8,7 @@ import { permissionForWsProvider, isRequestAllowedAtLevel, isSelfServiceRoute, + isOwnerOnlyRoute, } from './registry'; import { assertPermissionTotality, isExemptApiPath } from './totality'; @@ -298,3 +299,67 @@ test('notify: a read-level member can register and deregister their own device', // Still scoped: read level does not become write level for the rest of the surface. expect(isRequestAllowedAtLevel(notify!, 'read', 'POST', '/api/notify/_officer/notify')).toBe(false); }); + +// ── ownerOnly: the administrative corner of a shareable surface ── +// +// The case: a plugin worth granting to a Member that also holds the credential everything else runs on. +// Jellyfin's `/_config` stores a server's admin token and drives account provisioning; the rest of the +// plugin is exactly what a Member should have. Before this the choice was all of it or none. +// +// The owner never reaches any of this — `isApiRequestAllowed` returns for them before the permission is +// even resolved — so every case here is about somebody who legitimately holds the permission. + +const withOwnerOnly: Permission = { + key: 'jellyfin', + label: 'Jellyfin', + description: 'films and shows', + kind: 'app', + api: ['/jellyfin'], + ownerOnly: ['/_config'], + personal: ['/_officer'], + selfService: ['POST /_config/ping'], +}; + +describe('isOwnerOnlyRoute', () => { + test('matches the declared sub-path', () => { + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_config')).toBe(true); + }); + + test('matches everything under it — a settings surface is a tree, not a list of leaves', () => { + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_config/3/users')).toBe(true); + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_config/3/activate')).toBe(true); + }); + + test('leaves the rest of the plugin alone', () => { + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_jf/Items')).toBe(false); + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin')).toBe(false); + }); + + test('does not match a path that merely starts with the same letters', () => { + // `/_configuration` is a different route. Prefix matching that ignored the boundary would silently + // lock a route nobody declared, which is the mirror of the bug this feature exists to prevent. + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_configuration')).toBe(false); + }); + + test('does not reach outside the permission that declared it', () => { + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/music/_config')).toBe(false); + }); + + test('a permission declaring none is unaffected', () => { + expect(isOwnerOnlyRoute({ ...withOwnerOnly, ownerOnly: undefined }, '/api/jellyfin/_config')).toBe(false); + }); + + // The two rules that could contradict each other, and which way it has to fall. `authorize.ts` checks + // ownerOnly FIRST, so these assertions are about the matchers agreeing that the overlap exists — the + // precedence is what makes it safe. + test('overlapping selfService does not unlock it', () => { + expect(isSelfServiceRoute(withOwnerOnly, 'POST', '/api/jellyfin/_config/ping')).toBe(true); + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_config/ping')).toBe(true); + }); + + test('a write grant does not unlock it either', () => { + // `isRequestAllowedAtLevel` says yes, because level is not the question ownerOnly answers. + expect(isRequestAllowedAtLevel(withOwnerOnly, 'write', 'POST', '/api/jellyfin/_config')).toBe(true); + expect(isOwnerOnlyRoute(withOwnerOnly, '/api/jellyfin/_config')).toBe(true); + }); +}); diff --git a/src/servers/permissions/registry.ts b/src/servers/permissions/registry.ts index a678f504..4e857014 100644 --- a/src/servers/permissions/registry.ts +++ b/src/servers/permissions/registry.ts @@ -83,6 +83,33 @@ export type Permission = { * permission permits `POST /api/notify/devices/123`. */ personal?: string[]; + /** + * Sub-paths, relative to each `api` prefix, that ONLY the owner may reach — at any level, by any + * method, however the permission is granted. + * + * For the administrative corner of an otherwise shareable surface. Jellyfin is the case: the plugin is + * exactly what a Member should have, and `/_config` — which stores a server's admin token and drives + * account provisioning — is exactly what they should not. Without this the only options were to grant + * the whole plugin or none of it. + * + * ── Why this is not a kind, and not `ownerOnly` on a permission ── + * + * `plugins/manifest.ts` refuses an `ownerOnly` PERMISSION, and that refusal still stands: a permission + * is a thing the owner delegates, and one nobody can delegate is a contradiction. This is narrower and + * different in kind — the permission stays fully grantable, and a few paths inside it are marked as + * administering the plugin rather than using it. + * + * ── Why it is declared here rather than enforced by each plugin ── + * + * A plugin could check this itself, and the first one would. The tenth would forget a route and nothing + * would notice — no boot failure, no warning, no test, exactly the shape `assertPermissionTotality` + * exists to prevent. Declared here it is enforced once, for every plugin, on the door they all pass + * through, and a route added later by someone who never read this is covered by default. + * + * Beaten by nothing. Checked before `selfService`, so a path that is somehow both is owner-only — + * fail closed, since the two together are a contradiction and the safe reading has to win. + */ + ownerOnly?: string[]; /** * Reads that must stay POST — see the note at the top. A read grant permits these paths at any method. * Written relative to the permission's `api` prefix, like `personal`. @@ -497,6 +524,22 @@ export function permissionForWsProvider(provider: string): Permission | null { return PERMISSIONS.find((c) => c.ws?.includes(provider)) ?? null; } +/** + * Is this path inside one of the permission's owner-only sub-paths? + * + * Prefix-matched, like `personal` — `/_config` covers `/_config/3/users`. A settings surface is a tree, + * and enumerating its leaves is how one gets missed. + */ +export function isOwnerOnlyRoute(permission: Permission, path: string): boolean { + if (!permission.ownerOnly?.length) return false; + const rest = path.startsWith('/api') ? path.slice('/api'.length) : path; + return permission.api.some((prefix) => { + if (!isPrefixOf(prefix, rest)) return false; + const sub = rest.slice(prefix.length) || '/'; + return permission.ownerOnly!.some((entry) => isPrefixOf(entry, sub)); + }); +} + /** * Is this an exact self-service route — one any authenticated account may call without holding the * permission at all? Matched exactly on method AND path, never as a prefix. diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index 2a05528a..d3bbf902 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -46,6 +46,26 @@ export type PluginPermission = { * broken feature where a withheld permission should be. */ readOnlyWrites?: string[]; + /** + * Sub-paths, relative to the plugin's mount prefix, that ONLY the owner may reach — at any granted + * level, by any method. + * + * For the administrative corner of a surface that is otherwise worth sharing. Jellyfin's `/_config` + * stores a server's admin token and drives account provisioning; the rest of the plugin is exactly what + * a Member should have. Without this the choice was the whole plugin or none of it. + * + * Note what this is NOT. It does not make the PERMISSION owner-only — see the comment above, which + * still stands: a permission nobody can delegate is a contradiction. The permission stays fully + * grantable and these few paths are marked as administering the plugin rather than using it. + * + * DECLARED here, ENFORCED by the platform's permission gate. The plugin knows which of its paths are + * settings; the platform is the thing that cannot forget to check. A plugin doing this itself would + * work until the tenth one added a route and did not, with no boot failure and no warning — the shape + * `assertPermissionTotality` exists to prevent. + * + * Prefix-matched: `/_config` covers `/_config/3/users`. + */ + ownerOnly?: string[]; }; /** The package managers the platform knows how to drive. Mirrors `PM` in machine-setup's lib/base.sh. */ @@ -542,6 +562,15 @@ export function manifestProblems(appName: string, manifest: Partial