Review fixes: re-mint path, real session lifetime, full secret checklist
CI / full-gate (push) Successful in 6s

This commit is contained in:
2026-08-02 17:14:20 +02:00
parent fb7d20a7db
commit c0fe8b0a82
2 changed files with 52 additions and 30 deletions
+51 -29
View File
@@ -837,14 +837,23 @@ both default to `localhost` (the dev override sets `APP_URL=http://localhost:300
A clean clone needs **none** of the above — `docker compose up` brings up the whole stack A clean clone needs **none** of the above — `docker compose up` brings up the whole stack
with dev-throwaway secrets, an auto-generated signing key, and a seeded admin (see with dev-throwaway secrets, an auto-generated signing key, and a seeded admin (see
[Quick start](#quick-start)). Exactly **two** things can't be auto-generated, and **both [Quick start](#quick-start)). What can't be auto-generated is **production-only** — none of it
are production-only** — neither blocks a clean clone: blocks a clean clone:
1. **Production secrets** — every value below ships as a committed dev throwaway that works
out of the box 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.
| Secret | Where | Protects |
| --- | --- | --- |
| `CSRF_SECRET` | web env | signs our double-submit CSRF token |
| JWT signing key | mount a real `jwks.json` or set `…_JWKS_URL` | mints/verifies the session JWT — see [rotation](#jwt-signing-key--rotation) |
| `SECRETS_COOKIE` | kratos env | signs Kratos' session + anti-CSRF cookies |
| `SECRETS_CIPHER` | kratos env (32 chars) | encrypts credentials at rest |
| `SECRETS_SYSTEM` | hydra env | encrypts OAuth2 tokens + consent at rest |
| `POSTGRES_USER` / `POSTGRES_PASSWORD` | compose env | the Ory databases (default `ory`/`ory`) |
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 `CSRF_SECRET` is supplied
and differs from the throwaway.
2. **SSO provider client id/secret** — **optional**; password login works without them. 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 Supplying a provider's creds via env activates it; no creds ⇒ no SSO button (see
[Social sign-in (SSO)](#social-sign-in-sso)). [Social sign-in (SSO)](#social-sign-in-sso)).
@@ -1008,17 +1017,19 @@ what defends what, and which guarantees are deliberately not offered.
- **The browser is not trusted.** Cookies, form fields, URLs and headers are attacker-controlled - **The browser is not trusted.** Cookies, form fields, URLs and headers are attacker-controlled
until verified or escaped. Nothing is believed because of where it arrived from. until verified or escaped. Nothing is believed because of where it arrived from.
- **The session JWT is trusted only after verification** — signature against the JWKS key its - **The session JWT is trusted only after verification** — signature against the JWKS key its
`kid` names, then `exp`/`nbf` and the optional `iss`/`aud`. Before that it is bytes. `kid` names (or the sole key, when the set has one), then a **mandatory** `exp`, plus `nbf`
- **The private container network is the *only* thing guarding the Ory admin APIs.** Kratos and the optional `iss`/`aud`. Before that it is bytes.
admin (`4434`), Hydra admin (`4445`) and Keto write (`4467`) authenticate no one — reaching - **The private container network is the *only* thing guarding the Ory APIs.** Kratos admin
them *is* full identity and permission control. `compose.yml` publishes none of them (guarded (`4434`), Hydra admin (`4445`) and Keto write (`4467`) authenticate no one — reaching them
by `src/compose.test.ts`); dev publishes only the two ports a browser must reach. Never *is* full identity and permission control. Keto **read** (`4466`) cannot write, but discloses
expose an admin port, and never front one with a proxy that lacks its own auth. the entire authorization graph, so treat it the same. `compose.yml` publishes none of the six
(guarded by `src/compose.test.ts`); dev publishes only the two Ory ports a browser must reach.
Never expose one, and never front one with a proxy that lacks its own auth.
- **Plugins are trusted code**, in-process and unsandboxed — a plugin can do anything the host - **Plugins are trusted code**, in-process and unsandboxed — a plugin can do anything the host
can. Vetting happens when you mount one, not at runtime (AGENTS.md: crash isolation is a can. Vetting happens when you mount one, not at runtime (AGENTS.md: crash isolation is a
deliberate non-goal). deliberate non-goal).
- **Row-level rules belong upstream**, in the service that owns the data — see - **Attribute-based row rules belong upstream**, in the service that owns the data;
[three tiers](#three-tiers-of-may-i). relationship-based ones go to Keto — see [three tiers](#three-tiers-of-may-i).
**The JWT is signed, not encrypted.** Claims are base64: a signed-in user can read their own **The JWT is signed, not encrypted.** Claims are base64: a signed-in user can read their own
`sub`, `email` and `roles`. `HttpOnly` keeps page JavaScript out of the cookie, not the user. `sub`, `email` and `roles`. `HttpOnly` keeps page JavaScript out of the cookie, not the user.
@@ -1028,29 +1039,40 @@ Never put anything in a claim you wouldn't show them.
| Threat | Defense | | Threat | Defense |
| --- | --- | | --- | --- |
| Forged or tampered token | signature verified against the `kid`'s JWKS key; the `alg` allowlist is `RS256`/`ES256` only — **never `HS*` or `none`**, either of which lets an attacker-supplied key verify (`src/auth/jwt.ts`) | | Forged or tampered token | signature verified by `kid`; `alg` allowlist is `RS256`/`ES256` only — **never `HS*` or `none`**, either of which lets a forged token verify (`src/auth/jwt.ts`) |
| `alg` confusion | a key's own `alg` must match the header's, and its type must match the family | | `alg` confusion | a key that pins an `alg` must match the header's; the key type must always match the family |
| Replayed expired token | `exp`/`nbf`, `JWT_CLOCK_SKEW_SEC` leeway | | Replayed expired token | `exp` is mandatory — a token without one is rejected, never treated as eternal; `JWT_CLOCK_SKEW_SEC` leeway |
| Token minted for another deployment | optional `JWT_ISSUER` / `JWT_AUDIENCE` pinning | | Token minted for another deployment | optional `JWT_ISSUER` / `JWT_AUDIENCE` pinning |
| Stolen session cookie | `HttpOnly`, `SameSite=Lax`, `Secure` (`SECURE_COOKIES`), ~10m TTL, plus the [denylist](#instant-revoke-the-optional-denylist) | | Stolen session cookie | `HttpOnly`, `SameSite=Lax`, `Secure` (`SECURE_COOKIES`) — but see the real session lifetime below |
| CSRF on our own forms | signed double-submit token (`src/auth/csrf.ts`) + `SameSite=Lax`; Kratos' flows carry Kratos' own token | | CSRF on our own forms | signed double-submit token, **opt-in per handler** via `ctx.verifyCsrf` (`src/auth/csrf.ts`) + `SameSite=Lax`; Kratos' flows carry Kratos' own token |
| XSS | EJS `<%= %>` escapes; CSP `script-src 'self'` with no `'unsafe-inline'` (`src/http/security-headers.ts`) | | XSS | EJS `<%= %>` escapes; CSP `script-src 'self'` with no `'unsafe-inline'` (`src/http/security-headers.ts`) — the `*.html` slots stay [raw by contract](#routes--handlers) |
| Clickjacking | `frame-ancestors 'none'` + `X-Frame-Options: DENY` | | Clickjacking | `frame-ancestors 'none'` + `X-Frame-Options: DENY` |
| Open redirect via `return_to` | validated host-relative (`localPath`, `src/http/safe-url.ts`) | | Open redirect via `return_to` | validated host-relative (`localPath`, `src/http/safe-url.ts`) |
| Privilege escalation | roles are authored **only** in Keto; the identity projection is a derived read-only cache, re-read from Keto at every mint | | Privilege escalation | roles authored only in Keto, re-read at every mint — see [login & the session JWT](#login-and-the-session-jwt) |
| Downgrade / MIME sniffing | HSTS when `SECURE_COOKIES=true`, `X-Content-Type-Options: nosniff` | | Downgrade / MIME sniffing | HSTS when `SECURE_COOKIES=true`, `X-Content-Type-Options: nosniff` |
| A hung Ory parking requests | `ORY_TIMEOUT_SEC` per outbound call | | A hung Ory parking requests | `ORY_TIMEOUT_SEC` per outbound call |
**Fail closed.** Every rejection converges on two outcomes: a missing, malformed, expired or **The JWT's ~10m TTL is not the session lifetime.** The browser also holds Kratos'
revoked token yields *anonymous* — never a partly-trusted user — and an anonymous or `plainpages_session` cookie (30 days, sliding), and *that* is what silently re-mints a lapsed
under-privileged request is denied (`requireSession` bounces to `/login`; `can`/`check` return JWT. So a stolen cookie jar is worth 30 days of re-mintable access, not ten minutes. Only our
`false`). No verification failure degrades into access. two cookies obey `SECURE_COOKIES`; the Kratos one takes its flags from Kratos' own config.
**Fail closed — with one deliberate exception.** A token that cannot be verified (missing,
malformed, bad signature, wrong `iss`/`aud`) yields *anonymous*, never a partly-trusted user,
and anonymous or under-privileged is denied (`requireSession` bounces to `/login`; `can`/`check`
return `false`). An **expired or revoked** token instead triggers a re-mint against the live
Kratos session — full re-authentication with roles re-read from Keto, or a cleared cookie if
that session is dead; Ory unreachable ⇒ anonymous. Revoking a role therefore *downgrades* a
user promptly without signing them out; **ending a session outright means deactivating or
deleting the identity.**
**Not guaranteed** — accepted, and stated where each mechanism is: role changes **Not guaranteed** — accepted, and stated where each mechanism is: role changes
[lag up to one token TTL and sign-in needs Ory up](#two-trade-offs--both-deliberate), and the [lag up to one token TTL and sign-in needs Ory up](#two-trade-offs--both-deliberate), and the
denylist is [single-instance and skips group changes](#instant-revoke-the-optional-denylist). denylist is [single-instance and skips group changes](#instant-revoke-the-optional-denylist).
Hardening a real deploy is `REQUIRE_SECURE_SECRETS=true` and `SECURE_COOKIES=true` over the Hardening a real deploy is `REQUIRE_SECURE_SECRETS=true`, `SECURE_COOKIES=true`, and replacing
production secrets — see [what you must supply](#what-you-must-supply-the-only-manual-prep). **every** committed dev secret — see
[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 or Postgres' throwaways.
## Email ## Email
@@ -1452,7 +1474,7 @@ container-relative; with the dev bind-mount they edit the real file).
2. **Restart Kratos** so it signs with the new first key: `docker compose restart kratos`. 2. **Restart Kratos** so it signs with the new first key: `docker compose restart kratos`.
(web needs no restart — it hot-reloads the file. The hot path verifies JWTs locally, so a (web needs no restart — it hot-reloads the file. The hot path verifies JWTs locally, so a
brief Kratos blip only touches login/re-mint.) brief Kratos blip only touches login/re-mint.)
3. **Verify** new logins mint the new `kid` — decode the `plainpages_session` cookie's JWT 3. **Verify** new logins mint the new `kid` — decode the `plainpages_jwt` cookie
header, or watch web's logs for a `jwks reload on kid miss` debug line as old clients header, or watch web's logs for a `jwks reload on kid miss` debug line as old clients
present the new key. present the new key.
4. **Wait ~12 min**, then **prune** the superseded key: 4. **Wait ~12 min**, then **prune** the superseded key:
+1 -1
View File
@@ -15,7 +15,7 @@
- [x] CI/CD - When renovate updates a dependency - also release a new version of plainpages based on what got updated with Renovate. Major typescript? New apiVersion + new major. A tiny patch to ejs? Only patch release etc. Before implementing, explain in detail how you will solve this. (`renovate.yml` gains an `auto-release` job (`needs: renovate`) that cuts one `vX.Y.Z` tag per run for what Renovate merged; level = highest `Release-Bump:` trailer Renovate stamps via `commitBody`, any dep's major/minor/patch mapped straight through (default patch). Decoupled from `apiVersion` (tag-only, `HOST_API_VERSION` untouched — a "major" is just a bigger image tag, never a plugin break); pre-1.0 shifts down so nothing auto-crosses into 1.0.0. Pure `auto-release/next-version.ts` + unit tests; tag pushed with renovate-bot's PAT so `release.yml` fires; documented in README → CI/CD.) - [x] CI/CD - When renovate updates a dependency - also release a new version of plainpages based on what got updated with Renovate. Major typescript? New apiVersion + new major. A tiny patch to ejs? Only patch release etc. Before implementing, explain in detail how you will solve this. (`renovate.yml` gains an `auto-release` job (`needs: renovate`) that cuts one `vX.Y.Z` tag per run for what Renovate merged; level = highest `Release-Bump:` trailer Renovate stamps via `commitBody`, any dep's major/minor/patch mapped straight through (default patch). Decoupled from `apiVersion` (tag-only, `HOST_API_VERSION` untouched — a "major" is just a bigger image tag, never a plugin break); pre-1.0 shifts down so nothing auto-crosses into 1.0.0. Pure `auto-release/next-version.ts` + unit tests; tag pushed with renovate-bot's PAT so `release.yml` fires; documented in README → CI/CD.)
- [x] Add an e2e test for the admin plugin's OAuth2-clients (Hydra) screen. The full-flow e2e suite runs without Hydra (compose.full.yml), so /admin/clients register/detail/delete is only unit-covered (src/http/app.test.ts); wire Hydra into an e2e stack and drive the screen in the browser. (compose.full.yml now includes Hydra (`serve all --dev`) and full-flow.spec.ts drives /admin/clients register → one-time secret → list → detail → delete in the browser; documented in README → Testing.) - [x] Add an e2e test for the admin plugin's OAuth2-clients (Hydra) screen. The full-flow e2e suite runs without Hydra (compose.full.yml), so /admin/clients register/detail/delete is only unit-covered (src/http/app.test.ts); wire Hydra into an e2e stack and drive the screen in the browser. (compose.full.yml now includes Hydra (`serve all --dev`) and full-flow.spec.ts drives /admin/clients register → one-time secret → list → detail → delete in the browser; documented in README → Testing.)
- [x] Build and publish docker image as CI/CD. (Duplicate of the CI/CD items above: `ci.yml` builds and pushes `gitea.larvit.se/larvit/plainpages:<commit hash>` behind the green gate, `release.yml` re-tags it to semver and syncs those tags to Docker Hub.) - [x] Build and publish docker image as CI/CD. (Duplicate of the CI/CD items above: `ci.yml` builds and pushes `gitea.larvit.se/larvit/plainpages:<commit hash>` behind the green gate, `release.yml` re-tags it to semver and syncs those tags to Docker Hub.)
- [x] The human developer understands the security model in the auth in this project. (README → Auth → [Security model](README.md#security-model): trust boundaries — browser untrusted, JWT untrusted until verified, the private network as the *only* guard on the unauthenticated Ory admin APIs, plugins trusted and unsandboxed, row rules upstream — plus a threat→defense table, the fail-closed rule, and pointers to the limits that are deliberately not guaranteed. Signed-not-encrypted is called out so nothing secret lands in a claim. Every claim it makes is already enforced by an existing test.) - [x] The human developer understands the security model in the auth in this project. (README → Auth → [Security model](README.md#security-model): trust boundaries — browser untrusted, JWT untrusted until verified, the private network as the *only* guard on the unauthenticated Ory admin APIs, plugins trusted and unsandboxed, row rules upstream — plus a threat→defense table, the fail-closed rule, and pointers to the limits that are deliberately not guaranteed. Signed-not-encrypted is called out so nothing secret lands in a claim, and the JWT's ~10m TTL is separated from the 30-day Kratos session that re-mints it. Every row of the threat table is enforced by an existing test; the trust-boundary bullets are not testable claims. Review also corrected the hardening checklist — `REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`, so the committed Kratos/Hydra/Postgres dev secrets are now listed in "What you must supply".)
- [ ] Add i18n support. - [ ] Add i18n support.
## Architectural review findings (2026-07-02) ## Architectural review findings (2026-07-02)