From 10feb768800b4a94cd6e3cc4724efe81d7a5901b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 20 Aug 2026 22:56:10 +0000 Subject: [PATCH] permissions: a plugin can mark part of its surface owner-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A plugin worth granting to a Member can also hold the credential the rest of it 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. The choice was all of it or none of it. `ownerOnly` on a plugin permission takes sub-paths relative to the mount prefix. The permission stays fully grantable — this is not the ownerOnly PERMISSION that manifest.ts refuses, and that refusal still stands: a permission nobody can delegate is a contradiction. This says a few paths administer the plugin rather than use it. Declared by the plugin, enforced by the platform. A plugin could check this itself and the first one would; the tenth would add a route and forget, with no boot failure and no warning — the shape assertPermissionTotality exists to prevent. In the gate it is enforced once, on the door they all pass through, and a route added later by someone who never read the manifest is covered by default. Checked before selfService and before kind. A path that is somehow both self-service and owner-only is a contradiction, and the safe reading has to win. A write grant does not unlock it either — level is not the question this answers. Also forwards X-Officer-Owner and X-Officer-Level to a sidecar whose proxy names its permission key, for the finer decisions the platform cannot make for it: whose rows a read returns, which fields a level may see. Facts, never the role name — a plugin branching on "Super Admin" would give every plugin repository an opinion about the platform's role vocabulary, and renaming a role would change authorization in code the platform does not own. These are safe to trust only because the proxy copies a three-item allowlist off the inbound request; that is now written where it would be changed. The owner's level is reported as write rather than the absent 'none': they hold everything implicitly and appear in no grants map, so the raw lookup inverts the meaning for the one account that can do anything. No plugin declares one yet. Jellyfin cannot until connections stop being per-user — locking /_config today leaves a Member with a plugin that only shows a connect form they may not submit. Co-Authored-By: Claude Opus 5 --- docs/plugin-route-authorization.md | 29 +++++++---- src/servers/permissions/authorize.ts | 8 +++ src/servers/permissions/registry.test.ts | 65 ++++++++++++++++++++++++ src/servers/permissions/registry.ts | 43 ++++++++++++++++ src/servers/plugins/manifest.ts | 29 +++++++++++ src/servers/plugins/mount.ts | 4 ++ src/servers/sidecar/create-proxy.ts | 40 +++++++++++++-- 7 files changed, 205 insertions(+), 13 deletions(-) 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