From 4239ee12f0c1fbb7c0f38ecba2b5e4f147fd9335 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 14:18:39 +0100 Subject: [PATCH] =?UTF-8?q?Answer=20stability=20round=201:=200=20?= =?UTF-8?q?=F0=9F=94=B4,=203=20=F0=9F=9F=A1=20(0=20misread);=20gate=20test?= =?UTF-8?q?=20pins=20each=20source,=20builtin-route=20name,=20exact=20CSS?= =?UTF-8?q?=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 4 ++-- CHANGELOG.md | 6 +++++ README.md | 6 ++--- src/auth/routes.ts | 4 ++-- src/http/app.ts | 2 +- ...te-table.test.ts => builtin-route.test.ts} | 2 +- src/http/{route-table.ts => builtin-route.ts} | 0 src/plugin-host/plugin.test.ts | 22 +++++++++---------- src/plugin-host/plugin.ts | 7 +++--- src/test-tooling/stylesheet-floor.ts | 13 ++++++----- 10 files changed, 37 insertions(+), 29 deletions(-) rename src/http/{route-table.test.ts => builtin-route.test.ts} (95%) rename src/http/{route-table.ts => builtin-route.ts} (100%) diff --git a/AGENTS.md b/AGENTS.md index 4f7f9f2..f9ad06f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,8 +75,8 @@ Revisit only if the stated reason stops holding. ### Structure & contracts - **`src/` is grouped by concern**, not flat — `http/`, `auth/`, `i18n/`, `plugin-host/`, `ui/`, - and `test-tooling/` (modules only tests import), with `server.ts`/`config.ts`/`logger.ts` and the - topology-guard `*.test.ts` at the root; tests are co-located. Add a new module to the folder owning its concern. The core ships **no domain + and `test-tooling/` (modules only tests import), with `server.ts`/`config.ts`/`logger.ts` and + the topology-guard `*.test.ts` at the root; tests are co-located. Add a new module to the folder owning its concern. The core ships **no domain screens** — even the admin GUI is a drop-in plugin (`examples/plugins/admin/`). - **Plugins and config import the host only through a barrel** — `@plainpages/plugin-api` → `plugin-api/index.ts` → `src/plugin-host/plugin-api.ts`, `#menu-config` → `src/ui/menu-config.ts`, diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf59ff..c3427ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ at boot. Entries start at 0.3.0. ## Unreleased +### Discovery warns when a gate names a permission no plugin declares + +A route or nav node that gates on a permission no installed plugin declares now logs a boot warning +naming the plugin and the permission. The admin screens offer only declared permissions, so nobody can +be granted such a gate from them. Add the name to the plugin's `permissions` to silence the warning. + ### Discovery refuses a plugin with the id `error` `/error` is the page Kratos sends a failed self-service flow to, and a plugin named `error` would diff --git a/README.md b/README.md index c62b1e4..5e59558 100644 --- a/README.md +++ b/README.md @@ -730,7 +730,7 @@ The host detects collisions across all discovered plugins with `findConflicts` a | `route` | error | Two routes resolve to the same `method` + full path. Cross-plugin routes can't collide (the `/` prefix is unique), so this catches a plugin duplicating one of its own. | | `nav-id` | error | A nav node `id` is used more than once — the central override targets ids, so they must be unique. | | `home` / `dashboard` | error | More than one plugin declares `home` (or `dashboard`). Each landing page is a single slot, so only one may own it ([The landing pages](#the-landing-pages-home--dashboard)). | -| `permission` | warn | A permission name is declared by more than one plugin. Sharing is legitimate; pick a more specific [``](#naming-a-permission) if unintended. Also warned: a route or nav node gates on a permission no installed plugin declares, so the admin screens can't grant it. | +| `permission` | warn | A permission name is declared by more than one plugin. Sharing is legitimate; pick a more specific [``](#naming-a-permission) if unintended. Also warned: a route or nav node gates on a permission no installed plugin declares, so the admin screens can't grant it; declare it in that plugin's `permissions`. | Mount-path uniqueness needs no rule of its own — it follows from the id check. Discovery also rejects **per-manifest shape errors**: a non-array `nav`/`routes`/`permissions`, a non-function @@ -1830,7 +1830,7 @@ unnecessary here — the signature itself is already invalid. src/ The app — strict tsc, no build step. *.test.ts sit beside their module. server.ts Entry point; config.ts env loader; logger.ts structured log + trace span + tracedFetch *.test.ts Guards with no source counterpart (compose/kratos/keto/hydra/postgres/ci-gate/function-limits) - http/ Request pipeline: app.ts (createApp), route-table, context, body, cookie, + http/ Request pipeline: app.ts (createApp), builtin-route, context, body, cookie, static, safe-url (safeUrl + localPath), security-headers auth/ Identity + the session-JWT hot path: jwt · jwt-middleware · jwks · gen-jwks (CLI) · login · guards · csrf · denylist · flow-view · oauth-login · oauth-consent · @@ -1844,7 +1844,7 @@ src/ The app — strict tsc, no build step. *.test.ts sit beside ui/ chrome (`ctx.chrome`: menu, brand, user) · nav (composeNav) · menu-config (`#menu-config`) · icons (lucide sprite builder) · list-query · paginate test-tooling/ Test-only: app-harness (starts `createApp` over fakes for the `src/http` tests) · - stylesheet-floor (the browser-floor check; imports a devDependency) + stylesheet-floor (the browser-floor check) plugin-api/ The `@plainpages/plugin-api` package — the author barrel, linked into /node_modules views/ Core EJS in the one app shell: home, index, auth, oauth-consent, error, 403/404/500/503, diff --git a/src/auth/routes.ts b/src/auth/routes.ts index 9956048..fef901d 100644 --- a/src/auth/routes.ts +++ b/src/auth/routes.ts @@ -1,10 +1,10 @@ // The built-in auth/OAuth2 endpoints as named handlers on the host's internal route table -// (src/http/route-table.ts): the themed Kratos self-service pages and their code submits, the Hydra OAuth2 +// (src/http/builtin-route.ts): the themed Kratos self-service pages and their code submits, the Hydra OAuth2 // login/consent/logout challenges, login completion, the first-party logout, and Kratos' // flow-error sink. buildAuthRoutes registers only what the wired clients support — a missing // client means the path simply doesn't exist (404). `view` results render the core views. import { readFormBody } from "../http/body.ts"; -import type { BuiltinRoute, RequestCsrf } from "../http/route-table.ts"; +import type { BuiltinRoute, RequestCsrf } from "../http/builtin-route.ts"; import type { RequestContext } from "../http/context.ts"; import { CSRF_FIELD } from "./csrf.ts"; import { chosenLocale } from "../i18n/locale.ts"; diff --git a/src/http/app.ts b/src/http/app.ts index af0b767..4f9a629 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -3,7 +3,7 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import ejs from "ejs"; -import { type BuiltinRoute, matchBuiltinRoute, type PluginContextFactory, type RequestCsrf } from "./route-table.ts"; +import { type BuiltinRoute, matchBuiltinRoute, type PluginContextFactory, type RequestCsrf } from "./builtin-route.ts"; import { buildPluginChrome, type PageChrome } from "../ui/chrome.ts"; import { buildContext, type RequestContext, type User } from "./context.ts"; import { csrfCookie, ensureCsrfToken, verifyCsrfRequest } from "../auth/csrf.ts"; diff --git a/src/http/route-table.test.ts b/src/http/builtin-route.test.ts similarity index 95% rename from src/http/route-table.test.ts rename to src/http/builtin-route.test.ts index 0e66974..bbd4298 100644 --- a/src/http/route-table.test.ts +++ b/src/http/builtin-route.test.ts @@ -2,7 +2,7 @@ // router), method-distinct entries on the same path. import { test } from "node:test"; import assert from "node:assert/strict"; -import { type BuiltinRoute, matchBuiltinRoute } from "./route-table.ts"; +import { type BuiltinRoute, matchBuiltinRoute } from "./builtin-route.ts"; const entry = (method: "GET" | "POST", path: string): BuiltinRoute => ({ handler: () => null, method, path }); diff --git a/src/http/route-table.ts b/src/http/builtin-route.ts similarity index 100% rename from src/http/route-table.ts rename to src/http/builtin-route.ts diff --git a/src/plugin-host/plugin.test.ts b/src/plugin-host/plugin.test.ts index b18a29b..f991ffe 100644 --- a/src/plugin-host/plugin.test.ts +++ b/src/plugin-host/plugin.test.ts @@ -131,19 +131,19 @@ test("findConflicts: duplicate nav id is an error, a shared permission name only const noopHandler = () => {}; -test("findConflicts: a route or nav node gating on a permission no plugin declares warns once per plugin and name", () => { +test("findConflicts: a route or nav node gating on a permission no plugin declares warns once per plugin and name, sorted", () => { const gated = findConflicts([ - p({ id: "a", nav: [{ children: [{ id: "a:x", label: "X", permission: "ghost:read" }], id: "a:root", label: "A", session: true }], routes: [ - { handler: noopHandler, method: "GET", path: "/x", permission: "ghost:read" }, - { handler: noopHandler, method: "GET", path: "/y", permission: "other:write" }, - ] }), - p({ id: "b", permissions: [{ name: "other:write" }] }), + p({ id: "a", nav: [{ children: [{ id: "a:x", label: "X", permission: "navonly:read" }], id: "a:root", label: "A", session: true }], routes: [ + { handler: noopHandler, method: "GET", path: "/x", permission: "routeonly:read" }, + { handler: noopHandler, method: "GET", path: "/y", permission: "both:read" }, + { handler: noopHandler, method: "POST", path: "/y", permission: "both:read" }, + { handler: noopHandler, method: "GET", path: "/z", permission: "declared:write" }, + ], permissions: [] }), + p({ id: "b", nav: [{ id: "b:y", label: "Y", permission: "both:read" }], permissions: [{ name: "declared:write" }] }), ]); - const messages = gated.filter((c) => c.kind === "permission").map((c) => c.message); - assert.equal(messages.length, 1); - assert.match(messages[0] ?? "", /"ghost:read"/); - assert.deepEqual(gated[0]?.plugins, ["a"]); - assert.equal(gated[0]?.level, "warn"); + assert.ok(gated.every((c) => c.kind === "permission" && c.level === "warn")); + assert.deepEqual(gated.map((c) => c.plugins), [["a"], ["a"], ["a"], ["b"]]); + assert.deepEqual(gated.map((c) => /"([^"]+:[a-z]+)"/.exec(c.message)?.[1]), ["both:read", "navonly:read", "routeonly:read", "both:read"]); }); test("findConflicts: a gate on a permission another plugin declares, or on none, stays quiet", () => { diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index b702855..20767ca 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -231,8 +231,9 @@ export function findConflicts(plugins: Plugin[]): PluginConflict[] { return out; } -// Declaring is optional, but the admin screens offer only declared names: a gate on one nobody -// declares answers 403 until someone writes the Keto tuple by hand. +// Declaring is optional, but the admin screens offer only declared names: a route gated on one nobody +// declares answers 403 (a nav node is hidden) until the Keto tuple is written by hand or +// ADMIN_PERMISSIONS seeds it. function undeclaredGates(plugins: Plugin[]): PluginConflict[] { const declared = new Set(declaredPermissions(plugins).map((decl) => decl.name)); const out: PluginConflict[] = []; @@ -241,7 +242,7 @@ function undeclaredGates(plugins: Plugin[]): PluginConflict[] { for (const route of plugin.routes ?? []) if (route.permission != null) gated.add(route.permission); collectNavPermissions(plugin.nav, (name) => gated.add(name)); for (const name of [...gated].sort()) { - if (!declared.has(name)) out.push({ kind: "permission", level: "warn", message: `plugin "${plugin.id}" gates on "${name}", which no installed plugin declares, so the admin screens can't grant it; add it to a plugin's \`permissions\``, plugins: [plugin.id] }); + if (!declared.has(name)) out.push({ kind: "permission", level: "warn", message: `plugin "${plugin.id}" gates on "${name}", which no installed plugin declares, so the admin screens can't grant it; declare it in that plugin's \`permissions\``, plugins: [plugin.id] }); } } return out; diff --git a/src/test-tooling/stylesheet-floor.ts b/src/test-tooling/stylesheet-floor.ts index e2edc81..1ed3a79 100644 --- a/src/test-tooling/stylesheet-floor.ts +++ b/src/test-tooling/stylesheet-floor.ts @@ -7,9 +7,9 @@ // as false; media range and `or` syntax fail. A partial implementation counts as support, since MDN // marks edge gaps that way, such as `outline` ignoring `border-radius` before Chrome 94. A // vendor-prefixed name is one engine's by design and is skipped, as are `font-family` names: an engine -// skips a family it lacks. A custom property's value is checked too, for functions and units though -// not keywords: a value a floor engine can't parse makes every `var()` reading it invalid, so each -// declaration using it is dropped, wherever the property is declared. +// skips a family it lacks. A custom property's value is checked for functions and units, though not +// keywords: one a floor engine can't parse is invalid at computed-value time, so every declaration +// reading it through `var()` computes as `unset`, and a fallback declared before it doesn't help. // Not checked: the body of an at-rule other than @media and @supports. import bcd from "@mdn/browser-compat-data" with { type: "json" }; import type { CompatStatement, Identifier, SimpleSupportStatement, SupportBlock } from "@mdn/browser-compat-data"; @@ -116,8 +116,9 @@ function mediaQuery(sheet: Sheet, guards: string[], condition: string, offset: n } } -// The parts of an @supports condition that must hold for its block to apply, as text a guard can -// search: the condition without its `not (…)` groups and without any group joined by `or`, at every depth. +// The parts of an @supports condition that must hold for its block to apply, as text a +// `Feature.guard` can search: the condition without its `not (…)` groups and without any group +// joined by `or`, at every depth. function positive(condition: string): string { const { groups, tail } = parenGroups(condition); if (OR.test(groups.map((g) => g.head).join("") + tail)) return ""; @@ -125,7 +126,7 @@ function positive(condition: string): string { } interface Group { - // The text between the previous group and this one's "(". + // The text before this group's "(", back to the previous group or the start of the condition. head: string; inner: string; }