§8 review convergence (todo §8); re-ran the architecture + product reviewers to convergence — 5 rounds, until both returned zero new actionable findings. Fixed across rounds 1-4 (tests-first): bounded every outbound Ory fetch with a timeout (src/fetch-timeout.ts withTimeout + ORY_TIMEOUT_SEC default 5, incl. the http JWKS fetch) so a hung Ory can't park a request handler; anonymous on a permission-gated plugin route now 303→/login (was a dead-end 403; signed-in-without-role still 403); an already-signed-in user is sent home from /login + /registration; the onRequest hook short-circuit now sets the fresh CSRF cookie; admin-users malformed :id → 404 (was 500) via safeDecode; parseJwks validates key element shape (fails loud at load); removed the dead COOKIE_SECRET (loaded + enforced + documented but never read); documented HYDRA_ADMIN_URL; admin recovery shows the code + links to the public /recovery instead of the browser-unreachable admin-API link; reference-plugin breadcrumb-label + pagination/datetime README notes; corrected the contract doc to not over-promise a post-login "retry". Declined: unconditional base-ctx chrome (would build the menu per request, regressing the lazy hot path). Deferred → §9: return_to-preservation for deep-link login. Stability-reviewer on the cumulative diff: APPROVE, no Critical/High (addressed its Low nits). typecheck + 310 units + the full scripts/ci.sh gate (visual 9 · auth 1 · oauth 2 · full 6) green.

2026-06-20 00:42:23 +02:00
parent bd20d00714
commit a20f3507e0
19 changed files with 181 additions and 59 deletions
--- a/README.md
+++ b/README.md
@@ -148,13 +148,15 @@ auto-merged by `docker compose up`) turns them back off for live editing.
 | `PORT` | `3000` | web listen port |
 | `CACHE_TEMPLATES` | `false` | cache compiled EJS templates (`true` in prod) |
 | `SECURE_COOKIES` | `false` | mark our session/CSRF cookies `Secure` (`true` in prod https; off in dev http) |
-| `REQUIRE_SECURE_SECRETS` | `false` | when `true`, the two secrets must be supplied and differ from the dev throwaways |
+| `REQUIRE_SECURE_SECRETS` | `false` | when `true`, `CSRF_SECRET` must be supplied and differ from the dev throwaway |
 | `KRATOS_PUBLIC_URL` / `KRATOS_ADMIN_URL` | `http://kratos:4433` / `:4434` | identity (self-service / admin) |
 | `KETO_READ_URL` / `KETO_WRITE_URL` | `http://keto:4466` / `:4467` | permission check / write |
+| `HYDRA_ADMIN_URL` | `http://hydra:4445` | OAuth2 provider admin API (§6 login/consent handshake) |
 | `JWKS_URL` | `file://…/tokenizer/jwks.json` | the Kratos tokenizer signing key; verifies the session JWT (§4) |
 | `JWT_ISSUER` / `JWT_AUDIENCE` | _unset_ | optional: when set, the session JWT's `iss` / `aud` must match (the dev tokenizer sets neither) |
 | `JWT_CLOCK_SKEW_SEC` | `60` | exp/nbf leeway (s) for Kratos↔web clock drift (the auth E2E sets `0`) |
-| `COOKIE_SECRET` / `CSRF_SECRET` | dev throwaways | enforced by `REQUIRE_SECURE_SECRETS` |
+| `ORY_TIMEOUT_SEC` | `5` | per-call timeout for outbound Kratos/Keto/Hydra (and http JWKS) fetches, so a hung Ory can't park a request |
+| `CSRF_SECRET` | dev throwaway | signs our double-submit CSRF token; enforced by `REQUIRE_SECURE_SECRETS` |

 ### What you must supply (the only manual prep)

@@ -163,11 +165,11 @@ stack with dev-throwaway secrets, an auto-generated signing key, and a seeded ad
 (see [Development](#development)). Exactly **two** things can't be auto-generated, and
 **both are production-only** — neither blocks a clean clone:

-1. **Production secrets** — replace the committed dev throwaways: `COOKIE_SECRET` and
-   `CSRF_SECRET` (env), plus the **JWT signing key** (mount a real `jwks.json` or set
+1. **Production secrets** — replace the committed dev throwaway `CSRF_SECRET` (env), plus the
+   **JWT signing key** (mount a real `jwks.json` or set
   `…_JWKS_URL` — see [JWT signing key & rotation](#jwt-signing-key--rotation)). Set
-   `REQUIRE_SECURE_SECRETS=true` and the app refuses to boot until the two secrets are
-   supplied and differ from the throwaways.
+   `REQUIRE_SECURE_SECRETS=true` and the app refuses to boot until `CSRF_SECRET` is
+   supplied and differs from the throwaway.
 2. **SSO provider client id/secret** — **optional**; password login works without them.
   Supplying a provider's creds via env activates it; no creds ⇒ no SSO button (see
   [Social sign-in (SSO)](#social-sign-in-sso)).
--- a/compose.yml
+++ b/compose.yml
@@ -6,7 +6,7 @@ services:
    ports:
      - "3000:3000"
    # Explicit behaviour toggles (the app is environment-agnostic — see AGENTS.md).
-    # Supply COOKIE_SECRET / CSRF_SECRET via env; REQUIRE_SECURE_SECRETS refuses dev throwaways.
+    # Supply CSRF_SECRET via env; REQUIRE_SECURE_SECRETS refuses the dev throwaway.
    environment:
      CACHE_TEMPLATES: "true"
      REQUIRE_SECURE_SECRETS: "true"
--- a/docs/plugin-contract.md
+++ b/docs/plugin-contract.md
@@ -111,7 +111,10 @@ A route is `{ method, path, permission?, handler }`. `path` is **relative to the
 path `/<id>`** (so `/shifts` in the `scheduling` plugin serves `/scheduling/shifts`); the host
 matches `method` + the resolved full path, extracts `:name` segments into `ctx.params.name`,
 runs the `permission` gate (a coarse JWT-claim check — see the README), and only then calls the
-handler with the [request context](#requestcontext).
+handler with the [request context](#requestcontext). When the gate fails, an **anonymous** visitor
+is redirected to `/login` to sign in (same as the built-in admin screens; after login they land on
+the dashboard, not back on the requested page); a **signed-in** user who simply lacks the role gets
+the **403** page.

 `method` is one of `GET HEAD POST PUT PATCH DELETE`. A `GET` route also answers `HEAD`.

--- a/e2e/visual.spec.ts
+++ b/e2e/visual.spec.ts
@@ -123,11 +123,13 @@ test("unknown routes serve the 404 page (a real user-facing flow, covered end-to
 });

 // The reference plugin (plugins/scheduling) ships discovered in the image. Its nav + routes are
-// permission-gated, so an anonymous visitor never sees or reaches them (the authenticated list/form
-// flow needs cross-host login infra — deferred to the §8 full E2E, todo line 121). Side-effect-free.
-test("the reference plugin is permission-gated: anonymous → 403, hidden from the dashboard nav", async ({ page }) => {
-  const res = await page.goto("/scheduling/shifts");
-  expect(res?.status()).toBe(403);
+// permission-gated, so an anonymous visitor is bounced to sign in (and never sees it in the nav).
+// The authenticated list/form flow is the §8 full E2E (full-flow.spec). Side-effect-free.
+test("the reference plugin is permission-gated: anonymous → redirect to /login, hidden from the dashboard nav", async ({ page }) => {
+  // Don't follow the redirect — this Ory-free suite has no /login handler; assert the gate's 303 itself.
+  const res = await page.request.get("/scheduling/shifts", { maxRedirects: 0 });
+  expect(res.status()).toBe(303);
+  expect(res.headers()["location"]).toBe("/login");

  await page.goto("/");
  await expect(page.locator(".sidebar")).toContainText("People"); // dashboard nav renders
--- a/plugins/scheduling/README.md
+++ b/plugins/scheduling/README.md
@@ -7,7 +7,9 @@ What it demonstrates:

 - **A list page that fetches upstream data** — `GET /scheduling/shifts` calls the upstream REST
  service and renders the rows with the core building blocks (`shifts.ejs` → app shell, filter-bar,
-  data-table). Search round-trips the URL; zero-JS.
+  data-table). Search round-trips the URL; zero-JS. (It fetches **all** rows for brevity — for a
+  large list, parse `page`/`pageSize` from `parseListQuery`, forward them upstream as a `?limit`/
+  `?offset`, and render `pagination.ejs` with `paginate()`, exactly as the built-in admin screens do.)
 - **A form that forwards a write upstream** — `GET /scheduling/shifts/new` renders the form,
  `POST /scheduling/shifts` CSRF-verifies it (`ctx.verifyCsrf`) and forwards the create upstream,
  then POST-redirect-GET. The form body lives in the plugin's own `views/partials/shift-form.ejs`,
@@ -37,6 +39,10 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera
 Domain rules (overlap, capacity, time ordering) live in your backend — reject with a 4xx and the
 form re-renders. The plugin only validates that `title` and `assignee` are non-empty.

+`start`/`end` come from the form's `datetime-local` inputs as `YYYY-MM-DDTHH:mm` and are stored and
+shown verbatim (the dev mock seeds a space-separated style, so created vs seeded rows differ only
+cosmetically) — normalise to your backend's format there if it matters.
+
 ## Granting access

 A user sees Scheduling once they hold the `scheduling:read` role in Keto (and `scheduling:write`
--- a/plugins/scheduling/shifts.ts
+++ b/plugins/scheduling/shifts.ts
@@ -87,7 +87,7 @@ function toShift(raw: unknown): Shift {

 export function buildListModel(opts: { canWrite: boolean; chrome: PageChrome; error?: string; q: string; shifts: Shift[] }) {
  return {
-    breadcrumbs: [{ href: SHIFTS_PATH, label: "Scheduling" }, { label: "Shifts" }],
+    breadcrumbs: [{ label: "Shifts" }], // SHIFTS_PATH is the list itself; the form links back to it as "Shifts"
    canWrite: opts.canWrite,
    chrome: opts.chrome,
    ...(opts.error ? { error: opts.error } : {}),
--- a/public/css/styles.css
+++ b/public/css/styles.css
@@ -684,4 +684,4 @@ th[aria-sort="descending"] .sort-ico { transform: rotate(180deg); }
 .detail-list dd { margin: 0; word-break: break-word; }
 .btn-danger { color: var(--neg); border-color: var(--neg-bd); }
 .btn-danger:hover { background: var(--neg-bg); }
-.recovery-link { word-break: break-all; }
+.recovery-code code { font-size: 1.15rem; font-weight: 600; letter-spacing: 0.04em; }
--- a/src/admin-users.ts
+++ b/src/admin-users.ts
@@ -4,6 +4,7 @@
 // models; `handleAdminUsers` is the imperative shell app.ts dispatches to — gated admin-only,
 // CSRF-guarded, each action mapped to a RouteResult (render, or redirect after a write — PRG).

+import { safeDecode } from "./admin-groups.ts";
 import { ADMIN_USERS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts";
 import type { RequestContext, User } from "./context.ts";
 import type { Identity, KratosAdmin, RecoveryCode } from "./kratos-admin.ts";
@@ -253,7 +254,7 @@ export function buildUserFormModel(opts: {
    { id: "first", label: "First name", name: "first", optional: true, value: np.first },
    { id: "last", label: "Last name", name: "last", optional: true, value: np.last },
  ];
-  if (!editing) fields.push({ autocomplete: "new-password", hint: "Optional — leave blank to have the user set one via a recovery link.", icon: "i-lock", id: "password", label: "Password", name: "password", optional: true, type: "password" });
+  if (!editing) fields.push({ autocomplete: "new-password", hint: "Optional — leave blank to have the user set one via a recovery code.", icon: "i-lock", id: "password", label: "Password", name: "password", optional: true, type: "password" });

  return {
    edit: editing ? {
@@ -335,7 +336,8 @@ export async function handleAdminUsers(ctx: RequestContext, csrfToken: string, d
  if (seg.length === 1 && seg[0] === "new" && method === "GET") return renderForm({});

  // /admin/users/:id …
-  const targetId = decodeURIComponent(seg[0]!);
+  const targetId = safeDecode(seg[0]!); // malformed %-encoding → 404, not a 500 (matches groups/roles/clients)
+  if (targetId === null) return { html: await render("404", { title: "Not found" }), status: 404 };
  const identity = await kratosAdmin.getIdentity(targetId);
  if (!identity) return { html: await render("404", { title: "Not found" }), status: 404 };
  const back = `${ADMIN_USERS_BASE}/${encodeURIComponent(targetId)}`;
--- a/src/app.test.ts
+++ b/src/app.test.ts
@@ -179,12 +179,10 @@ test("mounts plugin routes: params, html/json/redirect/view results, and the per
  assert.match(await css.text(), /\.demo/);
  assert.equal((await fetch(url + "/public/demo/..%2f..%2fplugin.ts")).status, 403); // traversal still blocked

-  // gated route with no session → the rendered 403 page (covers the gate + 403.ejs over HTTP)
-  const denied = await fetch(url + "/demo/secret");
-  assert.equal(denied.status, 403);
-  const deniedBody = await denied.text();
-  assert.match(deniedBody, /403/);
-  assert.match(deniedBody, /styles\.css/);
+  // gated route, anonymous → redirect to sign in (like the built-in screens), not a dead-end 403
+  const denied = await fetch(url + "/demo/secret", { redirect: "manual" });
+  assert.equal(denied.status, 303);
+  assert.equal(denied.headers.get("location"), "/login");

  // known path + wrong method → 405 with Allow; unknown path → 404
  const wrong = await fetch(url + "/demo/data", { method: "DELETE" });
@@ -255,22 +253,24 @@ function mintJwt(payload: Record<string, unknown>): string {
  return `${input}.${b64url(sign("SHA256", Buffer.from(input), { dsaEncoding: "ieee-p1363", key: ec.privateKey }))}`;
 }

-test("a verified session JWT authorizes a role-gated route; no cookie / expired token → 403", async (t) => {
+test("a verified session JWT authorizes a role-gated route; no cookie / expired token → sign in", async (t) => {
  const app = createApp({ jwks: staticJwks([ecJwk]), plugins: [demoPlugin] });
  await new Promise<void>((r) => app.listen(0, r));
  t.after(() => app.close());
  const url = `http://localhost:${(app.address() as AddressInfo).port}`;
  const nowSec = Math.floor(Date.now() / 1000);
-  const secret = (cookie?: string) => fetch(url + "/demo/secret", cookie ? { headers: { cookie } } : {});
+  const secret = (cookie?: string) => fetch(url + "/demo/secret", { redirect: "manual", ...(cookie ? { headers: { cookie } } : {}) });

  // Token carrying the gating role → the handler runs (200).
  const ok = await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec + 600, roles: ["demo:read"], sub: "u1" })}`);
  assert.equal(ok.status, 200);
  assert.equal(await ok.text(), "secret");

-  // No cookie and an expired token both render anonymous → the gate denies (403).
-  assert.equal((await secret()).status, 403);
-  assert.equal((await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, roles: ["demo:read"], sub: "u1" })}`)).status, 403);
+  // No cookie and an expired token both render anonymous → the gate bounces to sign in (303 → /login).
+  const noCookie = await secret();
+  assert.equal(noCookie.status, 303);
+  assert.equal(noCookie.headers.get("location"), "/login");
+  assert.equal((await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, roles: ["demo:read"], sub: "u1" })}`)).status, 303);

  // The home menu wires in the permission-gated Admin section: an admin's roles surface the links.
  const home = (cookie?: string) => fetch(url + "/", cookie ? { headers: { cookie } } : {});
@@ -295,21 +295,23 @@ test("session re-mint: an expired JWT backed by a live Kratos session is silentl
  assert.equal(await ok.text(), "secret");
  assert.match(ok.headers.get("set-cookie") ?? "", /^plainpages_jwt=/);

-  // Kratos session gone: no re-mint, the stale cookie is cleared, the gate denies.
+  // Kratos session gone: no re-mint, the stale cookie is cleared, the now-anonymous request bounces to sign in.
  const dead = createApp({ jwks: staticJwks([ecJwk]), keto, kratos: withWhoami(async () => null), kratosAdmin: stubAdmin({}), plugins: [demoPlugin] });
  await new Promise<void>((r) => dead.listen(0, r));
  t.after(() => dead.close());
-  const denied = await fetch(`http://localhost:${(dead.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } });
-  assert.equal(denied.status, 403);
+  const denied = await fetch(`http://localhost:${(dead.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired }, redirect: "manual" });
+  assert.equal(denied.status, 303);
+  assert.equal(denied.headers.get("location"), "/login");
  assert.match(denied.headers.get("set-cookie") ?? "", /^plainpages_jwt=;.*Max-Age=0/);

-  // Ory unreachable (not a dead session): whoami throws → degrade to anonymous (403, not 500),
+  // Ory unreachable (not a dead session): whoami throws → degrade to anonymous (bounce to /login, not 500),
  // and leave the cookie untouched so the token can re-mint once Ory recovers.
  const down = createApp({ jwks: staticJwks([ecJwk]), keto, kratos: withWhoami(async () => { throw new KratosError("kratos down", 503, ""); }), kratosAdmin: stubAdmin({}), plugins: [demoPlugin] });
  await new Promise<void>((r) => down.listen(0, r));
  t.after(() => down.close());
-  const outage = await fetch(`http://localhost:${(down.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } });
-  assert.equal(outage.status, 403);
+  const outage = await fetch(`http://localhost:${(down.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired }, redirect: "manual" });
+  assert.equal(outage.status, 303);
+  assert.equal(outage.headers.get("location"), "/login");
  assert.equal(outage.headers.get("set-cookie"), null);
 });

@@ -322,6 +324,7 @@ test("guards map to responses: requireSession → /login, a failed can/check →
      { handler: (ctx) => ({ html: `hi ${requireSession(ctx).email}` }), method: "GET", path: "/me" },
      { handler: (ctx) => { if (!can(ctx, "admin")) throw new GuardError(403, "no"); return { html: "ok" }; }, method: "GET", path: "/admin-only" },
      { handler: async (ctx) => { if (!(await check(keto, ctx, { namespace: "Resource", object: ctx.params.id ?? "", relation: "view" }))) throw new GuardError(403, "no"); return { html: "seen" }; }, method: "GET", path: "/doc/:id" },
+      { handler: () => ({ html: "gated" }), method: "GET", path: "/gated", permission: "secret:read" }, // declarative route gate
    ],
  };
  const app = createApp({ jwks: staticJwks([ecJwk]), plugins: [guarded] });
@@ -346,6 +349,15 @@ test("guards map to responses: requireSession → /login, a failed can/check →
  // check (live Keto): the keto verdict gates the handler.
  assert.equal((await fetch(url + "/guarded/doc/open", auth([]))).status, 200);
  assert.equal((await fetch(url + "/guarded/doc/shut", auth([]))).status, 403);
+
+  // declarative route `permission` gate: anonymous → sign in, signed-in-without-role → the 403 page, with → 200.
+  const gAnon = await fetch(url + "/guarded/gated", { redirect: "manual" });
+  assert.equal(gAnon.status, 303);
+  assert.equal(gAnon.headers.get("location"), "/login");
+  const gDenied = await fetch(url + "/guarded/gated", auth([]));
+  assert.equal(gDenied.status, 403);
+  assert.match(await gDenied.text(), /403/); // the rendered 403.ejs over HTTP
+  assert.equal((await fetch(url + "/guarded/gated", auth(["secret:read"]))).status, 200);
 });

 test("plugin hooks: onRequest can short-circuit a request and onResponse observes the handler result", async (t) => {
@@ -361,10 +373,12 @@ test("plugin hooks: onRequest can short-circuit a request and onResponse observe
  };
  const url = await startApp(t, [hooked]);

-  // onRequest short-circuits before routing — handler never runs.
+  // onRequest short-circuits before routing — handler never runs; a fresh CSRF cookie still rides the
+  // response so a form the hook renders has its matching double-submit cookie.
  const blocked = await fetch(url + "/hooked/blocked");
  assert.equal(blocked.status, 403);
  assert.match(await blocked.text(), /blocked by hook/);
+  assert.match(blocked.headers.get("set-cookie") ?? "", /plainpages_csrf=/);

  // A normal route runs the handler; onResponse observed its result.
  assert.match(await (await fetch(url + "/hooked/ok")).text(), /handler ran/);
@@ -416,6 +430,24 @@ test("themed flow init: no ?flow= initialises one, relays Kratos' CSRF cookie, a
  assert.equal(stale.headers.get("location"), "/login");
 });

+test("themed auth: an already-signed-in user is sent home from /login and /registration, not /settings", async (t) => {
+  const app = createApp({ jwks: staticJwks([ecJwk]), kratos: mockKratos(async (_t, id) => loginFlow(id)) });
+  await new Promise<void>((r) => app.listen(0, r));
+  t.after(() => app.close());
+  const url = `http://localhost:${(app.address() as AddressInfo).port}`;
+  const signedIn = { headers: { cookie: `${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: Math.floor(Date.now() / 1000) + 600, roles: [], sub: "u1" })}` }, redirect: "manual" as const };
+
+  for (const path of ["/login", "/registration"]) {
+    const res = await fetch(url + path, signedIn);
+    assert.equal(res.status, 303, `${path} while signed in → 303`);
+    assert.equal(res.headers.get("location"), "/");
+  }
+  // /settings stays reachable when signed in (inits its flow, not bounced home).
+  assert.equal((await fetch(url + "/settings", signedIn)).headers.get("location"), "/settings?flow=new1");
+  // Anonymous still gets the login flow (no short-circuit).
+  assert.equal((await fetch(url + "/login", { redirect: "manual" })).headers.get("location"), "/login?flow=new1");
+});
+
 test("renders a fetched flow as the themed auth page: fields post straight to Kratos, errors surface", async (t) => {
  const app = createApp({ kratos: mockKratos(async (_t, id) => loginFlow(id)) });
  await new Promise<void>((r) => app.listen(0, r));
@@ -751,10 +783,13 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r
  await post(`/admin/users/${target.id}/state`, `_csrf=${token}`);
  assert.equal(target.state, "inactive");

-  // Recovery: renders the edit page (200) carrying the generated link.
+  // Recovery: renders the edit page (200) showing the generated code (code-based; no admin-host link).
  const rec = await post(`/admin/users/${target.id}/recovery`, `_csrf=${token}`);
  assert.equal(rec.status, 200);
-  assert.match(await rec.text(), /self-service\/recovery\?code=123456/);
+  const recHtml = await rec.text();
+  assert.match(recHtml, /Recovery code generated/);
+  assert.match(recHtml, /<code>123456<\/code>/);
+  assert.doesNotMatch(recHtml, /self-service\/recovery\?code=/); // the unreachable admin-API link is gone

  // Delete needs a deliberate confirm step (zero-JS): GET renders the interstitial, POST performs it.
  const confirm = await (await get(`/admin/users/${target.id}/delete`)).text();
@@ -770,8 +805,9 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r
  assert.equal((await post(`/admin/users/admin1/state`, `_csrf=${token}`)).status, 400);
  assert.equal(store.find((x) => x.id === "admin1")!.state, "active");

-  // Unknown id → 404.
+  // Unknown id → 404; malformed %-encoding → 404 (not a 500), matching groups/roles/clients.
  assert.equal((await get(`/admin/users/${randomUUID()}`)).status, 404);
+  assert.equal((await get("/admin/users/%ZZ")).status, 404);
 });

 // Built-in Groups admin screen (§5): gate + list/create/membership/delete over HTTP against a
--- a/src/app.ts
+++ b/src/app.ts
@@ -152,6 +152,9 @@ export function createApp(options: AppOptions = {}): Server {
      if (anyRequestHooks) {
        const short = await runRequestHooks(plugins, ctx);
        if (short) {
+          // Set the fresh CSRF cookie like every other page-emitting path, so a form the hook
+          // renders (its token is in ctx.chrome.csrfToken) has the matching double-submit cookie.
+          if (csrf.fresh) res.appendHeader("set-cookie", csrfCookie(csrf.token, { secure: secureCookies }));
          await sendResult(res, short.result, (view, data) => renderView(short.plugin.id, view, data));
          return;
        }
@@ -164,6 +167,9 @@ export function createApp(options: AppOptions = {}): Server {
      if (match) {
        const routeCtx = buildContext(req, res, { chrome: chrome(), params: match.params, user, verifyCsrf });
        if (!isAuthorized(match.route, routeCtx.roles)) {
+          // Anonymous → sign in (like the built-in screens' requireSession); a signed-in user who
+          // simply lacks the role gets the 403 page.
+          if (!routeCtx.user) { res.writeHead(303, { location: "/login" }).end(); return; }
          sendHtml(res, 403, await render("403", { title: "Forbidden" }));
          return;
        }
@@ -213,6 +219,12 @@ export function createApp(options: AppOptions = {}): Server {
      // Themed Kratos self-service pages (login/registration/recovery/verification/settings).
      const flowType = AUTH_FLOWS[pathname];
      if (kratos && flowType && (method === "GET" || method === "HEAD")) {
+        // Already signed in? Re-authenticating / re-registering is pointless — send them home.
+        // (/settings, /recovery, /verification stay reachable — a signed-in user can use those.)
+        if (ctx.user && (pathname === "/login" || pathname === "/registration")) {
+          res.writeHead(303, { location: "/" }).end();
+          return;
+        }
        const cookie = req.headers.cookie;
        const flowId = ctx.url.searchParams.get("flow");
        if (!flowId) {
--- a/src/config.test.ts
+++ b/src/config.test.ts
@@ -5,7 +5,6 @@ import { loadConfig } from "./config.ts";
 // Explicit secure-secret enforcement (no environment sniffing): secrets are the only
 // thing a hardened deploy must supply.
 const secureEnv = {
-  COOKIE_SECRET: "real-cookie-secret",
  CSRF_SECRET: "real-csrf-secret",
  REQUIRE_SECURE_SECRETS: "true",
 };
@@ -20,7 +19,6 @@ test("loads dev defaults when the environment is empty", () => {
  assert.equal(c.ketoReadUrl, "http://keto:4466");
  assert.equal(c.ketoWriteUrl, "http://keto:4467");
  assert.equal(c.hydraAdminUrl, "http://hydra:4445");
-  assert.match(c.cookieSecret, /dev-insecure/);
  assert.match(c.csrfSecret, /dev-insecure/);
  assert.equal(c.jwtClockSkewSec, 60); // default exp/nbf leeway for Kratos↔web clock drift
 });
@@ -51,10 +49,10 @@ test("parses explicit boolean toggles and rejects non-boolean values", () => {
 });

 test("reads overrides from the environment", () => {
-  const c = loadConfig({ COOKIE_SECRET: "x", KRATOS_PUBLIC_URL: "https://id.example.com", PORT: "8080" });
+  const c = loadConfig({ CSRF_SECRET: "x", KRATOS_PUBLIC_URL: "https://id.example.com", PORT: "8080" });
  assert.equal(c.port, 8080);
  assert.equal(c.kratosPublicUrl, "https://id.example.com");
-  assert.equal(c.cookieSecret, "x");
+  assert.equal(c.csrfSecret, "x");
 });

 test("rejects an invalid PORT", () => {
@@ -67,22 +65,26 @@ test("JWT_CLOCK_SKEW_SEC: parses a non-negative integer, rejects junk (E2E short
  for (const v of ["-1", "1.5", "abc"]) assert.throws(() => loadConfig({ JWT_CLOCK_SKEW_SEC: v }), /JWT_CLOCK_SKEW_SEC/);
 });

+test("ORY_TIMEOUT_SEC: defaults to 5 and must be a positive integer (0 would abort every Ory call)", () => {
+  assert.equal(loadConfig({}).oryTimeoutSec, 5);
+  assert.equal(loadConfig({ ORY_TIMEOUT_SEC: "10" }).oryTimeoutSec, 10);
+  for (const v of ["0", "-1", "1.5", "abc"]) assert.throws(() => loadConfig({ ORY_TIMEOUT_SEC: v }), /ORY_TIMEOUT_SEC/);
+});
+
 test("rejects a malformed Ory URL", () => {
  assert.throws(() => loadConfig({ KETO_READ_URL: "not a url" }), /KETO_READ_URL/);
 });

 test("REQUIRE_SECURE_SECRETS rejects a missing or dev-throwaway secret", () => {
-  assert.throws(() => loadConfig({ REQUIRE_SECURE_SECRETS: "true" }), /COOKIE_SECRET/);
-  assert.throws(() => loadConfig({ COOKIE_SECRET: "real", REQUIRE_SECURE_SECRETS: "true" }), /CSRF_SECRET/);
+  assert.throws(() => loadConfig({ REQUIRE_SECURE_SECRETS: "true" }), /CSRF_SECRET/);
  assert.throws(
-    () => loadConfig({ COOKIE_SECRET: "dev-insecure-cookie-secret", CSRF_SECRET: "real", REQUIRE_SECURE_SECRETS: "true" }),
-    /COOKIE_SECRET/,
+    () => loadConfig({ CSRF_SECRET: "dev-insecure-csrf-secret", REQUIRE_SECURE_SECRETS: "true" }),
+    /CSRF_SECRET/,
  );
 });

 test("REQUIRE_SECURE_SECRETS succeeds with real secrets and still defaults the Ory URLs", () => {
  const c = loadConfig(secureEnv);
-  assert.equal(c.cookieSecret, "real-cookie-secret");
  assert.equal(c.csrfSecret, "real-csrf-secret");
  assert.equal(c.kratosPublicUrl, "http://kratos:4433"); // only secrets are enforced; URLs still default
 });
--- a/src/config.ts
+++ b/src/config.ts
@@ -10,7 +10,6 @@

 export interface Config {
  cacheTemplates: boolean;
-  cookieSecret: string;
  csrfSecret: string;
  hydraAdminUrl: string;
  jwksUrl: string;
@@ -21,6 +20,7 @@ export interface Config {
  ketoWriteUrl: string;
  kratosAdminUrl: string;
  kratosPublicUrl: string;
+  oryTimeoutSec: number; // per-call timeout for outbound Kratos/Keto/Hydra fetches (bounds a hung Ory)
  port: number;
  secureCookies: boolean;
 }
@@ -82,11 +82,18 @@ function readNonNegInt(env: Env, key: string, devDefault: number): number {
  return n;
 }

+function readPosInt(env: Env, key: string, devDefault: number): number {
+  const raw = env[key];
+  if (raw === undefined) return devDefault;
+  const n = Number(raw);
+  if (!Number.isInteger(n) || n < 1) throw new Error(`config: ${key} must be a positive integer, got "${raw}"`);
+  return n;
+}
+
 export function loadConfig(env: Env = process.env): Config {
  const requireSecure = readBool(env, "REQUIRE_SECURE_SECRETS", false);
  return {
    cacheTemplates: readBool(env, "CACHE_TEMPLATES", false),
-    cookieSecret: readSecret(env, "COOKIE_SECRET", "dev-insecure-cookie-secret", requireSecure),
    csrfSecret: readSecret(env, "CSRF_SECRET", "dev-insecure-csrf-secret", requireSecure),
    // Hydra admin API — the OAuth2 login/consent challenge handshake (§6); not on the first-party path.
    hydraAdminUrl: readUrl(env, "HYDRA_ADMIN_URL", "http://hydra:4445"),
@@ -103,6 +110,7 @@ export function loadConfig(env: Env = process.env): Config {
    ketoWriteUrl: readUrl(env, "KETO_WRITE_URL", "http://keto:4467"),
    kratosAdminUrl: readUrl(env, "KRATOS_ADMIN_URL", "http://kratos:4434"),
    kratosPublicUrl: readUrl(env, "KRATOS_PUBLIC_URL", "http://kratos:4433"),
+    oryTimeoutSec: readPosInt(env, "ORY_TIMEOUT_SEC", 5),
    port: readPort(env),
    // Set Secure on our session/CSRF cookies. Off by default (dev runs http); prod (https) sets it.
    secureCookies: readBool(env, "SECURE_COOKIES", false),
--- a/src/fetch-timeout.test.ts
+++ b/src/fetch-timeout.test.ts
@@ -0,0 +1,24 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { withTimeout } from "./fetch-timeout.ts";
+
+test("withTimeout injects an abort signal that fires after the deadline", async () => {
+  let seenSignal: AbortSignal | undefined;
+  const slow: typeof fetch = ((_input, init) => {
+    seenSignal = (init as RequestInit | undefined)?.signal ?? undefined;
+    return new Promise<Response>((_resolve, reject) => {
+      seenSignal?.addEventListener("abort", () => reject(seenSignal!.reason));
+    });
+  }) as typeof fetch;
+
+  await assert.rejects(withTimeout(slow, 20)("http://x/"), (e: unknown) => (e as Error).name === "TimeoutError");
+  assert.ok(seenSignal instanceof AbortSignal); // the wrapped call received a real signal
+});
+
+test("withTimeout keeps a caller-supplied signal instead of overriding it", async () => {
+  let seen: AbortSignal | undefined;
+  const fake: typeof fetch = ((_input, init) => { seen = (init as RequestInit | undefined)?.signal ?? undefined; return Promise.resolve(new Response("ok")); }) as typeof fetch;
+  const mine = new AbortController().signal;
+  await withTimeout(fake, 50)("http://x/", { signal: mine });
+  assert.equal(seen, mine);
+});
--- a/src/fetch-timeout.ts
+++ b/src/fetch-timeout.ts
@@ -0,0 +1,9 @@
+// Bound every outbound Ory call (todo §8 review): a reachable-but-silent host — a hung container, a
+// black-holed socket, an LB holding the connection — would otherwise park a request handler forever
+// (and exhaust the pool under load). Wrap the injected `fetch` so each call aborts after `ms` unless
+// the caller already passed its own signal. server.ts wires this into the Kratos/Keto/Hydra clients.
+
+export function withTimeout(fetchImpl: typeof fetch, ms: number): typeof fetch {
+  // A caller-supplied signal wins (so an explicit abort still works); otherwise inject the timeout.
+  return (input, init) => fetchImpl(input, { ...init, signal: init?.signal ?? AbortSignal.timeout(ms) });
+}
--- a/src/jwks.test.ts
+++ b/src/jwks.test.ts
@@ -26,6 +26,12 @@ test("loadJwks reads a file:// set and a base64:// inline set, rejects http", ()
  assert.equal(loadJwks(`base64://${Buffer.from(inline).toString("base64")}`)[0]?.kid, "inline");

  assert.throws(() => loadJwks("http://keto:4466/keys"), /unsupported/);
+
+  // Malformed sets fail loud at load, not as an opaque crypto error at verify time.
+  const b64 = (o: unknown) => `base64://${Buffer.from(JSON.stringify(o)).toString("base64")}`;
+  assert.throws(() => loadJwks(b64({})), /missing `keys`/);
+  assert.throws(() => loadJwks(b64({ keys: ["nope"] })), /string `kty`/); // a non-object key
+  assert.throws(() => loadJwks(b64({ keys: [{ kid: "x" }] })), /string `kty`/); // key missing kty
 });

 test("cachingJwks caches within TTL, reloads after expiry", async () => {
--- a/src/jwks.ts
+++ b/src/jwks.ts
@@ -24,6 +24,13 @@ export interface JwksCacheOptions {
 function parseJwks(text: string): JsonWebKey[] {
  const parsed = JSON.parse(text) as { keys?: unknown };
  if (!Array.isArray(parsed.keys)) throw new Error("JWKS: missing `keys` array");
+  // Validate element shape here so a malformed key fails loud at load, not as an opaque crypto
+  // error on the first authenticated request (the verifier keys off `kty`/`kid`).
+  for (const k of parsed.keys) {
+    if (typeof k !== "object" || k === null || typeof (k as JsonWebKey).kty !== "string") {
+      throw new Error("JWKS: each key must be an object with a string `kty`");
+    }
+  }
  return parsed.keys as JsonWebKey[];
 }

--- a/src/server.ts
+++ b/src/server.ts
@@ -1,6 +1,7 @@
 import { createApp } from "./app.ts";
 import { loadConfig } from "./config.ts";
 import { discoverPlugins } from "./discovery.ts";
+import { withTimeout } from "./fetch-timeout.ts";
 import { runBootHooks } from "./hooks.ts";
 import { createHydraAdmin } from "./hydra-admin.ts";
 import { createJwksProvider } from "./jwks.ts";
@@ -11,15 +12,17 @@ import { loadMenuConfig } from "./menu-config.ts";

 const config = loadConfig(); // validates the env (incl. enforced secrets) — fails loud at boot
 const menu = await loadMenuConfig(); // config/menu.ts override + branding — fails loud if malformed
+// Every outbound Ory call is bounded so a hung/silent Ory can't park a request handler forever.
+const oryFetch = withTimeout(fetch, config.oryTimeoutSec * 1000);
 // Ory clients for the themed self-service routes + login completion (§4).
-const kratos = createKratosPublic({ baseUrl: config.kratosPublicUrl });
-const kratosAdmin = createKratosAdmin({ baseUrl: config.kratosAdminUrl });
-const keto = createKetoClient({ readUrl: config.ketoReadUrl, writeUrl: config.ketoWriteUrl });
+const kratos = createKratosPublic({ baseUrl: config.kratosPublicUrl, fetchImpl: oryFetch });
+const kratosAdmin = createKratosAdmin({ baseUrl: config.kratosAdminUrl, fetchImpl: oryFetch });
+const keto = createKetoClient({ fetchImpl: oryFetch, readUrl: config.ketoReadUrl, writeUrl: config.ketoWriteUrl });
 // Hydra admin client for the OAuth2 login/consent challenge handshake (§6).
-const hydra = createHydraAdmin({ baseUrl: config.hydraAdminUrl });
+const hydra = createHydraAdmin({ baseUrl: config.hydraAdminUrl, fetchImpl: oryFetch });
 // Session-JWT verify key: primed at boot from the configured JWKS (file mount, base64 inline,
 // or fetched http), then served from cache with TTL refresh + rotation-on-miss (§4).
-const jwks = await createJwksProvider(config.jwksUrl);
+const jwks = await createJwksProvider(config.jwksUrl, { fetchImpl: oryFetch }); // bound an http JWKS fetch too

 const plugins = await discoverPlugins(); // scans plugins/, validates — fails loud on a bad plugin
 console.log(`Discovered ${plugins.length} plugin(s)${plugins.length ? `: ${plugins.map((p) => p.id).join(", ")}` : ""}`);
--- a/todo.md
+++ b/todo.md
--- a/views/partials/user-form-body.ejs
+++ b/views/partials/user-form-body.ejs
@@ -2,7 +2,7 @@
  Admin user create/edit form body (todo §5), captured into the shell content slot. Config:
    form  { action, csrfToken, submitLabel, cancelHref, fields: field.ejs config[] }
    edit? { nextLabel, stateAction, recoveryAction, deleteAction }   (edit mode only)
-    recovery? { code?, link? }   shown after a recovery link is generated
+    recovery? { code? }   shown after a recovery code is generated (recovery is code-based)
    error?   string              shown when a write was rejected
 %><%
  const form = locals.form;
@@ -14,7 +14,7 @@
 <%- include("alert", { text: locals.error, tone: "neg" }) %>
 <% } -%>
 <% if (recovery) { -%>
-  <div class="alert alert-pos" role="status"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-check-circle"/></svg><div class="alert-body"><strong>Recovery link generated</strong><% if (recovery.link) { %><span class="recovery-link"><a href="<%= recovery.link %>"><%= recovery.link %></a></span><% } %><% if (recovery.code) { %><span>Code: <code><%= recovery.code %></code></span><% } %></div></div>
+  <div class="alert alert-pos" role="status"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-check-circle"/></svg><div class="alert-body"><strong>Recovery code generated</strong><span>Give it to the user — they enter it on the <a href="/recovery">password-reset screen</a> to set a new password (generate a fresh one if it has expired).</span><% if (recovery.code) { %><span class="recovery-code"><code><%= recovery.code %></code></span><% } %></div></div>
 <% } -%>
  <form class="form-card" method="post" action="<%= form.action %>">
    <input type="hidden" name="_csrf" value="<%= form.csrfToken %>">
@@ -28,7 +28,7 @@
  </form>
 <% if (edit) { -%>
  <section class="form-card admin-actions" aria-label="Account actions">
-    <form method="post" action="<%= edit.recoveryAction %>"><input type="hidden" name="_csrf" value="<%= form.csrfToken %>"><button class="btn" type="submit"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-mail"/></svg>Send recovery link</button></form>
+    <form method="post" action="<%= edit.recoveryAction %>"><input type="hidden" name="_csrf" value="<%= form.csrfToken %>"><button class="btn" type="submit"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-mail"/></svg>Generate recovery code</button></form>
    <form method="post" action="<%= edit.stateAction %>"><input type="hidden" name="_csrf" value="<%= form.csrfToken %>"><button class="btn" type="submit"><%= edit.nextLabel %></button></form>
    <a class="btn btn-danger" href="<%= edit.deleteAction %>"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-trash"/></svg>Delete user</a>
  </section>