diff --git a/middleware/migrations/0046_plugin_public_path_grants.sql b/middleware/migrations/0046_plugin_public_path_grants.sql new file mode 100644 index 00000000..9ce08250 --- /dev/null +++ b/middleware/migrations/0046_plugin_public_path_grants.sql @@ -0,0 +1,27 @@ +-- ── Plugin → public-path grants (epic #470 C4 / H1) ──────────────────────── +-- The operator explicitly consents to a plugin serving a URL prefix WITHOUT an +-- operator session. Deny-by-default: a plugin may declare +-- `permissions.public_paths` in its manifest, but nothing is served publicly +-- until a row exists here for that exact prefix. +-- +-- `plugin_id` is the manifest identity string — plugins have no agents-table +-- row, which is why this is a sibling table rather than a scope on an existing +-- grants table (same decision recorded on #458 for `plugin_mcp_grants`). +-- +-- `path_prefix` is stored verbatim as declared. It is matched by exact-prefix +-- comparison on a segment boundary at request time, never as SQL LIKE and +-- never as a regex — a stored value can therefore not widen its own match. +-- The composite PK makes re-granting idempotent and makes it impossible for +-- one plugin to hold two conflicting rows for the same prefix; cross-plugin +-- exclusivity is enforced in `platform/publicPathGrants.ts` at activation +-- time, because it has to hold for DECLARED prefixes too, not just granted +-- ones. +CREATE TABLE IF NOT EXISTS plugin_public_path_grants ( + plugin_id TEXT NOT NULL, + path_prefix TEXT NOT NULL, + granted_by TEXT NOT NULL, + granted_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (plugin_id, path_prefix) +); + +-- rollback: DROP TABLE plugin_public_path_grants; diff --git a/middleware/src/api/admin-v1.ts b/middleware/src/api/admin-v1.ts index 26ba929c..ad3fc505 100644 --- a/middleware/src/api/admin-v1.ts +++ b/middleware/src/api/admin-v1.ts @@ -279,6 +279,21 @@ export interface PluginPermissionsSummary { * refresh tokens never reach plugin code). Surfaced as a store-detail chip. * Loader defaults to `false`. */ acquires_oauth?: boolean; + /** + * Epic #470 C4 / H1 (`permissions.public_paths`): URL prefixes the plugin + * asks to serve WITHOUT an operator session. + * + * This is a REQUEST, not a capability — declaring it grants nothing. The + * prefix is claimed exclusively at activation (first plugin wins, a second + * one overlapping it fails to activate), and it is only served publicly once + * the operator has consented and a row exists in `plugin_public_path_grants`. + * Until then the prefix stays behind `requireAuth` like everything else. + * + * The single most consequential thing a plugin can ask for, so it is + * surfaced on its own in the store consent block rather than folded in with + * the network/memory chips. Loader defaults to `[]`. + */ + public_paths?: string[]; } export type PluginInstallState = diff --git a/middleware/src/index.ts b/middleware/src/index.ts index 8b044eda..2e078861 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -301,6 +301,9 @@ import { ChatAgentWrapRegistry } from './platform/chatAgentWrapRegistry.js'; import { PromptContributionRegistry } from './platform/promptContributionRegistry.js'; import { installProcessGuards } from './platform/processGuards.js'; import { PluginRouteRegistry } from './platform/pluginRouteRegistry.js'; +import { PublicPathGrantRegistry } from './platform/publicPathGrants.js'; +import { createLazyPublicPathGrantStore } from './platform/publicPathGrantStore.js'; +import { createPublicPathMount } from './platform/publicPathMount.js'; import { NotificationRouter } from './platform/notificationRouter.js'; import { PluginStatusRegistry } from './platform/pluginStatusRegistry.js'; import { OAuthReadinessTracker } from './plugins/oauth/oauthReadinessTracker.js'; @@ -561,6 +564,18 @@ async function main(): Promise { // so it can fire onBeforeTurn / onAfterToolCall / onAfterTurn during turns. serviceRegistry.provide('turnHookRegistry', turnHookRegistry); const pluginRouteRegistry = new PluginRouteRegistry(); + // Epic #470 C4 / H1 — who owns which unauthenticated URL prefix, and which of + // those the operator has consented to. The registry decides routing; the + // store holds the durable consent. Both are wired into ToolPluginRuntime + // (claim on activate, release on deactivate) and into the terminating mount + // installed BEFORE the `/api` requireAuth line far below. + // + // The store is late-bound because `graphPool` is published into the service + // registry after plugins activate — see createLazyPublicPathGrantStore. + const publicPathGrants = new PublicPathGrantRegistry(); + const publicPathGrantStore = createLazyPublicPathGrantStore(() => + serviceRegistry.get('graphPool'), + ); const notificationRouter = new NotificationRouter(); // Phase B+ — directory aggregator for the /operator/channels dashboard. @@ -1075,6 +1090,13 @@ async function main(): Promise { serviceRegistry, nativeToolRegistry, pluginRouteRegistry, + // Epic #470 C4 / H1 — declared public-path prefixes are claimed here on + // activate and released on deactivate. `corePublicPaths` is the SAME array + // requireAuth runs against, so a plugin can never declare a prefix that is + // already a static core exemption. + publicPathGrants, + publicPathGrantStore, + corePublicPaths: publicPaths(), notificationRouter, uiRouteCatalog, jobScheduler, @@ -2730,6 +2752,35 @@ async function main(): Promise { // Pre-Phase-A / no-DB boot: the legacy default is the only Agent. return reg ? undefined : 'default'; }; + // ── Epic #470 C4 / H1 — the terminating public-path mount ──────────────── + // + // THE POSITION OF THIS LINE IS THE FEATURE. It sits immediately before the + // OB-106 `/api` requireAuth mount below, and everything about the design + // follows from that: + // + // * A request under a prefix that is manifest-declared, exclusively owned + // AND operator-granted is dispatched to the owning plugin's router right + // here, before any authentication runs. + // * If that router does not handle it, this mount answers 404. It does NOT + // call next(). A granted prefix is a closed world owned by one plugin — + // an unhandled subpath must never travel on into the authenticated stack + // with no session attached. That is the hole a plain `publicPaths` entry + // leaves open, and the reason `auth/publicPaths.ts` stays a frozen + // core-owned literal instead of becoming a dynamic set. + // * Anything else calls next() and meets requireAuth exactly as before. + // + // Fail-closed by construction: no grants, no store, no registry, no live + // plugin — every one of those is a next(), i.e. a 401. There is no failure + // mode of this mount that produces LESS authentication than a build without + // it. Mounted after express.json/cookieParser so plugin handlers see the + // same parsed request they see through the ordinary boot-time mount. + app.use( + createPublicPathMount({ + grants: publicPathGrants, + routes: pluginRouteRegistry, + }), + ); + // OB-106: gate the chat-inference endpoints (`POST /api/chat`, // `POST /api/chat/stream`) behind `requireAuth`. Without this, anonymous // callers could trigger LLM inference (cost) and reach the tool surface @@ -4107,6 +4158,10 @@ async function main(): Promise { catalog: pluginCatalog, reactivate: reactivateAgent, dynamicAgentRuntime, + // Epic #470 C4 / H1 — operator consent for unauthenticated plugin path + // prefixes. Behind requireAuth like every other runtime endpoint. + publicPathGrantStore, + publicPathGrants, }), ); console.log('[middleware] runtime introspection endpoint ready at /api/v1/admin/runtime (auth: required)'); diff --git a/middleware/src/platform/pluginRouteRegistry.ts b/middleware/src/platform/pluginRouteRegistry.ts index 8272da87..e932aab2 100644 --- a/middleware/src/platform/pluginRouteRegistry.ts +++ b/middleware/src/platform/pluginRouteRegistry.ts @@ -1,3 +1,4 @@ +import { Router as createRouter } from 'express'; import type { Express, RequestHandler, Router } from 'express'; /** @@ -33,10 +34,25 @@ interface RouteEntry { source: string; } +/** What the terminating public-path mount needs to hand a request to a plugin: + * the prefix the router was mounted at, plus a handler that runs it with real + * Express mount semantics. See {@link PluginRouteRegistry.resolvePublicDispatch}. */ +export interface PluginRouteDispatch { + readonly prefix: string; + readonly source: string; + /** Runs the plugin's router. Calls `next()` — and only `next()` — when the + * router did not handle the request, so the caller decides what "unhandled" + * means. The public-path mount decides it means 404, never fallthrough. */ + readonly handle: RequestHandler; +} + export class PluginRouteRegistry { private readonly entries: RouteEntry[] = []; private mounted = false; private app: Express | null = null; + /** Per-entry mini-app used by `resolvePublicDispatch`, built once and reused. + * Keyed on the entry object so a disposed entry's wrapper is collectable. */ + private readonly dispatchWrappers = new WeakMap(); /** * Register a router at the given prefix. `source` is for diagnostics — @@ -100,6 +116,59 @@ export class PluginRouteRegistry { app.use(entry.prefix, guardedRouter); } + /** + * Epic #470 C4 / H1 — resolve the live router that owns `path` for `source`. + * + * Returns `null` when the plugin registered nothing covering that path, or + * when the entry it did register has since been disposed. Both cases mean + * the same thing to the caller: nobody is entitled to answer this request + * without a session. + * + * The returned `handle` wraps the router in a one-line Express `Router` + * mounted at the entry's own prefix. That is deliberate rather than manual + * `req.url` surgery: `req.baseUrl`/`req.url`/`req.params` rewriting and its + * restoration on `next()` are subtle, Express already implements them + * correctly, and a plugin router must see exactly the same request it sees + * through the ordinary boot-time mount — otherwise the "public" path and the + * authenticated path diverge, which is precisely the drift this whole + * mechanism exists to prevent. + * + * The disposed check lives INSIDE the wrapper, not just at resolve time, so + * a plugin deactivated between resolution and dispatch still stops serving. + */ + resolvePublicDispatch( + source: string, + path: string, + ): PluginRouteDispatch | null { + let best: RouteEntry | null = null; + for (const entry of this.entries) { + if (entry.source !== source || entry.disposed) continue; + if (!isUnderPrefix(path, entry.prefix)) continue; + // Longest prefix wins — a plugin registering both `/api/plugins/x` and + // `/api/plugins/x/hooks` must get the more specific router. + if (best === null || entry.prefix.length > best.prefix.length) { + best = entry; + } + } + if (!best) return null; + const entry = best; + + let handle = this.dispatchWrappers.get(entry); + if (!handle) { + const wrapper = createRouter(); + wrapper.use(entry.prefix, (req, res, next) => { + if (entry.disposed) { + next(); + return; + } + entry.router(req, res, next); + }); + handle = wrapper as unknown as RequestHandler; + this.dispatchWrappers.set(entry, handle); + } + return { prefix: entry.prefix, source: entry.source, handle }; + } + /** Diagnostic: what routers are registered today. */ list(): readonly { prefix: string; source: string; disposed: boolean }[] { return this.entries.map((e) => ({ @@ -133,6 +202,15 @@ export class PluginRouteRegistry { } } +/** `child` is `parent`, or sits beneath it on a segment boundary. Duplicated + * deliberately from `publicPathGrants.ts` rather than imported: this registry + * is a generic Express concern and must not depend on the grant machinery. */ +function isUnderPrefix(child: string, parent: string): boolean { + if (child === parent) return true; + const withSlash = parent.endsWith('/') ? parent : `${parent}/`; + return child.startsWith(withSlash); +} + function isExpressRouter(value: unknown): value is Router { // Express routers are callable (they are RequestHandler themselves) AND // expose a `use` method. Duck-typing is safer than instanceof because diff --git a/middleware/src/platform/publicPathGrantStore.ts b/middleware/src/platform/publicPathGrantStore.ts new file mode 100644 index 00000000..93287c8b --- /dev/null +++ b/middleware/src/platform/publicPathGrantStore.ts @@ -0,0 +1,173 @@ +import type { Pool } from 'pg'; + +/** + * Epic #470 C4 / H1 — durable operator consent for plugin public paths. + * + * Backs `plugin_public_path_grants` (migration 0046). The in-memory + * `PublicPathGrantRegistry` decides routing; this decides what the operator + * actually agreed to, and it survives a restart. + * + * FAIL-CLOSED, EVERY PATH + * ----------------------- + * Reads that cannot be satisfied return the EMPTY set, never a permissive + * default. No pool configured, table missing, database unreachable, query + * throws — every one of those degrades to "no prefix is public", which sends + * the request to `requireAuth` and a 401. This is the opposite of the rule + * `postgresGrantStore` follows for DENIALS (where swallowing an error would + * silently remove a veto): here an unread row removes an EXEMPTION, so + * swallowing is the safe direction and being noisy about it is enough. + */ + +export interface PublicPathGrantRow { + readonly pluginId: string; + readonly pathPrefix: string; + readonly grantedBy: string; + readonly grantedAt: Date; +} + +export interface PublicPathGrantStore { + /** Prefixes the operator has consented to for one plugin. */ + listForPlugin(pluginId: string): Promise>; + /** Every grant, for the admin surface. */ + listAll(): Promise; + /** Idempotent by primary key — re-granting refreshes who and when. */ + grant(pluginId: string, pathPrefix: string, grantedBy: string): Promise; + /** Returns true when a row was actually removed. */ + revoke(pluginId: string, pathPrefix: string): Promise; + /** Uninstall hook. Returns how many rows went away. + * + * B6 in the epic's design notes: the existing per-plugin grant tables are + * never cleaned on uninstall, so a plugin that re-claims a used id silently + * inherits the previous one's consent. Repeating that here would mean + * inheriting an UNAUTHENTICATED surface, so this table does not. */ + revokeAllForPlugin(pluginId: string): Promise; +} + +/** The store used when no database is configured: consents to nothing, ever. */ +export const NULL_PUBLIC_PATH_GRANT_STORE: PublicPathGrantStore = { + listForPlugin: () => Promise.resolve(new Set()), + listAll: () => Promise.resolve([]), + grant: () => + Promise.reject( + new Error('public-path grants require a database — none is configured'), + ), + revoke: () => Promise.resolve(false), + revokeAllForPlugin: () => Promise.resolve(0), +}; + +export class PostgresPublicPathGrantStore implements PublicPathGrantStore { + constructor(private readonly pool: Pool) {} + + async listForPlugin(pluginId: string): Promise> { + try { + const result = await this.pool.query<{ path_prefix: string }>( + `SELECT path_prefix FROM plugin_public_path_grants WHERE plugin_id = $1`, + [pluginId], + ); + return new Set(result.rows.map((row) => row.path_prefix)); + } catch (err) { + // Read failure means "no consent on record", which costs availability of + // the plugin's public route and costs nothing in security. Loud, because + // a persistently unreadable table looks identical to a plugin that was + // simply never granted anything. + console.warn( + `[public-paths] grant lookup failed for '${pluginId}' — treating as no grants:`, + err instanceof Error ? err.message : String(err), + ); + return new Set(); + } + } + + async listAll(): Promise { + try { + const result = await this.pool.query<{ + plugin_id: string; + path_prefix: string; + granted_by: string; + granted_at: Date; + }>( + `SELECT plugin_id, path_prefix, granted_by, granted_at + FROM plugin_public_path_grants + ORDER BY plugin_id, path_prefix`, + ); + return result.rows.map((row) => ({ + pluginId: row.plugin_id, + pathPrefix: row.path_prefix, + grantedBy: row.granted_by, + grantedAt: row.granted_at, + })); + } catch (err) { + console.warn( + '[public-paths] grant listing failed — reporting none:', + err instanceof Error ? err.message : String(err), + ); + return []; + } + } + + async grant( + pluginId: string, + pathPrefix: string, + grantedBy: string, + ): Promise { + // The write path does NOT swallow: an operator clicking "grant" and getting + // a silent no-op is the one failure mode worse than a 500 here. + await this.pool.query( + `INSERT INTO plugin_public_path_grants (plugin_id, path_prefix, granted_by) + VALUES ($1, $2, $3) + ON CONFLICT (plugin_id, path_prefix) + DO UPDATE SET granted_by = EXCLUDED.granted_by, granted_at = now()`, + [pluginId, pathPrefix, grantedBy], + ); + } + + async revoke(pluginId: string, pathPrefix: string): Promise { + const result = await this.pool.query( + `DELETE FROM plugin_public_path_grants + WHERE plugin_id = $1 AND path_prefix = $2`, + [pluginId, pathPrefix], + ); + return (result.rowCount ?? 0) > 0; + } + + async revokeAllForPlugin(pluginId: string): Promise { + const result = await this.pool.query( + `DELETE FROM plugin_public_path_grants WHERE plugin_id = $1`, + [pluginId], + ); + return result.rowCount ?? 0; + } +} + +/** + * Late-bound store. + * + * The pg pool is published into the service registry well AFTER plugins + * activate, so the activation path cannot hold a `Pool` directly — it would + * capture `undefined` and every plugin would come up with zero grants on a + * perfectly healthy database. Resolving per call is the same late-binding + * pattern the kernel already uses for other post-activation services, and it + * means the database layer coming up late does not need a plugin restart to + * take effect. + * + * When the pool is not (yet) available the null store answers: no grants, so + * `requireAuth`. + */ +export function createLazyPublicPathGrantStore( + getPool: () => Pool | undefined, +): PublicPathGrantStore { + const resolve = (): PublicPathGrantStore => { + const pool = getPool(); + return pool + ? new PostgresPublicPathGrantStore(pool) + : NULL_PUBLIC_PATH_GRANT_STORE; + }; + return { + listForPlugin: (pluginId) => resolve().listForPlugin(pluginId), + listAll: () => resolve().listAll(), + grant: (pluginId, pathPrefix, grantedBy) => + resolve().grant(pluginId, pathPrefix, grantedBy), + revoke: (pluginId, pathPrefix) => resolve().revoke(pluginId, pathPrefix), + revokeAllForPlugin: (pluginId) => resolve().revokeAllForPlugin(pluginId), + }; +} diff --git a/middleware/src/platform/publicPathGrants.ts b/middleware/src/platform/publicPathGrants.ts new file mode 100644 index 00000000..eb814085 --- /dev/null +++ b/middleware/src/platform/publicPathGrants.ts @@ -0,0 +1,363 @@ +import { z } from 'zod'; + +/** + * Epic #470 C4 / H1 — manifest-declared, operator-consented public-path grants + * with exclusive prefix ownership. + * + * WHY THIS IS NOT A DYNAMIC `publicPaths` + * --------------------------------------- + * The obvious design is "let a plugin push an entry into `auth/publicPaths.ts`". + * It rebuilds the exact hole it is meant to close. `requireAuth` runs *before* + * routing: it sees a URL and nothing else, so it structurally cannot know which + * router will finally answer. An entry there says "this URL needs no session" + * — it does not say "and only plugin A may answer it". Plugin A gets a grant + * for a prefix, does not handle some subpath under it, and plugin B (or a core + * router, or a future mount) answers that subpath with no session at all. + * + * So `auth/publicPaths.ts` stays a frozen, core-owned literal, and the grant + * lives in a mount slot placed *before* `requireAuth` which **terminates**: + * a request under a granted prefix is dispatched to the owning plugin's router + * and, if that router does not handle it, answered 404 — never passed on into + * the authenticated stack. See `publicPathMount.ts`. + * + * That ordering is what makes the whole mechanism fail-closed. If the grant + * table is empty, the store is down, the plugin never activated, or this + * registry is not wired at all, nothing matches, the early mount calls + * `next()`, and `requireAuth` 401s exactly as it does today. Every failure mode + * degrades to "more authentication", never to "less". + * + * THREE INDEPENDENT GATES + * ----------------------- + * 1. **Declaration** — the plugin lists the prefix in its manifest under + * `permissions.public_paths`. Syntactically validated here. + * 2. **Ownership** — the prefix is claimed exclusively at activation time. + * First activation wins; a second plugin declaring an overlapping prefix + * fails to activate with a named-conflict error. + * 3. **Consent** — the operator has a row in `plugin_public_path_grants` for + * that exact prefix. Declared-but-not-granted prefixes are claimed (so + * nobody else can take them) but never mounted publicly. + * + * All three must hold. Any one missing and the request goes to `requireAuth`. + */ + +/** + * The reserved root for plugin-declared public paths. A third-party plugin + * should always namespace under `/api/plugins//…`: it is the one + * shape that cannot collide with a core route, present or future. + * + * A plugin may additionally declare a public path under a prefix it actually + * registers a router at (see `claim()`), which is what lets a plugin that owns + * a historical, frozen wire path keep serving it after core stops exempting it + * statically. That check runs against the LIVE route registry, not against a + * hardcoded list, so no core file has to name any particular plugin's paths. + */ +export const PLUGIN_PUBLIC_PATH_ROOT = '/api/plugins/'; + +/** + * Roots core owns unconditionally. Not a duplicate of `publicPaths()` — that + * list is about which URLs skip the session gate, this one is about which URLs + * a plugin may never *claim*, granted or not. `/api/v1/admin` is not in + * `publicPaths()` (it is firmly behind the gate) and precisely for that reason + * it must never become claimable: an operator clicking through a consent + * dialog should not be able to hand a plugin the admin surface. + */ +const CORE_RESERVED_ROOTS: readonly string[] = [ + '/api/auth', + '/api/chat', + '/api/hooks', + '/api/messages', + '/api/public', + '/api/v1/admin', + '/api/v1/auth', + '/api/v1/install', + '/api/v1/memory', + '/api/v1/operator', + '/api/v1/setup', +]; + +/** Hard cap so a pathological manifest cannot blow up the per-request match. */ +export const MAX_DECLARED_PUBLIC_PATHS = 16; +const MAX_PATH_LENGTH = 256; + +/** + * Syntactic shape of one `permissions.public_paths` entry. + * + * Deliberately strict and deliberately zod: this string ends up deciding + * whether a URL skips authentication, so every character it may contain is + * enumerated rather than filtered. No wildcards (the match is prefix-based + * already), no percent-encoding (Express does NOT decode `req.path` — an + * encoded declaration could therefore never match a raw request path, so + * banning it removes the second representation outright rather than relying on + * a decode that never happens), + * no query or fragment (the match runs against `req.path`, which has neither), + * no dot segments. + */ +export const publicPathEntrySchema = z + .string() + .min(2) + .max(MAX_PATH_LENGTH) + .regex( + /^(?:\/[A-Za-z0-9._~-]+)+$/, + 'must be a slash-separated path of unreserved characters (no wildcards, no query, no percent-encoding)', + ) + .refine( + (p) => !p.split('/').some((seg) => seg === '.' || seg === '..'), + 'must not contain "." or ".." segments', + ) + .refine( + (p) => p.split('/').filter((s) => s.length > 0).length >= 2, + 'must be at least two segments deep — a one-segment claim is too broad', + ); + +/** `permissions.public_paths` as a whole. */ +export const publicPathsDeclarationSchema = z + .array(publicPathEntrySchema) + .max(MAX_DECLARED_PUBLIC_PATHS); + +/** True when `child` is `parent` itself or lies beneath it on a segment boundary. */ +export function isUnderPrefix(child: string, parent: string): boolean { + if (child === parent) return true; + const withSlash = parent.endsWith('/') ? parent : `${parent}/`; + return child.startsWith(withSlash); +} + +/** Two prefixes overlap when either one contains the other. */ +function prefixesOverlap(a: string, b: string): boolean { + return isUnderPrefix(a, b) || isUnderPrefix(b, a); +} + +export interface PublicPathValidationContext { + /** The core exemption list, passed in rather than imported so tests assert + * against the same array production runs (same reason `publicPaths.ts` + * exists as its own module). */ + readonly corePublicPaths: readonly RegExp[]; + /** Prefixes this plugin actually registered a router at. A plugin may only + * make public something it genuinely serves. */ + readonly ownRoutePrefixes: readonly string[]; +} + +export type PublicPathValidation = + | { readonly ok: true; readonly path: string } + | { readonly ok: false; readonly path: string; readonly reason: string }; + +/** + * Validate ONE declared path against everything that does not require knowing + * about other plugins. Cross-plugin exclusivity is `claim()`'s job. + */ +export function validateDeclaredPublicPath( + raw: unknown, + ctx: PublicPathValidationContext, +): PublicPathValidation { + const parsed = publicPathEntrySchema.safeParse(raw); + if (!parsed.success) { + const shown = typeof raw === 'string' ? raw : JSON.stringify(raw); + return { + ok: false, + path: String(shown), + reason: + parsed.error.issues[0]?.message ?? + 'is not a valid public-path declaration', + }; + } + const p = parsed.data; + + const reserved = CORE_RESERVED_ROOTS.find((root) => prefixesOverlap(p, root)); + if (reserved) { + return { + ok: false, + path: p, + reason: `overlaps the core-reserved root '${reserved}' — core routes are never grantable`, + }; + } + + // Already a core exemption? Then the grant is redundant AND ambiguous: two + // mechanisms would claim to own the same URL and only one of them terminates. + // Reject loudly rather than silently letting the static entry win. Once core + // drops a static exemption, the same declaration starts validating — which is + // exactly the handover this mechanism exists to make possible. + const probe = `${p}/`; + const collidingCoreEntry = ctx.corePublicPaths.find( + (re) => re.test(p) || re.test(probe), + ); + if (collidingCoreEntry) { + return { + ok: false, + path: p, + reason: `is already a static core public path (${String(collidingCoreEntry)}) — remove the core exemption first, or drop the declaration`, + }; + } + + const underReservedRoot = isUnderPrefix(p, PLUGIN_PUBLIC_PATH_ROOT); + const underOwnRoute = ctx.ownRoutePrefixes.some((prefix) => + isUnderPrefix(p, prefix), + ); + if (!underReservedRoot && !underOwnRoute) { + return { + ok: false, + path: p, + reason: + ctx.ownRoutePrefixes.length === 0 + ? `must start with '${PLUGIN_PUBLIC_PATH_ROOT}' — the plugin registered no routers, so it owns no other prefix` + : `must start with '${PLUGIN_PUBLIC_PATH_ROOT}' or lie under a prefix this plugin registers (${ctx.ownRoutePrefixes.join(', ')})`, + }; + } + + return { ok: true, path: p }; +} + +/** Thrown by `claim()`. Carries the conflicting plugin so the operator-facing + * error names both sides rather than saying "conflict". */ +export class PublicPathClaimError extends Error { + constructor( + readonly pluginId: string, + readonly path: string, + reason: string, + readonly conflictsWith?: string, + ) { + super( + `public-path declaration '${path}' from plugin '${pluginId}' ${reason}`, + ); + this.name = 'PublicPathClaimError'; + } +} + +export interface PublicPathClaim { + readonly pluginId: string; + readonly prefix: string; + /** Whether the operator has consented to this prefix. Claimed-but-ungranted + * prefixes hold the ownership reservation and serve nothing. */ + readonly granted: boolean; +} + +export interface PublicPathMatch { + readonly pluginId: string; + readonly prefix: string; +} + +/** + * Who owns which public prefix, and which of those the operator consented to. + * + * In-memory and authoritative for routing. Durable operator consent lives in + * `plugin_public_path_grants` (see `publicPathGrantStore.ts`); this registry is + * told about it at activation and whenever consent changes. + */ +export class PublicPathGrantRegistry { + /** prefix → owner. One map, so exclusivity is a `Map` invariant, not a scan. */ + private readonly byPrefix = new Map< + string, + { pluginId: string; granted: boolean } + >(); + + /** + * Claim every declared prefix for `pluginId`, exclusively. + * + * Re-claiming is idempotent for the SAME plugin (hot-reactivate releases its + * own prior claims first), and hard-fails for a different one: first + * activation wins and the second gets a named error. Nothing partial is left + * behind — a rejection rolls back the prefixes this call already took, so a + * failed activation cannot squat on a prefix nobody can then reclaim. + * + * @param grantedPrefixes prefixes the operator has already consented to. + * Anything outside this set is claimed but not served. + */ + claim( + pluginId: string, + declaredPaths: readonly unknown[], + ctx: PublicPathValidationContext & { + readonly grantedPrefixes: ReadonlySet; + }, + ): void { + if (declaredPaths.length > MAX_DECLARED_PUBLIC_PATHS) { + throw new PublicPathClaimError( + pluginId, + `${String(declaredPaths.length)} entries`, + `exceeds the maximum of ${String(MAX_DECLARED_PUBLIC_PATHS)} declared public paths`, + ); + } + // Hot-reactivate: drop this plugin's own reservations so re-declaring the + // same prefix is not a self-conflict. + this.releaseBySource(pluginId); + + const taken: string[] = []; + try { + for (const raw of declaredPaths) { + const check = validateDeclaredPublicPath(raw, ctx); + if (!check.ok) { + throw new PublicPathClaimError(pluginId, check.path, check.reason); + } + const prefix = check.path; + for (const [existing, owner] of this.byPrefix) { + if (prefixesOverlap(prefix, existing)) { + throw new PublicPathClaimError( + pluginId, + prefix, + `overlaps '${existing}', already owned by plugin '${owner.pluginId}'`, + owner.pluginId, + ); + } + } + this.byPrefix.set(prefix, { + pluginId, + granted: ctx.grantedPrefixes.has(prefix), + }); + taken.push(prefix); + } + } catch (err) { + for (const prefix of taken) this.byPrefix.delete(prefix); + throw err; + } + } + + /** + * Re-apply operator consent for an already-activated plugin, so granting or + * revoking in the admin UI takes effect without a restart. Prefixes the + * plugin never declared are ignored — consent cannot invent ownership. + */ + setGranted(pluginId: string, grantedPrefixes: ReadonlySet): void { + for (const [prefix, owner] of this.byPrefix) { + if (owner.pluginId !== pluginId) continue; + owner.granted = grantedPrefixes.has(prefix); + } + } + + /** + * The owning plugin for a request path, or `null` when the path is not + * publicly granted. Only GRANTED prefixes match: a declared-but-unconsented + * prefix resolves to `null` and the request goes to `requireAuth`. + * + * Longest prefix wins. Overlapping claims are rejected at claim time, so this + * is belt-and-braces rather than load-bearing — but if the invariant is ever + * broken, the more specific claim answering is the safer of the two outcomes. + */ + resolve(requestPath: string): PublicPathMatch | null { + let best: PublicPathMatch | null = null; + for (const [prefix, owner] of this.byPrefix) { + if (!owner.granted) continue; + if (!isUnderPrefix(requestPath, prefix)) continue; + if (best === null || prefix.length > best.prefix.length) { + best = { pluginId: owner.pluginId, prefix }; + } + } + return best; + } + + /** Drop every claim held by `pluginId`. Returns how many were released. */ + releaseBySource(pluginId: string): number { + let count = 0; + for (const [prefix, owner] of [...this.byPrefix]) { + if (owner.pluginId !== pluginId) continue; + this.byPrefix.delete(prefix); + count += 1; + } + return count; + } + + /** Diagnostic + admin surface: who owns what, and what is consented. */ + list(): readonly PublicPathClaim[] { + return [...this.byPrefix].map(([prefix, owner]) => ({ + pluginId: owner.pluginId, + prefix, + granted: owner.granted, + })); + } +} diff --git a/middleware/src/platform/publicPathMount.ts b/middleware/src/platform/publicPathMount.ts new file mode 100644 index 00000000..589f8e03 --- /dev/null +++ b/middleware/src/platform/publicPathMount.ts @@ -0,0 +1,148 @@ +import type { NextFunction, Request, RequestHandler, Response } from 'express'; + +import type { PluginRouteRegistry } from './pluginRouteRegistry.js'; +import type { PublicPathGrantRegistry } from './publicPathGrants.js'; + +/** + * Epic #470 C4 / H1 — the terminating early mount. + * + * Mounted in `index.ts` BEFORE the blanket `app.use('/api', requireAuth, …)` + * line. That position is the entire design: + * + * request + * → [this mount] granted prefix? → dispatch to the owning plugin, TERMINATE + * otherwise → next() + * → requireAuth 401 unless the URL is a static core public path + * → the authenticated router stack + * + * WHY IT TERMINATES + * ----------------- + * If this mount called `next()` when the owning plugin's router did not handle + * a path under its granted prefix, the request would carry on into the stack + * with no session — and some other router, mounted now or added in two years + * by someone who never read this file, would answer it. That is exactly the + * hole a plain `publicPaths` entry leaves open: an exemption grants a URL, not + * a router. So an unhandled path under a granted prefix is a 404 from here and + * goes no further. The granted prefix is a closed world owned by one plugin. + * + * There is a test that proves this is load-bearing rather than decorative: + * `terminate: false` reproduces the fallthrough, and the assertion flips. + * + * WHY IT IS FAIL-CLOSED + * --------------------- + * Every "I don't know" answer is `next()` — i.e. straight into `requireAuth`. + * No grants loaded, registry unwired, plugin not activated, store unreachable: + * all of them mean nothing matches, and the request is authenticated normally. + * The only way to reach a plugin handler without a session is for all three + * gates (declared, exclusively owned, operator-granted) to be satisfied at once + * AND for the plugin to be live and actually serving that prefix. + */ + +export interface PublicPathMountOptions { + readonly grants: PublicPathGrantRegistry; + readonly routes: PluginRouteRegistry; + /** + * Whether an unhandled path under a granted prefix terminates with 404. + * + * Defaults to `true` and production never passes anything else. It exists so + * the counter-proof test can switch the guarantee OFF and demonstrate the + * fallthrough it prevents — a test that cannot fail when the mechanism is + * removed is not evidence that the mechanism works. + */ + readonly terminate?: boolean; + /** Injected for tests; defaults to `console.warn`. */ + readonly logger?: (message: string) => void; +} + +/** Only these reach a plugin handler unauthenticated. Anything exotic (TRACE, + * CONNECT, arbitrary WebDAV verbs) goes to `requireAuth` instead — a grant is + * for a prefix, and widening it to every conceivable method for free is not + * what the operator consented to. */ +const ALLOWED_METHODS = new Set([ + 'GET', + 'HEAD', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + 'OPTIONS', +]); + +export function createPublicPathMount( + opts: PublicPathMountOptions, +): RequestHandler { + const terminate = opts.terminate !== false; + const warn = opts.logger ?? ((message: string) => console.warn(message)); + + return function publicPathMount( + req: Request, + res: Response, + next: NextFunction, + ): void { + // `req.path` excludes the query string but is NOT decoded — it is the raw + // pathname (`parseurl(req).pathname`), so `/x/%68i` stays `/x/%68i` here. + // The grant match is therefore a raw byte-prefix comparison, and that is + // why the declaration validator forbids percent-encoding: an encoded + // declaration could never match a raw request path, so banning it removes + // the second representation instead of trusting a decode that never + // happens. An encoded REQUEST path simply fails to match and falls through + // to `requireAuth` — the fail-closed direction. Containment of anything + // exotic that does match (dot segments, `..;/`, doubled slashes) comes from + // TERMINATION below, never from normalisation. + const match = opts.grants.resolve(req.path); + if (!match) { + next(); + return; + } + if (!ALLOWED_METHODS.has(req.method)) { + next(); + return; + } + + const dispatch = opts.routes.resolvePublicDispatch( + match.pluginId, + req.path, + ); + if (!dispatch) { + // Granted, owned — and nothing live is serving it. The plugin is + // deactivated, failed to activate, or never registered a router covering + // its own declaration. Terminate: falling through here would hand an + // unauthenticated request to whatever else happens to match. + finish(req, res, next, terminate, warn, match.prefix, 'no_live_router'); + return; + } + + dispatch.handle(req, res, (err?: unknown) => { + if (err) { + next(err); + return; + } + finish(req, res, next, terminate, warn, match.prefix, 'unhandled'); + }); + }; +} + +function finish( + req: Request, + res: Response, + next: NextFunction, + terminate: boolean, + warn: (message: string) => void, + prefix: string, + reason: 'no_live_router' | 'unhandled', +): void { + if (!terminate) { + // Counter-proof mode only. Never taken in production — see the + // `terminate` option's doc comment. + warn( + `[public-paths] TERMINATION DISABLED — '${req.path}' falls through from granted prefix '${prefix}' (${reason})`, + ); + next(); + return; + } + if (res.headersSent) return; + res.status(404).json({ + code: 'public_path.not_found', + message: `no handler for '${req.path}' under the public prefix '${prefix}'`, + }); +} diff --git a/middleware/src/plugins/manifestLoader.ts b/middleware/src/plugins/manifestLoader.ts index bfc82c25..1c47bc4a 100644 --- a/middleware/src/plugins/manifestLoader.ts +++ b/middleware/src/plugins/manifestLoader.ts @@ -25,6 +25,11 @@ import type { SetupAudience, SetupProfile, } from '../api/admin-v1.js'; +import { + MAX_DECLARED_PUBLIC_PATHS, + publicPathEntrySchema, + publicPathsDeclarationSchema, +} from '../platform/publicPathGrants.js'; import { compileSetupPattern } from './setupFieldPattern.js'; import { normalizeLocalized } from './manifestLocalized.js'; @@ -427,7 +432,7 @@ export function adaptManifestV1(doc: Record): Plugin | null { // Spec 005 — declarative OAuth-provider descriptors. Inert data the kernel // broker reads at flow time; no plugin code runs during the OAuth dance. const oauthProviders = extractOAuthProviders(doc['oauth_providers'], id); - const permissionsSummary = extractPermissions(permissions); + const permissionsSummary = extractPermissions(permissions, id); if (oauthProviders.length > 0) { permissionsSummary.acquires_oauth = true; } @@ -693,7 +698,9 @@ function extractChannelBlock( function extractPermissions( permissions: Record | undefined, + pluginId: string, ): PluginPermissionsSummary { + warnOnUnknownPermissionKeys(permissions, pluginId); const memory = asRecord(permissions?.['memory']); const graph = asRecord(permissions?.['graph']); const network = asRecord(permissions?.['network']); @@ -768,9 +775,106 @@ function extractPermissions( // Spec 005 — overridden to true in adaptManifestV1 when the manifest // declares >=1 valid oauth_providers descriptor. acquires_oauth: false, + // Epic #470 C4 / H1 — the prefixes the plugin ASKS to serve without a + // session. Validated here for shape only; ownership and operator consent + // are decided at activation (`platform/publicPathGrants.ts`). A malformed + // entry is dropped with a warning rather than rejecting the manifest, + // matching this loader's graceful-degradation rule everywhere else — and + // dropping is the safe direction, because a dropped entry is one fewer + // unauthenticated surface, never one more. + public_paths: extractPublicPaths(permissions?.['public_paths'], pluginId), }; } +/** + * Keys this loader understands under `permissions:`. Anything else is a typo, + * a key from a newer core, or a key from a core that dropped it — all three of + * which used to be silently ignored (recorded in `implementation.md` §2.5 as + * the reason a plugin could declare `permissions.public_paths` against an + * unpatched core and activate with no grant and no error). + * + * The manifest is still not rejected over an unknown key — that would make + * every core upgrade a breaking change for plugins built against a newer + * schema. It is now VISIBLE, which is the part that was missing. + */ +const KNOWN_PERMISSION_KEYS: ReadonlySet = new Set([ + 'events', + 'flows', + 'graph', + 'llm', + 'mcp', + 'memory', + 'network', + 'public_paths', + 'secrets', + 'subAgents', + 'templates', +]); + +/** + * Retired keys are deliberately NOT listed above. A manifest still declaring + * one keeps loading and activating exactly as before — the warning is the + * whole point: "core removed this, your manifest still asks for it" is + * information the plugin author needs, and it is the same signal a typo gets. + */ + +function warnOnUnknownPermissionKeys( + permissions: Record | undefined, + pluginId: string, +): void { + if (!permissions) return; + const unknown = Object.keys(permissions).filter( + (key) => !KNOWN_PERMISSION_KEYS.has(key), + ); + if (unknown.length === 0) return; + console.warn( + `[catalog] plugin '${pluginId}' declares unknown permission key(s) ${unknown + .map((k) => `permissions.${k}`) + .join(', ')} — ignored. Check the spelling, or the core version this ` + + 'manifest was written against.', + ); +} + +/** + * Epic #470 C4 / H1 — shape-validate `permissions.public_paths`. + * + * Only the syntactic gate lives here; it deliberately does NOT decide whether + * the path is claimable. Ownership needs to know about every other installed + * plugin, and the "must be a prefix this plugin actually serves" rule needs the + * live route registry — neither exists at catalog-load time. Both run at + * activation, where a violation is a loud activation failure rather than a + * quietly-shortened list. + */ +function extractPublicPaths(raw: unknown, pluginId: string): string[] { + if (raw === undefined || raw === null) return []; + if (!Array.isArray(raw)) { + console.warn( + `[catalog] plugin '${pluginId}': permissions.public_paths must be an array of path prefixes — ignored.`, + ); + return []; + } + const parsed = publicPathsDeclarationSchema.safeParse(raw); + if (parsed.success) return [...parsed.data]; + + // Keep the entries that are individually well-formed, name the ones that are + // not. A single bad entry must not silently take a plugin's whole + // declaration with it, and it must not pass unmentioned either. + const kept: string[] = []; + for (const entry of raw) { + const one = publicPathEntrySchema.safeParse(entry); + if (one.success) { + kept.push(one.data); + continue; + } + console.warn( + `[catalog] plugin '${pluginId}': permissions.public_paths entry ${JSON.stringify(entry)} rejected — ${ + one.error.issues[0]?.message ?? 'invalid' + }`, + ); + } + return kept.slice(0, MAX_DECLARED_PUBLIC_PATHS); +} + /** * Spec 005 — parse + validate the top-level `oauth_providers:` block. Each * descriptor must carry id/authorize_url/token_url/client_id_field/ diff --git a/middleware/src/plugins/toolPluginRuntime.ts b/middleware/src/plugins/toolPluginRuntime.ts index a808ba8f..c89abb6f 100644 --- a/middleware/src/plugins/toolPluginRuntime.ts +++ b/middleware/src/plugins/toolPluginRuntime.ts @@ -5,6 +5,9 @@ import { promises as fs } from 'node:fs'; import { createPluginContext } from '../platform/pluginContext.js'; import { eventEmitIds } from '../platform/eventCatalogRegistry.js'; import type { PluginRouteRegistry } from '../platform/pluginRouteRegistry.js'; +import { PublicPathClaimError } from '../platform/publicPathGrants.js'; +import type { PublicPathGrantRegistry } from '../platform/publicPathGrants.js'; +import type { PublicPathGrantStore } from '../platform/publicPathGrantStore.js'; import type { NotificationRouter } from '../platform/notificationRouter.js'; import type { PluginStatusRegistry } from '../platform/pluginStatusRegistry.js'; import type { UiRouteCatalog } from '../platform/uiRouteCatalog.js'; @@ -82,6 +85,20 @@ export interface ToolPluginRuntimeDeps { serviceRegistry: ServiceRegistry; nativeToolRegistry: NativeToolRegistry; pluginRouteRegistry: PluginRouteRegistry; + /** Epic #470 C4 / H1 — exclusive ownership of manifest-declared public path + * prefixes. Optional so narrow test wiring can omit it; when absent, NO + * prefix is ever claimed and every plugin route stays behind `requireAuth` + * (the fail-closed direction). */ + publicPathGrants?: PublicPathGrantRegistry; + /** Operator consent backing `publicPathGrants`. Optional for the same + * reason, and with the same consequence: no store, no consent, no public + * path. */ + publicPathGrantStore?: PublicPathGrantStore; + /** The core exemption list, injected rather than imported so a declaration + * is checked against the SAME array `requireAuth` runs (see the doc comment + * on `auth/publicPaths.ts`). Optional; absent means "no core exemptions to + * collide with", which only widens what a plugin may declare in tests. */ + corePublicPaths?: readonly RegExp[]; notificationRouter: NotificationRouter; uiRouteCatalog: UiRouteCatalog; jobScheduler: JobScheduler; @@ -317,6 +334,62 @@ export class ToolPluginRuntime { throw err; } + // Epic #470 C4 / H1 — claim the manifest-declared public-path prefixes. + // + // AFTER activate(), deliberately. The rule "a plugin may only make public + // something it actually serves" needs the prefixes the plugin registered, + // and those only exist once activate() has run. Claiming earlier would mean + // trusting the manifest about which routers exist, which is the same + // mistake as trusting it about authentication. + // + // A rejected claim is a hard activation failure with the SAME rollback the + // catch above performs. Half-activating a plugin whose public-path + // declaration conflicts with another plugin's is the one outcome worse than + // refusing it: the operator would see a healthy plugin serving a prefix + // somebody else owns. + if (this.deps.publicPathGrants) { + const declared = catalogEntry.plugin.permissions_summary?.public_paths ?? []; + if (declared.length > 0) { + try { + const granted = + (await this.deps.publicPathGrantStore?.listForPlugin(agentId)) ?? + new Set(); + this.deps.publicPathGrants.claim(agentId, declared, { + corePublicPaths: this.deps.corePublicPaths ?? [], + ownRoutePrefixes: this.deps.pluginRouteRegistry + .list() + .filter((r) => r.source === agentId && !r.disposed) + .map((r) => r.prefix), + grantedPrefixes: granted, + }); + const ungranted = declared.filter((p) => !granted.has(p)); + log( + `[tool-runtime] public paths for ${agentId}: ${String(granted.size)} granted, ` + + `${String(ungranted.length)} declared-but-awaiting-consent` + + (ungranted.length > 0 ? ` (${ungranted.join(', ')})` : ''), + ); + } catch (err) { + this.deps.publicPathGrants.releaseBySource(agentId); + this.deps.pluginRouteRegistry.disposeBySource(agentId); + this.deps.uiRouteCatalog.disposeBySource(agentId); + this.deps.serviceRegistry.disposeBySource(agentId); + this.deps.jobScheduler.stopForPlugin(agentId); + this.deps.pluginStatusRegistry?.clear(agentId); + // The plugin's own close() still has to run — it may hold a socket or + // a timer that activate() opened. Best-effort; the claim error is + // what propagates. + await Promise.resolve(handle.close()).catch(() => undefined); + if (err instanceof PublicPathClaimError) { + throw new Error( + `tool-runtime: ${agentId} cannot activate — ${err.message}`, + { cause: err }, + ); + } + throw err; + } + } + } + // Plugin self-extension (Theme B): if the module opted into the selfExtend // SDK, register its declarative templates and re-materialise every // operator-approved extension via the plugin's OWN `apply()`, passing the @@ -417,6 +490,13 @@ export class ToolPluginRuntime { this.deps.pluginRouteRegistry.disposeBySource(agentId); this.deps.uiRouteCatalog.disposeBySource(agentId); this.deps.serviceRegistry.disposeBySource(agentId); + // Epic #470 C4 / H1 — release the public-path ownership in the SAME breath + // as the routers. These two must never drift apart: an ownership claim that + // outlives its routers is a granted prefix with nothing behind it, and a + // router disposed while the claim stands is a prefix nobody else can take. + // (The mount answers 404 for the window in between either way — it resolves + // the live router on every request, not once at claim time.) + this.deps.publicPathGrants?.releaseBySource(agentId); try { await withTimeout( entry.handle.close(), diff --git a/middleware/src/routes/runtime.ts b/middleware/src/routes/runtime.ts index 393ff56c..f4a6c50e 100644 --- a/middleware/src/routes/runtime.ts +++ b/middleware/src/routes/runtime.ts @@ -19,6 +19,10 @@ import { checkSetupFieldPattern } from '../plugins/setupFieldPattern.js'; import type { PatternViolation } from '../plugins/setupFieldPattern.js'; import { extractFromJsonFile } from '../plugins/setupJsonFile.js'; import type { JsonFileFailureCode, JsonFileFieldSpec } from '../plugins/setupJsonFile.js'; +import type { PublicPathGrantRegistry } from '../platform/publicPathGrants.js'; +import { validateDeclaredPublicPath } from '../platform/publicPathGrants.js'; +import type { PublicPathGrantStore } from '../platform/publicPathGrantStore.js'; +import { publicPaths } from '../auth/publicPaths.js'; import type { SecretVault } from '../secrets/vault.js'; /** Per-call budget for invoking a plugin's dynamic options provider. */ @@ -74,6 +78,45 @@ interface RuntimeDeps { input: unknown, ): Promise; }; + /** Epic #470 C4 / H1 — durable operator consent for unauthenticated plugin + * path prefixes. Optional so existing test wiring keeps compiling; the + * public-path endpoints 503 when absent. */ + publicPathGrantStore?: PublicPathGrantStore; + /** The live ownership registry, so a consent change takes effect without a + * restart. Optional for the same reason — without it, a grant is persisted + * and applies on the plugin's next activation. */ + publicPathGrants?: PublicPathGrantRegistry; +} + +/** + * Epic #470 C4 / H1 — put the routing registry back in step with the consent + * table after a failed consent write. + * + * Called only from the PUT error path. The registry, not the table, decides + * whether a URL skips authentication, so it must never be left describing + * consent the table no longer records. If the re-read itself fails there is no + * trustworthy answer available, and the only safe assumption is that nothing is + * consented: that costs the plugin's public route and costs nothing in + * security. + */ +async function resyncPublicPathGrants( + deps: Pick, + pluginId: string, +): Promise { + const registry = deps.publicPathGrants; + if (!registry) return; + try { + const truth = + (await deps.publicPathGrantStore?.listForPlugin(pluginId)) ?? + new Set(); + registry.setGranted(pluginId, new Set(truth)); + } catch (err) { + registry.setGranted(pluginId, new Set()); + console.warn( + `[public-paths] could not re-read consent for '${pluginId}' after a failed update — closing every granted prefix:`, + err instanceof Error ? err.message : String(err), + ); + } } export function createRuntimeRouter(deps: RuntimeDeps): Router { @@ -288,6 +331,158 @@ export function createRuntimeRouter(deps: RuntimeDeps): Router { }, ); + // ── Epic #470 C4 / H1 — public-path consent ────────────────────────────── + // + // GET /installed/:id/public-paths what the manifest asks for, and + // which of those the operator granted + // PUT /installed/:id/public-paths { paths: string[] } — the complete + // consented set (grants what is new, + // revokes what is missing) + // + // PUT takes the FULL set rather than a single path on purpose: consent to an + // unauthenticated surface should be reviewed as a whole. "Add one more" and + // "here is the complete list I agree to" are different operator intents, and + // only the second one is safe to build a UI on. + router.get('/installed/:id/public-paths', async (req: Request, res: Response) => { + const id = typeof req.params['id'] === 'string' ? req.params['id'] : undefined; + if (!id) { + res.status(400).json({ code: 'runtime.invalid_id', message: 'missing id' }); + return; + } + if (!deps.publicPathGrantStore) { + res.status(503).json({ + code: 'runtime.public_paths_unavailable', + message: 'public-path grants require a database — none is configured', + }); + return; + } + const declared = + deps.catalog?.get(id)?.plugin.permissions_summary?.public_paths ?? []; + const granted = await deps.publicPathGrantStore.listForPlugin(id); + res.json({ + id, + declared, + // Reported per declared path rather than as a bare list, so the UI can + // render "asked for, not granted" — the state that matters — without + // having to diff two arrays and get the semantics wrong. + paths: declared.map((path) => ({ path, granted: granted.has(path) })), + // A grant whose declaration disappeared (plugin downgraded, manifest + // edited). Surfaced rather than hidden: it grants nothing today, but an + // operator should be able to see and clear it. + orphaned: [...granted].filter((path) => !declared.includes(path)), + }); + }); + + router.put('/installed/:id/public-paths', async (req: Request, res: Response) => { + const id = typeof req.params['id'] === 'string' ? req.params['id'] : undefined; + if (!id) { + res.status(400).json({ code: 'runtime.invalid_id', message: 'missing id' }); + return; + } + if (!deps.publicPathGrantStore) { + res.status(503).json({ + code: 'runtime.public_paths_unavailable', + message: 'public-path grants require a database — none is configured', + }); + return; + } + const body = req.body as { paths?: unknown } | null; + const requested = body?.paths; + if (!Array.isArray(requested) || requested.some((p) => typeof p !== 'string')) { + res.status(400).json({ + code: 'runtime.invalid_public_paths', + message: 'body.paths must be an array of strings', + }); + return; + } + const installed = deps.installedRegistry.get(id); + if (!installed) { + res.status(404).json({ + code: 'runtime.not_installed', + message: `agent '${id}' is not installed`, + }); + return; + } + const declared = + deps.catalog?.get(id)?.plugin.permissions_summary?.public_paths ?? []; + + // An operator may only consent to what the plugin actually ASKED for. + // Without this check the consent endpoint would itself be a way to make an + // arbitrary URL public — a bigger hole than the one this epic closes. + const undeclared = (requested as string[]).filter((p) => !declared.includes(p)); + if (undeclared.length > 0) { + res.status(400).json({ + code: 'runtime.public_path_not_declared', + message: + `agent '${id}' does not declare ${undeclared.join(', ')} in ` + + 'permissions.public_paths — consent cannot exceed the declaration', + }); + return; + } + + // Re-run the syntactic gate at consent time too. The catalog entry could + // have been produced by an older core, and this is the last point before a + // prefix becomes unauthenticated. `ownRoutePrefixes` is intentionally the + // declaration itself: cross-plugin exclusivity and the "must actually + // serve it" rule are activation-time concerns and are re-checked there. + for (const path of requested as string[]) { + const check = validateDeclaredPublicPath(path, { + corePublicPaths: publicPaths(), + ownRoutePrefixes: declared, + }); + if (!check.ok) { + res.status(400).json({ + code: 'runtime.invalid_public_path', + message: `'${check.path}' ${check.reason}`, + }); + return; + } + } + + const actor = req.session?.email ?? req.session?.sub ?? 'unknown'; + const next = new Set(requested as string[]); + try { + const current = await deps.publicPathGrantStore.listForPlugin(id); + const revoked = [...current].filter((path) => !next.has(path)); + + // NARROW THE REGISTRY BEFORE THE TABLE. + // + // The in-memory registry — not `plugin_public_path_grants` — is what the + // terminating mount consults on every request. So "revoke the row first" + // is the wrong thing to reason about: a revoke that reaches the database + // but not the registry is a prefix that keeps answering WITHOUT A SESSION + // until the process restarts. Closing it in the registry first means the + // surface is already shut no matter which write below fails, and the + // worst case becomes a row that outlives its routing effect — the + // restrictive direction. + if (revoked.length > 0) { + deps.publicPathGrants?.setGranted( + id, + new Set([...current].filter((path) => next.has(path))), + ); + } + + for (const path of revoked) { + await deps.publicPathGrantStore.revoke(id, path); + } + for (const path of next) { + await deps.publicPathGrantStore.grant(id, path, actor); + } + // Apply live so granting takes effect immediately rather than at next + // boot. Revocations already took effect above. + deps.publicPathGrants?.setGranted(id, next); + res.json({ id, paths: [...next] }); + } catch (err) { + // A half-written update leaves the registry describing a state the table + // does not agree with, and the registry is the one that decides whether a + // URL needs a session. Re-sync from the table so it can never keep + // serving a prefix whose row is already gone. + await resyncPublicPathGrants(deps, id); + const message = err instanceof Error ? err.message : String(err); + res.status(500).json({ code: 'runtime.update_failed', message }); + } + }); + // PATCH /installed/:id/audit-mode — #91 operator mode switch for an // audit/scanner plugin. Body: { mode: 'single-host'|'allowlist'|'public-web' }. // Rejected unless the manifest declares permissions.network.web_scanner. diff --git a/middleware/src/routes/store.ts b/middleware/src/routes/store.ts index ab19b060..3d5bfe17 100644 --- a/middleware/src/routes/store.ts +++ b/middleware/src/routes/store.ts @@ -533,5 +533,10 @@ function emptyPermissionsSummary(): PluginPermissionsSummary { graph_reads: [], graph_writes: [], network_outbound: [], + // Epic #470 C4 / H1 — the empty summary must claim NO public paths. This + // is the fallback used when a package has no readable manifest, and a + // fallback that invented an unauthenticated prefix would be the worst + // possible default. + public_paths: [], }; } diff --git a/middleware/test/publicPathGrants.test.ts b/middleware/test/publicPathGrants.test.ts new file mode 100644 index 00000000..e5db643e --- /dev/null +++ b/middleware/test/publicPathGrants.test.ts @@ -0,0 +1,696 @@ +import { describe, it, beforeEach } from 'node:test'; +import { strict as assert } from 'node:assert'; +import express from 'express'; +import { Router } from 'express'; +import cookieParser from 'cookie-parser'; +import type { Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; + +import { createRequireAuth } from '../src/auth/requireAuth.js'; +import { EmailWhitelist } from '../src/auth/whitelist.js'; +import { publicPaths } from '../src/auth/publicPaths.js'; +import { PluginRouteRegistry } from '../src/platform/pluginRouteRegistry.js'; +import { + PublicPathClaimError, + PublicPathGrantRegistry, + validateDeclaredPublicPath, +} from '../src/platform/publicPathGrants.js'; +import { createPublicPathMount } from '../src/platform/publicPathMount.js'; +import { listenLoopback } from './_helpers/listenLoopback.js'; +import { createRuntimeRouter } from '../src/routes/runtime.js'; +import { InMemoryInstalledRegistry } from '../src/plugins/installedRegistry.js'; +import type { + PluginCatalog, + PluginCatalogEntry, +} from '../src/plugins/manifestLoader.js'; +import type { PublicPathGrantStore } from '../src/platform/publicPathGrantStore.js'; + +/** + * Epic #470 C4 / H1 — manifest-declared, operator-consented public paths. + * + * The tests below build the PRODUCTION mount order, not a bare `express()`. + * That is the whole point: this epic's own runner router once passed an e2e + * test against a bare app while 401'ing in production behind the blanket + * `/api` gate (recorded in `auth/publicPaths.ts`). A test for an + * authentication bypass that does not include the authentication is not a test. + */ + +const SIGNING_KEY = new Uint8Array(32).fill(7); + +interface Harness { + server: Server; + baseUrl: string; + routes: PluginRouteRegistry; + grants: PublicPathGrantRegistry; +} + +/** + * Production topology, in order: + * express.json → cookieParser → [public-path mount] → requireAuth → routers + */ +async function buildApp(opts: { + routes: PluginRouteRegistry; + grants: PublicPathGrantRegistry; + terminate?: boolean; +}): Promise { + const app = express(); + app.use(express.json()); + app.use(cookieParser()); + + const mountOpts: Parameters[0] = { + grants: opts.grants, + routes: opts.routes, + ...(opts.terminate === undefined ? {} : { terminate: opts.terminate }), + logger: () => undefined, + }; + app.use(createPublicPathMount(mountOpts)); + + const requireAuth = createRequireAuth({ + signingKey: SIGNING_KEY, + whitelist: new EmailWhitelist('operator@example.com'), + publicPaths: publicPaths(), + }); + // The blanket OB-106 gate. Everything under /api that is not a static core + // public path and was not answered by the mount above lands here. + app.use('/api', requireAuth, (_req, res) => { + res.status(200).json({ reached: 'authenticated-stack' }); + }); + + // Plugin routers mount LAST in production (boot flush). Kept here so the + // fallthrough counter-proof can show what the terminating mount prevents. + opts.routes.mountAll(app); + + const server = await listenLoopback(app); + const { port } = server.address() as AddressInfo; + return { + server, + baseUrl: `http://127.0.0.1:${String(port)}`, + routes: opts.routes, + grants: opts.grants, + }; +} + +/** A plugin router that answers exactly one path under its prefix. */ +function pluginRouter(): Router { + const r = Router(); + r.get('/ping', (_req, res) => { + res.status(200).json({ ok: true, from: 'plugin' }); + }); + return r; +} + +const PREFIX = '/api/plugins/acme'; +const CTX = { + corePublicPaths: publicPaths(), + ownRoutePrefixes: [PREFIX], +}; + +describe('#470 C4/H1 — public-path declaration validation', () => { + it('accepts a well-formed prefix under the reserved plugin root', () => { + const result = validateDeclaredPublicPath(PREFIX, CTX); + assert.equal(result.ok, true); + }); + + it('rejects a path containing ".." traversal', () => { + const result = validateDeclaredPublicPath('/api/plugins/../v1/admin', CTX); + assert.equal(result.ok, false); + assert.ok( + !result.ok && /segments|unreserved/.test(result.reason), + `unexpected reason: ${!result.ok ? result.reason : ''}`, + ); + }); + + it('rejects percent-encoded traversal — the raw path has no second form', () => { + const result = validateDeclaredPublicPath('/api/plugins/%2e%2e/admin', CTX); + assert.equal(result.ok, false); + }); + + it('rejects a wildcard, which would widen the grant beyond the prefix', () => { + assert.equal(validateDeclaredPublicPath('/api/plugins/*', CTX).ok, false); + }); + + it('rejects a core-reserved root even though the operator could consent', () => { + const result = validateDeclaredPublicPath('/api/v1/admin/settings', CTX); + assert.equal(result.ok, false); + assert.ok(!result.ok && result.reason.includes('core-reserved')); + }); + + it('rejects a prefix that is already a static core public path', () => { + // `/p/` is a live entry in STATIC_PUBLIC_PATHS. Two mechanisms + // claiming the same URL is exactly the ambiguity this rejects. + const result = validateDeclaredPublicPath('/p/acme/dash', CTX); + assert.equal(result.ok, false); + assert.ok(!result.ok && result.reason.includes('static core public path')); + }); + + it('rejects a prefix outside both the reserved root and the plugin routes', () => { + const result = validateDeclaredPublicPath('/api/v1/something-else', CTX); + assert.equal(result.ok, false); + assert.ok(!result.ok && result.reason.includes('/api/plugins/')); + }); + + it('accepts a prefix the plugin actually registers a router at', () => { + // The rule that lets a plugin keep serving a historical wire path once + // core stops exempting it statically — without core naming that path. + const result = validateDeclaredPublicPath('/legacy/hook/inbound', { + corePublicPaths: publicPaths(), + ownRoutePrefixes: ['/legacy/hook'], + }); + assert.equal(result.ok, true); + }); + + it('rejects a one-segment claim as too broad', () => { + assert.equal(validateDeclaredPublicPath('/api', CTX).ok, false); + }); +}); + +describe('#470 C4/H1 — exclusive prefix ownership', () => { + let grants: PublicPathGrantRegistry; + beforeEach(() => { + grants = new PublicPathGrantRegistry(); + }); + + it('lets the first plugin claim a prefix', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.equal(grants.list().length, 1); + }); + + it('fails the SECOND plugin declaring the same prefix, naming the owner', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + let caught: unknown; + try { + grants.claim('evilcorp', [PREFIX], { + ...CTX, + grantedPrefixes: new Set(), + }); + } catch (err) { + caught = err; + } + assert.ok( + caught instanceof PublicPathClaimError, + 'second claim must throw PublicPathClaimError', + ); + assert.equal(caught.conflictsWith, 'acme'); + assert.equal(caught.pluginId, 'evilcorp'); + // The message must name BOTH sides — an operator reading a boot log needs + // to know who lost and who already held it. + assert.ok(caught.message.includes('acme')); + assert.ok(caught.message.includes('evilcorp')); + // First-come wins: the incumbent still owns it. + assert.deepEqual( + grants.list().map((c) => c.pluginId), + ['acme'], + ); + }); + + it('fails a plugin claiming a prefix NESTED under another plugin', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.throws( + () => + grants.claim('evilcorp', [`${PREFIX}/sub`], { + corePublicPaths: publicPaths(), + ownRoutePrefixes: [`${PREFIX}/sub`], + grantedPrefixes: new Set(), + }), + PublicPathClaimError, + ); + }); + + it('rolls back partial claims when a later entry in the same call fails', () => { + assert.throws( + () => + grants.claim('acme', [PREFIX, '/api/v1/admin/oops'], { + ...CTX, + grantedPrefixes: new Set(), + }), + PublicPathClaimError, + ); + // The valid first entry must NOT be left squatting. + assert.equal(grants.list().length, 0); + }); + + it('is idempotent for the same plugin re-activating', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.equal(grants.list().length, 1); + }); + + it('frees the prefix for another plugin once released', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.equal(grants.releaseBySource('acme'), 1); + grants.claim('other', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.deepEqual( + grants.list().map((c) => c.pluginId), + ['other'], + ); + }); + + it('resolves ONLY granted prefixes — a claim alone is not consent', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + assert.equal(grants.resolve(`${PREFIX}/ping`), null); + grants.setGranted('acme', new Set([PREFIX])); + assert.deepEqual(grants.resolve(`${PREFIX}/ping`), { + pluginId: 'acme', + prefix: PREFIX, + }); + }); + + it('will not let consent invent ownership of an undeclared prefix', () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + grants.setGranted('acme', new Set(['/api/plugins/never-declared'])); + assert.equal(grants.resolve('/api/plugins/never-declared/x'), null); + }); + + it('does not match a sibling prefix that merely shares a string prefix', () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + // `/api/plugins/acme-evil` starts with `/api/plugins/acme` as a STRING but + // is a different path segment. + assert.equal(grants.resolve('/api/plugins/acme-evil/ping'), null); + }); +}); + +describe('#470 C4/H1 — terminating early mount (production order)', () => { + let routes: PluginRouteRegistry; + let grants: PublicPathGrantRegistry; + let dispose: () => void; + + beforeEach(() => { + routes = new PluginRouteRegistry(); + grants = new PublicPathGrantRegistry(); + dispose = routes.register(PREFIX, pluginRouter(), 'acme'); + }); + + it('GRANTED: an unauthenticated GET reaches the plugin handler', async () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const h = await buildApp({ routes, grants }); + try { + const res = await fetch(`${h.baseUrl}${PREFIX}/ping`); + assert.equal(res.status, 200); + assert.deepEqual(await res.json(), { ok: true, from: 'plugin' }); + } finally { + h.server.close(); + } + }); + + it('DECLARED BUT NOT GRANTED: requireAuth 401s — consent is load-bearing', async () => { + grants.claim('acme', [PREFIX], { ...CTX, grantedPrefixes: new Set() }); + const h = await buildApp({ routes, grants }); + try { + const res = await fetch(`${h.baseUrl}${PREFIX}/ping`); + assert.equal(res.status, 401); + } finally { + h.server.close(); + } + }); + + it('NOT DECLARED AT ALL: requireAuth 401s (fail-closed baseline)', async () => { + const h = await buildApp({ routes, grants }); + try { + const res = await fetch(`${h.baseUrl}${PREFIX}/ping`); + assert.equal(res.status, 401); + } finally { + h.server.close(); + } + }); + + it('TERMINATION: an unhandled path under a granted prefix is 404, NOT fallthrough', async () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const h = await buildApp({ routes, grants }); + try { + const res = await fetch(`${h.baseUrl}${PREFIX}/not-a-route`); + assert.equal(res.status, 404); + const body = (await res.json()) as { code?: string }; + assert.equal(body.code, 'public_path.not_found'); + } finally { + h.server.close(); + } + }); + + it('AFTER DEACTIVATE: the granted prefix 404s instead of serving', async () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const h = await buildApp({ routes, grants }); + try { + assert.equal((await fetch(`${h.baseUrl}${PREFIX}/ping`)).status, 200); + dispose(); + const res = await fetch(`${h.baseUrl}${PREFIX}/ping`); + assert.equal(res.status, 404); + } finally { + h.server.close(); + } + }); + + it('AFTER disposeBySource: same, via the kernel deactivate fail-safe', async () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const h = await buildApp({ routes, grants }); + try { + assert.equal(routes.disposeBySource('acme'), 1); + assert.equal((await fetch(`${h.baseUrl}${PREFIX}/ping`)).status, 404); + } finally { + h.server.close(); + } + }); + + it('POST to a granted GET-only route terminates 404 rather than falling through', async () => { + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const h = await buildApp({ routes, grants }); + try { + const res = await fetch(`${h.baseUrl}${PREFIX}/ping`, { method: 'POST' }); + assert.equal(res.status, 404); + } finally { + h.server.close(); + } + }); + + it('a static core public path still works — the mount does not shadow it', async () => { + const h = await buildApp({ routes, grants }); + try { + // `/api/v1/auth/...` is a STATIC_PUBLIC_PATHS entry: it must pass + // requireAuth and reach the authenticated-stack sentinel unauthenticated. + const res = await fetch(`${h.baseUrl}/api/v1/auth/login-providers`); + assert.equal(res.status, 200); + assert.deepEqual(await res.json(), { reached: 'authenticated-stack' }); + } finally { + h.server.close(); + } + }); +}); + +describe('#470 C4/H1 — COUNTER-PROOF: termination is load-bearing', () => { + /** + * Same fixture as the TERMINATION test above, with `terminate: false`. + * + * If termination were decorative, this would still 404 and the assertion + * below would fail. It does not: the request falls out of the mount, meets + * `requireAuth`, and — because it is under `/api` with no session — 401s. + * On a deployment where the plugin's granted prefix happened to sit under a + * core static exemption, or where any later router matched, that same + * fallthrough would have been served with NO authentication at all. + * + * This is the difference between "the plugin owns this prefix" and "this URL + * skips auth", and it is why `auth/publicPaths.ts` stays a frozen literal. + */ + it('with termination OFF the unhandled path escapes the mount', async () => { + const routes = new PluginRouteRegistry(); + const grants = new PublicPathGrantRegistry(); + routes.register(PREFIX, pluginRouter(), 'acme'); + grants.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + + const withTermination = await buildApp({ routes, grants }); + let terminated: number; + try { + terminated = (await fetch(`${withTermination.baseUrl}${PREFIX}/nope`)) + .status; + } finally { + withTermination.server.close(); + } + + const routes2 = new PluginRouteRegistry(); + const grants2 = new PublicPathGrantRegistry(); + routes2.register(PREFIX, pluginRouter(), 'acme'); + grants2.claim('acme', [PREFIX], { + ...CTX, + grantedPrefixes: new Set([PREFIX]), + }); + const without = await buildApp({ + routes: routes2, + grants: grants2, + terminate: false, + }); + let fellThrough: number; + try { + fellThrough = (await fetch(`${without.baseUrl}${PREFIX}/nope`)).status; + } finally { + without.server.close(); + } + + assert.equal(terminated, 404, 'termination ON must answer 404'); + assert.notEqual( + fellThrough, + 404, + 'termination OFF must NOT 404 — otherwise this test proves nothing', + ); + assert.equal( + fellThrough, + 401, + 'termination OFF lets the request escape into the authenticated stack', + ); + }); +}); + + +// ── Consent writes: the registry is the authority, not the table ─────────── +// +// `PUT /installed/:id/public-paths` writes rows AND updates the in-memory +// `PublicPathGrantRegistry`. Only the registry is consulted per request, so a +// half-applied update that leaves the registry more permissive than the table +// is a prefix still answering without a session. These tests drive the real +// endpoint and then probe the real public surface, sharing ONE registry +// instance between the two mounts exactly as the process does. + +const ACME = 'de.byte5.agent.acme'; +const P_ONE = '/api/plugins/acme/one'; +const P_TWO = '/api/plugins/acme/two'; + +interface StoreState { + rows: Set; + /** Reject `grant()` for this path, simulating a write that dies partway. */ + failGrantFor?: string; + /** Reject `listForPlugin()` from this call number on (1-based), so the + * error-path re-read can be failed without failing the first read. */ + failListFromCall?: number; + listCalls: number; +} + +function stubStore(state: StoreState): PublicPathGrantStore { + return { + listForPlugin: () => { + state.listCalls += 1; + if ( + state.failListFromCall !== undefined && + state.listCalls >= state.failListFromCall + ) { + return Promise.reject(new Error('grant table unreadable')); + } + return Promise.resolve(new Set(state.rows)); + }, + listAll: () => Promise.resolve([]), + grant: (_id: string, path: string) => { + if (state.failGrantFor === path) { + return Promise.reject(new Error(`grant write failed for ${path}`)); + } + state.rows.add(path); + return Promise.resolve(); + }, + revoke: (_id: string, path: string) => + Promise.resolve(state.rows.delete(path)), + revokeAllForPlugin: () => Promise.resolve(0), + }; +} + +interface ConsentHarness { + publicUrl: string; + consentUrl: string; + close(): void; +} + +async function makeConsentHarness(opts: { + granted: readonly string[]; + store: PublicPathGrantStore; +}): Promise { + const routes = new PluginRouteRegistry(); + routes.register(P_ONE, pluginRouter(), ACME); + routes.register(P_TWO, pluginRouter(), ACME); + + const grants = new PublicPathGrantRegistry(); + grants.claim(ACME, [P_ONE, P_TWO], { + corePublicPaths: publicPaths(), + ownRoutePrefixes: [P_ONE, P_TWO], + grantedPrefixes: new Set(opts.granted), + }); + + const pub = await buildApp({ routes, grants }); + + const installed = new InMemoryInstalledRegistry(); + await installed.register({ + id: ACME, + installed_version: '0.1.0', + installed_at: new Date().toISOString(), + status: 'active', + config: {}, + }); + const entry = { + plugin: { + id: ACME, + name: 'Acme', + version: '0.1.0', + permissions_summary: { public_paths: [P_ONE, P_TWO] }, + }, + manifest: {}, + source_path: '', + source_kind: 'manifest-v1', + } as unknown as PluginCatalogEntry; + const catalog = { + get: (id: string): PluginCatalogEntry | undefined => + id === ACME ? entry : undefined, + } as unknown as PluginCatalog; + const stub = { names: () => [], counts: () => ({}) }; + + const consentApp = express(); + consentApp.use(express.json()); + consentApp.use( + '/api/v1/admin/runtime', + createRuntimeRouter({ + installedRegistry: installed, + serviceRegistry: stub as never, + turnHookRegistry: stub as never, + backgroundJobRegistry: stub as never, + chatAgentWrapRegistry: { labels: () => [], count: () => 0 } as never, + promptContributionRegistry: { labels: () => [], count: () => 0 } as never, + catalog, + publicPathGrantStore: opts.store, + // THE SAME registry the public mount above reads. + publicPathGrants: grants, + }), + ); + const consentServer = await listenLoopback(consentApp); + const { port } = consentServer.address() as AddressInfo; + + return { + publicUrl: pub.baseUrl, + consentUrl: `http://127.0.0.1:${String(port)}/api/v1/admin/runtime/installed/${ACME}/public-paths`, + close: () => { + pub.server.close(); + consentServer.close(); + }, + }; +} + +function putPaths(url: string, paths: readonly string[]): Promise { + return fetch(url, { + method: 'PUT', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ paths }), + }); +} + +describe('#470 C4/H1 — consent writes close the live surface', () => { + it('closes a revoked prefix even when a later grant write fails', async () => { + // Both consented. The operator drops P_ONE and keeps P_TWO — and the + // re-grant of P_TWO dies after P_ONE's row is already gone. + const state: StoreState = { + rows: new Set([P_ONE, P_TWO]), + failGrantFor: P_TWO, + listCalls: 0, + }; + const h = await makeConsentHarness({ + granted: [P_ONE, P_TWO], + store: stubStore(state), + }); + try { + const before = await fetch(`${h.publicUrl}${P_ONE}/ping`); + assert.equal(before.status, 200, 'precondition: P_ONE served publicly'); + + const res = await putPaths(h.consentUrl, [P_TWO]); + assert.equal(res.status, 500, 'the failed write must surface as a 500'); + + // The whole point. Before the fix the registry still carried + // granted=true for P_ONE, so this answered 200 from the plugin with no + // session — a revoked prefix serving unauthenticated until restart. + const after = await fetch(`${h.publicUrl}${P_ONE}/ping`); + assert.equal( + after.status, + 401, + 'a revoked prefix must stop serving even when the update failed', + ); + } finally { + h.close(); + } + }); + + it('applies a successful consent update to the live surface', async () => { + const state: StoreState = { + rows: new Set([P_ONE, P_TWO]), + listCalls: 0, + }; + const h = await makeConsentHarness({ + granted: [P_ONE, P_TWO], + store: stubStore(state), + }); + try { + const res = await putPaths(h.consentUrl, [P_TWO]); + assert.equal(res.status, 200); + assert.deepEqual((await res.json()) as unknown, { + id: ACME, + paths: [P_TWO], + }); + + assert.equal( + (await fetch(`${h.publicUrl}${P_ONE}/ping`)).status, + 401, + 'the revoked prefix is closed', + ); + assert.equal( + (await fetch(`${h.publicUrl}${P_TWO}/ping`)).status, + 200, + 'the still-consented prefix keeps serving', + ); + assert.deepEqual([...state.rows], [P_TWO], 'the table agrees'); + } finally { + h.close(); + } + }); + + it('grants nothing when the error-path re-read also fails', async () => { + // The write fails AND the registry cannot be re-synced from the table. + // With no trustworthy answer available the only safe state is "nothing is + // public" — never "keep whatever was there". + const state: StoreState = { + rows: new Set([P_ONE, P_TWO]), + failGrantFor: P_TWO, + failListFromCall: 2, + listCalls: 0, + }; + const h = await makeConsentHarness({ + granted: [P_ONE, P_TWO], + store: stubStore(state), + }); + try { + const res = await putPaths(h.consentUrl, [P_TWO]); + assert.equal(res.status, 500); + + assert.equal( + (await fetch(`${h.publicUrl}${P_ONE}/ping`)).status, + 401, + 'revoked prefix closed', + ); + assert.equal( + (await fetch(`${h.publicUrl}${P_TWO}/ping`)).status, + 401, + 'unreadable consent must close every prefix, not preserve the old set', + ); + } finally { + h.close(); + } + }); +}); diff --git a/specs/470-dev-platform-plugin/README.md b/specs/470-dev-platform-plugin/README.md index e62e23e2..2639f444 100644 --- a/specs/470-dev-platform-plugin/README.md +++ b/specs/470-dev-platform-plugin/README.md @@ -86,6 +86,28 @@ it at all. contract still holds, so a snapshot updated without a bump is the same silent break C1 exists to stop. Full table in `middleware/packages/plugin-api/README.md`. +### C4 / H1 — public-path grants (shipped) + +- **Manifest-declared, operator-consented public-path grants + exclusive prefix ownership + + the terminating early mount.** Closes G6. `permissions.public_paths` is a REQUEST; three + gates must all hold before a prefix is served without a session: declaration (zod-validated + shape), exclusive ownership (claimed at activation, first plugin wins, released on + deactivate), and operator consent (`plugin_public_path_grants`, migration `0046`). +- **`auth/publicPaths.ts` stays a frozen literal, and both static exemptions stay** — they + leave in C12, not here. Making that list dynamic was rejected in `implementation.md` §1: + `requireAuth` runs before routing and cannot know who will answer, so a grant there says + "this URL needs no session" without saying "and only plugin A may answer it". +- **The mount terminates.** An unhandled path under a granted prefix is answered 404 and does + NOT fall through into the authenticated stack. A counter-proof test disables termination + and shows the request escaping to `requireAuth`, so the guarantee is evidenced rather than + asserted. Two mutation checks confirm the suite fails when termination or the consent gate + is removed. +- **Fail-closed:** no grants, no store, no registry, no live plugin — each is a `next()` into + `requireAuth`. No failure mode of the new machinery yields less authentication than a build + without it. +- Unknown `permissions.*` keys now warn instead of vanishing silently (`implementation.md` + §2.5). Ratchet unchanged at 3296. + ### C8 — the abandonment checkpoint (G7 / P3b) **Shipped.** The plugin UI mechanism, end to end. Everything before this point was diff --git a/web-ui/app/_lib/errorHelp.ts b/web-ui/app/_lib/errorHelp.ts index 4309fb25..1dbc04eb 100644 --- a/web-ui/app/_lib/errorHelp.ts +++ b/web-ui/app/_lib/errorHelp.ts @@ -79,6 +79,12 @@ export const ERROR_HELP_CODES = [ // "try another file" and "report this". 'runtime.invalid_json_file_body', 'runtime.invalid_multiselect', + // #470 C4/H1 — the public-path consent endpoint. `public_path_not_declared` + // is the one an operator is most likely to hit: consent is capped by what the + // manifest asks for, so a path the plugin never declared is refused rather + // than quietly granted. + 'runtime.invalid_public_path', + 'runtime.invalid_public_paths', 'runtime.invalid_secrets_body', 'runtime.json_file_bad_extract_path', 'runtime.json_file_invalid_spec', @@ -95,6 +101,8 @@ export const ERROR_HELP_CODES = [ 'runtime.options_provider_failed', 'runtime.options_provider_timeout', 'runtime.options_unavailable', + 'runtime.public_path_not_declared', + 'runtime.public_paths_unavailable', 'runtime.setup_field_invalid', 'runtime.update_failed', 'runtime.value_not_offered', diff --git a/web-ui/app/_lib/storeTypes.ts b/web-ui/app/_lib/storeTypes.ts index 6c2035cc..300fc211 100644 --- a/web-ui/app/_lib/storeTypes.ts +++ b/web-ui/app/_lib/storeTypes.ts @@ -118,6 +118,12 @@ export interface PluginPermissionsSummary { secrets_runtime_write?: boolean; /** Spec 004 — plugin runs credential-acquisition flows on its own routes. */ flows?: boolean; + /** Epic #470 C4 / H1 — URL prefixes the plugin ASKS to serve without an + * operator session (`permissions.public_paths`). A declaration is a + * request, never a grant: the prefix stays behind the session gate until + * the operator consents and the middleware records the grant. Optional: + * absent on store payloads from a pre-#470-C4 core. */ + public_paths?: string[]; /** Spec 005 — plugin acquires standard authorization-code credentials via * the kernel OAuth broker (tokens stored + refreshed kernel-side). */ acquires_oauth?: boolean; diff --git a/web-ui/app/store/[id]/page.tsx b/web-ui/app/store/[id]/page.tsx index 11d22d02..85e413f1 100644 --- a/web-ui/app/store/[id]/page.tsx +++ b/web-ui/app/store/[id]/page.tsx @@ -600,7 +600,15 @@ async function PermissionsBlock({ }); } - if (active.length === 0 && flags.length === 0) { + // Epic #470 C4 / H1 — the plugin asks to serve these URL prefixes with NO + // operator session. Deliberately NOT a chip in the group list above: it is + // the single most consequential thing a plugin can request, and burying it + // among "memory reads" chips would be the wrong visual weight. Rendered as + // its own labelled block, and the copy says plainly that declaring is not + // granting. + const publicPaths = perms.public_paths ?? []; + + if (active.length === 0 && flags.length === 0 && publicPaths.length === 0) { return (

{t('none')} @@ -610,6 +618,30 @@ async function PermissionsBlock({ return (

+ {publicPaths.length > 0 ? ( +
+
+ + {t('publicPathsTitle')} +
+

+ {t('publicPathsBody')} +

+
    + {publicPaths.map((path) => ( + + {path} + + ))} +
+

+ {t('publicPathsNotGranted')} +

+
+ ) : null} {flags.length > 0 ? (
diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index a6b5ce8b..8a2286c0 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -1034,6 +1034,14 @@ "what": "Einer der ausgewählten Werte hat nicht die Form, die der Server akzeptiert.", "next": "Leere die Auswahl und wähle die Einträge erneut aus der Liste." }, + "invalid_public_path": { + "what": "Einer dieser Pfade ist kein gültiges öffentliches Präfix — zu breit, prozentkodiert oder für Core-Routen reserviert.", + "next": "Prüfe den Pfad gegen die Präfixe, die das Plugin deklariert, und gib ihn dann erneut frei." + }, + "invalid_public_paths": { + "what": "Die freigegebenen Pfade kamen nicht als Liste von Pfad-Strings an.", + "next": "Schicke die vollständige Menge der Pfade, die du freigibst, als Liste von Strings." + }, "invalid_secrets_body": { "what": "Die Zugangsdaten kamen in einer Form an, die der Server ablehnt.", "next": "Lade die Seite neu und trage die Zugangsdaten erneut ein." @@ -1095,6 +1103,14 @@ "what": "Dieser Core kann keine Setup-Optionen aus einem laufenden Plugin holen.", "next": "Trag den Wert von Hand ein." }, + "public_path_not_declared": { + "what": "Einen dieser Pfade fordert das Plugin in seinem Manifest gar nicht an, und eine Freigabe kann nie weiter reichen als die Anforderung.", + "next": "Gib nur die Pfade frei, die das Plugin deklariert, oder installiere eine Version, die den gewünschten Pfad deklariert." + }, + "public_paths_unavailable": { + "what": "Die Freigabe öffentlicher Pfade liegt in der Datenbank, und in diesem Deployment ist keine verdrahtet.", + "next": "Richte im Deployment eine Datenbank ein und setze die freigegebenen Pfade danach erneut." + }, "setup_field_invalid": { "what": "Einer der Werte passt nicht zum Format, das dieses Feld erwartet.", "next": "Korrigiere das markierte Feld und speichere erneut." @@ -3036,6 +3052,9 @@ "networkOutbound": "Netzwerk · Outbound", "flagRuntimeSecrets": "Schreibt eigene Credentials zur Laufzeit", "flagCredentialFlows": "Führt Credential-Flows auf eigenen Routes aus", + "publicPathsTitle": "Öffentliche (nicht authentifizierte) Pfade", + "publicPathsBody": "Dieses Plugin möchte die folgenden URL-Präfixe ohne Operator-Sitzung beantworten. Jede und jeder mit Zugriff auf diese Installation erreicht sie, und das Plugin allein verantwortet die Authentifizierung dieser Anfragen.", + "publicPathsNotGranted": "Deklarieren ist nicht Erteilen: Jedes Präfix bleibt hinter der Sitzungsprüfung, bis Sie zustimmen. Die Zustimmung wird automatisch entzogen, sobald das Plugin deaktiviert wird.", "none": "Keine Berechtigungen deklariert.", "runtimeCredentials": "Laufzeit-Credentials" }, diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index e15403a2..03afafae 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -1034,6 +1034,14 @@ "what": "One of the selected values is not in a shape the server accepts.", "next": "Clear the selection and pick the entries again from the list." }, + "invalid_public_path": { + "what": "One of those paths is not a valid public prefix — it is too broad, percent-encoded, or reserved for core routes.", + "next": "Check the path against the prefixes the plugin declares, then consent again." + }, + "invalid_public_paths": { + "what": "The consented paths did not arrive as a list of path strings.", + "next": "Send the complete set of paths you consent to, as a list of strings." + }, "invalid_secrets_body": { "what": "The credential change was sent in a shape the server rejected.", "next": "Reload the page and enter the credentials again." @@ -1095,6 +1103,14 @@ "what": "This core cannot fetch setup options from a running plugin.", "next": "Enter the value by hand." }, + "public_path_not_declared": { + "what": "One of those paths is not something this plugin asks for in its manifest, and consent can never reach further than the request.", + "next": "Consent only to the paths the plugin declares, or install a version that declares the one you want." + }, + "public_paths_unavailable": { + "what": "Public-path consent is stored in the database, and this deployment has none wired.", + "next": "Configure a database for the deployment, then set the consented paths again." + }, "setup_field_invalid": { "what": "One of the values does not match the format this field expects.", "next": "Correct the flagged field and save again." @@ -3036,6 +3052,9 @@ "networkOutbound": "Network · Outbound", "flagRuntimeSecrets": "Writes its own credentials at runtime", "flagCredentialFlows": "Runs credential flows on its own routes", + "publicPathsTitle": "Public (unauthenticated) paths", + "publicPathsBody": "This plugin asks to answer the URL prefixes below without an operator session. Anyone who can reach this deployment can reach them, and the plugin alone is responsible for authenticating those requests.", + "publicPathsNotGranted": "Declaring is not granting: each prefix stays behind the session gate until you consent to it, and consent is withdrawn automatically when the plugin is deactivated.", "none": "No permissions declared.", "runtimeCredentials": "Runtime credentials" },