diff --git a/AGENTS.md b/AGENTS.md index f5287df..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/`, - 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 3738399..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. | +| `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 @@ -1064,7 +1064,7 @@ stylesheet using a feature Chromium 87, Safari 15 or Firefox 115 lacks, unless a around it names that feature. It checks every stylesheet in `public/`, `examples/` and `plugins/`, so mount your plugin in `plugins//` and run it. A few constructs fail even when guarded, such as `@layer`, nested rules and media range syntax, while a media feature passes unguarded, since a -browser without it skips the query; `src/ui/stylesheet-floor.ts` lists the rules. +browser without it skips the query; `src/test-tooling/stylesheet-floor.ts` lists the rules. **A field whose options depend on another field is asked for in steps, one GET form each**, so every choice is in the URL, and nothing typed is lost to one because nothing is typed until they are made. @@ -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), builtin-routes, 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 · @@ -1843,6 +1843,8 @@ src/ The app — strict tsc, no build step. *.test.ts sit beside storage-provisioning (the DDL; bootstrap-only, holds the driver) 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) 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/docs/comprehension-history.md b/docs/comprehension-history.md index 49719f0..0228ea3 100644 --- a/docs/comprehension-history.md +++ b/docs/comprehension-history.md @@ -290,3 +290,28 @@ Inherited architect, would restructure first: - Locality, Shape, Overall: `src/http/app.ts:185` `handleRequest`: stage order lives only in line order, in a grandfathered complexity-43 function - Shape, Navigation: `src/auth/routes.ts:275` `buildAuthRoutes`: 19 flat modules in `src/auth/`, and the next flow lands here - Shape, Navigation: `src/ui/stylesheet-floor.ts:15` `FLOOR`: the 3am rule's home is a test-only module under `ui/` + +## 2026-11-08T13:22:47Z, PR #214 at 84368ea, against c2389f8 + +Ruling: better + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Mid A | better | same | better | better | better | +| Inherited architect | better | same | better | better | better | + +Mid A, decided by: + +- Navigation, Shape, Overall: `src/test-tooling/stylesheet-floor.ts:1` `floorViolations`: test-only modules no longer sit beside runtime code in `ui/` and `http/`, and `AGENTS.md:77` names the folder +- Self-sufficiency: `src/auth/login.ts:40` `readPermissions`: the comment now names `#granted`, which the code queries +- Shape: `src/test-tooling/stylesheet-floor.ts:122` `positive`: grouping and the per-group decision are separate named units +- Self-sufficiency: `src/test-tooling/stylesheet-floor.ts:10` file header: says why a custom property's value is checked for functions and units +- Self-sufficiency: `src/plugin-host/plugin.ts:234` `undeclaredGates`: its comment says why an undeclared gate matters + +Inherited architect, decided by: + +- Self-sufficiency, Shape, Overall: `src/auth/login.ts:40` `readPermissions`: the comment now names the relation the code queries +- Self-sufficiency: `src/auth/bootstrap.ts:31` `permissionTuple`: the contradicting comment is gone +- Navigation: `src/test-tooling/stylesheet-floor.ts:1` `floorViolations`: the folder is named in AGENTS.md and the README file map +- Shape: `src/test-tooling/stylesheet-floor.ts:122` `positive`: one loop split into `parenGroups`, `Group` and `keepGroup` +- Self-sufficiency: `src/auth/routes.ts:117` `codeFlowSubmit`: a comment says which `flowFailure` branch can fire there diff --git a/docs/reading-cost.md b/docs/reading-cost.md index f3e42f8..58cfecf 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -320,3 +320,20 @@ Read most (opens and searches): - `src/config.ts` 1 - `src/plugin-host/storage-provisioning.ts` 1 - `src/plugin-host/storage.ts` 1 + +## Reading cost, chunk 80e22d10c810, finished 2026-11-08T13:06:23Z + +130001 new input tokens before the first commit. + +Read most (opens and searches): + +- `README.md` 6 +- `todo.md` 4 +- `examples/plugins/admin/admin-groups.ts` 2 +- `src/auth/bootstrap.ts` 2 +- `src/auth/login.ts` 2 +- `src/http/app-admin.test.ts` 2 +- `AGENTS.md` 1 +- `CHANGELOG.md` 1 +- `examples/plugins/admin/i18n/sv-SE.ts` 1 +- `src/auth/routes.ts` 1 diff --git a/src/auth/routes.ts b/src/auth/routes.ts index e27a02c..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/builtin-routes.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/builtin-routes.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-admin.test.ts b/src/http/app-admin.test.ts index f9c8cde..3755628 100644 --- a/src/http/app-admin.test.ts +++ b/src/http/app-admin.test.ts @@ -16,7 +16,7 @@ import type { Plugin } from "../plugin-host/plugin.ts"; import adminManifest from "../../examples/plugins/admin/plugin.ts"; import { createI18n } from "../i18n/runtime.ts"; import { loadI18n } from "../i18n/load.ts"; -import { ecJwk, fakeKeto, matchesTuple, mintJwt, stubAdmin, stubHydra } from "./app-harness.ts"; +import { ecJwk, fakeKeto, matchesTuple, mintJwt, stubAdmin, stubHydra } from "../test-tooling/app-harness.ts"; // The HTTP-level admin tests mount the example plugin via createApp — stub Ory clients on // ctx.system, views from examples/plugins — exactly as an operator would after copying it in. diff --git a/src/http/app-auth.test.ts b/src/http/app-auth.test.ts index 1d68273..23bdcaa 100644 --- a/src/http/app-auth.test.ts +++ b/src/http/app-auth.test.ts @@ -10,7 +10,7 @@ import { KratosError, type Flow, type FlowSubmission, type FlowType, type Kratos import { SESSION_COOKIE } from "../auth/login.ts"; import { createI18n } from "../i18n/runtime.ts"; import { loadI18n } from "../i18n/load.ts"; -import { ecJwk, fakeKeto, mintJwt, mockKratos, OAUTH_SUBJECT, stubAdmin, stubHydra, withWhoami } from "./app-harness.ts"; +import { ecJwk, fakeKeto, mintJwt, mockKratos, OAUTH_SUBJECT, stubAdmin, stubHydra, withWhoami } from "../test-tooling/app-harness.ts"; // A re-rendered login flow: csrf hidden, themed fields, a submit, and a failed-attempt message. const node = (attrs: Record, label?: string): UiNode => ({ attributes: attrs, group: "default", messages: [], meta: label ? { label: { id: 1, text: label, type: "info" } } : {}, type: "input" }); diff --git a/src/http/app-i18n.test.ts b/src/http/app-i18n.test.ts index f78d115..6ed8e0b 100644 --- a/src/http/app-i18n.test.ts +++ b/src/http/app-i18n.test.ts @@ -11,7 +11,7 @@ import type { Plugin } from "../plugin-host/plugin.ts"; import { createI18n } from "../i18n/runtime.ts"; import type { MenuConfig } from "../ui/menu-config.ts"; import { loadI18n } from "../i18n/load.ts"; -import { ecJwk, mintJwt } from "./app-harness.ts"; +import { ecJwk, mintJwt } from "../test-tooling/app-harness.ts"; // ---- language (i18n) ---- diff --git a/src/http/app-plugins.test.ts b/src/http/app-plugins.test.ts index 831e722..8474599 100644 --- a/src/http/app-plugins.test.ts +++ b/src/http/app-plugins.test.ts @@ -14,7 +14,7 @@ import type { Identity } from "../auth/kratos-admin.ts"; import { KratosError, type Session } from "../auth/kratos-public.ts"; import { SESSION_COOKIE } from "../auth/login.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; -import { ecJwk, fakeKeto, mintJwt, startApp, stubAdmin, withWhoami } from "./app-harness.ts"; +import { ecJwk, fakeKeto, mintJwt, startApp, stubAdmin, withWhoami } from "../test-tooling/app-harness.ts"; // A test plugin exercising each RouteResult shape, a path param, and the permission gate. const demoPlugin: Plugin = { diff --git a/src/http/app.test.ts b/src/http/app.test.ts index b881b14..e103c7f 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -14,7 +14,7 @@ import { CSRF_COOKIE } from "../auth/csrf.ts"; import { staticJwks } from "../auth/jwks.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; import { ENGLISH_LOCALS } from "../i18n/view-locals.ts"; -import { ecJwk, session, startApp } from "./app-harness.ts"; +import { ecJwk, session, startApp } from "../test-tooling/app-harness.ts"; const viewsDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "views"); diff --git a/src/http/app.ts b/src/http/app.ts index a774d72..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 "./builtin-routes.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/builtin-routes.test.ts b/src/http/builtin-route.test.ts similarity index 99% rename from src/http/builtin-routes.test.ts rename to src/http/builtin-route.test.ts index c572048..bbd4298 100644 --- a/src/http/builtin-routes.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 "./builtin-routes.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/builtin-routes.ts b/src/http/builtin-route.ts similarity index 100% rename from src/http/builtin-routes.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 4f0d976..f991ffe 100644 --- a/src/plugin-host/plugin.test.ts +++ b/src/plugin-host/plugin.test.ts @@ -129,6 +129,30 @@ test("findConflicts: duplicate nav id is an error, a shared permission name only assert.ok(permissionDup.some((c) => c.kind === "permission" && c.level === "warn")); }); +const noopHandler = () => {}; + +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: "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" }] }), + ]); + 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", () => { + assert.deepEqual(findConflicts([ + p({ id: "a", permissions: [{ name: "a:read" }], routes: [{ handler: noopHandler, method: "GET", path: "/x", permission: "a:read" }] }), + p({ id: "b", routes: [{ handler: noopHandler, method: "GET", path: "/y", permission: "a:read" }, { handler: noopHandler, method: "GET", path: "/z", session: true }] }), + ]), []); +}); + test("findConflicts: each single slot (`home`/`dashboard`) may have one owner — two is a loud error", () => { const handler = () => ({ html: "x" }); const homeDup = findConflicts([p({ id: "a", home: handler }), p({ id: "b", home: handler })]); diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index cce30a1..20767ca 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -218,6 +218,8 @@ export function findConflicts(plugins: Plugin[]): PluginConflict[] { if (owners.length > 1) out.push({ kind: "permission", level: "warn", message: `permission "${name}" declared by ${uniq(owners).length} plugins; pick a more specific "" unless shared on purpose`, plugins: uniq(owners) }); }); + out.push(...undeclaredGates(plugins)); + // Both the id's dashes and the key's camel humps become underscores, so plugin "a-b" key "c" and // plugin "a" key "bC" name one variable — one plugin would silently read the other's value. collect(plugins, (plugin, push) => { @@ -229,6 +231,30 @@ export function findConflicts(plugins: Plugin[]): PluginConflict[] { return out; } +// 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[] = []; + for (const plugin of plugins) { + const gated = new Set(); + 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; declare it in that plugin's \`permissions\``, plugins: [plugin.id] }); + } + } + return out; +} + +function collectNavPermissions(nodes: NavNode[] | undefined, push: (name: string) => void): void { + for (const node of nodes ?? []) { + if (node.permission != null) push(node.permission); + collectNavPermissions(node.children, push); + } +} + // Map each emitted key → the plugin ids that emitted it (repeats kept, so within-plugin dups count). function collect(plugins: Plugin[], emit: (plugin: Plugin, push: (key: string) => void) => void): Map { const owners = new Map(); diff --git a/src/http/app-harness.ts b/src/test-tooling/app-harness.ts similarity index 99% rename from src/http/app-harness.ts rename to src/test-tooling/app-harness.ts index 6f8cb0c..4adaa79 100644 --- a/src/http/app-harness.ts +++ b/src/test-tooling/app-harness.ts @@ -1,12 +1,12 @@ import { generateKeyPairSync, sign, type JsonWebKey } from "node:crypto"; import type { AddressInfo } from "node:net"; import type { TestContext } from "node:test"; -import { createApp } from "./app.ts"; import type { HydraAdmin } from "../auth/hydra-admin.ts"; import type { KetoClient, RelationTuple, SubjectSet } from "../auth/keto-client.ts"; import type { KratosAdmin } from "../auth/kratos-admin.ts"; import type { FlowType, KratosPublic } from "../auth/kratos-public.ts"; import { SESSION_COOKIE } from "../auth/login.ts"; +import { createApp } from "../http/app.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; // A session JWT signed with a throwaway test key; `staticJwks([ecJwk])` is the matching verify side. diff --git a/src/ui/stylesheet-floor.test.ts b/src/test-tooling/stylesheet-floor.test.ts similarity index 100% rename from src/ui/stylesheet-floor.test.ts rename to src/test-tooling/stylesheet-floor.test.ts diff --git a/src/ui/stylesheet-floor.ts b/src/test-tooling/stylesheet-floor.ts similarity index 89% rename from src/ui/stylesheet-floor.ts rename to src/test-tooling/stylesheet-floor.ts index 59f2660..1ed3a79 100644 --- a/src/ui/stylesheet-floor.ts +++ b/src/test-tooling/stylesheet-floor.ts @@ -7,7 +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. +// 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"; @@ -114,23 +116,37 @@ function mediaQuery(sheet: Sheet, guards: string[], condition: string, offset: n } } -// The parts of an @supports condition that must hold for its block to apply: neither a `not (…)` -// group nor an operand of an `or`, at any 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 { - let top = ""; - let kept = ""; + const { groups, tail } = parenGroups(condition); + if (OR.test(groups.map((g) => g.head).join("") + tail)) return ""; + return groups.map(keepGroup).join("") + tail; +} + +interface Group { + // The text before this group's "(", back to the previous group or the start of the condition. + head: string; + inner: string; +} + +// The top-level parenthesised groups of `condition`, and the text after the last. +function parenGroups(condition: string): { groups: Group[]; tail: string } { + const groups: Group[] = []; let from = 0; for (let open = condition.indexOf("("); open >= 0; open = condition.indexOf("(", from)) { const close = closing(condition, open); - const head = condition.slice(from, open); - const inner = condition.slice(open + 1, close); - top += head; - if (NEGATED.test(head)) kept += head.replace(NEGATED, ""); - else if (/[\w-]$/.test(head)) kept += `${head}(${inner})`; // a function such as selector() - else kept += `${head}(${positive(inner)})`; + groups.push({ head: condition.slice(from, open), inner: condition.slice(open + 1, close) }); from = close + 1; } - return OR.test(top + condition.slice(from)) ? "" : kept + condition.slice(from); + return { groups, tail: condition.slice(from) }; +} + +function keepGroup({ head, inner }: Group): string { + if (NEGATED.test(head)) return head.replace(NEGATED, ""); + if (/[\w-]$/.test(head)) return `${head}(${inner})`; // a function such as selector(): its argument names the feature + return `${head}(${positive(inner)})`; } const OR = /(?