From fefc7cee304238f3fdad9d9b461dab923a161791 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Tue, 3 Nov 2026 21:43:57 +0100 Subject: [PATCH] Plan 0.5.0 from the ten highest-scoring items; reserve the plugin id error, translate the duplicate-group refusal, and correct four comments and README passages --- CHANGELOG.md | 10 ++++ README.md | 10 ++-- examples/plugins/admin/admin-groups.ts | 2 +- examples/plugins/admin/i18n/en-US.ts | 1 + examples/plugins/admin/i18n/sv-SE.ts | 1 + src/auth/bootstrap.ts | 2 +- src/auth/login.ts | 4 +- src/auth/routes.ts | 3 +- src/http/app-admin.test.ts | 4 +- src/plugin-host/plugin.test.ts | 5 +- src/plugin-host/plugin.ts | 2 +- todo.md | 73 ++++++-------------------- 12 files changed, 47 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2281811..2ab04f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ at boot. Entries start at 0.3.0. ## Unreleased +### Discovery refuses a plugin with the id `error` + +`/error` is the page Kratos sends a failed sign-in flow to, and a plugin named `error` would shadow +it. Discovery now refuses that id at boot, like the other reserved ids. Rename such a plugin's folder +before upgrading. + +### The admin's "group already exists" refusal is translated + +Creating a group whose name is taken shows the refusal in the page's language, not always in English. + ### A malformed URL variable's boot error no longer prints the value When a URL variable such as `PLUGIN_DB_URL`, `PLUGIN_DB_ADMIN_URL` or `OTLP_ENDPOINT` can't be parsed, diff --git a/README.md b/README.md index f70825e..2a1cffc 100644 --- a/README.md +++ b/README.md @@ -1271,9 +1271,9 @@ override sets `APP_URL=http://localhost:3000`, and `127.0.0.1` is canonicalised A clean clone needs **none** of the above. What can't be auto-generated is **production-only**: 1. **Production secrets** — every value below ships as a committed dev throwaway and **must** be - replaced before a deploy faces the internet. Only the first is enforced: - `REQUIRE_SECURE_SECRETS=true` refuses to boot on a missing or throwaway `CSRF_SECRET` and - **nothing else** — the rest fail silently, so treat this as a checklist. + replaced before a deploy faces the internet. Of these, `REQUIRE_SECURE_SECRETS=true` checks only + `CSRF_SECRET` (its [Configuration](#configuration) row lists the other secrets it checks); the + rest fail silently, so treat this as a checklist. | Secret | Where | Protects | | --- | --- | --- | @@ -1439,8 +1439,8 @@ unauthenticated Ory ports. Install only what you trust, and let the plugin's own Hardening a real deploy is `REQUIRE_SECURE_SECRETS=true`, `SECURE_COOKIES=true`, and replacing **every** committed dev secret ([what you must supply](#what-you-must-supply-the-only-manual-prep)). -`REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`; nothing fails loud if you ship Ory's, Postgres' -or the demo admin's throwaways. +`REQUIRE_SECURE_SECRETS` guards only the secrets its [Configuration](#configuration) row lists; +nothing fails loud if you ship Ory's, Postgres' or the demo admin's throwaways. ## Email diff --git a/examples/plugins/admin/admin-groups.ts b/examples/plugins/admin/admin-groups.ts index cb6ef8d..69e6acc 100644 --- a/examples/plugins/admin/admin-groups.ts +++ b/examples/plugins/admin/admin-groups.ts @@ -364,7 +364,7 @@ export const groupsCreate = withGroups(async (deps) => { const reject = async (error: string): Promise => ({ ...(await groupFormResult(deps, { error, values: { member, name } })), status: 400 }); if (!isValidGroupName(name)) return reject(ctx.t("admin.groups.validation.name")); if (!tuple) return reject(ctx.t("admin.groups.validation.member")); - if (await groupExists(keto, name)) return reject("A group with that name already exists."); + if (await groupExists(keto, name)) return reject(ctx.t("admin.groups.error.duplicate")); await keto.writeTuple(tuple); ctx.log.info("admin: group created", { actor: user.id, group: name }); return { redirect: detailHref(name) }; diff --git a/examples/plugins/admin/i18n/en-US.ts b/examples/plugins/admin/i18n/en-US.ts index b3bad1f..d7669eb 100644 --- a/examples/plugins/admin/i18n/en-US.ts +++ b/examples/plugins/admin/i18n/en-US.ts @@ -68,6 +68,7 @@ const messages = { "admin.groups.create": "Create group", "admin.groups.delete": "Delete group", "admin.groups.deleteMessage": "Delete group {{name}}? This can't be undone.", + "admin.groups.error.duplicate": "A group with that name already exists.", "admin.groups.error.selfLockout": "This change would lock you out of {{permissions}}, which you hold through this group. Ask another administrator to make it.", "admin.groups.field.name": "Group name", "admin.groups.field.nameHint": "Lowercase letters, digits, dashes and underscores.", diff --git a/examples/plugins/admin/i18n/sv-SE.ts b/examples/plugins/admin/i18n/sv-SE.ts index 1dec870..6df8e10 100644 --- a/examples/plugins/admin/i18n/sv-SE.ts +++ b/examples/plugins/admin/i18n/sv-SE.ts @@ -68,6 +68,7 @@ const messages: AdminMessages = { "admin.groups.create": "Skapa grupp", "admin.groups.delete": "Radera grupp", "admin.groups.deleteMessage": "Ta bort gruppen {{name}}? Det går inte att ångra.", + "admin.groups.error.duplicate": "Det finns redan en grupp med det namnet.", "admin.groups.error.selfLockout": "Ändringen skulle låsa ute dig från {{permissions}}, som du har genom den här gruppen. Be en annan administratör att göra den.", "admin.groups.field.name": "Gruppnamn", "admin.groups.field.nameHint": "Små bokstäver, siffror, bindestreck och understreck.", diff --git a/src/auth/bootstrap.ts b/src/auth/bootstrap.ts index 137ef59..ab08cae 100644 --- a/src/auth/bootstrap.ts +++ b/src/auth/bootstrap.ts @@ -28,7 +28,7 @@ export function identityPayload(email: string, password: string) { }; } -// Coarse-permission grant: `Permission:#members@user:`. Subject ids are `user:` +// Coarse-permission grant: `Permission:#granted@user:`. Subject ids are `user:` // (namespaces.keto.ts) — the source of truth the login flow projects into the JWT permissions. export function permissionTuple(userId: string, permission: string) { return { namespace: "Permission", object: permission, relation: "granted", subject_id: `user:${userId}` }; diff --git a/src/auth/login.ts b/src/auth/login.ts index 621c24d..65948ec 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -37,8 +37,8 @@ export interface CompletedLogin { permissions: string[]; } -// The coarse permissions a user holds — directly (`Permission:#members@user:`) or transitively via a -// group that is a member of the permission. Enumerates the defined permissions (the distinct objects in the Permission +// The coarse permissions a user holds — directly (`Permission:#granted@user:`) or transitively via a +// group granted the permission. Enumerates the defined permissions (the distinct objects in the Permission // namespace) and asks Keto to resolve each membership, so a permission granted to a group reaches the JWT — // matching the OPL model and the admin "Effective access" view. At login/refresh only, never per // request; permission count is small, so the per-permission checks are cheap and run in parallel. diff --git a/src/auth/routes.ts b/src/auth/routes.ts index 5938edc..a942537 100644 --- a/src/auth/routes.ts +++ b/src/auth/routes.ts @@ -117,7 +117,8 @@ function flowPage(kratos: KratosPublic, flowType: FlowType, secureCookies: boole // A code flow's POST (flow-view.ts → CODE_FLOWS). Trims the pasted one-time code, since Kratos // refuses one with whitespace around it, then passes the form on to Kratos and sends the browser // where Kratos redirects. Kratos' csrf_token field guards the POST, so the host's own CSRF token -// isn't checked here. +// isn't checked here. `submitFlow` throws only on a 5xx or an unreachable Kratos, so of +// `flowFailure`'s branches only the 503 fires here. function codeFlowSubmit(kratos: KratosPublic, flowType: FlowType, secureCookies: boolean): BuiltinRoute["handler"] { return async (ctx: RequestContext): Promise => { const pathname = ctx.url.pathname; diff --git a/src/http/app-admin.test.ts b/src/http/app-admin.test.ts index e54f843..f9c8cde 100644 --- a/src/http/app-admin.test.ts +++ b/src/http/app-admin.test.ts @@ -191,7 +191,9 @@ test("admin Groups screen: gate, list, create, detail/membership, delete (CSRF-g // An invalid name, a duplicate name, or a missing CSRF token are all refused, nothing written. const before = tuples.length; assert.equal((await post("/admin/groups", `_csrf=${token}&name=Bad Name&member=user:${grace}`)).status, 400); - assert.equal((await post("/admin/groups", `_csrf=${token}&name=eng&member=user:${grace}`)).status, 400); // already exists + const duplicate = await post("/admin/groups?locale=sv-SE", `_csrf=${token}&name=eng&member=user:${grace}`); + assert.equal(duplicate.status, 400); + assert.match(await duplicate.text(), /Det finns redan en grupp med det namnet\./); assert.equal((await post("/admin/groups", `name=x&member=user:${grace}`)).status, 403); assert.equal(tuples.length, before); diff --git a/src/plugin-host/plugin.test.ts b/src/plugin-host/plugin.test.ts index d4b123e..34e6e86 100644 --- a/src/plugin-host/plugin.test.ts +++ b/src/plugin-host/plugin.test.ts @@ -140,8 +140,8 @@ test("findConflicts: each single slot (`home`/`dashboard`) may have one owner }); // Drift guard: RESERVED_PLUGIN_IDS is a hand-maintained mirror of the host's own top-level mounts — -// a folder claiming one would silently shadow a built-in route. Derive the segments from the real -// route constants so adding a new auth flow or provider route without reserving its id fails here. +// a folder claiming one would silently shadow a built-in route. Only the auth flows derive from +// AUTH_FLOWS; every other mount is listed by hand, so a new top-level route needs a line here too. test("RESERVED_PLUGIN_IDS covers every built-in top-level mount; `home` (the / field) and `admin` (a plugin) are NOT reserved", () => { const seg = (path: string): string => path.split("/")[1] ?? ""; // first segment of "/x/y" const builtins = new Set([ @@ -151,6 +151,7 @@ test("RESERVED_PLUGIN_IDS covers every built-in top-level mount; `home` (the / f "oauth2", // /oauth2/login · /consent · /logout (Hydra provider) "dashboard", // the gated app home "public", // static assets + "error", // /error (Kratos' error sink) ]); for (const id of builtins) assert.ok(RESERVED_PLUGIN_IDS.has(id), `built-in mount "${id}" must be a reserved plugin id`); // "/" is owned by the `home` manifest field (not a / route), so it cannot be shadowed and is diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index 4c4716f..cce30a1 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -124,7 +124,7 @@ export function isValidPluginId(id: string): boolean { // one — discovery refuses it. "/" is owned by the `home` field, not a route, so it needs no // reservation; `admin` is deliberately absent, the admin screens being a drop-in plugin. export const RESERVED_PLUGIN_IDS: ReadonlySet = new Set([ - "auth", "dashboard", "login", "logout", "oauth2", "public", "recovery", "registration", "settings", "verification", + "auth", "dashboard", "error", "login", "logout", "oauth2", "public", "recovery", "registration", "settings", "verification", ]); export interface Semver { diff --git a/todo.md b/todo.md index ec3053a..90885c2 100644 --- a/todo.md +++ b/todo.md @@ -12,55 +12,23 @@ | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 56 | 0.4.22 | | | **Correct the README's two statements that `REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`.** | 1 | 2 | 1 | 2 | 1 | code health by 2 | 14.4 | -| 19 | 0.5.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | -| 16 | 0.5.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | -| 48 | 0.5.0 | | | **Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes.** | 1 | 1 | 1 | 1 | 1 | code health by 4 | 28.7 | | 51 | 0.5.0 | | | **Move the test-only tooling out of the runtime folders `src/ui/` and `src/http/`, and keep the README file map true.** | 2 | 1 | 1 | 2 | 3 | code health by 4 | 18.0 | | 52 | 0.5.0 | 51 | | **Make `positive` in `stylesheet-floor.ts` readable without tracing it by hand, and state why a custom property's value is checked.** | 2 | 1 | 1 | 2 | 3 | code health by 5 | 22.5 | -| 53 | 0.5.0 | | | **Say at `codeFlowSubmit` which branches of `flowFailure` can fire there.** | 1 | 1 | 1 | 1 | 1 | code health by 3 | 21.6 | -| 49 | 0.5.0 | | | **Render the admin's "group already exists" refusal from the catalog, in every locale.** | 1 | 2 | 1 | 2 | 1 | code health by 2 | 14.4 | | 6 | 0.5.0 | | | **Warn at discovery when a route or nav node gates on a permission no plugin declares.** | 2 | 1 | 1 | 2 | 2 | 7 by 3, 9 by 3 | 8.0 | | 23 | 0.5.0 | | | **Give list pages one view-model helper from query to pagination, instead of ~40 hand-written lines each.** | 3 | 1 | 2 | 4 | 5 | 1 by 4, 9 by 2, code health by 3 | 7.9 | | 43 | 0.5.0 | | | **Let a plugin version the scripts and other `/public/` files its own markup links, so they are cached as immutable too.** | 2 | 2 | 1 | 3 | 3 | 8 by 3, 7 by 2 | 5.0 | -| 47 | 0.5.0 | | | **Refuse the plugin id `error`, and make the drift guard cover the `/error` mount.** | 1 | 1 | 2 | 2 | 1 | 7 by 3 | 4.9 | -| 28 | 0.5.0 | | | **Let a plugin add entries to the profile menu through the plugin API.** | 2 | 2 | 2 | 4 | 4 | 7 by 4, 1 by 2 | 4.2 | -| 50 | 0.5.0 | | | **Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once.** | 3 | 1 | 3 | 5 | 7 | code health by 7 | 3.7 | -| 54 | 0.5.0 | | | **List the `setting` conflict kind in the README's Conflict rules table.** | 1 | 1 | 1 | 1 | 1 | 9 by 2 | 3.1 | -| 2 | 0.5.0 | | | **Rename the admin example plugin to a name that says what it administers.** | 3 | 1 | 2 | 4 | 6 | 9 by 3, code health by 2 | 2.6 | -| 5 | 0.5.0 | | | **List grants that no installed plugin declares, with a remove action.** | 2 | 2 | 2 | 4 | 4 | 2 by 3 | 2.1 | -| 41 | 0.5.0 | | | **Apply a permission save as one Keto call, so it cannot stop part-way.** | 2 | 2 | 2 | 4 | 3 | 2 by 2 | 2.1 | -| 14 | 0.5.0 | | | **Drop a double-clicked duplicate submit, with no client-side JavaScript.** | 4 | 2 | 3 | 7 | 7 | 2 by 7, 1 by 4 | 1.4 | +| 19 | 0.6.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | +| 16 | 0.6.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | +| 28 | 0.6.0 | | | **Let a plugin add entries to the profile menu through the plugin API.** | 2 | 2 | 2 | 4 | 4 | 7 by 4, 1 by 2 | 4.2 | +| 50 | 0.6.0 | | | **Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once.** | 3 | 1 | 3 | 5 | 7 | code health by 7 | 3.7 | +| 54 | 0.6.0 | | | **List the `setting` conflict kind in the README's Conflict rules table.** | 1 | 1 | 1 | 1 | 1 | 9 by 2 | 3.1 | +| 2 | 0.6.0 | | | **Rename the admin example plugin to a name that says what it administers.** | 3 | 1 | 2 | 4 | 6 | 9 by 3, code health by 2 | 2.6 | +| 5 | 0.6.0 | | | **List grants that no installed plugin declares, with a remove action.** | 2 | 2 | 2 | 4 | 4 | 2 by 3 | 2.1 | +| 41 | 0.6.0 | | | **Apply a permission save as one Keto call, so it cannot stop part-way.** | 2 | 2 | 2 | 4 | 3 | 2 by 2 | 2.1 | +| 14 | 0.6.0 | | | **Drop a double-clicked duplicate submit, with no client-side JavaScript.** | 4 | 2 | 3 | 7 | 7 | 2 by 7, 1 by 4 | 1.4 | ## Details -### 56. Correct the README's two statements that `REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`. - -README → What you must supply (about `:1275`: "refuses to boot on a missing or throwaway -`CSRF_SECRET` and **nothing else**") and README → Security model (about `:1442`: -"`REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`") contradict the Configuration table row (about -`:1216`), `src/config.ts:59` and `:190`, and `src/plugin-host/settings.ts`. The flag also refuses a -missing or throwaway `PLUGIN_DB_SECRET` once `PLUGIN_DB_URL` is set, and every plugin setting -declared `secret`. An operator following the checklist underrates the toggle. Make both passages say -what the flag enforces by pointing to the table row, its one home. Four comprehension seats raised it -independently. Markdown only. - -### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. - -Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id -g` is `20`, which is -`dialout` inside the noble image, and Docker Desktop remaps bind-mount ownership in its VM. - -### 16. Decide whether the single Keto `Resource` namespace splits into per-domain namespaces. - -Ory's own examples model `Shift`, `Document` and so on. One global `Resource` bucket is the -project's "no catch-all names" rule applied to namespaces. - -### 48. Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes. - -`login.ts:40` and `bootstrap.ts:31` write `Permission:#members@user:`; the code, the OPL -file and the README use `granted`, and `members` is the Group relation. Raised by every comprehension -seat that read them. - ### 51. Move the test-only tooling out of the runtime folders `src/ui/` and `src/http/`, and keep the README file map true. `src/ui/stylesheet-floor.ts` imports a devDependency and only its test uses it; `src/http/app-harness.ts` @@ -74,17 +42,6 @@ into named steps, say in the header that an unsupported value in a custom proper `var()` that reads it, and keep the README's "Two browser floors" pointing here. It is the home of the 3am answer. -### 53. Say at `codeFlowSubmit` which branches of `flowFailure` can fire there. - -`submitFlow` throws only on a 5xx, so on the submit path only the 503 branch can fire; three seats -found that by opening a second file. - -### 49. Render the admin's "group already exists" refusal from the catalog, in every locale. - -`examples/plugins/admin/admin-groups.ts` returns the English string "A group with that name already -exists." where AGENTS.md puts every user-visible string in a catalog (`admin/i18n/en-US.ts`, then -`sv-SE.ts`). - ### 6. Warn at discovery when a route or nav node gates on a permission no plugin declares. Declaring stays optional on purpose, but the picker is built from declarations only, so such a route @@ -104,11 +61,15 @@ favicon and logo, and a plugin's `styles`. A file a plugin's own markup links, s the plugin API, hence a minor. Pick one with the plugin authors' panel, then document it in README → Reserved view locals. -### 47. Refuse the plugin id `error`, and make the drift guard cover the `/error` mount. +### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. -`/error` is Kratos' error sink (`src/auth/routes.ts`), and plugin routes resolve before built-ins, so -a plugin with id `error` and a `/` route shadows it. Add `error` to `RESERVED_PLUGIN_IDS` and to the -mount list in `plugin.test.ts`'s drift guard, whose comment claims every built-in is derived. +Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id -g` is `20`, which is +`dialout` inside the noble image, and Docker Desktop remaps bind-mount ownership in its VM. + +### 16. Decide whether the single Keto `Resource` namespace splits into per-domain namespaces. + +Ory's own examples model `Shift`, `Document` and so on. One global `Resource` bucket is the +project's "no catch-all names" rule applied to namespaces. ### 28. Let a plugin add entries to the profile menu through the plugin API.