浏览代码

Security fixes and tests

Efren Yevale Varela 3 周之前
父节点
当前提交
3150922d0a
共有 57 个文件被更改,包括 4600 次插入 和 537 次删除
  1. 1 1
      README.md
  2. 10 7
      docs/ARCHITECTURE.md
  3. 69 3
      docs/CHANGELOG.md
  4. 5 1
      docs/CONFIGURATION.md
  5. 2 1
      docs/DATABASE.md
  6. 13 7
      docs/OAUTH.md
  7. 7 2
      docs/OPERATIONS.md
  8. 12 2
      docs/REDIS.md
  9. 10 6
      docs/SECURITY.md
  10. 3 8
      src/cli/secret.rs
  11. 6 0
      src/config.rs
  12. 20 7
      src/config/build.rs
  13. 25 0
      src/config/tests.rs
  14. 158 41
      src/crypto/tokens.rs
  15. 114 10
      src/crypto/totp.rs
  16. 133 6
      src/crypto/totp/tests.rs
  17. 0 6
      src/jose/client_assertion.rs
  18. 16 10
      src/jose/jar/request_uri.rs
  19. 107 7
      src/jose/jar/verify.rs
  20. 1 2
      src/jose/mod.rs
  21. 34 0
      src/keys.rs
  22. 2 0
      src/protocol/client_auth/assertion.rs
  23. 43 14
      src/protocol/code.rs
  24. 17 0
      src/protocol/code/tests.rs
  25. 1 6
      src/protocol/dcr.rs
  26. 2 7
      src/protocol/dcr/manage.rs
  27. 3 9
      src/protocol/device.rs
  28. 142 3
      src/protocol/par/tests/consume.rs
  29. 18 4
      src/protocol/revoke.rs
  30. 1 0
      src/protocol/revoke/tests.rs
  31. 337 0
      src/protocol/revoke/tests/mtls.rs
  32. 131 53
      src/protocol/token/grants/authorization_code.rs
  33. 19 48
      src/protocol/token/grants/device_code/interval.rs
  34. 2 2
      src/protocol/token/grants/device_code/issue.rs
  35. 1 0
      src/protocol/token/grants/device_code/tests.rs
  36. 75 0
      src/protocol/token/grants/device_code/tests/race.rs
  37. 19 11
      src/protocol/token/grants/refresh_token.rs
  38. 1 0
      src/protocol/token/grants/refresh_token/tests.rs
  39. 135 0
      src/protocol/token/grants/refresh_token/tests/barrier.rs
  40. 239 19
      src/protocol/token/service.rs
  41. 1 0
      src/protocol/token/tests.rs
  42. 40 0
      src/protocol/token/tests/mint_find.rs
  43. 147 0
      src/protocol/token/tests/shape.rs
  44. 55 3
      src/protocol/token/types.rs
  45. 128 8
      src/routes.rs
  46. 119 3
      src/store/codes.rs
  47. 13 2
      src/store/dcr.rs
  48. 71 17
      src/store/device.rs
  49. 68 31
      src/store/grants.rs
  50. 155 11
      src/store/mod.rs
  51. 108 1
      src/store/par.rs
  52. 868 16
      src/store/tests_store.rs
  53. 349 49
      src/store/tokens.rs
  54. 136 74
      src/ui/login.rs
  55. 380 19
      src/ui/login/tests/auth.rs
  56. 15 0
      tests/proptest_rotation.rs
  57. 13 0
      tests/resilience.rs

+ 1 - 1
README.md

@@ -259,7 +259,7 @@ certificate.
 - Redirect URIs: `redirect-uri create|list|enable|disable|delete` and
   `client attach-redirect-uri|detach-redirect-uri|redirect-uris`. Each accepts
   `--kind authorize|post_logout` (default `authorize`); `post_logout` URIs are
-  the return targets for `GET /logout`.
+  the return targets for `POST /logout` (`GET /logout` is read-only).
 - Users and entitlements: `user create|enroll-totp|disable|grant-scope|revoke-scope|scopes`.
 - Admin: `admin bootstrap` (see [RUNBOOK.md](RUNBOOK.md)).
 - Declarative apply: `config sync --file` and `config check`.

+ 10 - 7
docs/ARCHITECTURE.md

@@ -48,16 +48,19 @@ is fixed in `build_router_with_options` in `src/routes.rs`:
 
 1. Request id: mints or propagates `X-Request-Id` and opens the redacted
    `http_request` tracing span. Tracing and redaction live here.
-2. CORS: answers preflights from the explicit allowlist. Absent when the
+2. Security headers: adds `Referrer-Policy`, `X-Content-Type-Options`, a CSP
+   with `frame-ancestors 'none'`, HSTS on the TLS listener, and
+   `Cache-Control: no-store` on every response except `/.well-known/…` metadata.
+   Sits just inside the request-id layer so short-circuiting layers below
+   (CORS preflights, resilience 503s, rate-limit 429/503s, body-limit 413s)
+   still carry the headers.
+3. CORS: answers preflights from the explicit allowlist. Absent when the
    allowlist is empty, which is the default.
-3. Resilience: sheds excess load, caps concurrency, and times out slow
+4. Resilience: sheds excess load, caps concurrency, and times out slow
    handlers before they reach rate limiting.
-4. Rate limit: enforces the per-IP and per-client budgets. Health and metadata
+5. Rate limit: enforces the per-IP and per-client budgets. Health and metadata
    have no rule and pass through; those handlers fail closed on their own.
-5. Body limit: caps request bodies at 2 MiB.
-6. Security headers: adds `Referrer-Policy`, `X-Content-Type-Options`, a CSP
-   with `frame-ancestors 'none'`, HSTS on the TLS listener, and
-   `Cache-Control: no-store` on every response except `/.well-known/…` metadata.
+6. Body limit: caps request bodies at 2 MiB.
 
 A handler that needs a dependency takes `AppState` and calls a repository or the
 store. Errors are converted to `AppError`, which maps to an OAuth error body and

+ 69 - 3
docs/CHANGELOG.md

@@ -25,9 +25,13 @@ While the version stays below `1.0.0`, a minor bump may carry a breaking change.
   expand/dual-write/scale-to-zero-backfill/cutover rollout, and the PII/TOTP
   rotation runbooks across `docs/DATABASE.md`, `docs/SECURITY.md`,
   `docs/CONFIGURATION.md`, and `docs/OPERATIONS.md`.
-- `GET /logout` (a server-specific session endpoint, not an OAuth 2.1 or OIDC
-  feature) ends the browser session so a different user can sign in. It destroys
-  the session and expires the cookie, then renders a neutral page or redirects to
+- `GET /logout` and `POST /logout` (a server-specific session endpoint, not
+  an OAuth 2.1 or OIDC feature) end the browser session so a different user
+  can sign in. `GET /logout` is read-only: it renders a confirm page (live
+  session) or a neutral signed-out page (anonymous) and never destroys the
+  session or writes cookies. Only `POST /logout`, carrying the session-bound
+  CSRF `form_token`, destroys the session and expires the cookie, then renders
+  a neutral page or redirects to
   the flow's registered `post_logout` URI. It accepts no query parameters: the
   redirect target is frozen in session meta when the authorization flow is bound
   and used only while the owning client is active and still registers that exact
@@ -65,6 +69,11 @@ While the version stays below `1.0.0`, a minor bump may carry a breaking change.
   revoke, standalone tokens, and the lockout counter) before flipping
   `users.disabled`; a Redis failure aborts before the flag changes, so the
   command fails closed.
+- `POST /revoke` authenticates mTLS-registered clients (RFC 8705 §2) with the
+  verified peer chain injected by the TLS accept loop, exactly as the token and
+  introspection endpoints do; a missing or unregistered chain fails closed as
+  `invalid_client`. Client certificates are expected to be short-lived, so
+  expiry bounds exposure without a revocation list.
 
 ### Changed
 
@@ -88,6 +97,14 @@ While the version stays below `1.0.0`, a minor bump may carry a breaking change.
   proxy-header fallback is removed.
 - The login lockout now also covers TOTP enrollment confirmation, and the rate
   limiter classifies requests by their full path.
+- Key ids are `String`-backed and preserved verbatim end to end: a `vN` id
+  mixes the 4-byte big-endian generation into the token HMAC message
+  (byte-identical to the pre-change scheme) and any other id mixes its UTF-8
+  bytes, stored as `<kid>.<digest>`, so non-`vN` labels hash distinctly and
+  every `trim/parse::<u32>` collapse site is gone. Back-compat is scoped to
+  `vN` ids: hashes issued under an old `vN` id keep verifying through the
+  retained-key chain (newest-first, capped at 8 generations), so rotation
+  needs no re-issuance.
 
 ### Security
 
@@ -106,6 +123,27 @@ While the version stays below `1.0.0`, a minor bump may carry a breaking change.
   algorithm allowlist.
 - DCR `open` redirect policy now accepts only `https` or RFC 8252 loopback
   URIs, and the SSRF guard pins the fetch to the addresses it pre-validated.
+- Secret-holding token types keep a locked `Debug`: the pepper,
+  `TokenService`, and minted pairs print ids/lengths plus a `[REDACTED]`
+  marker, never raw pepper bytes or token strings, and the located-token
+  wrappers stay transitive through identifier-only records. Positive
+  `[REDACTED]` assertions pin the markers.
+- Regression tests pin the hardening headers (`no-store`, `nosniff`) on
+  every short-circuit path: limiter 429/503, CORS preflights, body-limit
+  413, and resilience 503.
+- The TOTP replay fingerprint is an HMAC under the token pepper instead of
+  unkeyed SHA-256, so a Redis dump no longer exposes the 6-digit code space.
+- JAR request objects are verified against an explicit asymmetric allowlist
+  (`RS256`/`PS256`/`ES256`/`EdDSA`, kept in sync with the `private_key_jwt`
+  assertion allowlist); anything else is rejected before key selection.
+- Token lookup validates the 43-char base64url shape before HMAC and Redis
+  work, so junk tokens are rejected fast. This is a cost guard only:
+  well-formed lookups still retry retained peppers newest-first.
+- `POST /logout` verifies the session-bound CSRF `form_token` and rejects a
+  missing or forged token with `invalid_request` leaving the session intact,
+  so a cross-site POST cannot sign the user out; `GET /logout` is read-only
+  (confirm page, no `Set-Cookie`) and the clearing cookie mirrors every
+  attribute of the session cookie.
 
 ### Fixed
 
@@ -123,6 +161,34 @@ While the version stays below `1.0.0`, a minor bump may carry a breaking change.
 - `GET /login?request_id=…` rebinds an already-authenticated session to the
   fresh authorization request instead of redirecting on the session's stale
   pending binding.
+- Permanent Redis infrastructure failures (`OOM`, `READONLY`, `CROSSSLOT`,
+  auth, and connection-type errors) fail closed as `503
+  temporarily_unavailable` instead of `500`, while programming-error classes
+  stay masked as `server_error` so 500-alerts still fire on real bugs.
+- Dead audit-misleading code is removed: the `client_assertion` JOSE stub
+  (the real logic lives in `protocol::client_auth::assertion`) and the
+  caller-less `store::grants::add_token` helper.
+- Auditor-facing comments corrected: the `jose` module TODO no longer lists
+  the deleted stub, the token shape-gate comment no longer claims to mask
+  generation probing, and the two algorithm allowlists carry keep-in-sync
+  pointers.
+- Device `decide` runs a single conditional-`HSET` Lua script instead of
+  `EXISTS`-then-`HSET`, so a concurrent consume cannot leave a zombie record;
+  the device poll marker is a Lua compare-and-set admitting exactly one poller
+  (the rest get `slow_down`).
+- PAR/code consume binds the ownership check into the consume Lua
+  (`consume_bound` over `client_id`, plus `redirect_uri`/PKCE challenge for
+  codes): a foreign-client or wrong-verifier presentation leaves the single-use
+  record intact instead of burning the victim's value.
+- Grant cascade `revoke` drains in a bounded loop until the member set reads
+  empty, and the rotation script carries a grant-record fence (`NOGRANT`
+  before any write), so a token minted mid-cascade is deleted by a later pass
+  or refused outright.
+- A durability-barrier failure after the originating write returns the minted
+  pair (`RotatedUnbarriered` on rotation) or deletes the won DPoP marker so the
+  originating `jti` can re-claim, instead of locking the client out; other
+  callers stay fail-closed (a `Replay` hit discards its orphan pre-mint and
+  rejects with `invalid_grant`).
 
 ## [0.1.0] - 2026-10-03
 

+ 5 - 1
docs/CONFIGURATION.md

@@ -179,7 +179,11 @@ The exact environment spellings:
 
 The token pepper and the PII field-encryption key are required: without either
 the key ring cannot be built and the server refuses to start with
-`MissingPiiKey` (or `MissingPepper`). The shipped cluster manifests set
+`MissingPiiKey` (or `MissingPepper`). Key ids (`token_pepper_id`,
+`totp_key_id`, `pii_key_id`, and any `*_previous_key_id`) are stored verbatim:
+token hashes as `<kid>.<digest>` (split on the first `.`) and PII blind
+indexes as `<kid>:<digest>`, so an id must contain neither `.` (token split)
+nor `:` (PII ambiguity); keep to `[A-Za-z0-9_-]+`. The shipped cluster manifests set
 `D_OAUTH_TOKEN_PEPPER_FILE`, `D_OAUTH_TOTP_ENCRYPTION_KEY_FILE`, and
 `D_OAUTH_PII_ENCRYPTION_KEY_FILE` rather than the inline keys; see
 [DEPLOYMENT.md](DEPLOYMENT.md).

+ 2 - 1
docs/DATABASE.md

@@ -146,7 +146,8 @@ once every `uri` is NULL.
 
 `kind` separates the two browser landings: `authorize` URIs are valid
 authorization-response callbacks (they carry `code`/`state`); `post_logout` URIs
-are bare navigation targets used only by `GET /logout`. Both kinds are matched
+are bare navigation targets used only by `POST /logout` (`GET /logout` is
+read-only and issues no redirect). Both kinds are matched
 with the same exact-string + RFC 8252 loopback rule.
 
 ### client_redirect_uris

+ 13 - 7
docs/OAUTH.md

@@ -112,15 +112,20 @@ mounts only `/health/live` and `/health/ready`.
 | GET, PUT, DELETE | `/register/{client_id}` | RFC 7592 §2 | Wired |
 
 `GET /logout` and `POST /logout` are a server-specific session endpoint, not a
-feature defined by OAuth 2.1 or OIDC. They destroy the authorization-server
-session and expire the `session_id` cookie. `GET /logout` takes **no query
-parameters** and issues no client-supplied redirect, so a forced navigation
-cannot cause an open redirect; the only redirect it may issue is to a
+feature defined by OAuth 2.1 or OIDC. `GET /logout` is read-only: it renders
+a confirm page (live session) or a neutral signed-out page (anonymous) and
+never destroys the session or writes cookies. Only `POST /logout`, carrying
+the session-bound CSRF `form_token`, destroys the authorization-server
+session and expires the `session_id` cookie (the clearing cookie mirrors the
+session cookie's `Path`/`HttpOnly`/`SameSite`/`Secure`/`Domain`). `GET /logout` takes **no query
+parameters** and issues no redirect at all, so a forced navigation
+cannot cause an open redirect; only `POST /logout` may redirect, and only to a
 `post_logout` URI that was **registered for the client and frozen in session
 meta when the authorization flow was bound**. It redirects there only when the
 owning client is still active and still registers that exact URI (exact-string
 match, with the RFC 8252 loopback exception); otherwise it renders a neutral
-signed-out page. `POST /logout` clears the session and redirects to `GET /login`.
+signed-out page. A `POST /logout` with a live session but no bound redirect
+target renders the neutral signed-out page instead of redirecting.
 This is what lets a client end the browser SSO session so a different user can
 sign in. There is no `post_logout_redirect_uri` parameter, no `id_token_hint`,
 and no `end_session_endpoint` in metadata.
@@ -326,8 +331,9 @@ Reference: RFC 9101 §4.1 and §5.2, RFC 9126 §2 and §3.
   `request_uri` or a signed `request` object. A public client may use the query
   string.
 - A signed request object must be signed, must have `iss` equal to `client_id`,
-  must target the issuer as `aud`, and must not use `none` or a symmetric
-  algorithm. Conflicts between the object and the query string are rejected.
+  must target the issuer as `aud`, and must use `RS256`, `PS256`, `ES256`, or
+  `EdDSA` (`none` and everything else are rejected before key selection).
+  Conflicts between the object and the query string are rejected.
 - A `request_uri` is a one-time, client-bound PAR handle or an exactly
   pre-registered URI. It is fetched only with the SSRF guard: https only, no
   redirects, global IPs only, size and time capped. The connection is pinned to

+ 7 - 2
docs/OPERATIONS.md

@@ -100,8 +100,9 @@ unaffected. Deleting a referenced URI disables it instead.
 
 `kind` selects the URI's role. `authorize` (the default) is an
 authorization-response callback. `post_logout` is a bare return target used only
-by `GET /logout` to send the browser back to the client after the session ends.
-A client may register at most one enabled `post_logout` URI; `GET /logout`
+by `POST /logout` to send the browser back to the client after the session ends
+(`GET /logout` is read-only and issues no redirect).
+A client may register at most one enabled `post_logout` URI; `POST /logout`
 redirects only to it, and only while the URI is enabled and the client active.
 
 ### Declarative config
@@ -372,6 +373,10 @@ survives a Redis loss.
   needed, restrict `pods/exec` to a break-glass role and audit the subresource.
 - Health: liveness `GET /health/live`, readiness `GET /health/ready` on the
   plaintext health listener.
+- Durability barrier: a `RotatedUnbarriered` refresh response means the
+  originating rotation applied but the barrier failed after the write, so the
+  returned pair is the live one; every other barrier failure stays
+  fail-closed (`503`).
 
 ## File and directory references
 

+ 12 - 2
docs/REDIS.md

@@ -39,7 +39,7 @@ differ.
 | `oauth:{state}:client_assertion_jti:<hash>` | `private_key_jwt` `jti` replay marker. | Remaining assertion validity (`exp + 30s skew - now`), clamped `[1s, 360s]` (300s max lifetime + 2×30s skew) |
 | `oauth:{state}:rotation:<request-id>` | Rotation idempotency mapping. | `min(900s, refresh TTL)` |
 | `oauth:{state}:loginfail:<sha256>` | Failed-login counter (hashed label). | `ui::login::LOCKOUT_WINDOW` (15 min) |
-| `oauth:{state}:totp_replay:<fingerprint>` | TOTP used-code replay marker. | Time step window |
+| `oauth:{state}:totp_replay:<fingerprint>` | TOTP used-code replay marker. The fingerprint is an HMAC under the token pepper (never the code itself), so a dump does not reveal the code. | Time step window |
 | `oauth:{rl}:ratelimit:<bucket-hash>` | Fixed-window rate-limit counter. | `rate_limit.window` (60s) |
 
 `<hash>` is always a keyed hash. Tokens are opaque, so the server can look a
@@ -81,12 +81,19 @@ token hash. The script returns one of:
   family.
 - `ABSENT`: unknown token.
 - `MISMATCH`: issued to a different client.
+- `NOGRANT`: the grant record is gone (a mint racing a cascade is fenced
+  before any write, so nothing is stored and no cascade runs).
 
 The old refresh record is not deleted. It becomes a tombstone with a `used`
 flag and a `rotated_to` member, kept for the refresh lifetime so reuse stays
 detectable. The new tokens are minted before the script runs, so a crash during
 the write cannot mint a token that was never stored.
 
+Grant cascade `revoke` drains in a bounded loop: one script pass is atomic but
+blind to a mint racing the cascade, so the pass repeats until the member set
+reads empty and a token minted mid-cascade is deleted by a later pass (or
+refused outright by the rotation `NOGRANT` fence) instead of surviving.
+
 ## The durability barrier
 
 A rotation, tombstone, or replay-marker write is followed by a durability
@@ -101,7 +108,10 @@ token is returned to the client. It is never run inside a Lua script or
 | `appendfsync_always` | Single node | Assert `CONFIG GET appendfsync` is `always`. This is instance-wide, not per-write-path. |
 
 A barrier below the required level fails closed with `503`, mapped from
-`StoreError::Durability`. The server-side wait leaves 500 ms of headroom under
+`StoreError::Durability`. Exception: a refresh rotation whose originating
+write already applied returns the minted pair as `RotatedUnbarriered` instead
+of `503` (any other caller, including a `Replay` hit, stays fail-closed).
+The server-side wait leaves 500 ms of headroom under
 the client response timeout so an expired barrier surfaces as a clean failure
 rather than an ambiguous timeout.
 

+ 10 - 6
docs/SECURITY.md

@@ -164,8 +164,8 @@ Reference: RFC 9126, RFC 9101.
 
 - A confidential client at `/authorize` must use a PAR `request_uri` or a signed
   `request` object. This is mandatory, not optional.
-- A JAR request object must be signed; `none` and symmetric algorithms are
-  rejected. `iss` must equal `client_id`, `aud` must be the issuer, and conflicts
+- A JAR request object must be signed with `RS256`, `PS256`, `ES256`, or `EdDSA`;
+  `none` and everything else are rejected before key selection. `iss` must equal `client_id`, `aud` must be the issuer, and conflicts
   with the query string are rejected.
 - A `request_uri` is one-time and client-bound, or exactly pre-registered, and
   is fetched only through the SSRF guard (https only, no redirects, global IPs
@@ -323,13 +323,17 @@ configure one as fail-open is a load-time error.
   `frame-ancestors 'none'`. They use server-side sessions, CSRF protection, and
   no open redirect.
 - `GET /logout` is a server-specific session endpoint (not OAuth 2.1/OIDC
-  defined). It destroys the session and clears the cookie, then renders a
-  neutral page or redirects to a `post_logout` URI. It accepts **no query
+  defined). It is read-only: it renders a confirm page and never destroys the
+  session or writes cookies, so a forced navigation cannot log a victim out.
+  Only `POST /logout` with the session-bound CSRF `form_token` destroys the
+  session and clears the cookie. It accepts **no query
   parameters**: the target is read only from session meta, frozen when the
   authorization flow was bound, and used only if the owning client is still
   active and still registers that exact URI (exact-string match + RFC 8252
-  loopback). A forced navigation to `/logout` can therefore only end a session
-  and, at most, send the browser to that client's own pre-registered URI — never
+  loopback). `GET /logout` issues no redirect; only `POST /logout` redirects,
+  and only to that bound URI. A forced navigation to `/logout` therefore only
+  renders the confirm page — it never ends the session and never leaves the
+  page, so it cannot log a victim out nor send the browser
   to an attacker target. There is no `post_logout_redirect_uri` parameter and no
   `id_token_hint`.
 - Failed-login and lockout counters live in Redis, not Postgres, so a login

+ 3 - 8
src/cli/secret.rs

@@ -78,8 +78,8 @@ pub fn hash_password(config: &Config, password: &str) -> Result<String, CliError
 
 /// Keyed token hash for at-rest storage under the active pepper.
 ///
-/// The numeric generation is derived from the configured key id (`v1` → `1`)
-/// so rotated peppers retain distinct storage strings.
+/// The configured key id is preserved verbatim, so rotated peppers retain
+/// distinct storage strings.
 ///
 /// # Errors
 /// Returns [`CliError::Crypto`] when no active pepper is configured.
@@ -88,9 +88,8 @@ pub fn token_hash(config: &Config, token: &str) -> Result<String, CliError> {
     .keys
     .active_pepper()
     .ok_or_else(|| CliError::Crypto("no active token pepper configured".to_owned()))?;
-  let generation = numeric_key_id(key_id.as_str());
   let hash = crate::crypto::tokens::token_hash(
-    &crate::crypto::tokens::KeyId::new(generation),
+    &crate::crypto::tokens::KeyId::new(key_id.as_str()),
     pepper.expose_secret().as_bytes(),
     token,
   );
@@ -466,10 +465,6 @@ impl hkdf::KeyType for HkdfAesKey {
   }
 }
 
-fn numeric_key_id(key_id: &str) -> u32 {
-  key_id.trim_start_matches('v').parse::<u32>().unwrap_or(1)
-}
-
 fn pem_encode(label: &str, der: &[u8]) -> String {
   let encoded = STANDARD.encode(der);
   let mut out = format!("-----BEGIN {label}-----\n");

+ 6 - 0
src/config.rs

@@ -322,6 +322,12 @@ pub enum ConfigError {
   /// distinct previous key id.
   #[error("previous key requires an explicit distinct previous key id")]
   MissingPreviousKeyId,
+  /// A key id contained characters outside `[A-Za-z0-9_-]`.
+  #[error("invalid key id: {id}")]
+  InvalidKeyId {
+    /// The rejected value.
+    id: String,
+  },
 }
 
 /// Read an optional secret from a mounted file, trimming trailing newlines.

+ 20 - 7
src/config/build.rs

@@ -178,9 +178,9 @@ fn build_key_ring(raw: &RawConfig) -> Result<KeyRing, ConfigError> {
     )?),
     "pii_previous_key",
   )?;
-  let pepper_id = key_id_or_default(raw.token_pepper_id.as_deref());
-  let totp_id = key_id_or_default(raw.totp_key_id.as_deref());
-  let pii_id = key_id_or_default(raw.pii_key_id.as_deref());
+  let pepper_id = key_id_or_default(raw.token_pepper_id.as_deref())?;
+  let totp_id = key_id_or_default(raw.totp_key_id.as_deref())?;
+  let pii_id = key_id_or_default(raw.pii_key_id.as_deref())?;
   let peppers = pepper
     .map(|secret| vec![(pepper_id, secret)])
     .unwrap_or_default();
@@ -219,7 +219,7 @@ fn active_first_chain(
         .map(str::trim)
         .filter(|value| !value.is_empty())
         .ok_or(ConfigError::MissingPreviousKeyId)?;
-      let id = KeyId::new(id_raw);
+      let id = validate_key_id(id_raw)?;
       if id == active_pair.0 {
         return Err(ConfigError::MissingPreviousKeyId);
       }
@@ -245,10 +245,23 @@ fn resolve_secret(
   }
 }
 
-fn key_id_or_default(raw: Option<&str>) -> KeyId {
+fn key_id_or_default(raw: Option<&str>) -> Result<KeyId, ConfigError> {
   match raw.map(str::trim).filter(|value| !value.is_empty()) {
-    Some(value) => KeyId::new(value),
-    None => KeyId::new(DEFAULT_KEY_ID),
+    Some(value) => validate_key_id(value),
+    None => Ok(KeyId::new(DEFAULT_KEY_ID)),
+  }
+}
+
+/// Reject a configured key id outside `[A-Za-z0-9_-]+`: `.` would collide
+/// with the `<kid>.<digest>` storage format, and empty/whitespace values are
+/// never valid ids (absent ids fall back to the default above).
+fn validate_key_id(value: &str) -> Result<KeyId, ConfigError> {
+  if crate::keys::is_valid_key_id(value) {
+    Ok(KeyId::new(value))
+  } else {
+    Err(ConfigError::InvalidKeyId {
+      id: value.to_owned(),
+    })
   }
 }
 

+ 25 - 0
src/config/tests.rs

@@ -550,6 +550,31 @@ fn config_treats_empty_previous_key_as_absent() -> Result<(), Box<dyn Error>> {
   Ok(())
 }
 
+#[test]
+fn config_rejects_dotted_key_id() {
+  for key in ["token_pepper_id", "totp_key_id", "pii_key_id"] {
+    let mut map = base();
+    map.insert(key.to_owned(), Value::String("v1.evil".to_owned()));
+    assert!(
+      matches!(try_load(&map), Err(ConfigError::InvalidKeyId { .. })),
+      "{key} must reject a dotted key id"
+    );
+  }
+  let mut previous = base();
+  previous.insert(
+    "pii_previous_key".to_owned(),
+    Value::String("SUPER_SECRET_PII_PREVIOUS-0123456789abcdef".to_owned()),
+  );
+  previous.insert(
+    "pii_previous_key_id".to_owned(),
+    Value::String("v0.evil".to_owned()),
+  );
+  assert!(matches!(
+    try_load(&previous),
+    Err(ConfigError::InvalidKeyId { .. })
+  ));
+}
+
 #[test]
 fn read_secret_file_preserves_surrounding_spaces() -> Result<(), Box<dyn Error>> {
   use secrecy::ExposeSecret as _;

+ 158 - 41
src/crypto/tokens.rs

@@ -38,35 +38,53 @@ pub enum TokenError {
   /// The operating system RNG failed.
   #[error("operating-system randomness unavailable")]
   UnavailableRandomness,
-  /// A stored hash did not parse as `v<key-id>.<base64url-digest>`.
+  /// A stored hash did not parse as `<key-id>.<base64url-digest>`.
   #[error("malformed stored token hash")]
   MalformedHash,
 }
 
 /// Pepper-generation identifier selecting the HMAC key for [`token_hash`].
 ///
+/// The full configured key id string is preserved verbatim (`v1`, `v2`, or any
+/// custom label). Ids of the form `v<N>` mix the 4-byte big-endian generation
+/// number into the HMAC message (the pre-string-id encoding, so stored hashes
+/// keep verifying); any other id mixes its UTF-8 bytes, so two generations
+/// hash differently even when their peppers match. The id is never trimmed.
+///
 /// Bumping the id while retaining the old pepper keeps previously issued
 /// hashes verifiable via [`verify_token_hash`].
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct KeyId(u32);
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct KeyId(String);
 
 impl KeyId {
-  /// Build a generation identifier.
+  /// Build a generation identifier, preserving the id verbatim.
   #[must_use]
-  pub const fn new(id: u32) -> Self {
-    Self(id)
+  pub fn new(id: impl Into<String>) -> Self {
+    Self(id.into())
   }
 
-  /// The raw generation number.
+  /// Borrow the identifier as a string slice.
   #[must_use]
-  pub const fn value(self) -> u32 {
-    self.0
+  pub fn as_str(&self) -> &str {
+    &self.0
+  }
+}
+
+impl From<&str> for KeyId {
+  fn from(value: &str) -> Self {
+    Self::new(value)
+  }
+}
+
+impl From<String> for KeyId {
+  fn from(value: String) -> Self {
+    Self::new(value)
   }
 }
 
 impl core::fmt::Display for KeyId {
   fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-    write!(f, "{}", self.0)
+    f.write_str(&self.0)
   }
 }
 
@@ -74,7 +92,7 @@ impl core::fmt::Display for KeyId {
 ///
 /// Holds the issuing [`KeyId`] plus the 32-byte HMAC-SHA256 digest. Never
 /// contains the raw token.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct TokenHash {
   key_id: KeyId,
   digest: [u8; 32],
@@ -83,8 +101,8 @@ pub struct TokenHash {
 impl TokenHash {
   /// The key id this hash was computed under.
   #[must_use]
-  pub const fn key_id(self) -> KeyId {
-    self.key_id
+  pub fn key_id(&self) -> &KeyId {
+    &self.key_id
   }
 
   /// The raw 32-byte digest. Compare with [`ct_eq`], never `==`.
@@ -93,21 +111,26 @@ impl TokenHash {
     &self.digest
   }
 
-  /// Encode for storage as `v<key-id>.<base64url-nopad-digest>`.
+  /// Encode for storage as `<key-id>.<base64url-nopad-digest>`.
   #[must_use]
-  pub fn to_storage_string(self) -> String {
-    format!("v{}.{}", self.key_id, URL_SAFE_NO_PAD.encode(self.digest))
+  pub fn to_storage_string(&self) -> String {
+    format!("{}.{}", self.key_id, URL_SAFE_NO_PAD.encode(self.digest))
   }
 
   /// Parse [`TokenHash::to_storage_string`] output.
   ///
+  /// The key id is taken verbatim (everything before the first `.`); only an
+  /// empty id or a malformed digest is rejected.
+  ///
   /// # Errors
   ///
   /// Returns [`TokenError::MalformedHash`] for any deviation from the format.
   pub fn parse_storage_string(s: &str) -> Result<Self, TokenError> {
-    let rest = s.strip_prefix('v').ok_or(TokenError::MalformedHash)?;
-    let (id, digest) = rest.split_once('.').ok_or(TokenError::MalformedHash)?;
-    let key_id = KeyId::new(id.parse::<u32>().map_err(|_| TokenError::MalformedHash)?);
+    let (id, digest) = s.split_once('.').ok_or(TokenError::MalformedHash)?;
+    if id.is_empty() {
+      return Err(TokenError::MalformedHash);
+    }
+    let key_id = KeyId::new(id.to_owned());
     let raw = URL_SAFE_NO_PAD
       .decode(digest)
       .map_err(|_| TokenError::MalformedHash)?;
@@ -154,15 +177,38 @@ fn hmac_key(pepper: &[u8]) -> Zeroizing<[u8; HMAC_BLOCK_BYTES]> {
   key
 }
 
+/// Domain-separation prefix for [`token_hash`]: the 4-byte big-endian
+/// generation for ids matching `v<N>`, `None` for any other id (which mixes
+/// its UTF-8 bytes instead). An out-of-`u32`-range suffix falls back to the
+/// string encoding; such ids cannot verify against legacy rows, but the hash
+/// stays deterministic.
+fn numeric_key_id_prefix(id: &str) -> Option<[u8; 4]> {
+  let digits = id.strip_prefix('v')?;
+  if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) {
+    return None;
+  }
+  digits.parse::<u32>().ok().map(|n| n.to_be_bytes())
+}
+
 /// Keyed hash of `token` under (`key_id`, `pepper`).
 ///
-/// The key id is domain-mixed into the message, so the same token hashes
-/// differently per generation even if a pepper were reused. Deterministic
-/// per (`key_id`, `pepper`, `token`); the raw token is never retained.
+/// A `v<N>` id mixes the 4-byte big-endian generation number ahead of the
+/// token bytes (byte-identical to the pre-string-id scheme); any other id
+/// mixes its UTF-8 bytes. Deterministic per (`key_id`, `pepper`, `token`);
+/// the raw token is never retained.
+///
+/// The returned [`TokenHash`] owns a clone of `key_id`: ids are short
+/// (`v<N>`) so this is one small allocation, cheaper than reference-counting
+/// across every [`verify_token_hash`] retry.
 #[must_use]
 pub fn token_hash(key_id: &KeyId, pepper: &[u8], token: &str) -> TokenHash {
-  let mut msg = Zeroizing::new(Vec::with_capacity(4 + token.len()));
-  msg.extend_from_slice(&key_id.value().to_be_bytes());
+  let id = key_id.as_str();
+  let prefix = numeric_key_id_prefix(id);
+  let mut msg = Zeroizing::new(Vec::with_capacity(4.max(id.len()) + token.len()));
+  match prefix {
+    Some(bytes) => msg.extend_from_slice(&bytes),
+    None => msg.extend_from_slice(id.as_bytes()),
+  }
   msg.extend_from_slice(token.as_bytes());
   let key = hmac_key(pepper);
   let mac_key: &hmac::digest::Key<Hmac<Sha256>> = (&*key).into();
@@ -172,7 +218,7 @@ pub fn token_hash(key_id: &KeyId, pepper: &[u8], token: &str) -> TokenHash {
   let mut digest = [0u8; 32];
   digest.copy_from_slice(&out);
   TokenHash {
-    key_id: *key_id,
+    key_id: key_id.clone(),
     digest,
   }
 }
@@ -231,16 +277,16 @@ mod crypto_tests {
 
   #[test]
   fn crypto_token_hash_deterministic_per_key_id() {
-    let id = KeyId::new(3);
+    let id = KeyId::new("v3");
     let first = token_hash(&id, b"pepper-one", "token-abc");
     let second = token_hash(&id, b"pepper-one", "token-abc");
     assert_eq!(first, second);
-    assert_eq!(first.key_id(), id);
+    assert_eq!(first.key_id(), &id);
   }
 
   #[test]
   fn crypto_token_hash_differs_per_pepper() {
-    let id = KeyId::new(3);
+    let id = KeyId::new("v3");
     let a = token_hash(&id, b"pepper-one", "token-abc");
     let b = token_hash(&id, b"pepper-two", "token-abc");
     assert_ne!(a, b);
@@ -248,14 +294,23 @@ mod crypto_tests {
 
   #[test]
   fn crypto_token_hash_differs_per_key_id() {
-    let a = token_hash(&KeyId::new(1), b"same-pepper", "token-abc");
-    let b = token_hash(&KeyId::new(2), b"same-pepper", "token-abc");
+    let a = token_hash(&KeyId::new("v1"), b"same-pepper", "token-abc");
+    let b = token_hash(&KeyId::new("v2"), b"same-pepper", "token-abc");
     assert_ne!(a, b);
   }
 
+  #[test]
+  fn crypto_token_hash_differs_per_non_numeric_key_id() {
+    let a = token_hash(&KeyId::new("pepper-2026"), b"same-pepper", "token-abc");
+    let b = token_hash(&KeyId::new("v1"), b"same-pepper", "token-abc");
+    assert_ne!(a, b);
+    assert_eq!(a.key_id().as_str(), "pepper-2026");
+    assert!(a.to_storage_string().starts_with("pepper-2026."));
+  }
+
   #[test]
   fn crypto_token_hash_matches_library_hmac() -> Result<(), TokenError> {
-    let id = KeyId::new(7);
+    let id = KeyId::new("v7");
     let ours = token_hash(
       &id,
       b"a-much-longer-pepper-than-thirty-two-bytes-for-sure-0123456789",
@@ -265,19 +320,36 @@ mod crypto_tests {
       b"a-much-longer-pepper-than-thirty-two-bytes-for-sure-0123456789",
     )
     .map_err(|_| TokenError::MalformedHash)?;
-    mac.update(&id.value().to_be_bytes());
+    mac.update(&7u32.to_be_bytes());
     mac.update(b"t");
     let out = mac.finalize().into_bytes();
     assert_eq!(ours.digest(), out.as_slice());
     Ok(())
   }
 
+  #[test]
+  fn crypto_token_hash_legacy_v1_known_answer() {
+    // Pre-change scheme mixed the numeric generation as 4-byte big-endian
+    // (`v1` -> `[0, 0, 0, 1]`) ahead of the token bytes. Stored hashes
+    // (Postgres DCR registration hashes, Redis token/code/device/PAR hashes)
+    // must keep verifying, so this vector pins the legacy `v1` digest.
+    let hash = token_hash(
+      &KeyId::new("v1"),
+      b"0123456789abcdef0123456789abcdef",
+      "kat-legacy-v1-token",
+    );
+    assert_eq!(
+      URL_SAFE_NO_PAD.encode(hash.digest()),
+      "GPdRQqgBMQi_frpfRECk3H0dBQZndDsxxxQczlg3IqI"
+    );
+  }
+
   #[test]
   fn crypto_old_key_id_verifies_during_rotation() {
-    let old = (KeyId::new(1), b"old-pepper".as_slice());
-    let new = (KeyId::new(2), b"new-pepper".as_slice());
+    let old = (KeyId::new("v1"), b"old-pepper".as_slice());
+    let new = (KeyId::new("v2"), b"new-pepper".as_slice());
     let stored = token_hash(&old.0, old.1, "refresh-token-xyz");
-    let keys = [new, old];
+    let keys = [new.clone(), old];
     assert!(verify_token_hash(&stored, &keys, "refresh-token-xyz"));
     assert!(!verify_token_hash(&stored, &keys, "wrong-token"));
     assert!(!verify_token_hash(&stored, &[new], "refresh-token-xyz"));
@@ -285,13 +357,13 @@ mod crypto_tests {
 
   #[test]
   fn crypto_verify_is_bounded_to_retained_keys() {
-    let id = KeyId::new(99);
+    let id = KeyId::new("v99");
     let stored = token_hash(&id, b"pepper-99", "bounded-token");
     let mut keys: Vec<(KeyId, &[u8])> = Vec::with_capacity(MAX_RETAINED_KEYS + 1);
     for _ in 0..MAX_RETAINED_KEYS {
-      keys.push((KeyId::new(1), b"decoy".as_slice()));
+      keys.push((KeyId::new("v1"), b"decoy".as_slice()));
     }
-    keys.push((id, b"pepper-99".as_slice()));
+    keys.push((id.clone(), b"pepper-99".as_slice()));
     assert!(!verify_token_hash(&stored, &keys, "bounded-token"));
     let mut head: Vec<(KeyId, &[u8])> = vec![(id, b"pepper-99".as_slice())];
     head.extend(keys.into_iter().take(MAX_RETAINED_KEYS - 1));
@@ -301,7 +373,7 @@ mod crypto_tests {
   #[test]
   fn crypto_hash_never_exposes_raw_token() {
     let raw = "super-secret-token-value";
-    let hash = token_hash(&KeyId::new(1), b"pepper", raw);
+    let hash = token_hash(&KeyId::new("v1"), b"pepper", raw);
     let stored = hash.to_storage_string();
     assert!(!stored.contains(raw));
     assert!(!stored.contains(&raw[..8]));
@@ -311,16 +383,61 @@ mod crypto_tests {
 
   #[test]
   fn crypto_storage_string_round_trip() -> Result<(), TokenError> {
-    let hash = token_hash(&KeyId::new(12), b"pepper", "round-trip-token");
+    let hash = token_hash(&KeyId::new("v12"), b"pepper", "round-trip-token");
     let parsed = TokenHash::parse_storage_string(&hash.to_storage_string())?;
     assert_eq!(parsed, hash);
     Ok(())
   }
 
+  #[test]
+  fn crypto_storage_string_preserves_non_numeric_id() -> Result<(), TokenError> {
+    let hash = token_hash(&KeyId::new("pepper-2026"), b"pepper", "round-trip-token");
+    let stored = hash.to_storage_string();
+    assert!(stored.starts_with("pepper-2026."));
+    let parsed = TokenHash::parse_storage_string(&stored)?;
+    assert_eq!(parsed, hash);
+    assert_eq!(parsed.key_id().as_str(), "pepper-2026");
+    Ok(())
+  }
+
   #[test]
   fn crypto_storage_string_rejects_malformed() {
-    for bad in ["", "1.abc", "v.abc", "vx.abc", "v1.!!!", "v1.aGk"] {
+    for bad in ["", ".abc", "1.abc", "v.abc", "vx.abc", "v1.!!!", "v1.aGk"] {
       assert!(TokenHash::parse_storage_string(bad).is_err(), "{bad}");
     }
   }
+
+  #[test]
+  fn crypto_token_hash_leading_zero_id_matches_v1() {
+    // `v01` parses to generation 1, so its digest must equal `v1`; the id is
+    // preserved verbatim, so the storage strings must differ.
+    let plain = token_hash(&KeyId::new("v1"), b"pepper", "pin-token");
+    let padded = token_hash(&KeyId::new("v01"), b"pepper", "pin-token");
+    assert_eq!(padded.digest(), plain.digest());
+    assert_ne!(padded.to_storage_string(), plain.to_storage_string());
+    assert!(padded.to_storage_string().starts_with("v01."));
+    assert!(plain.to_storage_string().starts_with("v1."));
+  }
+
+  #[test]
+  fn crypto_token_hash_overflow_suffix_is_deterministic() -> Result<(), TokenError> {
+    // `v4294967296` overflows `u32`, so it falls back to the UTF-8 string
+    // encoding: deterministic, distinct from `v1`, and byte-identical to the
+    // library HMAC over the id bytes plus the token bytes.
+    let id = KeyId::new("v4294967296");
+    let first = token_hash(&id, b"pepper", "pin-token");
+    let second = token_hash(&id, b"pepper", "pin-token");
+    assert_eq!(first, second);
+    let v1 = token_hash(&KeyId::new("v1"), b"pepper", "pin-token");
+    assert_ne!(first.digest(), v1.digest());
+    let mut mac =
+      Hmac::<Sha256>::new_from_slice(b"pepper").map_err(|_| TokenError::MalformedHash)?;
+    mac.update(b"v4294967296");
+    mac.update(b"pin-token");
+    let out = mac.finalize().into_bytes();
+    assert_eq!(first.digest(), out.as_slice());
+    let parsed = TokenHash::parse_storage_string(&first.to_storage_string())?;
+    assert_eq!(parsed, first);
+    Ok(())
+  }
 }

+ 114 - 10
src/crypto/totp.rs

@@ -243,17 +243,54 @@ pub fn verify_window(
 
 /// Domain-separated fingerprint binding a used code to its owner and step.
 ///
-/// The fingerprint (never the code) becomes the Redis replay-marker key.
+/// HMAC-SHA256 under a token pepper over the domain-separated
+/// message, so the Redis replay marker reveals nothing to brute force: a
+/// dump of the marker key cannot be reversed into the 6-digit code within
+/// its TTL without the pepper. The fingerprint (never the code) becomes the
+/// Redis replay-marker key. Markers are stored under the active pepper;
+/// verification probes every retained pepper generation newest-first (see
+/// [`replay_fingerprints`] and [`verify_and_claim_with_ring`]), so a code
+/// claimed before a rotation still counts as a replay after it within the
+/// 180 s claim window.
+///
+/// The HMAC key schedule is RFC 2104 §2 verbatim (long peppers are hashed,
+/// short peppers zero-padded), infallible by construction, mirroring
+/// `store::hmac_sha256` without depending on `store`. It is byte-equivalent
+/// with `Hmac::<Sha256>::new_from_slice` over the same message.
 #[must_use]
-pub fn replay_fingerprint(user_id: &str, code: &str, step: u64) -> String {
-  let mut hasher = Sha256::new();
-  hasher.update(b"totp-replay-v1|");
-  hasher.update(user_id.as_bytes());
-  hasher.update(b"|");
-  hasher.update(code.as_bytes());
-  hasher.update(b"|");
-  hasher.update(step.to_be_bytes());
-  hex_bytes(&hasher.finalize())
+pub fn replay_fingerprint(pepper: &[u8], user_id: &str, code: &str, step: u64) -> String {
+  use hmac::digest::{KeyInit, Mac as _};
+  use sha2::digest::Key;
+  let mut block = Zeroizing::new([0u8; 64]);
+  if pepper.len() > block.len() {
+    let hashed = Sha256::digest(pepper);
+    block[..hashed.len()].copy_from_slice(&hashed);
+  } else {
+    block[..pepper.len()].copy_from_slice(pepper);
+  }
+  let key: &Key<hmac::Hmac<Sha256>> = (&*block).into();
+  let mut mac = <hmac::Hmac<Sha256> as KeyInit>::new(key);
+  mac.update(b"totp-replay-v1|");
+  mac.update(user_id.as_bytes());
+  mac.update(b"|");
+  mac.update(code.as_bytes());
+  mac.update(b"|");
+  mac.update(&step.to_be_bytes());
+  hex_bytes(&mac.finalize().into_bytes())
+}
+
+/// Fingerprints for one code under every retained pepper generation,
+/// newest-first (one entry per [`KeyRing::peppers_newest_first`] item).
+///
+/// Callers store new markers under the first entry (the active pepper) and
+/// treat any entry as a replay hit, so a pepper rotation inside the claim
+/// window cannot reopen a consumed code.
+#[must_use]
+pub fn replay_fingerprints(keys: &KeyRing, user_id: &str, code: &str, step: u64) -> Vec<String> {
+  keys
+    .peppers_newest_first()
+    .map(|(_, secret)| replay_fingerprint(secret.expose_secret().as_bytes(), user_id, code, step))
+    .collect()
 }
 
 /// Atomically claim a replay-marker key (`SET key 1 PX ttl NX`).
@@ -316,6 +353,73 @@ pub async fn verify_and_claim(
   }
 }
 
+/// One TOTP presentation to verify and claim across pepper generations.
+pub struct TotpClaim<'a> {
+  /// Owner label bound into the replay fingerprint (user id string).
+  pub user_id: &'a str,
+  /// Stored base32 secret to verify against.
+  pub secret_b32: &'a str,
+  /// Presented code (whitespace trimmed before fingerprinting).
+  pub code: &'a str,
+  /// Current Unix time selecting the ±1-step window.
+  pub now_unix: u64,
+  /// How long a consumed code stays claimed.
+  pub ttl: Duration,
+}
+
+/// Verify `code` and claim it across every retained pepper generation.
+///
+/// The matched step's fingerprint is computed under each pepper newest-first
+/// (see [`replay_fingerprints`]); any hit is a replay. New markers are stored
+/// under the active pepper only. `replay_key` maps a fingerprint hex to the
+/// full Redis key (store prefix). Returns `false` for a wrong code AND for a
+/// replayed one; callers must not distinguish the two.
+///
+/// Ordering (F7): older generations are probed read-only (`EXISTS`) before
+/// the active generation is claimed atomically (`SET key 1 PX ttl NX`). This
+/// order is deliberate. The only shared write is the single `SET NX` on the
+/// active key, so concurrent presentations of the same code collapse there:
+/// exactly one observes `Fresh`, every other observes `Replay`, and no
+/// interleaving can accept the same code twice. Probing older generations
+/// first also avoids writing a spurious active marker when the code was
+/// already claimed before a pepper rotation (the pre-rotation fingerprint
+/// still hits within the claim window). Reversing the order — claim first,
+/// then probe — would widen that race and leave junk markers on replays.
+///
+/// # Errors
+///
+/// Returns [`TotpError::InvalidSecret`] for a malformed stored secret,
+/// [`TotpError::Encrypt`] when the ring holds no pepper, and
+/// [`TotpError::Store`] when the replay store fails.
+pub async fn verify_and_claim_with_ring(
+  conn: &mut impl redis::aio::ConnectionLike,
+  keys: &KeyRing,
+  replay_key: impl Fn(&str) -> String,
+  claim: &TotpClaim<'_>,
+) -> Result<bool, TotpError> {
+  let Some(step) = verify_window(claim.secret_b32, claim.code, claim.now_unix)? else {
+    return Ok(false);
+  };
+  let trimmed = claim.code.trim();
+  let fingerprints = replay_fingerprints(keys, claim.user_id, trimmed, step);
+  let Some(active) = fingerprints.first() else {
+    return Err(TotpError::Encrypt);
+  };
+  for fingerprint in fingerprints.iter().skip(1) {
+    let exists: i64 = redis::cmd("EXISTS")
+      .arg(replay_key(fingerprint))
+      .query_async(conn)
+      .await?;
+    if exists != 0 {
+      return Ok(false);
+    }
+  }
+  match claim_code(conn, &replay_key(active), claim.ttl).await? {
+    Claim::Fresh => Ok(true),
+    Claim::Replay => Ok(false),
+  }
+}
+
 /// Encrypt a TOTP secret under the active TOTP-encryption key.
 ///
 /// The writer is version-gated for the rolling upgrade: `envelope = false`

+ 133 - 6
src/crypto/totp/tests.rs

@@ -6,7 +6,8 @@ use secrecy::{ExposeSecret as _, SecretString};
 use super::{
   RECOVERY_CODE_COUNT, TOTP_BLOB_VERSION, TOTP_STEP_SECS, TotpError, build_totp, decrypt_secret,
   derive_key, derive_key_legacy, encrypt_secret, generate_recovery_codes, generate_secret,
-  normalize_recovery_code, otpauth_uri, qr_svg, replay_fingerprint, verify_window,
+  normalize_recovery_code, otpauth_uri, qr_svg, replay_fingerprint, replay_fingerprints,
+  verify_window,
 };
 use crate::{
   error::{AppError, OAuthErrorCode},
@@ -178,23 +179,41 @@ fn login_totp_recovery_codes_shape_and_normalize() -> Result<(), TotpError> {
 
 #[test]
 fn login_totp_replay_fingerprint_binds_owner_code_and_step() {
-  let base = replay_fingerprint("user-1", "287082", 1);
+  let pepper = b"test-pepper-0123456789abcdef";
+  let base = replay_fingerprint(pepper, "user-1", "287082", 1);
   assert_eq!(base.len(), 64, "fingerprint must be sha256 hex");
   let variants = [
-    replay_fingerprint("user-2", "287082", 1),
-    replay_fingerprint("user-1", "287083", 1),
-    replay_fingerprint("user-1", "287082", 2),
+    replay_fingerprint(pepper, "user-2", "287082", 1),
+    replay_fingerprint(pepper, "user-1", "287083", 1),
+    replay_fingerprint(pepper, "user-1", "287082", 2),
   ];
   for other in variants {
     assert_ne!(other, base, "fingerprint must bind owner, code, and step");
   }
   assert_eq!(
-    replay_fingerprint("user-1", "287082", 1),
+    replay_fingerprint(pepper, "user-1", "287082", 1),
     base,
     "fingerprint must be deterministic"
   );
 }
 
+/// T3-A5: the replay fingerprint is HMAC-keyed under the token pepper, so a
+/// Redis dump cannot brute-force the 6-digit code within the marker TTL.
+#[test]
+fn login_totp_replay_fingerprint_is_keyed_by_pepper() {
+  let first = replay_fingerprint(b"pepper-one-0123456789abcdef", "user-1", "287082", 1);
+  let second = replay_fingerprint(b"pepper-two-0123456789abcdef", "user-1", "287082", 1);
+  assert_ne!(
+    first, second,
+    "different peppers must yield different fingerprints for the same user|code|step"
+  );
+  assert_eq!(
+    first,
+    replay_fingerprint(b"pepper-one-0123456789abcdef", "user-1", "287082", 1),
+    "fingerprint must be deterministic per pepper"
+  );
+}
+
 #[test]
 fn login_totp_generated_code_verifies_in_window() -> Result<(), TotpError> {
   let secret = generate_secret()?;
@@ -446,3 +465,111 @@ fn totp_error_maps_fail_closed() {
   assert_eq!(qr.code(), OAuthErrorCode::ServerError);
   assert_eq!(qr.status(), StatusCode::INTERNAL_SERVER_ERROR);
 }
+
+/// HMAC known-answer test: `replay_fingerprint` must equal
+/// `Hmac::<Sha256>::new_from_slice` over the same domain-separated message,
+/// for both a short pepper and a long (>64 B block) pepper exercising the
+/// RFC 2104 hashed-key branch.
+#[test]
+fn login_totp_replay_hmac_kat_matches_library() -> Result<(), TotpError> {
+  use hmac::{Hmac, Mac as _, digest::KeyInit};
+  use sha2::Sha256;
+  fn expected(pepper: &[u8], user_id: &str, code: &str, step: u64) -> Result<String, TotpError> {
+    let mut mac = Hmac::<Sha256>::new_from_slice(pepper).map_err(|_| TotpError::Encrypt)?;
+    mac.update(b"totp-replay-v1|");
+    mac.update(user_id.as_bytes());
+    mac.update(b"|");
+    mac.update(code.as_bytes());
+    mac.update(b"|");
+    mac.update(&step.to_be_bytes());
+    let out = mac.finalize().into_bytes();
+    let mut hex = String::with_capacity(64);
+    for b in out {
+      hex.push_str(&format!("{b:02x}"));
+    }
+    Ok(hex)
+  }
+  for pepper in [
+    b"short-pepper".as_slice(),
+    b"a-much-longer-pepper-than-sixty-four-bytes-for-sure-0123456789abcdef0123456789".as_slice(),
+  ] {
+    assert!(
+      pepper.len() != 64 || pepper.len() > 64,
+      "test must cover both key-schedule branches"
+    );
+    assert_eq!(
+      replay_fingerprint(pepper, "user-1", "287082", 1),
+      expected(pepper, "user-1", "287082", 1)?,
+      "fingerprint must match library HMAC"
+    );
+  }
+  // The long-pepper branch specifically (>64 B) must hash the key first.
+  let long =
+    b"a-much-longer-pepper-than-sixty-four-bytes-for-sure-0123456789abcdef0123456789".as_slice();
+  assert!(long.len() > 64, "long pepper must exceed the SHA-256 block");
+  assert_eq!(
+    replay_fingerprint(long, "user-1", "287082", 1),
+    expected(long, "user-1", "287082", 1)?,
+  );
+  Ok(())
+}
+
+/// Rotation replay: a marker stored under pepper N must be detected after the
+/// active rotates to N+1, because the check spans every retained generation
+/// while new markers are stored under the active pepper only.
+#[test]
+fn login_totp_replay_spans_pepper_generations() -> Result<(), TotpError> {
+  let old_ring = KeyRing::from_parts(
+    vec![(
+      KeyId::new("v1"),
+      SecretString::from("old-pepper-0123456789abcdef"),
+    )],
+    vec![(
+      KeyId::new("v1"),
+      SecretString::from("test-totp-key-0123456789abcdef"),
+    )],
+    vec![(
+      KeyId::new("v1"),
+      SecretString::from("test-pii-key-0123456789abcdef"),
+    )],
+  )
+  .map_err(|_| TotpError::Encrypt)?;
+  let new_ring = KeyRing::from_parts(
+    vec![
+      (
+        KeyId::new("v2"),
+        SecretString::from("new-pepper-0123456789abcdef"),
+      ),
+      (
+        KeyId::new("v1"),
+        SecretString::from("old-pepper-0123456789abcdef"),
+      ),
+    ],
+    vec![(
+      KeyId::new("v1"),
+      SecretString::from("test-totp-key-0123456789abcdef"),
+    )],
+    vec![(
+      KeyId::new("v1"),
+      SecretString::from("test-pii-key-0123456789abcdef"),
+    )],
+  )
+  .map_err(|_| TotpError::Encrypt)?;
+  let old_fp = replay_fingerprints(&old_ring, "user-1", "287082", 1);
+  let rotated = replay_fingerprints(&new_ring, "user-1", "287082", 1);
+  assert_eq!(
+    old_fp.len(),
+    1,
+    "single-generation ring yields one fingerprint"
+  );
+  assert_eq!(rotated.len(), 2, "rotated ring spans both generations");
+  assert!(
+    rotated.contains(&old_fp[0]),
+    "post-rotation check must still cover the pre-rotation fingerprint"
+  );
+  assert_ne!(
+    rotated[0], old_fp[0],
+    "active fingerprint must differ after rotation"
+  );
+  Ok(())
+}

+ 0 - 6
src/jose/client_assertion.rs

@@ -1,6 +0,0 @@
-//! `private_key_jwt` client authentication.
-//!
-//! Client authentication ONLY: it produces no `cnf` and never satisfies a
-//! sender-constraint binding on its own.
-//!
-//! TODO(task 18): implement assertion verification and client lookup.

+ 16 - 10
src/jose/jar/request_uri.rs

@@ -57,6 +57,10 @@ fn is_registered_request_uri(client: &Client, uri: &str) -> bool {
 
 /// Consume a PAR-issued `request_uri` one-time, bound to `client`.
 ///
+/// The expected client travels inside the consume script: a foreign presenter
+/// observes `Mismatched` and the victim record survives for its owner, so a
+/// cross-client presentation can never burn the single-use value.
+///
 /// # Errors
 ///
 /// Returns `invalid_request` for an empty, unknown, consumed, or
@@ -72,17 +76,19 @@ async fn resolve_par(
   if id.is_empty() {
     return Err(AppError::invalid_request("request_uri is malformed"));
   }
-  let record = par::consume(conn, store, id).await?;
-  let Some(record) = record else {
-    return Err(AppError::invalid_request(
-      "request_uri is unknown, expired, or already used",
-    ));
+  let record = match par::consume_bound(conn, store, id, client.client_id.as_str()).await? {
+    par::ConsumeOutcome::Consumed(record) => record,
+    par::ConsumeOutcome::Mismatched => {
+      return Err(AppError::invalid_request(
+        "request_uri was issued to a different client",
+      ));
+    }
+    par::ConsumeOutcome::Absent => {
+      return Err(AppError::invalid_request(
+        "request_uri is unknown, expired, or already used",
+      ));
+    }
   };
-  if record.client_id != client.client_id {
-    return Err(AppError::invalid_request(
-      "request_uri was issued to a different client",
-    ));
-  }
   let stored: Value = serde_json::from_str(&record.request).map_err(AppError::internal)?;
   let Some(object) = stored.as_object() else {
     return Err(AppError::internal(std::io::Error::new(

+ 107 - 7
src/jose/jar/verify.rs

@@ -1,11 +1,12 @@
 //! Signed request-object verification against the client JWKS.
 //!
-//! Rejects `alg:none` and symmetric algorithms, selects the verification
+//! Rejects `alg:none` and every algorithm outside the `RS256`/`PS256`/
+//! `ES256`/`EdDSA` allowlist, selects the verification
 //! key by `kid` (or tries every key when unpinned), and enforces
 //! `iss`/`aud`/`exp` through `jsonwebtoken` with zero clock-skew leeway.
 
 use jsonwebtoken::{
-  Algorithm, AlgorithmFamily, DecodingKey, Validation, decode, decode_header,
+  Algorithm, DecodingKey, Validation, decode, decode_header,
   jwk::{Jwk, JwkSet},
 };
 use serde_json::Value;
@@ -15,12 +16,21 @@ use crate::{db::client_repo::Client, error::AppError};
 /// `exp`/`nbf` clock-skew leeway in seconds. Zero: strict expiry.
 const LEEWAY_SECS: u64 = 0;
 
+/// Allowed request-object signing algorithms (RFC 9101 §4.1 asymmetric
+/// only; mirrors `protocol::client_auth::assertion::ALLOWED_ALGS`).
+const ALLOWED_ALGS: [Algorithm; 4] = [
+  Algorithm::RS256,
+  Algorithm::PS256,
+  Algorithm::ES256,
+  Algorithm::EdDSA,
+];
+
 /// Verify a signed request object and return its claim map.
 ///
 /// # Errors
 ///
 /// Returns `invalid_request_object` for an unparsable header, `alg:none`,
-/// symmetric algorithms, unknown `kid`, signature failure, or `iss`/`aud`/
+/// a non-allowlisted algorithm, unknown `kid`, signature failure, or `iss`/`aud`/
 /// `exp` violations.
 pub(crate) fn verify_request_object(
   client: &Client,
@@ -30,11 +40,12 @@ pub(crate) fn verify_request_object(
   let header = decode_header(jwt)
     .map_err(|_| AppError::invalid_request_object("request object header is malformed"))?;
   // Unsigned objects (`alg:none` fails header parsing above) and every
-  // symmetric algorithm are rejected: client secrets are Argon2 hashes and
-  // can never serve as HMAC keys, so no `oct` key is ever usable here.
-  if header.alg.family() == AlgorithmFamily::Hmac {
+  // algorithm outside the asymmetric allowlist are rejected before key
+  // selection: client secrets are Argon2 hashes and can never serve as
+  // HMAC keys, so no `oct` key is ever usable here.
+  if !ALLOWED_ALGS.contains(&header.alg) {
     return Err(AppError::invalid_request_object(
-      "symmetric request objects are not accepted",
+      "request object signing algorithm is not allowed",
     ));
   }
   let set = client
@@ -94,3 +105,92 @@ fn key_alg_compatible(key: &Jwk, alg: Algorithm) -> bool {
     }
   }
 }
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+  use crate::error::OAuthErrorCode;
+  use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
+
+  /// Test body result (no `unwrap`/`expect` under the deny lints).
+  type TestResult = Result<(), TestError>;
+
+  /// Errors raised by the `jar::verify` test scaffolding itself.
+  #[derive(Debug, thiserror::Error)]
+  enum TestError {
+    /// The verifier rejected (or accepted) unexpectedly.
+    #[error("verifier error")]
+    App(#[from] AppError),
+    /// A test assertion observed an unexpected value.
+    #[error("unexpected test value: {0}")]
+    Unexpected(String),
+  }
+
+  fn fixture() -> Client {
+    Client {
+      id: uuid::Uuid::nil(),
+      client_id: "jar-verify-fixture".to_owned(),
+      client_secret_expires_at: None,
+      client_id_issued_at: time::OffsetDateTime::UNIX_EPOCH,
+      client_name: "JAR Verify Fixture".to_owned(),
+      grant_types: Vec::new(),
+      response_types: Vec::new(),
+      scope: String::new(),
+      token_endpoint_auth_method: "none".to_owned(),
+      jwks: None,
+      jwks_uri: None,
+      tls_client_auth_subject_dn: None,
+      tls_client_auth_cert_thumbprint: None,
+      request_uris: None,
+      contacts: None,
+      software_id: None,
+      dpop_bound_access_tokens: false,
+      active: true,
+      created_at: time::OffsetDateTime::UNIX_EPOCH,
+      updated_at: time::OffsetDateTime::UNIX_EPOCH,
+    }
+  }
+
+  fn forged_jwt(alg: &str) -> String {
+    let header =
+      URL_SAFE_NO_PAD.encode(serde_json::json!({ "alg": alg, "typ": "JWT" }).to_string());
+    let payload =
+      URL_SAFE_NO_PAD.encode(serde_json::json!({ "iss": "jar-verify-fixture" }).to_string());
+    format!("{header}.{payload}.AAAA")
+  }
+
+  fn assert_rejected_at_allowlist(jwt: &str, alg: &str) -> TestResult {
+    let client = fixture();
+    match verify_request_object(&client, "https://issuer.example.test", jwt) {
+      Ok(_) => Err(TestError::Unexpected(format!(
+        "{alg} request object was accepted"
+      ))),
+      Err(err) => {
+        assert_eq!(err.code(), OAuthErrorCode::InvalidRequestObject);
+        if err.description().contains("algorithm is not allowed") {
+          Ok(())
+        } else {
+          Err(TestError::Unexpected(format!(
+            "{alg} unexpected description: {}",
+            err.description()
+          )))
+        }
+      }
+    }
+  }
+
+  #[test]
+  fn jar_verify_rejects_hs256_request_object() -> TestResult {
+    assert_rejected_at_allowlist(&forged_jwt("HS256"), "HS256")
+  }
+
+  #[test]
+  fn jar_verify_rejects_non_allowlisted_asymmetric_alg() -> TestResult {
+    for alg in [
+      "RS384", "RS512", "PS384", "PS512", "ES384", "HS384", "HS512",
+    ] {
+      assert_rejected_at_allowlist(&forged_jwt(alg), alg)?;
+    }
+    Ok(())
+  }
+}

+ 1 - 2
src/jose/mod.rs

@@ -3,9 +3,8 @@
 //! Single verifier (`jsonwebtoken`) for every spec-signed artifact. The AS
 //! signs nothing itself, so there is no AS signing key and no `jwks_uri`.
 //!
-//! TODO(task 20): wire re-exports for `dpop`/`jar`/`client_assertion`/`jwk`.
+//! TODO(task 20): wire re-exports for `dpop`/`jar`/`jwk`.
 
-pub mod client_assertion;
 pub mod dpop;
 pub mod jar;
 pub mod jwk;

+ 34 - 0
src/keys.rs

@@ -53,6 +53,19 @@ impl KeyId {
   }
 }
 
+/// Whether `id` is storage-safe: non-empty `[A-Za-z0-9_-]+`.
+///
+/// A `.` would collide with the `<kid>.<digest>` storage format and split
+/// stored hashes at the wrong boundary, so it is rejected here and at config
+/// load; empty and whitespace-only values never reach this check as valid.
+#[must_use]
+pub fn is_valid_key_id(id: &str) -> bool {
+  !id.is_empty()
+    && id
+      .bytes()
+      .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')
+}
+
 impl fmt::Display for KeyId {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     f.write_str(&self.0)
@@ -128,6 +141,12 @@ pub enum KeyRingError {
   /// A key id was empty.
   #[error("key id must not be empty")]
   EmptyKeyId,
+  /// A key id contained characters outside `[A-Za-z0-9_-]`.
+  #[error("invalid key id: {id}")]
+  InvalidKeyId {
+    /// The rejected value.
+    id: String,
+  },
   /// The same key id was supplied twice within one purpose chain.
   #[error("duplicate key id: {id}")]
   DuplicateKeyId {
@@ -240,6 +259,11 @@ fn build_chain(
     if id.is_empty() {
       return Err(KeyRingError::EmptyKeyId);
     }
+    if !is_valid_key_id(id.as_str()) {
+      return Err(KeyRingError::InvalidKeyId {
+        id: id.as_str().to_owned(),
+      });
+    }
     if out.iter().any(|existing| existing.id() == &id) {
       return Err(KeyRingError::DuplicateKeyId {
         id: id.as_str().to_owned(),
@@ -366,6 +390,16 @@ mod tests {
     Ok(())
   }
 
+  #[test]
+  fn keyring_rejects_dotted_ids() {
+    let dotted = KeyRing::from_parts(
+      vec![pepper("v1.evil", "a")],
+      vec![pepper("v1", "b")],
+      vec![pepper("v1", "c")],
+    );
+    assert!(matches!(dotted, Err(KeyRingError::InvalidKeyId { .. })));
+  }
+
   #[test]
   fn keygen_purpose_documents_no_as_signing_key() {
     assert!(KEYGEN_PURPOSE.contains("never an AS signing key"));

+ 2 - 0
src/protocol/client_auth/assertion.rs

@@ -26,6 +26,8 @@ use crate::{
 
 /// Allowed client-assertion signing algorithms (matches the metadata
 /// `token_endpoint_auth_signing_alg_values_supported` allowlist).
+/// Mirrored by `crate::jose::jar::verify` (`ALLOWED_ALGS`): keep the two
+/// allowlists in sync when adding or removing an algorithm.
 pub(crate) const ALLOWED_ALGS: [Algorithm; 4] = [
   Algorithm::RS256,
   Algorithm::PS256,

+ 43 - 14
src/protocol/code.rs

@@ -22,6 +22,7 @@ use std::{fmt, time::Duration};
 
 use secrecy::ExposeSecret;
 use uuid::Uuid;
+use zeroize::Zeroizing;
 
 use crate::{
   config::TtlConfig,
@@ -141,16 +142,35 @@ impl From<CodeIssueError> for AppError {
 }
 
 /// One retained pepper generation, newest-first.
-#[derive(Debug, Clone)]
+///
+/// The raw pepper never reaches `Debug`: only the generation, label, and
+/// material length are printed (the [`KeyRing`] ids-only pattern).
+#[derive(Clone)]
 struct PepperKey {
-  /// Numeric generation for [`crypto_tokens::token_hash`].
+  /// Verbatim generation for [`crypto_tokens::token_hash`].
   generation: crypto_tokens::KeyId,
-  /// Raw pepper bytes.
-  secret: Vec<u8>,
+  /// String label (`v<N>`) for audit.
+  label: String,
+  /// Raw pepper bytes, wiped on drop.
+  secret: Zeroizing<Vec<u8>>,
+}
+
+impl fmt::Debug for PepperKey {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    f.debug_struct("PepperKey")
+      .field("generation", &self.generation)
+      .field("label", &self.label)
+      .field("secret", &"[REDACTED]")
+      .field("secret_len", &self.secret.len())
+      .finish()
+  }
 }
 
 /// Authorization-code minting plus hash-at-rest persistence.
-#[derive(Debug, Clone)]
+///
+/// `Debug` prints the lifetime plus the retained pepper ids and material
+/// lengths only, never raw pepper bytes.
+#[derive(Clone)]
 pub struct CodeIssuer {
   /// Code lifetime, capped at [`CODE_TTL_CAP`] at construction.
   ttl: Duration,
@@ -159,6 +179,19 @@ pub struct CodeIssuer {
   keys: Vec<PepperKey>,
 }
 
+impl fmt::Debug for CodeIssuer {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    let key_labels: Vec<&str> = self.keys.iter().map(|key| key.label.as_str()).collect();
+    let secret_lens: Vec<usize> = self.keys.iter().map(|key| key.secret.len()).collect();
+    f.debug_struct("CodeIssuer")
+      .field("ttl", &self.ttl)
+      .field("key_labels", &key_labels)
+      .field("secret", &"[REDACTED]")
+      .field("secret_lens", &secret_lens)
+      .finish()
+  }
+}
+
 impl CodeIssuer {
   /// Build an issuer from the validated TTLs and key ring.
   ///
@@ -171,14 +204,10 @@ impl CodeIssuer {
       .peppers_newest_first()
       .take(crypto_tokens::MAX_RETAINED_KEYS)
     {
-      let generation = id
-        .as_str()
-        .trim_start_matches('v')
-        .parse::<u32>()
-        .unwrap_or(1);
       keys.push(PepperKey {
-        generation: crypto_tokens::KeyId::new(generation),
-        secret: secret.expose_secret().as_bytes().to_vec(),
+        generation: crypto_tokens::KeyId::new(id.as_str()),
+        label: id.as_str().to_owned(),
+        secret: Zeroizing::new(secret.expose_secret().as_bytes().to_vec()),
       });
     }
     if keys.is_empty() {
@@ -217,8 +246,8 @@ impl CodeIssuer {
     let cnf = resolve_cnf(request.client_kind, request.dpop_jkt)?;
     let raw = crypto_tokens::mint_token()?;
     let active = self.keys.first().ok_or(CodeIssueError::NoPepper)?;
-    let code_hash =
-      crypto_tokens::token_hash(&active.generation, &active.secret, &raw).to_storage_string();
+    let code_hash = crypto_tokens::token_hash(&active.generation, active.secret.as_slice(), &raw)
+      .to_storage_string();
     let grant_id = Uuid::new_v4().to_string();
     let record = codes::CodeRecord {
       client_id: request.client_id.to_owned(),

+ 17 - 0
src/protocol/code/tests.rs

@@ -396,6 +396,23 @@ async fn code_issue_ttl_clamped_to_sixty_seconds() -> TestResult {
   Ok(())
 }
 
+#[test]
+fn code_issuer_debug_redacts_pepper() -> TestResult {
+  let issuer = CodeIssuer::new(&TtlConfig::default(), &test_ring()?)?;
+  let rendered = format!("{issuer:?}");
+  if rendered.contains("code-issue-pepper") {
+    return Err(TestError::Unexpected(
+      "Debug must never print raw pepper bytes".to_owned(),
+    ));
+  }
+  if !rendered.contains("[REDACTED]") {
+    return Err(TestError::Unexpected(
+      "Debug must mark pepper material as [REDACTED]".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
 #[tokio::test]
 async fn code_issue_debug_redacts_raw_code() -> TestResult {
   let (_guard, url) = redis_container().await?;

+ 1 - 6
src/protocol/dcr.rs

@@ -693,13 +693,8 @@ fn keyed_hash(config: &Config, token: &str) -> Result<String, AppError> {
     .keys
     .active_pepper()
     .ok_or_else(AppError::server_error)?;
-  let generation = key_id
-    .as_str()
-    .trim_start_matches('v')
-    .parse::<u32>()
-    .unwrap_or(1);
   let hash = tokens_crypto::token_hash(
-    &tokens_crypto::KeyId::new(generation),
+    &tokens_crypto::KeyId::new(key_id.as_str()),
     pepper.expose_secret().as_bytes(),
     token,
   );

+ 2 - 7
src/protocol/dcr/manage.rs

@@ -201,20 +201,15 @@ fn verify_registration_token(config: &Config, stored: &str, presented: &str) ->
     .keys
     .peppers_newest_first()
     .map(|(id, secret)| {
-      let generation = id
-        .as_str()
-        .trim_start_matches('v')
-        .parse::<u32>()
-        .unwrap_or(1);
       (
-        tokens_crypto::KeyId::new(generation),
+        tokens_crypto::KeyId::new(id.as_str()),
         secret.expose_secret().as_bytes().to_vec(),
       )
     })
     .collect();
   let candidates: Vec<(tokens_crypto::KeyId, &[u8])> = owned
     .iter()
-    .map(|(id, secret)| (*id, secret.as_slice()))
+    .map(|(id, secret)| (id.clone(), secret.as_slice()))
     .collect();
   tokens_crypto::verify_token_hash(&parsed, &candidates, presented)
 }

+ 3 - 9
src/protocol/device.rs

@@ -157,19 +157,13 @@ pub fn binding_key(store: &Store, device_hash: &str) -> String {
 
 /// Versioned keyed hash of a raw code, for use as a store key.
 ///
-/// Mirrors the CLI hashing convention: the numeric generation comes from the
-/// active key id (`v1` → `1`, unparseable → `1`), so rotated peppers keep
-/// distinct storage strings.
+/// Mirrors the CLI hashing convention: the active key id is preserved verbatim,
+/// so rotated peppers keep distinct storage strings.
 pub fn hash_token(keys: &crate::keys::KeyRing, token: &str) -> Result<String, AppError> {
   let (id, pepper) = keys.active_pepper().ok_or_else(AppError::server_error)?;
-  let generation = id
-    .as_str()
-    .trim_start_matches('v')
-    .parse::<u32>()
-    .unwrap_or(1);
   Ok(
     tokens::token_hash(
-      &tokens::KeyId::new(generation),
+      &tokens::KeyId::new(id.as_str()),
       pepper.expose_secret().as_bytes(),
       token,
     )

+ 142 - 3
src/protocol/par/tests/consume.rs

@@ -147,9 +147,8 @@ async fn par_authorize_binds_client_and_rejects_replay() -> Result<(), TestError
     Err(error) => expect_code(&error, OAuthErrorCode::InvalidRequest)?,
   }
 
-  // The failed foreign attempt consumed the record (single-use Lua): a fresh
-  // push is required for the legitimate first use.
-  let request_uri = push_public(&mut h, &endpoint, client_a, redirect_uri).await?;
+  // The foreign attempt must not burn the victim record: the legitimate
+  // first use resolves the same `request_uri`.
   let client_row_a = client_repo::find_active_by_client_id(&h.pool, &h.keys, client_a)
     .await?
     .ok_or_else(|| TestError::Unexpected("client A missing".to_owned()))?;
@@ -189,6 +188,146 @@ async fn par_authorize_binds_client_and_rejects_replay() -> Result<(), TestError
   Ok(())
 }
 
+#[tokio::test]
+async fn par_foreign_presentation_does_not_consume_victim_record() -> Result<(), TestError> {
+  let (_guard, mut h) = harness().await?;
+  let client_a = "par-victim-client";
+  let client_b = "par-foreign-client";
+  let redirect_uri = "https://client.example.test/cb";
+  insert_client(
+    &h.pool,
+    &h.keys,
+    client_a,
+    TokenEndpointAuthMethod::None,
+    None,
+  )
+  .await?;
+  insert_client(
+    &h.pool,
+    &h.keys,
+    client_b,
+    TokenEndpointAuthMethod::None,
+    None,
+  )
+  .await?;
+  attach_redirect(&h.pool, &h.keys, client_a, redirect_uri).await?;
+  attach_redirect(&h.pool, &h.keys, client_b, redirect_uri).await?;
+
+  let endpoint = h.endpoint.clone();
+  let request_uri = push_public(&mut h, &endpoint, client_a, redirect_uri).await?;
+  let issuer = "https://auth.example.test";
+
+  let client_row_b = client_repo::find_active_by_client_id(&h.pool, &h.keys, client_b)
+    .await?
+    .ok_or_else(|| TestError::Unexpected("client B missing".to_owned()))?;
+  let query_b = HashMap::from([
+    ("client_id".to_owned(), client_b.to_owned()),
+    ("request_uri".to_owned(), request_uri.clone()),
+  ]);
+  let foreign = JarRequest {
+    client: &client_row_b,
+    issuer,
+    query: &query_b,
+    request: None,
+    request_uri: Some(request_uri.as_str()),
+  };
+  match resolve(&mut h.conn, &h.store, &foreign).await {
+    Ok(_) => {
+      return Err(TestError::Unexpected(
+        "cross-client request_uri accepted".to_owned(),
+      ));
+    }
+    Err(error) => expect_code(&error, OAuthErrorCode::InvalidRequest)?,
+  }
+
+  let client_row_a = client_repo::find_active_by_client_id(&h.pool, &h.keys, client_a)
+    .await?
+    .ok_or_else(|| TestError::Unexpected("client A missing".to_owned()))?;
+  let query_a = HashMap::from([
+    ("client_id".to_owned(), client_a.to_owned()),
+    ("request_uri".to_owned(), request_uri.clone()),
+  ]);
+  let legitimate = JarRequest {
+    client: &client_row_a,
+    issuer,
+    query: &query_a,
+    request: None,
+    request_uri: Some(request_uri.as_str()),
+  };
+  resolve(&mut h.conn, &h.store, &legitimate)
+    .await
+    .map_err(|error| {
+      TestError::Unexpected(format!(
+        "victim record burned by foreign use: {}",
+        error.code()
+      ))
+    })?;
+  Ok(())
+}
+
+#[tokio::test]
+async fn par_consume_bound_transport_error_recovery() -> Result<(), TestError> {
+  let (_guard, mut h) = harness().await?;
+  let client_id = "par-recovery-client";
+  let record = par::ParRecord {
+    client_id: client_id.to_owned(),
+    request: "{\"response_type\":\"code\"}".to_owned(),
+  };
+  par::put(
+    &mut h.conn,
+    &h.store,
+    "par-recovery-id",
+    &record,
+    Duration::from_secs(300),
+  )
+  .await
+  .map_err(|error| TestError::Unexpected(format!("{error}")))?;
+  // Poison the key type through the public key layout: the guarded `HGETALL`
+  // script fails indeterminately on a string key, the recovery re-read sees
+  // the surviving key, and the consume fails closed without deleting it.
+  let key = h.store.par_key("par-recovery-id");
+  let _: () = redis::cmd("DEL")
+    .arg(&key)
+    .query_async(&mut h.conn)
+    .await
+    .map_err(TestError::Redis)?;
+  let _: () = redis::cmd("SET")
+    .arg(&key)
+    .arg("poison")
+    .query_async(&mut h.conn)
+    .await
+    .map_err(TestError::Redis)?;
+  match par::consume_bound(&mut h.conn, &h.store, "par-recovery-id", client_id).await {
+    Err(_) => {}
+    Ok(_) => {
+      return Err(TestError::Unexpected(
+        "indeterminate consume with a surviving key must fail closed".to_owned(),
+      ));
+    }
+  }
+  let present: i64 = redis::cmd("EXISTS")
+    .arg(&key)
+    .query_async(&mut h.conn)
+    .await
+    .map_err(TestError::Redis)?;
+  if present != 1 {
+    return Err(TestError::Unexpected(
+      "failed consume must leave the surviving key intact".to_owned(),
+    ));
+  }
+  // An absent key reports `Absent` through the same public API: no record,
+  // no error, never a replay.
+  match par::consume_bound(&mut h.conn, &h.store, "par-recovery-absent", client_id).await {
+    Ok(par::ConsumeOutcome::Absent) => Ok(()),
+    Ok(_) => Err(TestError::Unexpected(
+      "absent PAR record must report Absent".to_owned(),
+    )),
+    Err(error) => Err(TestError::Unexpected(format!(
+      "absent consume error: {error}"
+    ))),
+  }
+}
+
 #[tokio::test]
 async fn par_expired_request_uri_is_rejected() -> Result<(), TestError> {
   let (_guard, mut h) = harness().await?;

+ 18 - 4
src/protocol/revoke.rs

@@ -20,7 +20,7 @@
 use std::collections::HashMap;
 
 use axum::{
-  Form, Json,
+  Extension, Form, Json,
   extract::State,
   http::{HeaderMap, StatusCode, header},
   response::{IntoResponse, Response},
@@ -42,6 +42,7 @@ use crate::{
   },
   state::{AppState, RedisConn},
   store::Store,
+  tls::PeerCertificates,
 };
 
 /// Supported `token_type_hint` value for access tokens (RFC 7009 §2.1).
@@ -236,8 +237,20 @@ struct ErrorBody {
 /// `POST /revoke` (RFC 7009 §2.1).
 ///
 /// Credentials come from the `Authorization` header and the form body only; a
-/// secret in the query string is never consulted. All well-formed requests
-/// answer `200 OK`, including unknown or foreign tokens.
+/// secret in the query string is never consulted. mTLS-registered clients
+/// (RFC 8705 §2) authenticate with `client_id` plus the verified peer chain
+/// injected by the TLS accept loop, exactly as the token and introspection
+/// endpoints do; a missing or unregistered chain fails closed as
+/// `invalid_client`. No forwarded header is ever trusted: only the in-process
+/// [`PeerCertificates`] extension counts as mTLS evidence.
+///
+/// Certificate revocation story: client certificates are expected to be
+/// short-lived, so expiry bounds exposure without any revocation list; a
+/// CRL/OCSP check at authentication time is the hook point if deployments
+/// later need explicit revocation (verify freshness there, fail closed on
+/// outage, never cache a good verdict past the response's validity window).
+/// All well-formed requests answer `200 OK`, including unknown or foreign
+/// tokens.
 ///
 /// # Errors
 ///
@@ -246,6 +259,7 @@ struct ErrorBody {
 /// `token_type_hint` is rendered as `400 unsupported_token_type`.
 pub async fn revoke_handler(
   State(state): State<AppState>,
+  peer_certs: Option<Extension<PeerCertificates>>,
   headers: HeaderMap,
   Form(form): Form<HashMap<String, String>>,
 ) -> Result<Response, AppError> {
@@ -260,7 +274,7 @@ pub async fn revoke_handler(
       .map(|secret| SecretString::from(secret.as_str())),
     assertion_type: form.get("client_assertion_type").map(String::as_str),
     assertion: form.get("client_assertion").map(String::as_str),
-    peer_certs: None,
+    peer_certs: peer_certs.as_deref(),
   };
   let mut redis = state.redis.clone();
   let store = Store::from_config(&state.config.redis);

+ 1 - 0
src/protocol/revoke/tests.rs

@@ -7,6 +7,7 @@
 
 mod auth;
 mod cascade;
+mod mtls;
 
 use std::{
   sync::{

+ 337 - 0
src/protocol/revoke/tests/mtls.rs

@@ -0,0 +1,337 @@
+//! mTLS revocation: a registered certificate self-revokes over the verified
+//! peer chain; an unknown certificate fails closed as `invalid_client`.
+
+use std::sync::{Arc, atomic::AtomicBool};
+
+use axum::{
+  Router,
+  body::Body,
+  http::{Request, StatusCode, header},
+  routing::post,
+};
+use rustls::pki_types::{CertificateDer, pem::PemObject};
+use tower::ServiceExt as _;
+use uuid::Uuid;
+
+use super::{
+  TestError, TestResult, mint_request, now_unix, service, test_keys, test_redis, unique,
+};
+use crate::{
+  db::client_repo::{self, NewClient},
+  protocol::{mtls, revoke::revoke_handler},
+  state::{AppState, Clock, RedisConn},
+  tls::PeerCertificates,
+};
+
+/// Self-signed client fixture (`CN=mtls-test-client, O=Example`).
+const CLIENT_SELF_PEM: &str = include_str!("../../../../tests/mtls/certs/client-self-cert.pem");
+/// Unregistered self-signed client (`CN=rogue, O=Example`).
+const CLIENT_ROGUE_PEM: &str = include_str!("../../../../tests/mtls/certs/client-rogue-cert.pem");
+
+/// Decode the first certificate from a PEM fixture.
+fn cert_der(pem: &str) -> Result<CertificateDer<'static>, TestError> {
+  CertificateDer::from_pem_slice(pem.as_bytes())
+    .map_err(|error| TestError::Unexpected(format!("fixture PEM: {error}")))
+}
+
+/// Register a `self_signed_tls_client_auth` client pinned to `thumbprint`.
+async fn insert_mtls_client(pool: &sqlx::PgPool, client_id: &str, thumbprint: &str) -> TestResult {
+  let keys = test_keys()?;
+  let new = NewClient {
+    id: Uuid::new_v4(),
+    client_id: client_id.to_owned(),
+    client_secret_hash: None,
+    client_secret_expires_at: None,
+    client_name: format!("{client_id} name"),
+    grant_types: vec!["authorization_code".to_owned()],
+    response_types: vec!["code".to_owned()],
+    scope: String::new(),
+    token_endpoint_auth_method: "self_signed_tls_client_auth".to_owned(),
+    jwks: None,
+    jwks_uri: None,
+    tls_client_auth_subject_dn: None,
+    tls_client_auth_cert_thumbprint: Some(thumbprint.to_owned()),
+    request_uris: None,
+    contacts: None,
+    software_id: None,
+    dpop_bound_access_tokens: false,
+    registration_access_token_hash: None,
+  };
+  client_repo::insert(pool, &keys, false, &new).await?;
+  Ok(())
+}
+
+/// Handler-level [`AppState`] over a real pool and Redis handle.
+fn app_state(pool: sqlx::PgPool, redis: RedisConn) -> Result<AppState, TestError> {
+  let fixture = serde_json::json!({
+    "issuer": "https://auth.example.test",
+    "environment": "development",
+    "tls": {"cert_path": "/tmp/revoke-test-cert.pem", "key_path": "/tmp/revoke-test-key.pem"},
+    "database": {"url": "postgres://127.0.0.1:1/revoke"},
+    "redis": {"url": "redis://127.0.0.1:1", "durability": "waitaof"},
+    "token_pepper": "revoke-test-pepper-0123456789abcdef",
+    "totp_encryption_key": "revoke-test-totp-key-0123456789abcdef",
+    "pii_encryption_key": "revoke-test-pii-key-0123456789abcdef",
+  });
+  let source = ::config::Config::try_from(&fixture)
+    .map_err(|error| TestError::Unexpected(format!("config fixture: {error:?}")))?;
+  let config = crate::config::Config::from_source(source)
+    .map_err(|error| TestError::Unexpected(format!("config fixture: {error:?}")))?;
+  Ok(AppState {
+    pool,
+    redis,
+    config: Arc::new(config),
+    keys: Arc::new(test_keys()?),
+    transition_unsafe: Arc::new(AtomicBool::new(false)),
+    clock: Clock::system(),
+  })
+}
+
+/// A `POST /revoke` carrying `token` + `client_id` with `certs` injected as
+/// the verified peer chain, exactly as the TLS accept loop provides it.
+async fn revoke_with_certs(
+  state: AppState,
+  token: &str,
+  client_id: &str,
+  certs: PeerCertificates,
+) -> Result<StatusCode, TestError> {
+  let app = Router::new()
+    .route("/revoke", post(revoke_handler))
+    .with_state(state);
+  let body = format!("token={token}&client_id={client_id}&token_type_hint=access_token");
+  let mut request = Request::post("/revoke")
+    .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded")
+    .body(Body::from(body))
+    .map_err(|error| TestError::Unexpected(format!("request build: {error}")))?;
+  request.extensions_mut().insert(certs);
+  let response = match app.oneshot(request).await {
+    Ok(response) => response,
+    Err(never) => match never {},
+  };
+  Ok(response.status())
+}
+
+/// A `POST /revoke` carrying `token` + `client_id` with `certs` injected as
+/// the verified peer chain, returning the status plus the parsed JSON body
+/// (empty object when the body is not JSON, e.g. the `200` empty response).
+async fn revoke_with_certs_full(
+  state: AppState,
+  token: &str,
+  client_id: &str,
+  certs: PeerCertificates,
+) -> Result<(StatusCode, serde_json::Value), TestError> {
+  let app = Router::new()
+    .route("/revoke", post(revoke_handler))
+    .with_state(state);
+  let body = format!("token={token}&client_id={client_id}&token_type_hint=access_token");
+  let mut request = Request::post("/revoke")
+    .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded")
+    .body(Body::from(body))
+    .map_err(|error| TestError::Unexpected(format!("request build: {error}")))?;
+  request.extensions_mut().insert(certs);
+  let response = match app.oneshot(request).await {
+    Ok(response) => response,
+    Err(never) => match never {},
+  };
+  let status = response.status();
+  let bytes = http_body_util::BodyExt::collect(response.into_body())
+    .await
+    .map_err(|error| TestError::Unexpected(format!("body read: {error}")))?
+    .to_bytes();
+  let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null);
+  Ok((status, json))
+}
+
+/// A registered-but-foreign chain fails closed as `invalid_client` and leaves
+/// the victim token live: the attacker presents its own valid certificate
+/// while claiming the victim `client_id`, so the thumbprint pin does not
+/// match the claimed client.
+#[tokio::test]
+async fn revoke_mtls_chain_does_not_match_claimed_client() -> TestResult {
+  let (_db, pool) = crate::db::test_support::migrated_pool()
+    .await
+    .map_err(|error| TestError::Unexpected(format!("postgres: {error}")))?;
+  let (_guard, mut conn, store_handle) = test_redis().await?;
+  let service = service()?;
+  let victim_leaf = cert_der(CLIENT_SELF_PEM)?;
+  let victim_thumbprint = mtls::thumbprint_b64url(victim_leaf.as_ref());
+  let victim_id = unique("mtls-victim");
+  insert_mtls_client(&pool, &victim_id, &victim_thumbprint).await?;
+  let attacker_leaf = cert_der(CLIENT_ROGUE_PEM)?;
+  let attacker_thumbprint = mtls::thumbprint_b64url(attacker_leaf.as_ref());
+  let attacker_id = unique("mtls-attacker");
+  insert_mtls_client(&pool, &attacker_id, &attacker_thumbprint).await?;
+  let minted = service
+    .mint(
+      &mut conn,
+      &store_handle,
+      &mint_request(&victim_id, &unique("grant")),
+      now_unix(),
+    )
+    .await?;
+  let access = minted.access_token.clone();
+  let peer = PeerCertificates(Arc::new(vec![attacker_leaf]));
+  let mut verify_conn = conn.clone();
+  let state = app_state(pool, conn)?;
+  let (status, body) = revoke_with_certs_full(state, &access, &victim_id, peer).await?;
+  if status != StatusCode::BAD_REQUEST {
+    return Err(TestError::Unexpected(format!(
+      "foreign mTLS chain must be invalid_client (400), got {status}"
+    )));
+  }
+  if body.get("error").and_then(|value| value.as_str()) != Some("invalid_client") {
+    return Err(TestError::Unexpected(
+      "foreign mTLS chain must render invalid_client".to_owned(),
+    ));
+  }
+  if service
+    .find_access(&mut verify_conn, &store_handle, &access)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "rejected mTLS revocation must leave the victim token live".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// A disabled mTLS client fails closed as `invalid_client`, even over its
+/// previously valid chain, and the token stays live.
+#[tokio::test]
+async fn revoke_mtls_disabled_client_rejected() -> TestResult {
+  let (_db, pool) = crate::db::test_support::migrated_pool()
+    .await
+    .map_err(|error| TestError::Unexpected(format!("postgres: {error}")))?;
+  let (_guard, mut conn, store_handle) = test_redis().await?;
+  let service = service()?;
+  let leaf = cert_der(CLIENT_SELF_PEM)?;
+  let thumbprint = mtls::thumbprint_b64url(leaf.as_ref());
+  let client_id = unique("mtls-disabled");
+  insert_mtls_client(&pool, &client_id, &thumbprint).await?;
+  let minted = service
+    .mint(
+      &mut conn,
+      &store_handle,
+      &mint_request(&client_id, &unique("grant")),
+      now_unix(),
+    )
+    .await?;
+  let access = minted.access_token.clone();
+  if !crate::db::client_repo::deactivate(&pool, &client_id).await? {
+    return Err(TestError::Unexpected(
+      "deactivate must update a row".to_owned(),
+    ));
+  }
+  let peer = PeerCertificates(Arc::new(vec![leaf]));
+  let mut verify_conn = conn.clone();
+  let state = app_state(pool, conn)?;
+  let (status, body) = revoke_with_certs_full(state, &access, &client_id, peer).await?;
+  if status != StatusCode::BAD_REQUEST {
+    return Err(TestError::Unexpected(format!(
+      "disabled mTLS client must be invalid_client (400), got {status}"
+    )));
+  }
+  if body.get("error").and_then(|value| value.as_str()) != Some("invalid_client") {
+    return Err(TestError::Unexpected(
+      "disabled mTLS client must render invalid_client".to_owned(),
+    ));
+  }
+  if service
+    .find_access(&mut verify_conn, &store_handle, &access)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "rejected mTLS revocation must leave the token live".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// An mTLS-registered client self-revokes its own access token over the
+/// verified peer chain.
+#[tokio::test]
+async fn revoke_mtls_client_self_revokes() -> TestResult {
+  let (_db, pool) = crate::db::test_support::migrated_pool()
+    .await
+    .map_err(|error| TestError::Unexpected(format!("postgres: {error}")))?;
+  let (_guard, mut conn, store_handle) = test_redis().await?;
+  let service = service()?;
+  let leaf = cert_der(CLIENT_SELF_PEM)?;
+  let thumbprint = mtls::thumbprint_b64url(leaf.as_ref());
+  let client_id = unique("mtls-client");
+  insert_mtls_client(&pool, &client_id, &thumbprint).await?;
+  let minted = service
+    .mint(
+      &mut conn,
+      &store_handle,
+      &mint_request(&client_id, &unique("grant")),
+      now_unix(),
+    )
+    .await?;
+  let access = minted.access_token.clone();
+  let peer = PeerCertificates(Arc::new(vec![leaf]));
+  let mut verify_conn = conn.clone();
+  let state = app_state(pool, conn)?;
+  let status = revoke_with_certs(state, &access, &client_id, peer).await?;
+  if status != StatusCode::OK {
+    return Err(TestError::Unexpected(format!(
+      "mTLS self-revoke must be 200, got {status}"
+    )));
+  }
+  if service
+    .find_access(&mut verify_conn, &store_handle, &access)
+    .await?
+    .is_some()
+  {
+    return Err(TestError::Unexpected(
+      "mTLS self-revoke must delete the access token".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// An unregistered certificate fails closed as `invalid_client` and leaves
+/// the token live.
+#[tokio::test]
+async fn revoke_mtls_unknown_certificate_rejected() -> TestResult {
+  let (_db, pool) = crate::db::test_support::migrated_pool()
+    .await
+    .map_err(|error| TestError::Unexpected(format!("postgres: {error}")))?;
+  let (_guard, mut conn, store_handle) = test_redis().await?;
+  let service = service()?;
+  let leaf = cert_der(CLIENT_SELF_PEM)?;
+  let thumbprint = mtls::thumbprint_b64url(leaf.as_ref());
+  let client_id = unique("mtls-client");
+  insert_mtls_client(&pool, &client_id, &thumbprint).await?;
+  let minted = service
+    .mint(
+      &mut conn,
+      &store_handle,
+      &mint_request(&client_id, &unique("grant")),
+      now_unix(),
+    )
+    .await?;
+  let access = minted.access_token.clone();
+  let rogue = cert_der(CLIENT_ROGUE_PEM)?;
+  let peer = PeerCertificates(Arc::new(vec![rogue]));
+  let mut verify_conn = conn.clone();
+  let state = app_state(pool, conn)?;
+  let status = revoke_with_certs(state, &access, &client_id, peer).await?;
+  if status != StatusCode::UNAUTHORIZED && status != StatusCode::BAD_REQUEST {
+    return Err(TestError::Unexpected(format!(
+      "unknown mTLS certificate must be invalid_client (401/400), got {status}"
+    )));
+  }
+  if service
+    .find_access(&mut verify_conn, &store_handle, &access)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "rejected mTLS revocation must leave the token live".to_owned(),
+    ));
+  }
+  Ok(())
+}

+ 131 - 53
src/protocol/token/grants/authorization_code.rs

@@ -102,11 +102,13 @@ pub struct CodeTokenResponse {
 
 /// Redeem an authorization code at the token endpoint (RFC 9700 §4.5.3).
 ///
-/// The client is authenticated, the code is consumed atomically (a missing
-/// record — wrong, expired, or already redeemed — fails with `invalid_grant`
-/// and is never re-consumed), and the consumed record is checked for client
-/// id, redirect URI, PKCE, and sender binding before anything is minted.
-/// Tokens join the code's `grant_id` chain.
+/// The client is authenticated, then the code is consumed atomically bound to
+/// the expected client id, redirect URI, and PKCE challenge: a foreign or
+/// mismatched presentation observes `Mismatched` and the victim record
+/// survives for its legitimate presenter, so a wrong, expired, or
+/// already-redeemed code fails with `invalid_grant` without ever burning a
+/// live value. The consumed record then feeds the sender-binding checks
+/// before anything is minted. Tokens join the code's `grant_id` chain.
 ///
 /// # Errors
 ///
@@ -146,21 +148,20 @@ pub async fn redeem_code(
     ));
   }
 
-  let record = consume_code(state, inputs.code.trim())
-    .await?
-    .ok_or_else(|| AppError::invalid_grant("authorization code is invalid or expired"))?;
-  if record.client_id != client.client_id {
-    return Err(AppError::invalid_grant(
-      "authorization code was issued to another client",
-    ));
-  }
-  if record.redirect_uri != inputs.redirect_uri {
-    return Err(AppError::invalid_grant(
-      "redirect_uri does not match the authorization request",
-    ));
-  }
-  pkce::verify(inputs.code_verifier, Some(record.code_challenge.as_str()))
+  let verifier = inputs
+    .code_verifier
+    .filter(|value| !value.is_empty())
+    .ok_or_else(|| AppError::invalid_grant("code_verifier does not match code_challenge"))?;
+  let expected_challenge = pkce::challenge_from_verifier(verifier)
     .map_err(|_| AppError::invalid_grant("code_verifier does not match code_challenge"))?;
+  let record = consume_code(
+    state,
+    inputs.code.trim(),
+    client.client_id.as_str(),
+    inputs.redirect_uri,
+    expected_challenge.as_str(),
+  )
+  .await?;
 
   let proofs = fresh_proofs(state, &client, inputs, used_basic).await?;
   require_dpop_binding(&client, &proofs)?;
@@ -238,40 +239,57 @@ fn candidate_client_id(inputs: &CodeGrantInputs<'_>) -> Option<String> {
   }
 }
 
-/// Atomically consume a code, retrying retained peppers newest-first.
+/// Atomically consume a code bound to its expected presenter values.
 ///
-/// Only the hash under the issuing generation names an existing key; consumes
-/// under the other generations observe absence (a no-op `None`) and never
-/// delete anything, so exactly one caller across all generations observes
-/// the record.
+/// The expected client id, redirect URI, and PKCE challenge travel inside the
+/// consume script: only the hash under the issuing generation names an
+/// existing key, and a generation holding the record but observing a binding
+/// mismatch reports `Mismatched` without deleting anything, so exactly one
+/// matching caller across all generations observes the record while a foreign
+/// presentation leaves the victim value live.
 async fn consume_code(
   state: &mut CodeGrantState<'_>,
   code: &str,
-) -> Result<Option<codes::CodeRecord>, AppError> {
+  expected_client_id: &str,
+  expected_redirect_uri: &str,
+  expected_challenge: &str,
+) -> Result<codes::CodeRecord, AppError> {
   for (id, secret) in state
     .keys
     .peppers_newest_first()
     .take(crypto_tokens::MAX_RETAINED_KEYS)
   {
-    let generation = id
-      .as_str()
-      .trim_start_matches('v')
-      .parse::<u32>()
-      .unwrap_or(1);
     let hash = crypto_tokens::token_hash(
-      &crypto_tokens::KeyId::new(generation),
+      &crypto_tokens::KeyId::new(id.as_str()),
       secret.expose_secret().as_bytes(),
       code,
     )
     .to_storage_string();
-    let consumed = codes::consume(state.conn, state.store, &hash)
-      .await
-      .map_err(AppError::from)?;
-    if consumed.is_some() {
-      return Ok(consumed);
+    match codes::consume_bound(
+      state.conn,
+      state.store,
+      &hash,
+      &[
+        ("client_id", expected_client_id),
+        ("redirect_uri", expected_redirect_uri),
+        ("code_challenge", expected_challenge),
+      ],
+    )
+    .await
+    .map_err(AppError::from)?
+    {
+      codes::ConsumeOutcome::Consumed(record) => return Ok(record),
+      codes::ConsumeOutcome::Mismatched => {
+        return Err(AppError::invalid_grant(
+          "authorization code is invalid or expired",
+        ));
+      }
+      codes::ConsumeOutcome::Absent => {}
     }
   }
-  Ok(None)
+  Err(AppError::invalid_grant(
+    "authorization code is invalid or expired",
+  ))
 }
 
 /// Public clients carry no secret; every other registered method is
@@ -886,20 +904,18 @@ mod tests {
         ));
       }
     }
-    // The code was consumed by the failed attempt: no second redemption.
+    // The mismatched attempt must not burn the victim value: the legitimate
+    // presenter redeems the same code on retry.
     let retry = CodeGrantInputs {
       redirect_uri: REDIRECT,
       ..inputs_clone(&inputs)
     };
-    match redeem_once(&mut h, &retry, now + 1).await {
-      Err(error) if is_invalid_grant(&error) => Ok(()),
-      Err(other) => Err(TestError::Unexpected(format!(
-        "consumed code must stay invalid_grant, got {other}"
-      ))),
-      Ok(_) => Err(TestError::Unexpected(
-        "a mismatched code must not redeem on retry".to_owned(),
-      )),
-    }
+    redeem_once(&mut h, &retry, now + 1)
+      .await
+      .map_err(|error| {
+        TestError::Unexpected(format!("mismatched redirect burned the code: {error}"))
+      })?;
+    Ok(())
   }
 
   #[tokio::test]
@@ -945,6 +961,73 @@ mod tests {
     }
   }
 
+  #[tokio::test]
+  async fn test_grant_code_foreign_client_does_not_consume_victim_code() -> TestResult {
+    let (_container, mut h) = harness().await?;
+    let client_a = unique("victim");
+    let client_b = unique("foreign");
+    for client_id in [&client_a, &client_b] {
+      insert_client(
+        &h,
+        client_id,
+        TokenEndpointAuthMethod::ClientSecretPost,
+        Some("shared-secret"),
+        &[
+          super::GRANT_TYPE_AUTHORIZATION_CODE,
+          GRANT_TYPE_REFRESH_TOKEN,
+        ],
+      )
+      .await?;
+    }
+    let issued = issue_code(&mut h, &client_a, CodeClientKind::Confidential, None).await?;
+    let now = now_unix();
+
+    let foreign = CodeGrantInputs {
+      code: issued.code.as_str(),
+      redirect_uri: REDIRECT,
+      code_verifier: Some(VERIFIER),
+      client_id: Some(client_b.as_str()),
+      basic_header: None,
+      body_secret: Some(secret("shared-secret")),
+      assertion_type: None,
+      assertion: None,
+      dpop_proof: None,
+      peer_certs: None,
+    };
+    match redeem_once(&mut h, &foreign, now).await {
+      Err(error) if is_invalid_grant(&error) => {}
+      Err(other) => {
+        return Err(TestError::Unexpected(format!(
+          "foreign client must be invalid_grant, got {other}"
+        )));
+      }
+      Ok(_) => {
+        return Err(TestError::Unexpected(
+          "a code must never redeem for another client".to_owned(),
+        ));
+      }
+    }
+
+    let legitimate = CodeGrantInputs {
+      code: issued.code.as_str(),
+      redirect_uri: REDIRECT,
+      code_verifier: Some(VERIFIER),
+      client_id: Some(client_a.as_str()),
+      basic_header: None,
+      body_secret: Some(secret("shared-secret")),
+      assertion_type: None,
+      assertion: None,
+      dpop_proof: None,
+      peer_certs: None,
+    };
+    redeem_once(&mut h, &legitimate, now + 1)
+      .await
+      .map_err(|error| {
+        TestError::Unexpected(format!("victim code burned by foreign use: {error}"))
+      })?;
+    Ok(())
+  }
+
   #[tokio::test]
   async fn test_grant_code_wrong_verifier_rejected() -> TestResult {
     let (_container, mut h) = harness().await?;
@@ -1145,13 +1228,8 @@ mod tests {
       .peppers_newest_first()
       .next()
       .ok_or_else(|| TestError::Unexpected("no pepper configured".to_owned()))?;
-    let generation = key_id
-      .as_str()
-      .trim_start_matches('v')
-      .parse::<u32>()
-      .unwrap_or(1);
     let hash = crypto_tokens::token_hash(
-      &crypto_tokens::KeyId::new(generation),
+      &crypto_tokens::KeyId::new(key_id.as_str()),
       secret.expose_secret().as_bytes(),
       raw,
     )

+ 19 - 48
src/protocol/token/grants/device_code/interval.rs

@@ -1,64 +1,35 @@
 //! Polling-interval enforcement (RFC 8628 §3.5 `slow_down`).
-//!
-//! The last attempt time rides a sidecar key derived from the device record
-//! key, so it shares the `{state}` hash slot and expires with the record.
 
 use super::{DevicePollError, PollState};
-use crate::{
-  error::AppError,
-  store::{self, Store},
-};
-
-/// Polling-attempt sidecar: last poll time (Unix seconds) for a device hash.
-///
-/// It carries the device TTL so it can never outlive its record by more than
-/// one issuance lifetime.
-pub(super) fn poll_key(store: &Store, device_hash: &str) -> String {
-  format!("{}:poll", store.device_key(device_hash))
-}
+use crate::{error::AppError, store::device as device_store};
 
 /// Enforce the polling interval, counting this attempt.
 ///
 /// A poll sooner than one interval after the previous attempt fails with
 /// `slow_down`; the attempt still refreshes the marker so repeated fast
 /// polls keep backing off instead of racing the window edge.
+///
+/// The read-compare-write runs as one Lua compare-and-set
+/// ([`device_store::claim_poll_slot`]), so concurrent polls cannot both pass
+/// the gate.
 pub(super) async fn check_interval(
   state: &mut PollState<'_>,
   device_hash: &str,
 ) -> Result<(), DevicePollError> {
-  let key = poll_key(state.store, device_hash);
-  let last: Option<String> = redis::cmd("GET")
-    .arg(&key)
-    .query_async(state.conn)
-    .await
-    .map_err(store::StoreError::Redis)
-    .map_err(AppError::from)?;
-  if let Some(raw) = last {
-    let last_secs = raw.parse::<i64>().map_err(|_| AppError::server_error())?;
-    let elapsed = state.now_unix.saturating_sub(last_secs);
-    let interval_secs = state.interval.as_secs() as i64;
-    if elapsed < interval_secs {
-      stamp_poll(state, &key).await?;
-      let wait = interval_secs.saturating_sub(elapsed).max(1) as u64;
-      return Err(DevicePollError::SlowDown {
-        retry_after_secs: wait,
-      });
+  let slot = device_store::claim_poll_slot(
+    state.conn,
+    state.store,
+    device_hash,
+    state.now_unix,
+    state.interval,
+    state.device_ttl,
+  )
+  .await
+  .map_err(AppError::from)?;
+  match slot {
+    device_store::PollSlot::Allowed => Ok(()),
+    device_store::PollSlot::SlowDown { retry_after_secs } => {
+      Err(DevicePollError::SlowDown { retry_after_secs })
     }
   }
-  stamp_poll(state, &key).await?;
-  Ok(())
-}
-
-/// Record this poll attempt under the device TTL.
-async fn stamp_poll(state: &mut PollState<'_>, key: &str) -> Result<(), DevicePollError> {
-  redis::cmd("SET")
-    .arg(key)
-    .arg(state.now_unix)
-    .arg("PX")
-    .arg(state.device_ttl.as_millis() as u64)
-    .query_async::<()>(state.conn)
-    .await
-    .map_err(store::StoreError::Redis)
-    .map_err(AppError::from)?;
-  Ok(())
 }

+ 2 - 2
src/protocol/token/grants/device_code/issue.rs

@@ -8,7 +8,7 @@
 use uuid::Uuid;
 
 use super::super::super::{ClientKind, MintRequest, SenderProof, TokenService};
-use super::{DevicePollError, DeviceTokenResponse, PollState, interval::poll_key};
+use super::{DevicePollError, DeviceTokenResponse, PollState};
 use crate::{
   config::GrantKind,
   db::{scope_repo, user_scope_repo},
@@ -92,7 +92,7 @@ pub(super) async fn issue_approved(
     .map_err(AppError::from)?;
 
   let stale = [
-    poll_key(state.store, redemption.device_hash),
+    device_store::poll_key(state.store, redemption.device_hash),
     binding_key(state.store, redemption.device_hash),
   ];
   redis::cmd("DEL")

+ 1 - 0
src/protocol/token/grants/device_code/tests.rs

@@ -24,6 +24,7 @@ mod flows;
 mod keys;
 mod matrix;
 mod poll;
+mod race;
 mod scope;
 mod setup;
 

+ 75 - 0
src/protocol/token/grants/device_code/tests/race.rs

@@ -0,0 +1,75 @@
+//! `test(grant_device)` races: concurrent polls admit exactly one attempt.
+
+use std::time::Duration;
+
+use super::super::DevicePollError;
+use super::super::PollState;
+use super::super::interval::check_interval;
+use super::flows::now_unix;
+use super::{TestError, TestResult, harness, unique};
+use crate::protocol::device::DEVICE_INTERVAL_SECS;
+use crate::store::device::poll_key;
+
+/// T3-B2: two polls racing at the same instant must not both pass the
+/// interval gate. A GET-then-SET gate admits every concurrent reader; the
+/// Lua compare-and-set admits exactly one and answers `slow_down` to the
+/// loser while still advancing the stamp.
+#[tokio::test]
+async fn test_grant_device_concurrent_interval_admits_only_one() -> TestResult {
+  let (_container, mut h) = harness().await?;
+  let device_hash = unique("device-race");
+  let now = now_unix();
+  let interval = Duration::from_secs(DEVICE_INTERVAL_SECS);
+  for round in 0..50u32 {
+    let sidecar = poll_key(&h.store, &device_hash);
+    let _: () = redis::cmd("DEL")
+      .arg(&sidecar)
+      .query_async(&mut h.conn)
+      .await?;
+    let mut left_conn = h.conn.clone();
+    let mut right_conn = h.conn.clone();
+    let mut left = PollState {
+      pool: &h.pool,
+      conn: &mut left_conn,
+      store: &h.store,
+      keys: &h.config.keys,
+      issuer: &h.config.issuer,
+      device_ttl: h.config.ttl.device,
+      interval,
+      ttl: &h.config.ttl,
+      now_unix: now,
+    };
+    let mut right = PollState {
+      pool: &h.pool,
+      conn: &mut right_conn,
+      store: &h.store,
+      keys: &h.config.keys,
+      issuer: &h.config.issuer,
+      device_ttl: h.config.ttl.device,
+      interval,
+      ttl: &h.config.ttl,
+      now_unix: now,
+    };
+    let (left_outcome, right_outcome) = tokio::join!(
+      check_interval(&mut left, device_hash.as_str()),
+      check_interval(&mut right, device_hash.as_str())
+    );
+    let admitted = u8::from(left_outcome.is_ok()) + u8::from(right_outcome.is_ok());
+    if admitted != 1 {
+      return Err(TestError::Unexpected(format!(
+        "round {round}: racing polls admitted {admitted}, want exactly one"
+      )));
+    }
+    let loser = if left_outcome.is_ok() {
+      right_outcome
+    } else {
+      left_outcome
+    };
+    if !matches!(loser, Err(DevicePollError::SlowDown { .. })) {
+      return Err(TestError::Unexpected(format!(
+        "round {round}: losing poll must be slow_down"
+      )));
+    }
+  }
+  Ok(())
+}

+ 19 - 11
src/protocol/token/grants/refresh_token.rs

@@ -8,13 +8,17 @@
 //! request-id mapping).
 //!
 //! Ordering is the whole contract: the new raw pair is minted server-side
-//! FIRST, keyed by the deterministic `rotation_request_id`, and only then is
-//! the Lua script invoked. A lost post-`EXEC` reply is recovered by re-reading
-//! the request-id mapping (same hashes, never a double rotation); only a
-//! tombstone with no valid mapping triggers the grant-cascade reuse path. A
-//! retry that hits a live mapping is rejected WITHOUT cascading: the raw
-//! tokens of the first rotation are unrecoverable from hashes alone, so the
-//! retry cannot be served, but the grant family is left intact.
+//! FIRST (barrier deferred to the rotation, which covers the pre-mint writes
+//! on the same connection), keyed by the deterministic `rotation_request_id`,
+//! and only then is the Lua script invoked. A barrier failure after the
+//! originating write applied resolves to the minted pair
+//! (`RotatedUnbarriered`, returned as-is); a lost post-`EXEC` reply is
+//! recovered by re-reading the request-id mapping (same hashes, never a
+//! double rotation, still rejected without cascading); only a tombstone with
+//! no valid mapping triggers the grant-cascade reuse path. A retry that hits
+//! a live mapping is rejected WITHOUT cascading: the raw tokens of the first
+//! rotation are unrecoverable from hashes alone, so the retry cannot be
+//! served, but the grant family is left intact.
 
 #[cfg(test)]
 mod tests;
@@ -163,8 +167,12 @@ pub async fn refresh_with_subject_check(
 ///
 /// The stored `cnf` alone never mints: every bound component must equal its
 /// freshly verified counterpart. Scope may only narrow (`invalid_scope`
-/// otherwise). The replacement pair is minted BEFORE the Lua script runs, so
-/// every non-`Rotated` outcome revokes the orphaned pre-mint.
+/// otherwise). The replacement pair is minted BEFORE the Lua script runs
+/// (barrier deferred to the rotation, which gates the response), so every
+/// non-`Rotated`/`RotatedUnbarriered` outcome revokes the orphaned pre-mint.
+/// A barrier failure after the originating write applied returns the minted
+/// pair as tokens instead of locking out; a mapping replay still discards
+/// the orphans and rejects without cascading.
 ///
 /// # Errors
 ///
@@ -216,7 +224,7 @@ pub async fn refresh(
     user_term,
   )?;
   let minted = service
-    .mint(
+    .mint_for_rotation(
       conn,
       store,
       &MintRequest {
@@ -263,7 +271,7 @@ pub async fn refresh(
     .await
     .map_err(AppError::from)?
   {
-    RotateOutcome::Rotated { .. } => Ok(response(
+    RotateOutcome::Rotated { .. } | RotateOutcome::RotatedUnbarriered { .. } => Ok(response(
       service,
       &minted,
       &narrowed,

+ 1 - 0
src/protocol/token/grants/refresh_token/tests.rs

@@ -3,6 +3,7 @@
 //! Child suites drive the public [`super::super::refresh`] API. Every test
 //! uses unique client/grant labels so the shared keyspace never collides.
 
+mod barrier;
 mod concurrent;
 mod disabled_subject;
 mod entitlements;

+ 135 - 0
src/protocol/token/grants/refresh_token/tests/barrier.rs

@@ -0,0 +1,135 @@
+//! Wait-mode barrier failure: the originating request still resolves.
+//!
+//! On a replica-less node a `WAIT`-mode barrier always fails closed after the
+//! rotation write already applied. The originating request id must resolve to
+//! its freshly minted pair (not `invalid_grant`): discarding it would lock
+//! the originator out while its successor sits live but unreachable. A retry
+//! of the spent token stays fail-closed through the mapping.
+
+use super::{
+  TestError, TestResult, dpop_proof, input, is_invalid_grant, mint_grant, service, test_redis,
+};
+use crate::{
+  config::DurabilityMode,
+  protocol::{
+    scope::ScopeSet,
+    token::{ClientKind, grants::refresh_token::refresh},
+  },
+  store::Store,
+};
+
+/// A barrier failure after the originating rotation write returns the minted
+/// pair as tokens; a later retry of the spent token is still `invalid_grant`.
+#[tokio::test]
+async fn grant_refresh_barrier_failure_returns_tokens_for_originator() -> TestResult {
+  let (_container, mut conn, durable, _url) = test_redis().await?;
+  let fragile = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::Wait,
+  );
+  let service = service()?;
+  let client = super::unique("barrier-client");
+  let proof = dpop_proof(&super::unique("barrier-jkt"));
+  let initial = mint_grant(
+    &service,
+    &mut conn,
+    &durable,
+    &client,
+    ClientKind::Public,
+    "read",
+    proof.clone(),
+  )
+  .await?;
+  let grant = service
+    .find_refresh(&mut conn, &durable, &initial.refresh_token)
+    .await?
+    .ok_or_else(|| TestError::Unexpected("initial grant missing".to_owned()))?
+    .record
+    .grant_id
+    .clone();
+  let enabled = ScopeSet::parse("read")?;
+
+  let rotated = refresh(
+    &mut conn,
+    &fragile,
+    &service,
+    &input(
+      &client,
+      ClientKind::Public,
+      &initial.refresh_token,
+      None,
+      &proof,
+      &enabled,
+    ),
+  )
+  .await?;
+  if rotated.refresh_token == initial.refresh_token || rotated.access_token == initial.access_token
+  {
+    return Err(TestError::Unexpected(
+      "barrier recovery returned the presented tokens".to_owned(),
+    ));
+  }
+  if rotated.grant_id != grant {
+    return Err(TestError::Unexpected(
+      "barrier recovery changed the grant chain".to_owned(),
+    ));
+  }
+  if service
+    .find_refresh(&mut conn, &durable, &rotated.refresh_token)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "recovered refresh token must stay live (not discarded)".to_owned(),
+    ));
+  }
+  if service
+    .find_access(&mut conn, &durable, &rotated.access_token)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "recovered access token must stay live (not discarded)".to_owned(),
+    ));
+  }
+
+  let retry = refresh(
+    &mut conn,
+    &durable,
+    &service,
+    &input(
+      &client,
+      ClientKind::Public,
+      &initial.refresh_token,
+      None,
+      &proof,
+      &enabled,
+    ),
+  )
+  .await;
+  let error = match retry {
+    Ok(_) => {
+      return Err(TestError::Unexpected(
+        "replayed old token rotated again".to_owned(),
+      ));
+    }
+    Err(error) => error,
+  };
+  if !is_invalid_grant(&error) {
+    return Err(TestError::Unexpected(format!(
+      "retry code: {}",
+      error.code()
+    )));
+  }
+  if service
+    .find_refresh(&mut conn, &durable, &rotated.refresh_token)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "replay cascaded the recovered pair".to_owned(),
+    ));
+  }
+  Ok(())
+}

+ 239 - 19
src/protocol/token/service.rs

@@ -11,9 +11,10 @@
 //! used as keys. `iat`/`exp` are enforced by the Redis TTL on each record
 //! (task-12 pattern) and reported on [`MintedTokens`] for response shaping.
 
-use std::time::Duration;
+use std::{fmt, time::Duration};
 
 use secrecy::ExposeSecret;
+use zeroize::Zeroizing;
 
 use super::types::{
   FoundAccess, FoundRefresh, MintRequest, MintedTokens, TokenServiceError, resolve_cnf,
@@ -27,18 +28,35 @@ use crate::{
 };
 
 /// One retained pepper generation, newest-first.
-#[derive(Debug, Clone)]
+///
+/// The raw pepper never reaches `Debug`: only the generation, label, and
+/// material length are printed (the [`KeyRing`] ids-only pattern).
+#[derive(Clone)]
 struct PepperKey {
-  /// Numeric generation for [`crypto_tokens::token_hash`].
+  /// Verbatim generation for [`crypto_tokens::token_hash`].
   generation: crypto_tokens::KeyId,
   /// String label (`v<N>`) for audit.
   label: String,
-  /// Raw pepper bytes.
-  secret: Vec<u8>,
+  /// Raw pepper bytes, wiped on drop.
+  secret: Zeroizing<Vec<u8>>,
+}
+
+impl fmt::Debug for PepperKey {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    f.debug_struct("PepperKey")
+      .field("generation", &self.generation)
+      .field("label", &self.label)
+      .field("secret", &"[REDACTED]")
+      .field("secret_len", &self.secret.len())
+      .finish()
+  }
 }
 
 /// Token minting plus hash-at-rest lookup, revocation, and rotation.
-#[derive(Debug, Clone)]
+///
+/// `Debug` prints lifetimes plus the retained pepper ids and material
+/// lengths only, never raw pepper bytes or token strings.
+#[derive(Clone)]
 pub struct TokenService {
   /// Access-token lifetime.
   access_ttl: Duration,
@@ -49,6 +67,19 @@ pub struct TokenService {
   keys: Vec<PepperKey>,
 }
 
+impl fmt::Debug for TokenService {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    let key_labels: Vec<&str> = self.keys.iter().map(|key| key.label.as_str()).collect();
+    let secret_lens: Vec<usize> = self.keys.iter().map(|key| key.secret.len()).collect();
+    f.debug_struct("TokenService")
+      .field("access_ttl", &self.access_ttl)
+      .field("refresh_ttl", &self.refresh_ttl)
+      .field("key_labels", &key_labels)
+      .field("secret_lens", &secret_lens)
+      .finish()
+  }
+}
+
 impl TokenService {
   /// Build a service from the validated TTLs and key ring.
   ///
@@ -61,15 +92,10 @@ impl TokenService {
       .peppers_newest_first()
       .take(crypto_tokens::MAX_RETAINED_KEYS)
     {
-      let generation = id
-        .as_str()
-        .trim_start_matches('v')
-        .parse::<u32>()
-        .unwrap_or(1);
       keys.push(PepperKey {
-        generation: crypto_tokens::KeyId::new(generation),
+        generation: crypto_tokens::KeyId::new(id.as_str()),
         label: id.as_str().to_owned(),
-        secret: secret.expose_secret().as_bytes().to_vec(),
+        secret: Zeroizing::new(secret.expose_secret().as_bytes().to_vec()),
       });
     }
     if keys.is_empty() {
@@ -101,10 +127,22 @@ impl TokenService {
 
   /// Hash a raw token under one retained generation for store lookup.
   fn hash_under(&self, key: &PepperKey, raw: &str) -> String {
-    crypto_tokens::token_hash(&key.generation, &key.secret, raw).to_storage_string()
+    crypto_tokens::token_hash(&key.generation, key.secret.as_slice(), raw).to_storage_string()
   }
 }
 
+/// Minted tokens are 32 random bytes base64url-encoded without padding
+/// ([`crypto_tokens::TOKEN_STR_LEN`] chars); anything else cannot resolve to
+/// a stored hash, so reject it before HMAC and Redis work. This is a cost
+/// guard only: well-formed lookups still retry retained peppers
+/// newest-first, so the gate does not mask generation probing.
+fn is_well_formed_token(raw: &str) -> bool {
+  raw.len() == crypto_tokens::TOKEN_STR_LEN
+    && raw
+      .bytes()
+      .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')
+}
+
 impl TokenService {
   /// Mint an access plus refresh pair and persist only their hashes.
   ///
@@ -122,6 +160,42 @@ impl TokenService {
     store: &Store,
     request: &MintRequest,
     now_unix: i64,
+  ) -> Result<MintedTokens, TokenServiceError> {
+    self.mint_inner(conn, store, request, now_unix, true).await
+  }
+
+  /// Mint a rotation-candidate pair, deferring the durability barrier to the
+  /// rotation that commits it.
+  ///
+  /// Only [`Self::rotate`] (via the refresh grant) may use this: the pair is
+  /// unreachable until the rotation script commits it, and the rotation's own
+  /// barrier runs on the same connection afterwards, so it covers these
+  /// writes before any response. A barrier failure after the originating
+  /// write applied then resolves to the minted pair instead of locking out.
+  ///
+  /// # Errors
+  ///
+  /// Returns [`TokenServiceError::MissingDpopBinding`] for a public client
+  /// without a `jkt`, or the crypto/store failure.
+  pub async fn mint_for_rotation(
+    &self,
+    conn: &mut RedisConn,
+    store: &Store,
+    request: &MintRequest,
+    now_unix: i64,
+  ) -> Result<MintedTokens, TokenServiceError> {
+    self.mint_inner(conn, store, request, now_unix, false).await
+  }
+
+  /// Mint body shared by [`Self::mint`] (barriered) and
+  /// [`Self::mint_for_rotation`] (barrier deferred to the rotation).
+  async fn mint_inner(
+    &self,
+    conn: &mut RedisConn,
+    store: &Store,
+    request: &MintRequest,
+    now_unix: i64,
+    apply_barrier: bool,
   ) -> Result<MintedTokens, TokenServiceError> {
     let cnf = resolve_cnf(request.client_kind, &request.proof)?;
     let cnf_json = cnf.to_storage()?;
@@ -156,8 +230,13 @@ impl TokenService {
       self.refresh_ttl,
     )
     .await?;
-    low::put_access(conn, store, &access_hash, &access, self.access_ttl).await?;
-    low::put_refresh(conn, store, &refresh_hash, &refresh, self.refresh_ttl).await?;
+    if apply_barrier {
+      low::put_access(conn, store, &access_hash, &access, self.access_ttl).await?;
+      low::put_refresh(conn, store, &refresh_hash, &refresh, self.refresh_ttl).await?;
+    } else {
+      low::put_access_pending(conn, store, &access_hash, &access, self.access_ttl).await?;
+      low::put_refresh_pending(conn, store, &refresh_hash, &refresh, self.refresh_ttl).await?;
+    }
     Ok(MintedTokens {
       access_token: access_raw,
       refresh_token: refresh_raw,
@@ -181,6 +260,9 @@ impl TokenService {
     store: &Store,
     raw_token: &str,
   ) -> Result<Option<FoundAccess>, TokenServiceError> {
+    if !is_well_formed_token(raw_token) {
+      return Ok(None);
+    }
     for key in &self.keys {
       let hash = self.hash_under(key, raw_token);
       if let Some(record) = low::get_access(conn, store, &hash).await? {
@@ -205,6 +287,9 @@ impl TokenService {
     store: &Store,
     raw_token: &str,
   ) -> Result<Option<FoundRefresh>, TokenServiceError> {
+    if !is_well_formed_token(raw_token) {
+      return Ok(None);
+    }
     for key in &self.keys {
       let hash = self.hash_under(key, raw_token);
       if let Some(record) = low::get_refresh(conn, store, &hash).await? {
@@ -230,6 +315,9 @@ impl TokenService {
     store: &Store,
     raw_token: &str,
   ) -> Result<bool, TokenServiceError> {
+    if !is_well_formed_token(raw_token) {
+      return Ok(false);
+    }
     let located = if let Some(found) = self.find_access(conn, store, raw_token).await? {
       Some((found.hash, found.record.grant_id))
     } else if let Some(found) = self.find_refresh(conn, store, raw_token).await? {
@@ -285,7 +373,8 @@ impl TokenService {
   /// Returns [`TokenServiceError::NoPepper`] when the ring is empty.
   pub fn request_id_for(&self, old_refresh_hash: &str) -> Result<String, TokenServiceError> {
     let active = self.active()?;
-    low::derive_rotation_id(&active.secret, old_refresh_hash).map_err(TokenServiceError::from)
+    low::derive_rotation_id(active.secret.as_slice(), old_refresh_hash)
+      .map_err(TokenServiceError::from)
   }
 
   /// Atomically rotate a refresh token (compare-and-set with reuse signal).
@@ -293,7 +382,9 @@ impl TokenService {
   /// The caller mints the new pair first and fixes `request_id`; a replay of
   /// the same request id returns the SAME hashes
   /// ([`low::RotateOutcome::Replay`]), while a tombstoned old token without a
-  /// mapping triggers the grant-cascade reuse path.
+  /// mapping triggers the grant-cascade reuse path. A barrier failure after
+  /// the originating write applied returns the minted pair as
+  /// [`low::RotateOutcome::RotatedUnbarriered`] instead of a lockout error.
   ///
   /// # Errors
   ///
@@ -305,7 +396,7 @@ impl TokenService {
     input: &low::RotationInput,
   ) -> Result<low::RotateOutcome, TokenServiceError> {
     let active = self.active()?;
-    low::rotate(conn, store, &active.secret, input)
+    low::rotate(conn, store, active.secret.as_slice(), input)
       .await
       .map_err(TokenServiceError::from)
   }
@@ -324,6 +415,9 @@ impl TokenService {
     store: &Store,
     raw_token: &str,
   ) -> Result<Option<String>, TokenServiceError> {
+    if !is_well_formed_token(raw_token) {
+      return Ok(None);
+    }
     for key in &self.keys {
       let hash = self.hash_under(key, raw_token);
       if let Some(target) = low::tombstone_of(conn, store, &hash).await? {
@@ -333,3 +427,129 @@ impl TokenService {
     Ok(None)
   }
 }
+
+#[cfg(test)]
+mod debug_redaction_tests {
+  use secrecy::SecretString;
+
+  use super::super::types::{Cnf, MintedTokens};
+  use super::*;
+  use crate::{
+    config::TtlConfig,
+    keys::{KeyId, KeyRing},
+    store::tokens::{AccessRecord, RefreshRecord},
+  };
+
+  #[test]
+  fn token_debug_redacts_secrets() -> Result<(), Box<dyn std::error::Error>> {
+    let access_sentinel = "access-sentinel-7f3a9c2e";
+    let refresh_sentinel = "refresh-sentinel-4b8d1f6a";
+    let minted = MintedTokens {
+      access_token: access_sentinel.to_owned(),
+      refresh_token: refresh_sentinel.to_owned(),
+      access_hash: "v1.test-access-hash".to_owned(),
+      refresh_hash: "v1.test-refresh-hash".to_owned(),
+      cnf: Cnf::default(),
+      issued_at: 1_700_000_000,
+      access_expires_at: 1_700_003_600,
+      refresh_expires_at: 1_700_086_400,
+    };
+    let minted_debug = format!("{minted:?}");
+    assert!(
+      minted_debug.contains("[REDACTED]"),
+      "MintedTokens Debug must carry the redaction marker"
+    );
+    assert!(
+      !minted_debug.contains(access_sentinel),
+      "MintedTokens Debug leaked the raw access token"
+    );
+    assert!(
+      !minted_debug.contains(refresh_sentinel),
+      "MintedTokens Debug leaked the raw refresh token"
+    );
+    assert!(minted_debug.contains("v1.test-access-hash"));
+    assert!(minted_debug.contains("v1.test-refresh-hash"));
+
+    let ttl = TtlConfig {
+      access: Duration::from_secs(3600),
+      refresh: Duration::from_secs(86_400),
+      ..TtlConfig::default()
+    };
+    let ring = KeyRing::from_parts(
+      vec![(
+        KeyId::new("v1"),
+        SecretString::from("redact-pepper-sentinel-0123456789abcdef"),
+      )],
+      vec![(KeyId::new("v1"), SecretString::from("totp-secret"))],
+      vec![(KeyId::new("v1"), SecretString::from("pii-secret"))],
+    )?;
+    let service = TokenService::new(&ttl, &ring)?;
+    let service_debug = format!("{service:?}");
+    assert!(
+      service_debug.contains("v1"),
+      "TokenService Debug must keep the pepper id"
+    );
+    assert!(
+      !service_debug.contains("redact-pepper-sentinel"),
+      "TokenService Debug leaked raw pepper bytes"
+    );
+    assert!(
+      !service_debug.contains("secret: ["),
+      "TokenService Debug leaked raw pepper byte array"
+    );
+
+    // The pepper itself redacts through the `[REDACTED]` marker: assert the
+    // marker is present (positive lock), not just that the sentinel is
+    // absent.
+    let pepper = PepperKey {
+      generation: crypto_tokens::KeyId::new("v1"),
+      label: "v1".to_owned(),
+      secret: Zeroizing::new(b"redact-pepper-sentinel-0123456789abcdef".to_vec()),
+    };
+    let pepper_debug = format!("{pepper:?}");
+    assert!(
+      pepper_debug.contains("[REDACTED]"),
+      "PepperKey Debug must carry the redaction marker"
+    );
+    assert!(
+      !pepper_debug.contains("redact-pepper-sentinel"),
+      "PepperKey Debug leaked raw pepper bytes"
+    );
+
+    // `FoundAccess`/`FoundRefresh` carry no raw token, so their `Debug`
+    // has no `[REDACTED]` marker by design; redaction is transitive through
+    // the identifier-only record `Debug` (see `AccessRecord`/`RefreshRecord`).
+    let found_access = FoundAccess {
+      record: AccessRecord {
+        client_id: "client-a".to_owned(),
+        grant_id: "grant-a".to_owned(),
+        scope: "orders:read".to_owned(),
+        subject: "user-a".to_owned(),
+        cnf: String::new(),
+      },
+      hash: "v1.test-access-hash".to_owned(),
+      key_label: "v1".to_owned(),
+    };
+    let access_debug = format!("{found_access:?}");
+    assert!(access_debug.contains("v1"));
+    assert!(!access_debug.contains(access_sentinel));
+    assert!(!access_debug.contains(refresh_sentinel));
+
+    let found_refresh = FoundRefresh {
+      record: RefreshRecord {
+        client_id: "client-a".to_owned(),
+        grant_id: "grant-a".to_owned(),
+        scope: "orders:read".to_owned(),
+        subject: "user-a".to_owned(),
+        cnf: String::new(),
+      },
+      hash: "v1.test-refresh-hash".to_owned(),
+      key_label: "v1".to_owned(),
+    };
+    let refresh_debug = format!("{found_refresh:?}");
+    assert!(refresh_debug.contains("v1"));
+    assert!(!refresh_debug.contains(access_sentinel));
+    assert!(!refresh_debug.contains(refresh_sentinel));
+    Ok(())
+  }
+}

+ 1 - 0
src/protocol/token/tests.rs

@@ -7,6 +7,7 @@ mod matrix;
 mod mint_find;
 mod peppers;
 mod rotate;
+mod shape;
 
 use std::{
   sync::atomic::{AtomicU64, Ordering},

+ 40 - 0
src/protocol/token/tests/mint_find.rs

@@ -192,3 +192,43 @@ async fn token_service_grant_set_holds_hashes_and_revoke_grant() -> TestResult {
   }
   Ok(())
 }
+
+/// A non-`vN` pepper id keeps its identity end to end: the stored hash carries
+/// the full string label and the minted token still resolves under it.
+#[tokio::test]
+async fn token_service_non_numeric_kid_preserved_end_to_end() -> TestResult {
+  let (_redis, mut conn, store) = test_redis().await?;
+  let service = service(&[("pepper-2026", "token-service-pepper-one")])?;
+  let grant = unique("grant");
+  let client = unique("client");
+
+  let minted = service
+    .mint(
+      &mut conn,
+      &store,
+      &mint_request(
+        &client,
+        ClientKind::Confidential,
+        &grant,
+        SenderProof::none(),
+      ),
+      now_unix(),
+    )
+    .await?;
+  if !minted.access_hash.starts_with("pepper-2026.")
+    || !minted.refresh_hash.starts_with("pepper-2026.")
+  {
+    return Err(TestError::Unexpected(
+      "non-numeric kid must survive in the stored hash".to_owned(),
+    ));
+  }
+  let found = service
+    .find_access(&mut conn, &store, &minted.access_token)
+    .await?;
+  match found {
+    Some(found) if found.hash == minted.access_hash && found.key_label == "pepper-2026" => Ok(()),
+    _ => Err(TestError::Unexpected(
+      "token minted under a non-numeric kid must resolve with its label".to_owned(),
+    )),
+  }
+}

+ 147 - 0
src/protocol/token/tests/shape.rs

@@ -0,0 +1,147 @@
+//! T3-A8: malformed tokens are rejected before HMAC + Redis work.
+//!
+//! Junk (wrong length or non-base64url charset) must resolve to `Ok(None)` /
+//! `Ok(false)` without touching Redis. The tests run against a dead Redis
+//! endpoint with short timeouts: any store hit surfaces as `Err`, so reaching
+//! `Ok` proves the precheck fired first. A well-formed but unknown token must
+//! still reach the store (and thus `Err` here), proving the precheck does not
+//! blanket-skip lookups.
+
+use std::time::Duration;
+
+use super::{TestError, TestResult, service};
+use crate::{config::DurabilityMode, state::RedisConn, store::Store};
+
+/// A connection/store pair pointed at a dead endpoint: any Redis I/O fails.
+///
+/// The manager is created lazily (no eager connect), so building it succeeds
+/// and only an actual store hit surfaces as `Err`.
+async fn dead_conn_store() -> Result<(RedisConn, Store), TestError> {
+  let client = redis::Client::open("redis://127.0.0.1:1/")?;
+  let manager_config = redis::aio::ConnectionManagerConfig::new()
+    .set_response_timeout(Some(Duration::from_millis(200)))
+    .set_connection_timeout(Some(Duration::from_millis(200)))
+    .set_number_of_retries(0);
+  let manager = redis::aio::ConnectionManager::new_lazy_with_config(client, manager_config)
+    .map_err(TestError::from)?;
+  let store = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::WaitAof,
+  );
+  Ok((RedisConn::Single(manager), store))
+}
+
+/// A real minter output: well-formed by construction, but unknown to any
+/// store. Coupling to [`crate::crypto::tokens::mint_token`] (instead of a
+/// hand-built `"A".repeat(43)`) keeps the positive case honest if the token
+/// shape ever changes.
+fn well_formed_unknown() -> Result<String, TestError> {
+  crate::crypto::tokens::mint_token()
+    .map_err(|err| TestError::Unexpected(format!("minter output must mint: {err}")))
+}
+
+/// Junk tokens are rejected without a store hit: `Ok(None)` / `Ok(false)`.
+#[tokio::test]
+async fn token_service_malformed_shape_rejected_before_store() -> TestResult {
+  let service = service(&[("v1", "token-service-pepper-one")])?;
+  let (mut conn, store) = dead_conn_store().await?;
+  let junk: Vec<String> = vec![
+    String::new(),
+    "short".to_owned(),
+    "no-such-token".to_owned(),
+    "!!!not-base64url!!!".to_owned(),
+    "A".repeat(42),
+    "A".repeat(44),
+    format!("{}!", "A".repeat(42)),
+    format!("{}+", "A".repeat(42)),
+  ];
+  for raw in &junk {
+    if service.find_access(&mut conn, &store, raw).await?.is_some() {
+      return Err(TestError::Unexpected(format!(
+        "malformed access token must resolve to None: len={}",
+        raw.len()
+      )));
+    }
+    if service
+      .find_refresh(&mut conn, &store, raw)
+      .await?
+      .is_some()
+    {
+      return Err(TestError::Unexpected(format!(
+        "malformed refresh token must resolve to None: len={}",
+        raw.len()
+      )));
+    }
+    if service.revoke_token(&mut conn, &store, raw).await? {
+      return Err(TestError::Unexpected(format!(
+        "malformed revoke must report false: len={}",
+        raw.len()
+      )));
+    }
+  }
+  Ok(())
+}
+
+/// A well-formed unknown token still reaches the store (fail-closed `Err`
+/// against the dead endpoint), proving the precheck only skips junk.
+#[tokio::test]
+async fn token_service_consume_tombstone_rejects_malformed_before_store() -> TestResult {
+  let service = service(&[("v1", "token-service-pepper-one")])?;
+  let (mut conn, store) = dead_conn_store().await?;
+  let junk: Vec<String> = vec![
+    String::new(),
+    "short".to_owned(),
+    "no-such-token".to_owned(),
+    "!!!not-base64url!!!".to_owned(),
+    "A".repeat(42),
+    "A".repeat(44),
+    format!("{}!", "A".repeat(42)),
+    format!("{}+", "A".repeat(42)),
+  ];
+  for raw in &junk {
+    if service
+      .consume_tombstone(&mut conn, &store, raw)
+      .await?
+      .is_some()
+    {
+      return Err(TestError::Unexpected(format!(
+        "malformed tombstone lookup must resolve to None: len={}",
+        raw.len()
+      )));
+    }
+  }
+  let raw = well_formed_unknown()?;
+  if service
+    .consume_tombstone(&mut conn, &store, &raw)
+    .await
+    .is_ok()
+  {
+    return Err(TestError::Unexpected(
+      "well-formed unknown tombstone lookup must hit the store".to_owned(),
+    ));
+  }
+  Ok(())
+}
+#[tokio::test]
+async fn token_service_well_formed_unknown_still_hits_store() -> TestResult {
+  let service = service(&[("v1", "token-service-pepper-one")])?;
+  let (mut conn, store) = dead_conn_store().await?;
+  let raw = well_formed_unknown()?;
+  if service.find_access(&mut conn, &store, &raw).await.is_ok() {
+    return Err(TestError::Unexpected(
+      "well-formed unknown access lookup must hit the store".to_owned(),
+    ));
+  }
+  if service.find_refresh(&mut conn, &store, &raw).await.is_ok() {
+    return Err(TestError::Unexpected(
+      "well-formed unknown refresh lookup must hit the store".to_owned(),
+    ));
+  }
+  if service.revoke_token(&mut conn, &store, &raw).await.is_ok() {
+    return Err(TestError::Unexpected(
+      "well-formed unknown revoke must hit the store".to_owned(),
+    ));
+  }
+  Ok(())
+}

+ 55 - 3
src/protocol/token/types.rs

@@ -6,6 +6,8 @@
 //! both were proven; an unconstrained confidential grant carries an empty
 //! `cnf`.
 
+use std::fmt;
+
 use serde::{Deserialize, Serialize};
 
 use crate::{crypto::tokens as crypto_tokens, error::AppError, store::StoreError};
@@ -160,7 +162,10 @@ pub struct MintRequest {
 
 /// Freshly minted pair: raw tokens (return to the caller once) plus their
 /// hash-at-rest identities and validity window.
-#[derive(Debug, Clone, PartialEq, Eq)]
+///
+/// `Debug` redacts the raw tokens (lengths only) and prints the storage
+/// identities, following the [`crate::protocol::code::IssuedCode`] pattern.
+#[derive(Clone, PartialEq, Eq)]
 pub struct MintedTokens {
   /// Raw access token (never stored).
   pub access_token: String,
@@ -180,8 +185,27 @@ pub struct MintedTokens {
   pub refresh_expires_at: i64,
 }
 
+impl fmt::Debug for MintedTokens {
+  /// Redacted [`fmt::Debug`]: raw tokens never reach logs or errors, only
+  /// their lengths plus the hash-at-rest identities.
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    f.debug_struct("MintedTokens")
+      .field("access_token", &"[REDACTED]")
+      .field("access_token_len", &self.access_token.len())
+      .field("refresh_token", &"[REDACTED]")
+      .field("refresh_token_len", &self.refresh_token.len())
+      .field("access_hash", &self.access_hash)
+      .field("refresh_hash", &self.refresh_hash)
+      .field("cnf", &self.cnf)
+      .field("issued_at", &self.issued_at)
+      .field("access_expires_at", &self.access_expires_at)
+      .field("refresh_expires_at", &self.refresh_expires_at)
+      .finish()
+  }
+}
+
 /// A located access token plus its hash-at-rest identity.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 pub struct FoundAccess {
   /// The stored record.
   pub record: crate::store::tokens::AccessRecord,
@@ -191,8 +215,22 @@ pub struct FoundAccess {
   pub key_label: String,
 }
 
+impl fmt::Debug for FoundAccess {
+  /// Locked [`fmt::Debug`]: the struct carries no raw token, only the stored
+  /// record plus its hash-at-rest identity and pepper label. Transitive: the
+  /// record's derived `Debug` holds identifiers only (see
+  /// [`crate::store::tokens::AccessRecord`]), so nothing secret formats here.
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    f.debug_struct("FoundAccess")
+      .field("record", &self.record)
+      .field("hash", &self.hash)
+      .field("key_label", &self.key_label)
+      .finish()
+  }
+}
+
 /// A located refresh token plus its hash-at-rest identity.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 pub struct FoundRefresh {
   /// The stored record.
   pub record: crate::store::tokens::RefreshRecord,
@@ -201,3 +239,17 @@ pub struct FoundRefresh {
   /// Pepper label (`v<N>`) that produced the match.
   pub key_label: String,
 }
+
+impl fmt::Debug for FoundRefresh {
+  /// Locked [`fmt::Debug`]: the struct carries no raw token, only the stored
+  /// record plus its hash-at-rest identity and pepper label. Transitive: the
+  /// record's derived `Debug` holds identifiers only (see
+  /// [`crate::store::tokens::RefreshRecord`]), so nothing secret formats here.
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    f.debug_struct("FoundRefresh")
+      .field("record", &self.record)
+      .field("hash", &self.hash)
+      .field("key_label", &self.key_label)
+      .finish()
+  }
+}

+ 128 - 8
src/routes.rs

@@ -1,9 +1,9 @@
 //! Router assembly: public listener routes plus the middleware chain.
 //!
 //! [`build_router`] mounts every endpoint behind one [`tower::ServiceBuilder`]
-//! whose execution order (outer-to-inner) is exactly {request-id → CORS (when
-//! configured) → resilience (load-shed → concurrency-limit → timeout) →
-//! rate-limit → body limit → security headers}:
+//! whose execution order (outer-to-inner) is exactly {request-id → security
+//! headers → CORS (when configured) → resilience (load-shed →
+//! concurrency-limit → timeout) → rate-limit → body limit}:
 //!
 //! 1. [`middleware::request_id::RequestIdLayer`] mints/propagates
 //!    `X-Request-Id` and opens the redacted `http_request` tracing span
@@ -107,9 +107,12 @@ pub fn build_router_with_options(state: AppState, options: RouterOptions) -> axu
   // Task 44 resilience insertion: timeout + concurrency-limit + load-shed sit
   // outside the rate limiter (so overload sheds 503 before it can queue into
   // `PoolTimedOut`) and inside request-id (so the span still covers them).
-  // Execution order outer-to-inner: load-shed → concurrency-limit → timeout →
-  // rate-limit → body-limit → security-headers; the `.layer` calls below read
-  // innermost-first.
+  // The security-headers layer sits just inside request-id (outside every
+  // short-circuiting layer: CORS preflights, resilience 503s, limiter
+  // 429/503s, body-limit 413s), so every rejection still carries the
+  // hardening headers. Execution order outer-to-inner: request-id →
+  // security-headers → load-shed → concurrency-limit → timeout → rate-limit
+  // → body-limit; the `.layer` calls below read innermost-first.
   let resilience = ResilienceConfig::from_policy(&state.config.failure_policy);
 
   let router = axum::Router::new()
@@ -153,7 +156,6 @@ pub fn build_router_with_options(state: AppState, options: RouterOptions) -> axu
     .route("/revoke", post(revoke::revoke_handler))
     .with_state(state.clone())
     .merge(health_router(state.clone()))
-    .layer(SecurityHeadersLayer::new(options.hsts))
     .layer(RequestBodyLimitLayer::new(options.max_body_bytes))
     .layer(RateLimitLayer::new(state))
     .layer(ResilienceLayer::from_config(&resilience));
@@ -162,7 +164,9 @@ pub fn build_router_with_options(state: AppState, options: RouterOptions) -> axu
     Some(layer) => router.layer(layer),
     None => router,
   };
-  router.layer(RequestIdLayer)
+  router
+    .layer(SecurityHeadersLayer::new(options.hsts))
+    .layer(RequestIdLayer)
 }
 
 /// Build the rustls server config for the public TLS listener.
@@ -735,6 +739,122 @@ mod router_tests {
     Ok(())
   }
 
+  /// Short-circuit rejections (limiter, CORS preflight, body limit,
+  /// resilience shed) return before any handler, so they must still carry
+  /// the hardening headers: `Cache-Control: no-store` +
+  /// `X-Content-Type-Options: nosniff`.
+  fn assert_hardening_headers(headers: &HeaderMap) -> TestResult {
+    let no_store = headers
+      .get(header::CACHE_CONTROL)
+      .and_then(|value| value.to_str().ok())
+      .unwrap_or_default();
+    if no_store != "no-store" {
+      return Err(format!("short-circuit must be no-store, got {no_store:?}").into());
+    }
+    let nosniff = headers
+      .get(header::X_CONTENT_TYPE_OPTIONS)
+      .and_then(|value| value.to_str().ok())
+      .unwrap_or_default();
+    if nosniff != "nosniff" {
+      return Err("short-circuit must carry X-Content-Type-Options: nosniff".into());
+    }
+    Ok(())
+  }
+
+  /// Limiter short-circuits (429 over budget, 503 fail-closed) must still
+  /// carry the hardening headers: `Cache-Control: no-store` +
+  /// `X-Content-Type-Options: nosniff`. Both `too_many_requests` and
+  /// `service_unavailable` return before the inner security layer, so this
+  /// drives the deterministic fail-closed 503 path (same bypass as 429).
+  #[tokio::test]
+  async fn router_limiter_short_circuit_carries_security_headers() -> TestResult {
+    let form = [("content-type", "application/x-www-form-urlencoded")];
+    let (_, headers, body) = drive(
+      build_router(limited_state(Vec::new())?),
+      "POST",
+      "/token",
+      &form,
+      "",
+    )
+    .await?;
+    if !body.contains(LIMITER_MARKER) {
+      return Err("expected the limiter fail-closed marker".into());
+    }
+    assert_hardening_headers(&headers)
+  }
+
+  /// CORS preflights short-circuit at the CORS layer (outside the limiter),
+  /// but inside the security-headers layer, so an allowlisted preflight must
+  /// echo the origin and still carry the hardening headers.
+  #[tokio::test]
+  async fn router_cors_preflight_carries_security_headers() -> TestResult {
+    let router = build_router(limited_state(vec!["https://app.example.test"])?);
+    let preflight = [
+      ("origin", "https://app.example.test"),
+      ("access-control-request-method", "POST"),
+    ];
+    let (_, headers, _) = drive(router, "OPTIONS", "/token", &preflight, "").await?;
+    let allowed = headers
+      .get(header::ACCESS_CONTROL_ALLOW_ORIGIN)
+      .and_then(|value| value.to_str().ok())
+      .unwrap_or_default();
+    if allowed != "https://app.example.test" {
+      return Err(format!("allowlisted origin must be echoed, got {allowed}").into());
+    }
+    assert_hardening_headers(&headers)
+  }
+
+  /// Body-limit 413s return before any handler, inside the security-headers
+  /// layer, so an oversized body must be rejected with the hardening headers.
+  #[tokio::test]
+  async fn router_body_limit_413_carries_security_headers() -> TestResult {
+    let options = RouterOptions {
+      max_body_bytes: 8,
+      ..RouterOptions::default()
+    };
+    let form = [
+      ("content-type", "application/x-www-form-urlencoded"),
+      ("content-length", "24"),
+    ];
+    let (status, headers, _) = drive(
+      build_router_with_options(passthrough_state()?, options),
+      "POST",
+      "/token",
+      &form,
+      "body-exceeds-eight-bytes",
+    )
+    .await?;
+    if status != StatusCode::PAYLOAD_TOO_LARGE {
+      return Err(format!("oversized body must be 413, got {status}").into());
+    }
+    assert_hardening_headers(&headers)
+  }
+
+  /// Resilience sheds (timeout/overload 503) return before any handler,
+  /// inside the security-headers layer, so a timed-out request must fail
+  /// closed with the hardening headers.
+  #[tokio::test]
+  async fn router_resilience_timeout_carries_security_headers() -> TestResult {
+    let hanging = axum::Router::new().route(
+      "/hang",
+      axum::routing::get(|| async {
+        tokio::time::sleep(Duration::from_secs(30)).await;
+        "never"
+      }),
+    );
+    let router = hanging
+      .layer(crate::resilience::ResilienceLayer::new(
+        Duration::from_millis(1),
+        1024,
+      ))
+      .layer(crate::middleware::security_headers::SecurityHeadersLayer::new(false));
+    let (status, headers, _) = drive(router, "GET", "/hang", &[], "").await?;
+    if status != StatusCode::SERVICE_UNAVAILABLE {
+      return Err(format!("resilience timeout must fail closed 503, got {status}").into());
+    }
+    assert_hardening_headers(&headers)
+  }
+
   /// Every mounted sensitive route is classified by `Rule::for_path`, and the
   /// exempt routes are not. The list mirrors the mount table in
   /// `build_router_with_options`; a newly mounted sensitive endpoint that is

+ 119 - 3
src/store/codes.rs

@@ -6,9 +6,9 @@
 //! indeterminate transport result the key is re-read instead of replaying the
 //! consume, so a surviving record stays available for the client's retry.
 
-use std::{collections::HashMap, time::Duration};
+use std::{collections::HashMap, sync::LazyLock, time::Duration};
 
-use redis::RedisError;
+use redis::{RedisError, Script};
 use serde::{Deserialize, Serialize};
 
 use super::{SCRIPT_CONSUME_HASH, Store, StoreError, flat_to_map, hset_record};
@@ -78,7 +78,123 @@ pub async fn issue(
   .map_err(StoreError::Redis)
 }
 
-/// Atomically consume a code record: exactly one caller observes `Some`.
+/// Guarded single-use consume: the record is deleted only when every
+/// expected field matches, otherwise it survives untouched.
+///
+/// KEYS: code record. ARGV: flattened expected field/value pairs. Returns the
+/// hash fields on a match, `{'MISMATCH'}` when a bound field differs (the key
+/// is left intact), or `false` when absent. Mirrors the refresh-rotation
+/// `client_id` compare-and-set.
+static SCRIPT_CONSUME_CODE_GUARDED: LazyLock<Script> = LazyLock::new(|| {
+  Script::new(
+    r"
+    local data = redis.call('HGETALL', KEYS[1])
+    if #data == 0 then
+      return false
+    end
+    local h = {}
+    for i = 1, #data, 2 do
+      h[data[i]] = data[i + 1]
+    end
+    for i = 1, #ARGV, 2 do
+      if h[ARGV[i]] ~= ARGV[i + 1] then
+        return {'MISMATCH'}
+      end
+    end
+    redis.call('DEL', KEYS[1])
+    return data
+    ",
+  )
+});
+
+/// Outcome of a guarded code consume.
+#[derive(Debug)]
+pub enum ConsumeOutcome {
+  /// Bindings matched; the record was deleted and is returned.
+  Consumed(CodeRecord),
+  /// Bindings differed; the record survives for the legitimate presenter.
+  Mismatched,
+  /// No record under this hash (wrong, expired, or already consumed).
+  Absent,
+}
+
+/// Atomically consume a code record bound to its expected values: exactly one
+/// matching presenter observes `Consumed`; a mismatched presenter observes
+/// `Mismatched` without deleting anything, so a foreign `client_id`,
+/// `redirect_uri`, or PKCE challenge cannot burn the victim single-use value.
+pub async fn consume_bound(
+  conn: &mut RedisConn,
+  store: &Store,
+  code_hash: &str,
+  expected: &[(&str, &str)],
+) -> Result<ConsumeOutcome, StoreError> {
+  let key = store.code_key(code_hash);
+  let mut invocation = SCRIPT_CONSUME_CODE_GUARDED.prepare_invoke();
+  invocation.key(key.as_str());
+  for (field, value) in expected {
+    invocation.arg(field).arg(value);
+  }
+  let outcome: Result<redis::Value, StoreError> = invocation
+    .invoke_async(conn)
+    .await
+    .map_err(StoreError::Redis);
+  match outcome {
+    Ok(redis::Value::Array(items)) => {
+      if is_mismatch(&items) {
+        Ok(ConsumeOutcome::Mismatched)
+      } else {
+        let flat = items
+          .into_iter()
+          .map(redis_string)
+          .collect::<Result<Vec<_>, _>>()?;
+        CodeRecord::from_map(&flat_to_map(flat)?).map(ConsumeOutcome::Consumed)
+      }
+    }
+    // Absent (nil or a `false` reply): no record.
+    Ok(_) => Ok(ConsumeOutcome::Absent),
+    // Indeterminate transport result: re-read, never replay.
+    Err(error) => recover_consume_bound(conn, store, code_hash, error).await,
+  }
+}
+
+/// Whether a script reply is the single-element `MISMATCH` tag rather than
+/// hash fields (field/value pairs always arrive in even length).
+fn is_mismatch(items: &[redis::Value]) -> bool {
+  match items {
+    [single] => {
+      matches!(
+        single,
+        redis::Value::BulkString(bytes) if bytes.as_slice() == b"MISMATCH"
+      ) || matches!(single, redis::Value::SimpleString(text) if text == "MISMATCH")
+    }
+    _ => false,
+  }
+}
+
+/// Re-read after an indeterminate guarded consume: a surviving key means the
+/// consume did not happen (fail closed, the record stays usable); an absent
+/// key means it did (report `Absent`, never replay).
+async fn recover_consume_bound(
+  conn: &mut RedisConn,
+  store: &Store,
+  code_hash: &str,
+  error: StoreError,
+) -> Result<ConsumeOutcome, StoreError> {
+  if !matches!(error, StoreError::Redis(_)) {
+    return Err(error);
+  }
+  let key = store.code_key(code_hash);
+  let present: bool = redis::cmd("EXISTS")
+    .arg(&key)
+    .query_async(conn)
+    .await
+    .map_err(StoreError::Redis)?;
+  if present {
+    Err(error)
+  } else {
+    Ok(ConsumeOutcome::Absent)
+  }
+}
 pub async fn consume(
   conn: &mut RedisConn,
   store: &Store,

+ 13 - 2
src/store/dcr.rs

@@ -110,7 +110,11 @@ pub async fn consume_one_use(
 ///
 /// A winning claim is durability-barriered BEFORE the caller treats the proof
 /// as fresh (same contract as token rotation in [`super::tokens`]): a marker
-/// that could vanish on failover would let a replayed `jti` through.
+/// that could vanish on failover would let a replayed `jti` through. When the
+/// barrier fails after the marker was written, the marker is removed before
+/// the `Durability` error surfaces, so the originating proof can be claimed
+/// again instead of reading back as a replay; a concurrent non-originating
+/// claimant still loses while the marker stands.
 pub async fn claim_marker(
   conn: &mut RedisConn,
   store: &Store,
@@ -130,7 +134,14 @@ pub async fn claim_marker(
   .map(|set| set.is_some())
   .map_err(StoreError::Redis)?;
   if fresh {
-    durability_barrier(conn, store).await?;
+    match durability_barrier(conn, store).await {
+      Ok(()) => {}
+      Err(StoreError::Durability) => {
+        let _ = redis::cmd("DEL").arg(key).query_async::<i64>(conn).await;
+        return Err(StoreError::Durability);
+      }
+      Err(other) => return Err(other),
+    }
   }
   Ok(fresh)
 }

+ 71 - 17
src/store/device.rs

@@ -75,6 +75,28 @@ impl DeviceRecord {
   }
 }
 
+/// Polling-attempt sidecar: last poll time (Unix seconds) for a device hash.
+///
+/// It derives from the device record key, so it shares the `{state}` hash
+/// slot, and it carries the device TTL so it can never outlive its record by
+/// more than one issuance lifetime.
+#[must_use]
+pub fn poll_key(store: &Store, device_hash: &str) -> String {
+  format!("{}:poll", store.device_key(device_hash))
+}
+
+/// Outcome of one atomic polling-interval gate attempt.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum PollSlot {
+  /// This attempt is allowed; the stamp now reads the attempt time.
+  Allowed,
+  /// Too fast: wait this many whole seconds (the stamp still advanced).
+  SlowDown {
+    /// Whole seconds the device should wait before polling again.
+    retry_after_secs: u64,
+  },
+}
+
 /// Store a device record and its user-code mapping (both idempotent rewrites).
 pub async fn put(
   conn: &mut RedisConn,
@@ -139,6 +161,10 @@ pub async fn resolve_user_code(
 
 /// Record the user's verdict and bind the grant; returns false when the
 /// device record is already gone.
+///
+/// The existence check and the write run as one Lua script, so a verdict
+/// racing the atomic consume can neither resurrect a zombie record nor lose
+/// the consume's delete. The key TTL is preserved.
 pub async fn decide(
   conn: &mut RedisConn,
   store: &Store,
@@ -149,24 +175,52 @@ pub async fn decide(
 ) -> Result<bool, StoreError> {
   let key = store.device_key(device_hash);
   let budget = store.retry_budget();
-  retry_op!(conn, budget, async {
-    let exists: bool = redis::cmd("EXISTS").arg(&key).query_async(conn).await?;
-    if !exists {
-      return Ok(false);
-    }
-    redis::cmd("HSET")
-      .arg(&key)
-      .arg("status")
-      .arg(status.as_str())
-      .arg("subject")
-      .arg(subject)
-      .arg("grant_id")
-      .arg(grant_id)
-      .query_async::<()>(conn)
-      .await?;
-    Ok(true)
+  let applied: i64 = retry_op!(conn, budget, async {
+    let mut invocation = super::SCRIPT_DEVICE_DECIDE.prepare_invoke();
+    invocation.key(key.as_str());
+    invocation.arg(status.as_str());
+    invocation.arg(subject);
+    invocation.arg(grant_id);
+    invocation.invoke_async(conn).await
   })
-  .map_err(StoreError::Redis)
+  .map_err(StoreError::Redis)?;
+  Ok(applied == 1)
+}
+
+/// Enforce the polling interval and count this attempt, atomically.
+///
+/// The read-compare-write runs as one Lua compare-and-set, so concurrent
+/// polls cannot both pass the gate: exactly one attempt is admitted and the
+/// rest observe `slow_down`. A corrupt marker surfaces as
+/// [`StoreError::InvalidData`] (fail closed, never fail open).
+pub async fn claim_poll_slot(
+  conn: &mut RedisConn,
+  store: &Store,
+  device_hash: &str,
+  now_unix: i64,
+  interval: Duration,
+  device_ttl: Duration,
+) -> Result<PollSlot, StoreError> {
+  let key = poll_key(store, device_hash);
+  let budget = store.retry_budget();
+  let interval_secs = interval.as_secs();
+  let ttl_ms = device_ttl.as_millis() as u64;
+  let outcome: i64 = retry_op!(conn, budget, async {
+    let mut invocation = super::SCRIPT_DEVICE_POLL_CAS.prepare_invoke();
+    invocation.key(key.as_str());
+    invocation.arg(now_unix);
+    invocation.arg(interval_secs);
+    invocation.arg(ttl_ms);
+    invocation.invoke_async(conn).await
+  })
+  .map_err(StoreError::Redis)?;
+  match outcome {
+    0 => Ok(PollSlot::Allowed),
+    wait if wait > 0 => Ok(PollSlot::SlowDown {
+      retry_after_secs: wait as u64,
+    }),
+    _ => Err(StoreError::InvalidData),
+  }
 }
 
 /// Atomically consume an approved device record for token issuance.

+ 68 - 31
src/store/grants.rs

@@ -78,25 +78,6 @@ pub async fn get(
     .transpose()
 }
 
-/// Index a token hash in its grant set (idempotent `SADD`).
-pub async fn add_token(
-  conn: &mut RedisConn,
-  store: &Store,
-  grant_id: &str,
-  token_hash: &str,
-) -> Result<(), StoreError> {
-  let set_key = store.grant_tokens_key(grant_id);
-  retry_op!(conn, store.retry_budget(), async {
-    let _: () = redis::cmd("SADD")
-      .arg(&set_key)
-      .arg(token_hash)
-      .query_async(conn)
-      .await?;
-    Ok(())
-  })
-  .map_err(StoreError::Redis)
-}
-
 /// List the token hashes indexed under a grant.
 pub async fn members(
   conn: &mut RedisConn,
@@ -113,32 +94,88 @@ pub async fn members(
   .map_err(StoreError::Redis)
 }
 
-/// Cascade-revoke a grant: atomically delete every member token key plus the
-/// grant set and record (single-slot Lua, no `KEYS` scan).
+/// Cascade-revoke a grant: delete every member token key plus the grant set
+/// and record (single-slot Lua, no `KEYS` scan).
+///
+/// One script pass is atomic but blind to a mint racing the cascade: a member
+/// `SADD`ed after the pass's `SMEMBERS` snapshot would survive with live
+/// records. The pass therefore repeats until the set reads empty (bounded),
+/// so a token minted mid-cascade is deleted by a later pass instead of
+/// surviving the revoke.
+///
+/// The bound is fail-closed: when the set is still non-empty after the pass
+/// budget (a mint landing after every snapshot), revocation reports
+/// [`super::StoreError::InvalidData`] instead of a silent success count, and
+/// a post-barrier re-read rejects a member `SADD`ed after the final pass the
+/// same way — a late `SADD` can never leave a live TTL-less member behind
+/// quietly.
 ///
 /// Returns the number of token keys deleted. Unknown grants revoke to zero.
 pub async fn revoke(
   conn: &mut RedisConn,
   store: &Store,
   grant_id: &str,
+) -> Result<u64, StoreError> {
+  revoke_with_passes(conn, store, grant_id, REVOKE_DRAIN_PASSES).await
+}
+
+/// Drain budget: enough passes to absorb a racing mint, small enough to fail
+/// closed fast under continuous churn.
+const REVOKE_DRAIN_PASSES: u32 = 8;
+
+/// Cascade-revoke with an explicit drain budget (the production path uses
+/// [`REVOKE_DRAIN_PASSES`]); a zero budget models an already-exhausted drain.
+pub(crate) async fn revoke_with_passes(
+  conn: &mut RedisConn,
+  store: &Store,
+  grant_id: &str,
+  max_passes: u32,
 ) -> Result<u64, StoreError> {
   let set_key = store.grant_tokens_key(grant_id);
   let grant_key = store.grant_key(grant_id);
   let refresh_prefix = store.refresh_key_prefix();
   let access_prefix = store.access_key_prefix();
-  let mut invocation = SCRIPT_GRANT_REVOKE.prepare_invoke();
-  invocation.key(set_key.as_str()).key(grant_key.as_str());
-  invocation
-    .arg(refresh_prefix.as_str())
-    .arg(access_prefix.as_str());
-  let deleted: u64 = invocation
-    .invoke_async(conn)
-    .await
-    .map_err(StoreError::Redis)?;
+  let mut total = 0_u64;
+  let mut drained = false;
+  for _ in 0..max_passes {
+    let mut invocation = SCRIPT_GRANT_REVOKE.prepare_invoke();
+    invocation.key(set_key.as_str()).key(grant_key.as_str());
+    invocation
+      .arg(refresh_prefix.as_str())
+      .arg(access_prefix.as_str());
+    let deleted: u64 = invocation
+      .invoke_async(conn)
+      .await
+      .map_err(StoreError::Redis)?;
+    total = total.saturating_add(deleted);
+    let remaining: Vec<String> = redis::cmd("SMEMBERS")
+      .arg(&set_key)
+      .query_async(conn)
+      .await
+      .map_err(StoreError::Redis)?;
+    if remaining.is_empty() {
+      drained = true;
+      break;
+    }
+  }
+  if !drained {
+    return Err(StoreError::InvalidData);
+  }
   // Revocation is a reuse-driven security response: barrier it like any
   // tombstone-class write before the caller reports the outcome.
   super::durability_barrier(conn, store).await?;
-  Ok(deleted)
+  // A member `SADD`ed after the final pass would otherwise survive with live
+  // records behind a success count: re-read after the barrier and fail closed.
+  let late: Vec<String> = redis::cmd("SMEMBERS")
+    .arg(&set_key)
+    .query_async(conn)
+    .await
+    .map_err(StoreError::Redis)?;
+  if late.is_empty() {
+    Ok(total)
+  } else {
+    Err(StoreError::InvalidData)
+  }
 }
 
 /// Cascade-revoke the grant family containing `token_hash`: resolve the grant

+ 155 - 11
src/store/mod.rs

@@ -86,20 +86,69 @@ pub enum StoreError {
 }
 
 impl From<StoreError> for crate::error::AppError {
-  /// Map a store failure to a client-safe error: transient Redis outages and
-  /// unmet durability barriers render `503 temporarily_unavailable` (fail
-  /// closed); everything else is masked as `server_error`.
+  /// Map a store failure to a client-safe error: genuine Redis infrastructure
+  /// failures (transient timeouts/connections, `OOM`, `READONLY`,
+  /// `CROSSSLOT`, auth, and connection-type errors) and unmet durability
+  /// barriers render `503 temporarily_unavailable` (fail closed);
+  /// programming-error classes (`Parse`, `UnexpectedReturnType`, `Client`,
+  /// `WRONGTYPE` type errors surfaced as `Extension` or `Server`, and generic
+  /// Lua-runtime `ERR`/`EXECABORT` server failures) are
+  /// masked as `server_error` (500) so 500-alerts still fire on real bugs.
+  /// Genuinely internal bugs (`InvalidData`, `Mismatch`) are likewise masked
+  /// as `server_error`.
   fn from(error: StoreError) -> Self {
     match error {
       StoreError::Durability => Self::temporarily_unavailable(),
-      StoreError::Redis(ref inner) if classify_redis(inner) == ErrorClass::Transient => {
-        Self::temporarily_unavailable()
-      }
+      StoreError::Redis(inner) if is_infra_redis_error(&inner) => Self::temporarily_unavailable(),
+      StoreError::Redis(inner) => Self::internal(inner),
       other => Self::internal(other),
     }
   }
 }
 
+/// Whether a Redis error is genuine infrastructure (503) rather than a
+/// programming error (500): transient errors plus `OOM`/`READONLY`/
+/// `CROSSSLOT`/auth/connection-type errors fail closed, while `Parse`,
+/// `UnexpectedReturnType`, `Client`, `WRONGTYPE` type errors, and generic
+/// Lua-runtime `ERR`/`EXECABORT` server failures stay visible as masked
+/// `server_error`.
+fn is_infra_redis_error(error: &RedisError) -> bool {
+  if is_wrongtype(error) {
+    return false;
+  }
+  // Generic Lua/script-runtime failures (`ERR` class, `EXECABORT`) are bugs
+  // in the script or its arguments, not sick infrastructure: report them as
+  // masked `server_error` (500) so 500-alerts fire. Transient kinds,
+  // `NOSCRIPT` (self-healed), `OOM`/`READONLY`/`CROSSSLOT`, auth, and
+  // connection errors keep their 503 fail-closed mapping below.
+  if matches!(
+    error.kind(),
+    ErrorKind::Server(ServerErrorKind::ResponseError | ServerErrorKind::ExecAbort)
+  ) {
+    return false;
+  }
+  !matches!(
+    error.kind(),
+    ErrorKind::Parse | ErrorKind::UnexpectedReturnType | ErrorKind::Client
+  )
+}
+
+/// Whether a Redis error is a `WRONGTYPE` type error: an `Extension` with code
+/// `WRONGTYPE`, or any server error whose code or detail carries it (the
+/// `WRONGTYPE ... wrong kind of value` detail on a generic `ERR`).
+fn is_wrongtype(error: &RedisError) -> bool {
+  if error
+    .code()
+    .is_some_and(|code| code.eq_ignore_ascii_case("WRONGTYPE"))
+  {
+    return true;
+  }
+  error.detail().is_some_and(|detail| {
+    let lower = detail.to_ascii_lowercase();
+    lower.contains("wrongtype") || lower.contains("wrong kind")
+  })
+}
+
 /// Whether a Redis error is worth retrying (idempotent ops only) or must fail
 /// closed immediately.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -555,6 +604,31 @@ pub(crate) static HSET_TTL: LazyLock<Script> = LazyLock::new(|| {
   )
 });
 
+/// Atomic grant-member write: `HSET` the record fields, `PEXPIRE` the
+/// record, `SADD` the member into the grant set, then arm-or-extend the set
+/// TTL (never shorten), before the script returns.
+///
+/// KEYS: record, grant token set. ARGV: record TTL ms, member, then the
+/// flattened record field/value pairs. The record and its index entry land
+/// together, so a revoke drain cannot interleave between the record write and
+/// the index write and leave an orphan in either direction.
+pub(crate) static SCRIPT_PUT_MEMBER: LazyLock<Script> = LazyLock::new(|| {
+  Script::new(
+    r"
+    if #ARGV > 2 then
+      redis.call('HSET', KEYS[1], unpack(ARGV, 3))
+      redis.call('PEXPIRE', KEYS[1], ARGV[1])
+    end
+    redis.call('SADD', KEYS[2], ARGV[2])
+    local set_pttl = redis.call('PTTL', KEYS[2])
+    if set_pttl < 0 or set_pttl < tonumber(ARGV[1]) then
+      redis.call('PEXPIRE', KEYS[2], ARGV[1])
+    end
+    return 1
+    ",
+  )
+});
+
 /// Single-use consume with `GETDEL` semantics: return the hash fields and
 /// delete the key atomically, or `false` when absent.
 pub(crate) static SCRIPT_CONSUME_HASH: LazyLock<Script> = LazyLock::new(|| {
@@ -570,15 +644,82 @@ pub(crate) static SCRIPT_CONSUME_HASH: LazyLock<Script> = LazyLock::new(|| {
   )
 });
 
+/// Device verdict conditional write (RFC 8628 user decision).
+///
+/// KEYS: device record. ARGV: status, subject, grant id. Sets the verdict
+/// fields only when the record still exists, so a verdict racing the atomic
+/// consume can never resurrect a zombie record (no lost update). The key TTL
+/// is left untouched. Returns 1 when applied, 0 when the record is gone.
+pub(crate) static SCRIPT_DEVICE_DECIDE: LazyLock<Script> = LazyLock::new(|| {
+  Script::new(
+    r"
+    if redis.call('EXISTS', KEYS[1]) == 0 then
+      return 0
+    end
+    redis.call('HSET', KEYS[1], 'status', ARGV[1], 'subject', ARGV[2], 'grant_id', ARGV[3])
+    return 1
+    ",
+  )
+});
+
+/// Device poll-interval compare-and-set (RFC 8628 §3.5 `slow_down`).
+///
+/// KEYS: poll sidecar. ARGV: now (Unix seconds), interval (seconds), TTL ms.
+/// Reads the last attempt, compares, and stamps this attempt in one step, so
+/// concurrent polls cannot both pass the gate. The stamp advances on every
+/// path, so fast polls keep backing off instead of racing the window edge.
+/// Returns 0 when this attempt is allowed, the whole seconds to wait when it
+/// is too fast, or -1 when the stored marker is corrupt.
+pub(crate) static SCRIPT_DEVICE_POLL_CAS: LazyLock<Script> = LazyLock::new(|| {
+  Script::new(
+    r"
+    local last = redis.call('GET', KEYS[1])
+    if not last then
+      redis.call('SET', KEYS[1], ARGV[1], 'PX', ARGV[3])
+      return 0
+    end
+    local last_secs = tonumber(last)
+    if not last_secs then
+      return -1
+    end
+    local elapsed = tonumber(ARGV[1]) - last_secs
+    if elapsed < 0 then
+      elapsed = 0
+    end
+    local interval_secs = tonumber(ARGV[2])
+    if elapsed < interval_secs then
+      redis.call('SET', KEYS[1], ARGV[1], 'PX', ARGV[3])
+      local wait = interval_secs - elapsed
+      if wait < 1 then
+        wait = 1
+      end
+      return wait
+    end
+    redis.call('SET', KEYS[1], ARGV[1], 'PX', ARGV[3])
+    return 0
+    ",
+  )
+});
+
 /// Refresh rotation compare-and-set.
 ///
-/// KEYS: old refresh, new refresh, new access, grant token set, request map.
-/// ARGV: expected client, new-refresh JSON, new-refresh TTL ms, new-access
-/// JSON, new-access TTL ms, request id, grant id, new refresh member,
-/// new access member, tombstone TTL ms, map TTL ms.
+/// KEYS: old refresh, new refresh, new access, grant token set, request map,
+/// grant record. ARGV: expected client, new-refresh JSON, new-refresh TTL ms,
+/// new-access JSON, new-access TTL ms, request id, grant id, new refresh
+/// member, new access member, tombstone TTL ms, map TTL ms.
 ///
 /// Returns `{TAG, ...}`: `ROTATED`, `REPLAY` (idempotent retry: same tokens),
-/// `REUSE` (tombstoned: rotated-to member), `ABSENT`, or `MISMATCH`.
+/// `REUSE` (tombstoned: rotated-to member), `NOGRANT` (grant record gone: a
+/// mint racing a cascade is fenced before writing, fail closed with no
+/// cascade), `ABSENT`, or `MISMATCH`.
+///
+/// Precedence is intentional: the idempotency-map `REPLAY` check runs before
+/// the `NOGRANT` grant-record fence, so a retried request whose grant was
+/// revoked mid-cascade still resolves to its already-minted pair instead of
+/// locking out. `REPLAY` mints nothing new — the re-resolved hashes name
+/// records the cascade already deleted, so the service layer safely discards
+/// them — and only map-miss rotations reach the fence, which writes nothing,
+/// indexes nothing, and cascades nothing.
 pub(crate) static SCRIPT_ROTATE_REFRESH: LazyLock<Script> = LazyLock::new(|| {
   Script::new(
     r"
@@ -587,6 +728,9 @@ pub(crate) static SCRIPT_ROTATE_REFRESH: LazyLock<Script> = LazyLock::new(|| {
       local parts = cjson.decode(mapped)
       return {'REPLAY', parts['refresh'], parts['access']}
     end
+    if redis.call('EXISTS', KEYS[6]) == 0 then
+      return {'NOGRANT'}
+    end
     local old = redis.call('HGETALL', KEYS[1])
     if #old == 0 then
       return {'ABSENT'}

+ 108 - 1
src/store/par.rs

@@ -1,7 +1,8 @@
 //! PAR `request_uri` records (RFC 9126, 90s TTL).
 
-use std::{collections::HashMap, time::Duration};
+use std::{collections::HashMap, sync::LazyLock, time::Duration};
 
+use redis::Script;
 use serde::{Deserialize, Serialize};
 
 use super::{SCRIPT_CONSUME_HASH, Store, StoreError, hset_record, retry_op};
@@ -51,6 +52,112 @@ pub async fn put(
   .map_err(StoreError::Redis)
 }
 
+/// Guarded single-use consume: the record is deleted only when the expected
+/// client matches, otherwise it survives untouched.
+///
+/// KEYS: PAR record. ARGV: expected `client_id`. Returns the hash fields on
+/// a match, `{'MISMATCH'}` when the presenter is foreign (the key is left
+/// intact), or `false` when absent. Mirrors the refresh-rotation `client_id`
+/// compare-and-set.
+static SCRIPT_CONSUME_PAR_GUARDED: LazyLock<Script> = LazyLock::new(|| {
+  Script::new(
+    r"
+    local data = redis.call('HGETALL', KEYS[1])
+    if #data == 0 then
+      return false
+    end
+    local h = {}
+    for i = 1, #data, 2 do
+      h[data[i]] = data[i + 1]
+    end
+    if h['client_id'] ~= ARGV[1] then
+      return {'MISMATCH'}
+    end
+    redis.call('DEL', KEYS[1])
+    return data
+    ",
+  )
+});
+
+/// Outcome of a guarded PAR consume.
+#[derive(Debug)]
+pub enum ConsumeOutcome {
+  /// The presenting client owns the record; it was deleted and is returned.
+  Consumed(ParRecord),
+  /// A foreign client presented the id; the record survives for its owner.
+  Mismatched,
+  /// No record under this id (unknown, expired, or already consumed).
+  Absent,
+}
+
+/// Atomically consume a PAR record bound to its owning client: exactly one
+/// matching presenter observes `Consumed`; a foreign presenter observes
+/// `Mismatched` without deleting anything.
+pub async fn consume_bound(
+  conn: &mut RedisConn,
+  store: &Store,
+  request_id: &str,
+  expected_client_id: &str,
+) -> Result<ConsumeOutcome, StoreError> {
+  let key = store.par_key(request_id);
+  let mut invocation = SCRIPT_CONSUME_PAR_GUARDED.prepare_invoke();
+  invocation.key(key.as_str());
+  invocation.arg(expected_client_id);
+  let outcome: Result<redis::Value, StoreError> = invocation
+    .invoke_async(conn)
+    .await
+    .map_err(StoreError::Redis);
+  match outcome {
+    Ok(redis::Value::Array(items)) => {
+      if is_mismatch(&items) {
+        Ok(ConsumeOutcome::Mismatched)
+      } else {
+        let mut flat = Vec::with_capacity(items.len());
+        for item in items {
+          match item {
+            redis::Value::BulkString(bytes) => {
+              flat.push(String::from_utf8(bytes).map_err(|_| StoreError::InvalidData)?);
+            }
+            redis::Value::SimpleString(text) => flat.push(text),
+            _ => return Err(StoreError::InvalidData),
+          }
+        }
+        ParRecord::from_map(&super::flat_to_map(flat)?).map(ConsumeOutcome::Consumed)
+      }
+    }
+    Ok(_) => Ok(ConsumeOutcome::Absent),
+    Err(error) => {
+      if !matches!(error, StoreError::Redis(_)) {
+        return Err(error);
+      }
+      let present: bool = redis::cmd("EXISTS")
+        .arg(&key)
+        .query_async(conn)
+        .await
+        .map_err(StoreError::Redis)?;
+      if present {
+        Err(error)
+      } else {
+        Ok(ConsumeOutcome::Absent)
+      }
+    }
+  }
+}
+
+/// Whether a script reply is the single-element `MISMATCH` tag rather than
+/// hash fields (field/value pairs always arrive in even length).
+fn is_mismatch(items: &[redis::Value]) -> bool {
+  match items {
+    [single] => {
+      matches!(
+        single,
+        redis::Value::BulkString(bytes) if bytes.as_slice() == b"MISMATCH"
+      ) || matches!(single, redis::Value::SimpleString(text) if text == "MISMATCH")
+    }
+    _ => false,
+  }
+}
+
 /// Atomically consume a PAR record: exactly one caller observes `Some`.
 pub async fn consume(
   conn: &mut RedisConn,

+ 868 - 16
src/store/tests_store.rs

@@ -1493,6 +1493,132 @@ fn store_error_classification_and_retry_scope() -> TestResult {
   Ok(())
 }
 
+#[test]
+fn store_permanent_infra_errors_map_to_503() -> TestResult {
+  use crate::error::{AppError, OAuthErrorCode};
+  use axum::http::StatusCode;
+
+  let cross_slot = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::CrossSlot),
+    "slot",
+  ));
+  let readonly = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::ReadOnly),
+    "ro",
+  ));
+  let auth = redis::RedisError::from((redis::ErrorKind::AuthenticationFailed, "auth"));
+  let oom_value = redis::parse_redis_value(b"-OOM command not allowed\r\n");
+  let oom = match oom_value {
+    Ok(redis::Value::ServerError(server)) => redis::RedisError::from(server),
+    _ => {
+      return Err(TestError::Unexpected(
+        "expected an OOM server error value".to_owned(),
+      ));
+    }
+  };
+
+  for error in [cross_slot, readonly, auth, oom] {
+    let mapped = AppError::from(StoreError::Redis(error));
+    if mapped.code() != OAuthErrorCode::TemporarilyUnavailable {
+      return Err(TestError::Unexpected(
+        "permanent Redis infra failure must map to temporarily_unavailable".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::SERVICE_UNAVAILABLE {
+      return Err(TestError::Unexpected(
+        "permanent Redis infra failure must render 503".to_owned(),
+      ));
+    }
+  }
+
+  // Genuinely internal bugs stay masked as server_error (500).
+  for error in [StoreError::InvalidData, StoreError::Mismatch] {
+    let mapped = AppError::from(error);
+    if mapped.code() != OAuthErrorCode::ServerError {
+      return Err(TestError::Unexpected(
+        "internal store bug must stay server_error".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::INTERNAL_SERVER_ERROR {
+      return Err(TestError::Unexpected(
+        "internal store bug must render 500".to_owned(),
+      ));
+    }
+  }
+  Ok(())
+}
+
+#[test]
+fn store_error_classification() -> TestResult {
+  use crate::error::{AppError, OAuthErrorCode};
+  use axum::http::StatusCode;
+
+  let parse = redis::RedisError::from((redis::ErrorKind::Parse, "parse"));
+  let unexpected = redis::RedisError::from((redis::ErrorKind::UnexpectedReturnType, "type"));
+  let client = redis::RedisError::from((redis::ErrorKind::Client, "client"));
+  let wrongtype_value = redis::parse_redis_value(
+    b"-WRONGTYPE Operation against a key holding the wrong kind of value\r\n",
+  );
+  let wrongtype = match wrongtype_value {
+    Ok(redis::Value::ServerError(server)) => redis::RedisError::from(server),
+    _ => {
+      return Err(TestError::Unexpected(
+        "expected a WRONGTYPE server error value".to_owned(),
+      ));
+    }
+  };
+  for error in [parse, unexpected, client, wrongtype] {
+    let mapped = AppError::from(StoreError::Redis(error));
+    if mapped.code() != OAuthErrorCode::ServerError {
+      return Err(TestError::Unexpected(
+        "programming Redis error must map to server_error".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::INTERNAL_SERVER_ERROR {
+      return Err(TestError::Unexpected(
+        "programming Redis error must render 500".to_owned(),
+      ));
+    }
+  }
+
+  let cross_slot = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::CrossSlot),
+    "slot",
+  ));
+  let readonly = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::ReadOnly),
+    "ro",
+  ));
+  let auth = redis::RedisError::from((redis::ErrorKind::AuthenticationFailed, "auth"));
+  let transient = redis::RedisError::from(std::io::Error::new(
+    std::io::ErrorKind::TimedOut,
+    "timed out",
+  ));
+  let oom_value = redis::parse_redis_value(b"-OOM command not allowed\r\n");
+  let oom = match oom_value {
+    Ok(redis::Value::ServerError(server)) => redis::RedisError::from(server),
+    _ => {
+      return Err(TestError::Unexpected(
+        "expected an OOM server error value".to_owned(),
+      ));
+    }
+  };
+  for error in [cross_slot, readonly, auth, transient, oom] {
+    let mapped = AppError::from(StoreError::Redis(error));
+    if mapped.code() != OAuthErrorCode::TemporarilyUnavailable {
+      return Err(TestError::Unexpected(
+        "infra Redis failure must map to temporarily_unavailable".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::SERVICE_UNAVAILABLE {
+      return Err(TestError::Unexpected(
+        "infra Redis failure must render 503".to_owned(),
+      ));
+    }
+  }
+  Ok(())
+}
+
 #[tokio::test]
 async fn store_retry_budget_bounds_idempotent_reads() -> TestResult {
   use std::sync::atomic::{AtomicU32, Ordering as AtomicOrdering};
@@ -1580,28 +1706,754 @@ async fn store_hset_record_writes_fields_and_ttl() -> TestResult {
   assert_ttl_within(pttl_ms(&mut conn, &key).await?, ttl)
 }
 
-/// A field-less `HSET` is a Redis arity error; the empty-pairs guard falls
-/// back to the TTL touch alone, so an existing record keeps its fields and a
-/// live expiry instead of becoming persistent (or erroring out).
+/// Revoke drain exhaustion must fail closed, never report silent success.
+///
+/// A zero-pass budget models the exhausted drain (a mint landing after every
+/// snapshot, so the set never reads empty): members still indexed must
+/// surface [`StoreError::InvalidData`], never a silent `Ok` count. The full
+/// production budget on the same grant then still drains to zero with no live
+/// member records behind it.
 #[tokio::test]
-async fn store_hset_record_empty_pairs_keeps_ttl() -> TestResult {
+async fn store_grant_revoke_exhaustion_is_not_silent() -> TestResult {
   let (_container, url) = redis_container(Vec::new()).await?;
-  let (mut conn, _store) = direct_store(&url, DurabilityMode::WaitAof).await?;
-  let key = unique("hset-empty");
-  let ttl = Duration::from_secs(300);
-  let pairs = vec![("client_id".to_owned(), "client-1".to_owned())];
+  let (mut conn, store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let grant = unique("grant");
+  let client = unique("client");
+  let seed = unique("seed-refresh");
+  grants::create(
+    &mut conn,
+    &store,
+    &grants::GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "orders:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_refresh(
+    &mut conn,
+    &store,
+    &seed,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
 
-  hset_record(&mut conn, &key, &pairs, ttl).await?;
-  hset_record(&mut conn, &key, &[], ttl).await?;
+  match grants::revoke_with_passes(&mut conn, &store, &grant, 0).await {
+    Err(StoreError::InvalidData) => {}
+    Ok(count) => {
+      return Err(TestError::Unexpected(format!(
+        "exhausted drain must fail closed, got Ok({count})"
+      )));
+    }
+    Err(error) => {
+      return Err(TestError::Unexpected(format!(
+        "exhausted drain must be InvalidData, got {error:?}"
+      )));
+    }
+  }
 
-  let record = hget_record(&mut conn, &key).await?;
-  if record
-    .as_ref()
-    .is_none_or(|fields| fields.get("client_id").map(String::as_str) != Some("client-1"))
+  // The production budget on the same grant drains fully: no live members.
+  let drained = grants::revoke(&mut conn, &store, &grant).await?;
+  if drained == 0 {
+    return Err(TestError::Unexpected(
+      "production revoke must count the seed member".to_owned(),
+    ));
+  }
+  if tokens::get_refresh(&mut conn, &store, &seed)
+    .await?
+    .is_some()
   {
     return Err(TestError::Unexpected(
-      "empty rewrite must preserve the fields".to_owned(),
+      "revoke must leave no live member record".to_owned(),
     ));
   }
-  assert_ttl_within(pttl_ms(&mut conn, &key).await?, ttl)
+  if !grants::members(&mut conn, &store, &grant).await?.is_empty() {
+    return Err(TestError::Unexpected(
+      "revoke must leave the grant set empty".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// A corrupt poll-interval marker must fail closed, never fail open.
+///
+/// A non-numeric sidecar value trips the script's `-1` path, which
+/// `claim_poll_slot` surfaces as `InvalidData` (500-masked, never an
+/// admission).
+#[tokio::test]
+async fn claim_poll_slot_corrupt_marker_fails_closed() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let device_hash = unique("device");
+  let key = device::poll_key(&store, &device_hash);
+  let _: () = redis::cmd("SET")
+    .arg(&key)
+    .arg("not-a-number")
+    .query_async(&mut conn)
+    .await?;
+
+  match device::claim_poll_slot(
+    &mut conn,
+    &store,
+    &device_hash,
+    1_700_000_000,
+    Duration::from_secs(5),
+    Duration::from_secs(600),
+  )
+  .await
+  {
+    Err(StoreError::InvalidData) => Ok(()),
+    Ok(slot) => Err(TestError::Unexpected(format!(
+      "corrupt poll marker must fail closed, admitted {slot:?}"
+    ))),
+    Err(error) => Err(TestError::Unexpected(format!(
+      "corrupt poll marker must be InvalidData, got {error:?}"
+    ))),
+  }
+}
+
+/// Generic Lua-runtime failures (`ERR`/`EXECABORT` server errors that are not
+/// `WRONGTYPE`) are programming errors: they must map to masked
+/// `server_error` (500) so 500-alerts fire, while genuine infrastructure
+/// (`OOM`, `READONLY`, `CROSSSLOT`, auth, transient/connection) stays 503.
+#[test]
+fn store_error_classification_lua_runtime_is_500() -> TestResult {
+  use crate::error::{AppError, OAuthErrorCode};
+  use axum::http::StatusCode;
+
+  let lua_err = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::ResponseError),
+    "ERR Error running script: @user_script:1: bad argument",
+  ));
+  let exec_abort = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::ExecAbort),
+    "EXECABORT Script aborted",
+  ));
+  for error in [lua_err, exec_abort] {
+    let mapped = AppError::from(StoreError::Redis(error));
+    if mapped.code() != OAuthErrorCode::ServerError {
+      return Err(TestError::Unexpected(
+        "generic Lua-runtime failure must map to server_error".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::INTERNAL_SERVER_ERROR {
+      return Err(TestError::Unexpected(
+        "generic Lua-runtime failure must render 500".to_owned(),
+      ));
+    }
+  }
+
+  // Genuine infrastructure stays fail-closed 503.
+  let oom_value = redis::parse_redis_value(b"-OOM command not allowed\r\n");
+  let oom = match oom_value {
+    Ok(redis::Value::ServerError(server)) => redis::RedisError::from(server),
+    _ => {
+      return Err(TestError::Unexpected(
+        "expected an OOM server error value".to_owned(),
+      ));
+    }
+  };
+  let readonly = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::ReadOnly),
+    "ro",
+  ));
+  let cross_slot = redis::RedisError::from((
+    redis::ErrorKind::Server(redis::ServerErrorKind::CrossSlot),
+    "slot",
+  ));
+  let auth = redis::RedisError::from((redis::ErrorKind::AuthenticationFailed, "auth"));
+  let transient = redis::RedisError::from(std::io::Error::new(
+    std::io::ErrorKind::TimedOut,
+    "timed out",
+  ));
+  for error in [oom, readonly, cross_slot, auth, transient] {
+    let mapped = AppError::from(StoreError::Redis(error));
+    if mapped.code() != OAuthErrorCode::TemporarilyUnavailable {
+      return Err(TestError::Unexpected(
+        "infra failure must map to temporarily_unavailable".to_owned(),
+      ));
+    }
+    if mapped.status() != StatusCode::SERVICE_UNAVAILABLE {
+      return Err(TestError::Unexpected(
+        "infra failure must render 503".to_owned(),
+      ));
+    }
+  }
+  Ok(())
+}
+
+/// T3-B1: `decide` (EXISTS then HSET) racing `consume` (atomic DEL) must
+/// never resurrect a zombie record. Every round puts a fresh record and runs
+/// both ops concurrently; after both settle the key must be gone in every
+/// case (decide won and consume reaped the verdict, or consume won and
+/// decide saw the miss) — a surviving record is a verdict recreated after
+/// the delete, i.e. a lost update.
+#[tokio::test]
+async fn store_device_decide_consume_leaves_no_zombie() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let client = unique("client");
+  for round in 0..100u32 {
+    let device_hash = format!("{}-{round:03}", unique("device-race"));
+    let user_code = format!("{}-{round:03}", unique("user-race"));
+    let grant = format!("{}-{round:03}", unique("grant-race"));
+    device::put(
+      &mut conn,
+      &store,
+      &device_hash,
+      &user_code,
+      &device::DeviceRecord {
+        client_id: client.clone(),
+        scope: "orders:read".to_owned(),
+        status: device::DeviceStatus::Pending,
+        subject: String::new(),
+        grant_id: String::new(),
+      },
+      Duration::from_secs(600),
+    )
+    .await?;
+    let decide_task = tokio::spawn({
+      let conn = conn.clone();
+      let store = store.clone();
+      let device_hash = device_hash.clone();
+      let grant = grant.clone();
+      async move {
+        let mut conn = conn;
+        device::decide(
+          &mut conn,
+          &store,
+          &device_hash,
+          device::DeviceStatus::Approved,
+          "user-1",
+          &grant,
+        )
+        .await
+      }
+    });
+    let consume_task = tokio::spawn({
+      let conn = conn.clone();
+      let store = store.clone();
+      let device_hash = device_hash.clone();
+      async move {
+        let mut conn = conn;
+        device::consume(&mut conn, &store, &device_hash).await
+      }
+    });
+    let decided = decide_task
+      .await
+      .map_err(|_| TestError::Join)?
+      .map_err(|error| TestError::Unexpected(format!("round {round} decide: {error:?}")))?;
+    let consumed_present = consume_task
+      .await
+      .map_err(|_| TestError::Join)?
+      .map_err(|error| TestError::Unexpected(format!("round {round} consume: {error:?}")))?
+      .is_some();
+    let observed = device::poll(&mut conn, &store, &device_hash)
+      .await
+      .map_err(|error| TestError::Unexpected(format!("round {round} poll: {error:?}")))?;
+    if observed.is_some() {
+      return Err(TestError::Unexpected(format!(
+        "round {round}: zombie (decided={decided}, consumed={consumed_present})"
+      )));
+    }
+  }
+  Ok(())
+}
+
+/// T3-B4: a rotation into a revoked grant must be fenced, never minted.
+///
+/// A token minted mid-cascade (live record, grant record already gone) must
+/// not rotate into fresh tokens: the fence rejects with `Absent`, writes no
+/// new records, indexes no new members, and leaves the presented record live
+/// (fail closed, no cascade on a fenced write).
+#[tokio::test]
+async fn store_grant_revoke_fence_rejects_rotation_into_revoked_grant() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let grant = unique("grant");
+  let client = unique("client");
+  let old = unique("old-refresh");
+  let pepper = b"store-test-pepper";
+
+  grants::create(
+    &mut conn,
+    &store,
+    &grants::GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "orders:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_refresh(
+    &mut conn,
+    &store,
+    &old,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+  grants::revoke(&mut conn, &store, &grant).await?;
+
+  // The mid-cascade mint: a live record re-indexed after the grant record
+  // died (the rotation SADD that slipped through the revoke snapshot).
+  let late_old = unique("late-refresh");
+  tokens::put_refresh(
+    &mut conn,
+    &store,
+    &late_old,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+
+  let new_refresh = unique("fenced-refresh");
+  let new_access = unique("fenced-access");
+  let outcome = tokens::rotate(
+    &mut conn,
+    &store,
+    pepper,
+    &rotation_input(&late_old, &client, &new_refresh, &new_access, &grant, None),
+  )
+  .await?;
+  if !matches!(outcome, tokens::RotateOutcome::Absent) {
+    return Err(TestError::Unexpected(
+      "rotation into a revoked grant must be fenced to Absent".to_owned(),
+    ));
+  }
+  if tokens::get_refresh(&mut conn, &store, &new_refresh)
+    .await?
+    .is_some()
+    || tokens::get_access(&mut conn, &store, &new_access)
+      .await?
+      .is_some()
+  {
+    return Err(TestError::Unexpected(
+      "fenced rotation must leave no minted records behind".to_owned(),
+    ));
+  }
+  if grants::members(&mut conn, &store, &grant)
+    .await?
+    .iter()
+    .any(|member| member == &new_refresh || member == &new_access)
+  {
+    return Err(TestError::Unexpected(
+      "fenced rotation must index no new members".to_owned(),
+    ));
+  }
+  if tokens::get_refresh(&mut conn, &store, &late_old)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "fenced rotation must not cascade on the presented record".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// T3-B4: one `revoke` call drains members indexed mid-cascade.
+///
+/// A single-shot `SMEMBERS`-snapshot revoke leaves a member `SADD`ed after
+/// its snapshot alive; `revoke` must loop until the grant set reads empty so
+/// a mint that lands mid-cascade is deleted by a later pass.
+#[tokio::test]
+async fn store_grant_revoke_drains_mid_cascade_members() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let grant = unique("grant");
+  let client = unique("client");
+
+  grants::create(
+    &mut conn,
+    &store,
+    &grants::GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "orders:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_refresh(
+    &mut conn,
+    &store,
+    &unique("seed-refresh"),
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_access(
+    &mut conn,
+    &store,
+    &unique("seed-access"),
+    &access_record(&client, &grant),
+    Duration::from_secs(300),
+  )
+  .await?;
+
+  // First pass: the cascade snapshot. A mint lands after it (the SADD the
+  // single-shot script can never see).
+  let set_key = store.grant_tokens_key(&grant);
+  let grant_key = store.grant_key(&grant);
+  let mut first = super::SCRIPT_GRANT_REVOKE.prepare_invoke();
+  first.key(set_key.as_str()).key(grant_key.as_str());
+  first
+    .arg(store.refresh_key_prefix().as_str())
+    .arg(store.access_key_prefix().as_str());
+  let _: u64 = first.invoke_async(&mut conn).await?;
+
+  let late_refresh = unique("late-refresh");
+  let late_access = unique("late-access");
+  tokens::put_refresh(
+    &mut conn,
+    &store,
+    &late_refresh,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_access(
+    &mut conn,
+    &store,
+    &late_access,
+    &access_record(&client, &grant),
+    Duration::from_secs(300),
+  )
+  .await?;
+
+  // One `revoke` call must drain the stragglers: no survivor records, an
+  // empty index, and a count that includes the late pair.
+  let drained = grants::revoke(&mut conn, &store, &grant).await?;
+  if drained < 2 {
+    return Err(TestError::Unexpected(
+      "revoke must count the mid-cascade pair".to_owned(),
+    ));
+  }
+  if tokens::get_refresh(&mut conn, &store, &late_refresh)
+    .await?
+    .is_some()
+    || tokens::get_access(&mut conn, &store, &late_access)
+      .await?
+      .is_some()
+  {
+    return Err(TestError::Unexpected(
+      "revoke must delete tokens minted mid-cascade".to_owned(),
+    ));
+  }
+  if !grants::members(&mut conn, &store, &grant).await?.is_empty() {
+    return Err(TestError::Unexpected(
+      "revoke must leave the grant set empty".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// T3-B5: a barrier failure after the rotation write returns the originating
+/// request's tokens instead of a whole-grant lockout.
+///
+/// On a replica-less `WAIT` node the barrier always fails closed; the
+/// originating rotation (this request id) must still resolve to its
+/// already-minted pair (`RotatedUnbarriered`), while a non-originating request id on the
+/// tombstoned old token still cascades (fail closed, no tokens).
+#[tokio::test]
+async fn store_rotation_barrier_failure_returns_replay_for_originating_id() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, durable) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let fragile = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::Wait,
+  );
+  let grant = unique("grant");
+  let client = unique("client");
+  let old = unique("old-refresh");
+  let new_refresh = unique("new-refresh");
+  let new_access = unique("new-access");
+  let pepper = b"store-test-pepper";
+
+  grants::create(
+    &mut conn,
+    &durable,
+    &grants::GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "orders:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_refresh(
+    &mut conn,
+    &durable,
+    &old,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+
+  let outcome = tokens::rotate(
+    &mut conn,
+    &fragile,
+    pepper,
+    &rotation_input(&old, &client, &new_refresh, &new_access, &grant, None),
+  )
+  .await?;
+  match outcome {
+    tokens::RotateOutcome::RotatedUnbarriered {
+      new_refresh_hash,
+      new_access_hash,
+    } if new_refresh_hash == new_refresh && new_access_hash == new_access => {}
+    _ => {
+      return Err(TestError::Unexpected(
+        "barrier failure must return the originating pair as RotatedUnbarriered".to_owned(),
+      ));
+    }
+  }
+  if tokens::get_refresh(&mut conn, &durable, &new_refresh)
+    .await?
+    .is_none()
+    || tokens::get_access(&mut conn, &durable, &new_access)
+      .await?
+      .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "originating pair must be readable after barrier failure".to_owned(),
+    ));
+  }
+
+  // A non-originating request id on the tombstoned old token gets no tokens:
+  // reuse, plus the cascade.
+  let request_id = tokens::derive_rotation_id(pepper, &old)?;
+  let map_key = durable.rotation_map_key(&request_id);
+  let _: () = redis::cmd("DEL")
+    .arg(&map_key)
+    .query_async(&mut conn)
+    .await?;
+  let foreign = tokens::rotate(
+    &mut conn,
+    &durable,
+    pepper,
+    &rotation_input(
+      &old,
+      &client,
+      &unique("foreign"),
+      &unique("foreign-a"),
+      &grant,
+      Some("req-foreign".to_owned()),
+    ),
+  )
+  .await?;
+  if !matches!(foreign, tokens::RotateOutcome::Reused { .. }) {
+    return Err(TestError::Unexpected(
+      "non-originating retry must stay fail-closed reuse".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// T3-B5: a barrier failure after a winning replay-marker claim must not burn
+/// the proof.
+///
+/// The marker is removed before the `Durability` error surfaces, so the
+/// originating `jti` can be claimed again; a concurrent non-originating
+/// claimant still loses while the marker stands.
+#[tokio::test]
+async fn store_replay_marker_barrier_failure_does_not_burn_proof() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, durable) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let fragile = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::Wait,
+  );
+  let jti = unique("jti");
+  let ttl = Duration::from_secs(300);
+
+  match dcr::claim_dpop_jti(&mut conn, &fragile, &jti, ttl).await {
+    Err(StoreError::Durability) => {}
+    _ => {
+      return Err(TestError::Unexpected(
+        "failing barrier must surface Durability".to_owned(),
+      ));
+    }
+  }
+  let marker: Option<String> = redis::cmd("GET")
+    .arg(durable.dpop_jti_key(&jti))
+    .query_async(&mut conn)
+    .await?;
+  if marker.is_some() {
+    return Err(TestError::Unexpected(
+      "failed claim must not leave the marker behind".to_owned(),
+    ));
+  }
+  if !dcr::claim_dpop_jti(&mut conn, &durable, &jti, ttl).await? {
+    return Err(TestError::Unexpected(
+      "originating jti must be claimable after the failure".to_owned(),
+    ));
+  }
+  Ok(())
+}
+
+/// A field-less `HSET` is a Redis arity error; the empty-pairs guard falls
+/// back to the TTL touch alone, so an existing record keeps its fields and a
+/// live expiry instead of becoming persistent (or erroring out).
+#[tokio::test]
+async fn store_hset_record_empty_pairs_keeps_ttl() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, _store) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let key = unique("hset-empty");
+  let ttl = Duration::from_secs(300);
+  let pairs = vec![("client_id".to_owned(), "client-1".to_owned())];
+
+  hset_record(&mut conn, &key, &pairs, ttl).await?;
+  hset_record(&mut conn, &key, &[], ttl).await?;
+
+  let record = hget_record(&mut conn, &key).await?;
+  if record
+    .as_ref()
+    .is_none_or(|fields| fields.get("client_id").map(String::as_str) != Some("client-1"))
+  {
+    return Err(TestError::Unexpected(
+      "empty rewrite must preserve the fields".to_owned(),
+    ));
+  }
+  assert_ttl_within(pttl_ms(&mut conn, &key).await?, ttl)
+}
+
+/// Wait-mode writes without a replica always fail the barrier; the pending
+/// pre-mint must skip it (`Ok`) while the barriered write fails closed.
+#[tokio::test]
+async fn store_put_access_pending_skips_barrier() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, _) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let fragile = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::Wait,
+  );
+  let grant = unique("grant");
+  let client = unique("client");
+  let pending_hash = unique("pending-access");
+  let barriered_hash = unique("barriered-access");
+
+  tokens::put_access_pending(
+    &mut conn,
+    &fragile,
+    &pending_hash,
+    &access_record(&client, &grant),
+    Duration::from_secs(300),
+  )
+  .await?;
+  if tokens::get_access(&mut conn, &fragile, &pending_hash)
+    .await?
+    .is_none()
+  {
+    return Err(TestError::Unexpected(
+      "pending write must be readable without a barrier".to_owned(),
+    ));
+  }
+
+  match tokens::put_access(
+    &mut conn,
+    &fragile,
+    &barriered_hash,
+    &access_record(&client, &grant),
+    Duration::from_secs(300),
+  )
+  .await
+  {
+    Err(StoreError::Durability) => Ok(()),
+    Ok(()) => Err(TestError::Unexpected(
+      "barriered write without replicas must fail closed".to_owned(),
+    )),
+    Err(error) => Err(TestError::Unexpected(format!(
+      "barriered write must be Durability, got {error:?}"
+    ))),
+  }
+}
+
+/// A barrier failure still mints a readable pair; revoking the grant must
+/// delete that recovered pair, so no unbarriered token survives revocation.
+#[tokio::test]
+async fn store_recovered_pair_revocable_after_unbarriered_rotation() -> TestResult {
+  let (_container, url) = redis_container(Vec::new()).await?;
+  let (mut conn, durable) = direct_store(&url, DurabilityMode::WaitAof).await?;
+  let fragile = Store::from_parts(
+    "oauth:{state}:".to_owned(),
+    "oauth:{rl}:".to_owned(),
+    DurabilityMode::Wait,
+  );
+  let grant = unique("grant");
+  let client = unique("client");
+  let old = unique("old-refresh");
+  let new_refresh = unique("new-refresh");
+  let new_access = unique("new-access");
+  let pepper = b"store-test-pepper";
+
+  grants::create(
+    &mut conn,
+    &durable,
+    &grants::GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "orders:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
+  tokens::put_refresh(
+    &mut conn,
+    &durable,
+    &old,
+    &refresh_record(&client, &grant),
+    Duration::from_secs(3600),
+  )
+  .await?;
+
+  match tokens::rotate(
+    &mut conn,
+    &fragile,
+    pepper,
+    &rotation_input(&old, &client, &new_refresh, &new_access, &grant, None),
+  )
+  .await?
+  {
+    tokens::RotateOutcome::RotatedUnbarriered {
+      new_refresh_hash,
+      new_access_hash,
+    } if new_refresh_hash == new_refresh && new_access_hash == new_access => {}
+    _ => {
+      return Err(TestError::Unexpected(
+        "barrier failure must force RotatedUnbarriered".to_owned(),
+      ));
+    }
+  }
+
+  grants::revoke(&mut conn, &durable, &grant).await?;
+  if tokens::get_refresh(&mut conn, &durable, &new_refresh)
+    .await?
+    .is_some()
+  {
+    return Err(TestError::Unexpected(
+      "revoke must delete the recovered refresh token".to_owned(),
+    ));
+  }
+  if tokens::get_access(&mut conn, &durable, &new_access)
+    .await?
+    .is_some()
+  {
+    return Err(TestError::Unexpected(
+      "revoke must delete the recovered access token".to_owned(),
+    ));
+  }
+  Ok(())
 }

+ 349 - 49
src/store/tokens.rs

@@ -21,8 +21,8 @@ use std::{collections::HashMap, time::Duration};
 use serde::{Deserialize, Serialize};
 
 use super::{
-  OpClass, SCRIPT_ROTATE_REFRESH, Store, StoreError, durability_barrier, hmac_sha256, hset_record,
-  may_retry_op, retry_op,
+  OpClass, SCRIPT_PUT_MEMBER, SCRIPT_ROTATE_REFRESH, Store, StoreError, durability_barrier,
+  hmac_sha256, may_retry_op, retry_op,
 };
 use crate::state::RedisConn;
 
@@ -30,6 +30,11 @@ use crate::state::RedisConn;
 const ROTATION_MAP_GRACE: Duration = Duration::from_secs(900);
 
 /// A stored access-token record, keyed by the token hash.
+///
+/// `Debug` is intentionally derived: the record holds only identifiers
+/// (`client_id`, `grant_id`, scope, subject, `cnf`) and never a raw token,
+/// so the [`crate::protocol::token::types::FoundAccess`] redaction stays
+/// transitive through this field.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct AccessRecord {
   /// Client the token was issued to.
@@ -45,6 +50,11 @@ pub struct AccessRecord {
 }
 
 /// A stored refresh-token record (or tombstone), keyed by the token hash.
+///
+/// `Debug` is intentionally derived: the record holds only identifiers
+/// (`client_id`, `grant_id`, scope, subject, `cnf`) and never a raw token,
+/// so the [`crate::protocol::token::types::FoundRefresh`] redaction stays
+/// transitive through this field.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct RefreshRecord {
   /// Client the token was issued to.
@@ -105,7 +115,20 @@ pub enum RotateOutcome {
     /// Hash of the new access token.
     new_access_hash: String,
   },
-  /// Idempotent retry: the mapping already held these tokens.
+  /// Rotation applied but the durability barrier failed after this request
+  /// id's write already landed. Only the originating request may resolve to
+  /// these tokens (returned as-is, never discarded); any non-originating
+  /// caller still fails closed through [`RotateOutcome::Replay`] or
+  /// [`RotateOutcome::Reused`].
+  RotatedUnbarriered {
+    /// Hash of the new refresh token.
+    new_refresh_hash: String,
+    /// Hash of the new access token.
+    new_access_hash: String,
+  },
+  /// Idempotent retry: the mapping already held these tokens. The raw pair of
+  /// the first rotation is unrecoverable from hashes alone, so the retry
+  /// cannot be served (fail closed without cascading the grant family).
   Replay {
     /// Hash of the new refresh token.
     new_refresh_hash: String,
@@ -146,6 +169,33 @@ pub async fn put_access(
   token_hash: &str,
   record: &AccessRecord,
   ttl: Duration,
+) -> Result<(), StoreError> {
+  put_access_inner(conn, store, token_hash, record, ttl, true).await
+}
+
+/// Store an access-token record and index it in its grant set WITHOUT the
+/// durability barrier.
+///
+/// Only for the refresh-rotation pre-mint: the pair is unreachable until the
+/// rotation script commits it, and the rotation's own barrier runs on the
+/// same connection afterwards, so it covers these writes before any response.
+pub async fn put_access_pending(
+  conn: &mut RedisConn,
+  store: &Store,
+  token_hash: &str,
+  record: &AccessRecord,
+  ttl: Duration,
+) -> Result<(), StoreError> {
+  put_access_inner(conn, store, token_hash, record, ttl, false).await
+}
+
+async fn put_access_inner(
+  conn: &mut RedisConn,
+  store: &Store,
+  token_hash: &str,
+  record: &AccessRecord,
+  ttl: Duration,
+  apply_barrier: bool,
 ) -> Result<(), StoreError> {
   let key = store.access_key(token_hash);
   let set_key = store.grant_tokens_key(&record.grant_id);
@@ -162,7 +212,11 @@ pub async fn put_access(
       .map_err(store_redis)
   })
   .map_err(StoreError::Redis)?;
-  durability_barrier(conn, store).await
+  if apply_barrier {
+    durability_barrier(conn, store).await
+  } else {
+    Ok(())
+  }
 }
 
 /// Store a refresh-token record and index it in its grant set, then apply the
@@ -173,6 +227,33 @@ pub async fn put_refresh(
   token_hash: &str,
   record: &RefreshRecord,
   ttl: Duration,
+) -> Result<(), StoreError> {
+  put_refresh_inner(conn, store, token_hash, record, ttl, true).await
+}
+
+/// Store a refresh-token record and index it in its grant set WITHOUT the
+/// durability barrier.
+///
+/// Only for the refresh-rotation pre-mint: the pair is unreachable until the
+/// rotation script commits it, and the rotation's own barrier runs on the
+/// same connection afterwards, so it covers these writes before any response.
+pub async fn put_refresh_pending(
+  conn: &mut RedisConn,
+  store: &Store,
+  token_hash: &str,
+  record: &RefreshRecord,
+  ttl: Duration,
+) -> Result<(), StoreError> {
+  put_refresh_inner(conn, store, token_hash, record, ttl, false).await
+}
+
+async fn put_refresh_inner(
+  conn: &mut RedisConn,
+  store: &Store,
+  token_hash: &str,
+  record: &RefreshRecord,
+  ttl: Duration,
+  apply_barrier: bool,
 ) -> Result<(), StoreError> {
   let key = store.refresh_key(token_hash);
   let set_key = store.grant_tokens_key(&record.grant_id);
@@ -189,7 +270,11 @@ pub async fn put_refresh(
       .map_err(store_redis)
   })
   .map_err(StoreError::Redis)?;
-  durability_barrier(conn, store).await
+  if apply_barrier {
+    durability_barrier(conn, store).await
+  } else {
+    Ok(())
+  }
 }
 
 /// Read an access-token record; `Ok(None)` when absent.
@@ -254,10 +339,12 @@ pub async fn tombstone_of(
 
 /// Atomically rotate a refresh token (compare-and-set with reuse signal).
 ///
-/// On `Rotated`/`Replay` the durability barrier is met BEFORE returning. On
-/// `Reused` the grant family is cascade-revoked before returning. On an
-/// indeterminate transport result state is re-read (mapping, then the old
-/// record), never blindly replayed.
+/// On `Rotated` the durability barrier was met before returning; when the
+/// barrier fails after the originating write applied the minted pair is
+/// returned as `RotatedUnbarriered` instead of a lockout error. On `Reused`
+/// the grant family is cascade-revoked before returning. On an indeterminate
+/// transport result state is re-read (mapping, then the old record), never
+/// blindly replayed.
 pub async fn rotate(
   conn: &mut RedisConn,
   store: &Store,
@@ -272,6 +359,7 @@ pub async fn rotate(
   let new_refresh_key = store.refresh_key(&input.new_refresh_hash);
   let new_access_key = store.access_key(&input.new_access_hash);
   let grant_key = store.grant_tokens_key(&input.new_refresh.grant_id);
+  let grant_record_key = store.grant_key(&input.new_refresh.grant_id);
   let map_key = store.rotation_map_key(&request_id);
   let new_refresh_json = input.new_refresh.to_json()?;
   let new_access_json = input.new_access.to_json()?;
@@ -285,6 +373,7 @@ pub async fn rotate(
         new_refresh_key: &new_refresh_key,
         new_access_key: &new_access_key,
         grant_key: &grant_key,
+        grant_record_key: &grant_record_key,
         map_key: &map_key,
         expected_client: &input.expected_client_id,
         new_refresh_json: &new_refresh_json,
@@ -320,7 +409,8 @@ async fn invoke_rotate(
     .key(args.new_refresh_key)
     .key(args.new_access_key)
     .key(args.grant_key)
-    .key(args.map_key);
+    .key(args.map_key)
+    .key(args.grant_record_key);
   invocation
     .arg(args.expected_client)
     .arg(args.new_refresh_json)
@@ -347,6 +437,7 @@ struct InvokeRotate<'a> {
   new_refresh_key: &'a str,
   new_access_key: &'a str,
   grant_key: &'a str,
+  grant_record_key: &'a str,
   map_key: &'a str,
   expected_client: &'a str,
   new_refresh_json: &'a str,
@@ -362,7 +453,7 @@ struct InvokeRotate<'a> {
 }
 
 /// Settle a script reply: barrier first for token-bearing outcomes, cascade
-/// for reuse.
+/// for reuse, fail closed for a fenced (revoked-grant) write.
 async fn finish_rotate(
   conn: &mut RedisConn,
   store: &Store,
@@ -372,15 +463,20 @@ async fn finish_rotate(
   match tag.as_str() {
     "ROTATED" => {
       let (new_refresh, new_access) = take_pair(&parts)?;
-      durability_barrier(conn, store).await?;
-      Ok(RotateOutcome::Rotated {
-        new_refresh_hash: new_refresh,
-        new_access_hash: new_access,
-      })
+      match barrier_or_recover(conn, store).await? {
+        BarrierVerdict::Held => Ok(RotateOutcome::Rotated {
+          new_refresh_hash: new_refresh,
+          new_access_hash: new_access,
+        }),
+        BarrierVerdict::FailedAfterWrite => Ok(RotateOutcome::RotatedUnbarriered {
+          new_refresh_hash: new_refresh,
+          new_access_hash: new_access,
+        }),
+      }
     }
     "REPLAY" => {
       let (new_refresh, new_access) = take_pair(&parts)?;
-      durability_barrier(conn, store).await?;
+      barrier_or_recover(conn, store).await?;
       Ok(RotateOutcome::Replay {
         new_refresh_hash: new_refresh,
         new_access_hash: new_access,
@@ -395,11 +491,37 @@ async fn finish_rotate(
       Ok(RotateOutcome::Reused { rotated_to })
     }
     "ABSENT" => Ok(RotateOutcome::Absent),
+    "NOGRANT" => Ok(RotateOutcome::Absent),
     "MISMATCH" => Ok(RotateOutcome::Mismatch),
     _ => Err(StoreError::InvalidData),
   }
 }
 
+/// Verdict of the durability barrier for a token-bearing outcome.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum BarrierVerdict {
+  /// The barrier held; the write is durable.
+  Held,
+  /// The barrier failed after this request id's write already applied: only
+  /// the originating request may resolve to its own pair. Any other failure
+  /// propagates (fail closed for every non-originating caller).
+  FailedAfterWrite,
+}
+
+/// Run the durability barrier for a token-bearing outcome, distinguishing a
+/// held barrier from a failure after this request id's write applied (the
+/// caller resolves the originating request instead of locking out).
+async fn barrier_or_recover(
+  conn: &mut RedisConn,
+  store: &Store,
+) -> Result<BarrierVerdict, StoreError> {
+  match durability_barrier(conn, store).await {
+    Ok(()) => Ok(BarrierVerdict::Held),
+    Err(StoreError::Durability) => Ok(BarrierVerdict::FailedAfterWrite),
+    Err(other) => Err(other),
+  }
+}
+
 /// Re-read after an indeterminate rotation: the mapping decides replay, a
 /// tombstone decides reuse (plus cascade), a live record fails closed so the
 /// client can safely retry, and absence reports `Absent`.
@@ -431,7 +553,7 @@ async fn recover_rotate(
     if new_refresh.is_empty() || new_access.is_empty() {
       return Err(StoreError::InvalidData);
     }
-    durability_barrier(conn, store).await?;
+    barrier_or_recover(conn, store).await?;
     return Ok(RotateOutcome::Replay {
       new_refresh_hash: new_refresh,
       new_access_hash: new_access,
@@ -477,6 +599,25 @@ fn record_pairs(
   ]
 }
 
+/// Write one grant member (record + index entry) in a single Lua script.
+///
+/// Previously this was three round-trips (`HSET`+`PEXPIRE`, `SADD`,
+/// arm-or-extend the set TTL), so a revoke drain could interleave between the
+/// record write and the `SADD`: with the record deleted but the `SADD` still
+/// landing afterwards, the set kept an orphan member (and, symmetrically, a
+/// record could sit briefly with no index entry for the drain to find). One
+/// script closes that post-barrier late-`SADD` orphan window at the producer:
+/// the record and its index entry become visible together.
+///
+/// Residual limits (by design, not by omission): the script is atomic but a
+/// revoke drain is still `SMEMBERS`-then-delete, so a member `SADD`ed after
+/// the drain's snapshot still survives that pass — the drain therefore loops
+/// until the set reads empty and re-reads after the barrier (see
+/// [`super::grants::revoke`]). A mint that lands after the final pass leaves
+/// a live record the cascade never saw; that survivor is contained, not
+/// exploitable: introspection re-checks `users.disabled` (so a
+/// disabled-subject orphan still introspects inactive) and refresh re-checks
+/// current entitlements before minting.
 async fn put_member(
   conn: &mut RedisConn,
   key: &str,
@@ -485,39 +626,19 @@ async fn put_member(
   set_key: &str,
   member: &str,
 ) -> Result<(), StoreError> {
-  hset_record(conn, key, pairs, ttl).await?;
-  let _: () = redis::cmd("SADD")
-    .arg(set_key)
-    .arg(member)
-    .query_async(conn)
+  let mut invocation = SCRIPT_PUT_MEMBER.prepare_invoke();
+  invocation
+    .key(key)
+    .key(set_key)
+    .arg(ttl.as_millis() as u64)
+    .arg(member);
+  for (field, value) in pairs {
+    invocation.arg(field).arg(value);
+  }
+  let _: i64 = invocation
+    .invoke_async(conn)
     .await
     .map_err(StoreError::Redis)?;
-  ensure_set_ttl(conn, set_key, ttl).await
-}
-
-/// Keep the per-grant token set alive at least as long as its longest-lived
-/// member: arm an expiry when the set is persistent, extend it when the member
-/// outlives the current one, never shorten it. The set "lives with the grant"
-/// (docs/REDIS.md) instead of leaking member hashes forever.
-///
-/// Both steps are single atomic commands, so a concurrent writer can never
-/// shorten a longer expiry: `NX` arms only a persistent set (a no-op once one
-/// is set), then `GT` extends only a shorter one.
-async fn ensure_set_ttl(
-  conn: &mut RedisConn,
-  set_key: &str,
-  ttl: Duration,
-) -> Result<(), StoreError> {
-  let ttl_ms = ttl.as_millis() as u64;
-  for mode in ["NX", "GT"] {
-    let _: () = redis::cmd("PEXPIRE")
-      .arg(set_key)
-      .arg(ttl_ms)
-      .arg(mode)
-      .query_async(conn)
-      .await
-      .map_err(StoreError::Redis)?;
-  }
   Ok(())
 }
 
@@ -549,3 +670,182 @@ fn store_redis(error: StoreError) -> redis::RedisError {
     _ => redis::RedisError::from((redis::ErrorKind::Client, "store token write failed")),
   }
 }
+
+#[cfg(test)]
+mod put_member_tests {
+  use std::time::Duration;
+
+  use testcontainers::{
+    GenericImage, ImageExt,
+    core::{IntoContainerPort, WaitFor},
+    runners::AsyncRunner,
+  };
+
+  use super::{AccessRecord, RefreshRecord, get_access, get_refresh, put_access, put_refresh};
+  use crate::{
+    config::{DurabilityMode, RedisConfig, redis::RawRedisConfig},
+    state::RedisConn,
+    store::{Store, StoreError},
+    test_guard::ContainerGuard,
+  };
+
+  type TestResult = Result<(), TestError>;
+
+  #[derive(Debug, thiserror::Error)]
+  enum TestError {
+    #[error("container error: {0}")]
+    Containers(#[from] testcontainers::TestcontainersError),
+    #[error("config error: {0}")]
+    Config(#[from] crate::config::ConfigError),
+    #[error("redis error: {0}")]
+    Redis(#[from] redis::RedisError),
+    #[error("store error: {0}")]
+    Store(#[from] StoreError),
+    #[error("unexpected test value: {0}")]
+    Unexpected(String),
+  }
+
+  /// `put_member` lands the record and its grant-set entry in one Lua script,
+  /// so a revoke drain can never observe one without the other: every stream
+  /// below asserts the no-orphan invariant in both directions (each readable
+  /// record is indexed, each indexed member resolves to a live record), the
+  /// record and set TTLs are armed, and a shorter-lived second member never
+  /// shortens the set TTL.
+  #[tokio::test]
+  async fn put_member_writes_record_and_index_atomically() -> TestResult {
+    let container = GenericImage::new("redis", "7.2-alpine")
+      .with_exposed_port(6379.tcp())
+      .with_wait_for(WaitFor::message_on_stdout("Ready to accept connections"))
+      .with_cmd(vec!["redis-server", "--appendonly", "yes"])
+      .start()
+      .await?;
+    let guarded = ContainerGuard::new(container);
+    let port = guarded.get_host_port_ipv4(6379).await?;
+    let url = format!("redis://127.0.0.1:{port}");
+    let client = redis::Client::open(url.as_str())?;
+    let manager_config = redis::aio::ConnectionManagerConfig::new()
+      .set_response_timeout(Some(Duration::from_secs(10)))
+      .set_connection_timeout(Some(Duration::from_secs(5)));
+    let manager = redis::aio::ConnectionManager::new_with_config(client, manager_config).await?;
+    let mut conn = RedisConn::Single(manager);
+    let raw = RawRedisConfig {
+      url: Some(url),
+      durability: Some(DurabilityMode::WaitAof),
+      response_timeout_secs: 10,
+      connect_timeout_secs: 5,
+      ..Default::default()
+    };
+    let store = Store::from_config(&RedisConfig::from_raw(raw)?);
+
+    let grant = "put-member-grant";
+    let client_id = "put-member-client";
+    let refresh_hash = "put-member-refresh";
+    let access_hash = "put-member-access";
+    put_refresh(
+      &mut conn,
+      &store,
+      refresh_hash,
+      &RefreshRecord {
+        client_id: client_id.to_owned(),
+        grant_id: grant.to_owned(),
+        scope: "orders:read".to_owned(),
+        subject: "user-1".to_owned(),
+        cnf: String::new(),
+      },
+      Duration::from_secs(3600),
+    )
+    .await?;
+    put_access(
+      &mut conn,
+      &store,
+      access_hash,
+      &AccessRecord {
+        client_id: client_id.to_owned(),
+        grant_id: grant.to_owned(),
+        scope: "orders:read".to_owned(),
+        subject: "user-1".to_owned(),
+        cnf: String::new(),
+      },
+      Duration::from_secs(300),
+    )
+    .await?;
+
+    // Direction 1: every readable record is indexed.
+    if get_refresh(&mut conn, &store, refresh_hash)
+      .await?
+      .is_none()
+    {
+      return Err(TestError::Unexpected(
+        "refresh record must be readable".to_owned(),
+      ));
+    }
+    if get_access(&mut conn, &store, access_hash).await?.is_none() {
+      return Err(TestError::Unexpected(
+        "access record must be readable".to_owned(),
+      ));
+    }
+    let set_key = store.grant_tokens_key(grant);
+    for hash in [refresh_hash, access_hash] {
+      let indexed: bool = redis::cmd("SISMEMBER")
+        .arg(&set_key)
+        .arg(hash)
+        .query_async(&mut conn)
+        .await?;
+      if !indexed {
+        return Err(TestError::Unexpected(format!(
+          "{hash} must be indexed in the grant set"
+        )));
+      }
+    }
+
+    // Direction 2: every indexed member resolves to a live record (no orphan
+    // index entries from a record write racing its own SADD).
+    let members: Vec<String> = redis::cmd("SMEMBERS")
+      .arg(&set_key)
+      .query_async(&mut conn)
+      .await?;
+    if members.len() != 2 {
+      return Err(TestError::Unexpected(format!(
+        "grant set must hold exactly the pair, saw {}",
+        members.len()
+      )));
+    }
+    for member in &members {
+      let live = get_refresh(&mut conn, &store, member).await?.is_some()
+        || get_access(&mut conn, &store, member).await?.is_some();
+      if !live {
+        return Err(TestError::Unexpected(format!(
+          "indexed {member} must resolve to a record"
+        )));
+      }
+    }
+
+    // TTLs preserved: the record and the set both carry an expiry.
+    let record_pttl: i64 = redis::cmd("PTTL")
+      .arg(store.refresh_key(refresh_hash))
+      .query_async(&mut conn)
+      .await?;
+    let set_pttl: i64 = redis::cmd("PTTL")
+      .arg(&set_key)
+      .query_async(&mut conn)
+      .await?;
+    if record_pttl <= 0 {
+      return Err(TestError::Unexpected(
+        "refresh record must carry a TTL".to_owned(),
+      ));
+    }
+    if set_pttl <= 0 {
+      return Err(TestError::Unexpected(
+        "grant set must carry a TTL".to_owned(),
+      ));
+    }
+    // The 300 s access member must not have shortened the set TTL armed by the
+    // 3600 s refresh member (arm-or-extend, never shorten).
+    if set_pttl <= 1_000_000 {
+      return Err(TestError::Unexpected(format!(
+        "grant set TTL must track the longest-lived member, saw {set_pttl} ms"
+      )));
+    }
+    Ok(())
+  }
+}

+ 136 - 74
src/ui/login.rs

@@ -198,13 +198,24 @@ pub fn session_cookie(config: &Config, session_id: &str) -> Cookie<'static> {
   builder.build()
 }
 
-/// Build an expired session cookie for logout.
+/// Build an expired session cookie for logout, mirroring every attribute of
+/// [`session_cookie`] (`Path`, `HttpOnly`, `SameSite`, `Secure`, `Domain`) so
+/// the clearing cookie overwrites the session cookie in the browser. A bare
+/// removal cookie without those attributes would leave the session cookie
+/// alive under the stricter attribute set.
 #[must_use]
-pub fn expired_session_cookie() -> Cookie<'static> {
-  Cookie::build((SESSION_COOKIE, String::new()))
+pub fn expired_session_cookie(config: &Config) -> Cookie<'static> {
+  let mut builder = Cookie::build((SESSION_COOKIE, String::new()))
     .path("/")
-    .removal()
-    .build()
+    .http_only(true)
+    .same_site(SameSite::Lax);
+  if config.cookie_secure {
+    builder = builder.secure(true);
+  }
+  if let Some(domain) = config.cookie_domain.clone() {
+    builder = builder.domain(domain);
+  }
+  builder.removal().build()
 }
 
 /// Mint the stateless pre-login CSRF token: HMAC under the active token
@@ -666,23 +677,17 @@ pub async fn confirm_totp(
   };
   let secret = totp::decrypt_secret(deps.keys, &blob)?;
   let now_unix = u64::try_from(deps.now.unix_timestamp().max(0)).unwrap_or(0);
-  let key_for = |step: u64| {
-    deps.store.totp_replay_key(&totp::replay_fingerprint(
-      &user.id.to_string(),
-      code.trim(),
-      step,
-    ))
-  };
-  if !totp::verify_and_claim(
-    deps.conn,
-    key_for,
-    &secret,
+  let user_label = user.id.to_string();
+  let store = deps.store;
+  let key_for = |fingerprint: &str| store.totp_replay_key(fingerprint);
+  let claim = totp::TotpClaim {
+    user_id: &user_label,
+    secret_b32: &secret,
     code,
     now_unix,
-    totp::USED_CODE_TTL,
-  )
-  .await?
-  {
+    ttl: totp::USED_CODE_TTL,
+  };
+  if !totp::verify_and_claim_with_ring(deps.conn, deps.keys, key_for, &claim).await? {
     user_repo::register_login_failure(deps.conn, deps.store, policy, &live.record.subject).await?;
     audit_failure(
       live.meta.pending_request_id.as_deref(),
@@ -734,21 +739,16 @@ async fn verify_second_factor(
   if let Some(blob) = user_repo::totp_secret_blob(deps.pool, user_id).await? {
     let secret = totp::decrypt_secret(deps.keys, &blob)?;
     let now_unix = u64::try_from(deps.now.unix_timestamp().max(0)).unwrap_or(0);
-    let key_for = |step: u64| {
-      deps
-        .store
-        .totp_replay_key(&totp::replay_fingerprint(&user_label, code.trim(), step))
-    };
-    if totp::verify_and_claim(
-      deps.conn,
-      key_for,
-      &secret,
+    let store = deps.store;
+    let key_for = |fingerprint: &str| store.totp_replay_key(fingerprint);
+    let claim = totp::TotpClaim {
+      user_id: &user_label,
+      secret_b32: &secret,
       code,
       now_unix,
-      totp::USED_CODE_TTL,
-    )
-    .await?
-    {
+      ttl: totp::USED_CODE_TTL,
+    };
+    if totp::verify_and_claim_with_ring(deps.conn, deps.keys, key_for, &claim).await? {
       return Ok(true);
     }
   }
@@ -1047,6 +1047,14 @@ pub struct ConfirmForm {
   pub csrf: String,
 }
 
+/// Form posted to `POST /logout`.
+#[derive(Debug, Deserialize)]
+pub struct LogoutForm {
+  /// Session-bound form token.
+  #[serde(default)]
+  pub csrf: String,
+}
+
 /// Login page.
 #[derive(Template)]
 #[template(
@@ -1164,10 +1172,35 @@ struct ConfirmTemplate {
   <title>Signed in</title></head><body>\
   <h1>Signed in</h1><p>You are signed in.</p>\
   <form method=\"post\" action=\"/logout\">\
+  <input type=\"hidden\" name=\"csrf\" value=\"{{ csrf }}\">\
+  <button type=\"submit\">Sign out</button></form></body></html>",
+  ext = "html"
+)]
+struct SuccessTemplate {
+  csrf: String,
+}
+
+/// Sign-out confirmation page rendered by `GET /logout`.
+///
+/// Read-only: rendering this page never destroys the session. The embedded
+/// session-bound form token authorizes the `POST /logout` that ends it, so a
+/// forced navigation to `GET /logout` (for example an `<img src="/logout">`)
+/// cannot sign the user out.
+#[derive(Template)]
+#[template(
+  source = "<!doctype html><html lang=\"en\"><head><meta charset=\"utf-8\">\
+  <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">\
+  <link rel=\"stylesheet\" href=\"/assets/app.css\">\
+  <title>Confirm sign out</title></head><body>\
+  <h1>Sign out?</h1><p>Use the button below to end your session.</p>\
+  <form method=\"post\" action=\"/logout\">\
+  <input type=\"hidden\" name=\"csrf\" value=\"{{ csrf }}\">\
   <button type=\"submit\">Sign out</button></form></body></html>",
   ext = "html"
 )]
-struct SuccessTemplate;
+struct LogoutConfirmTemplate {
+  csrf: String,
+}
 
 /// Neutral landing page after an RP-initiated logout. Deliberately has no
 /// auto-redirect and no form post: a forced navigation to `GET /logout` (for
@@ -1458,34 +1491,18 @@ pub async fn post_totp_confirm(
   }
 }
 
-/// `POST /logout`: destroy the session and expire the cookie.
-pub async fn post_logout(State(state): State<AppState>, jar: CookieJar) -> Response {
-  let mut conn = state.redis.clone();
-  let store = Store::from_config(&state.config.redis);
-  let session_id = jar
-    .get(SESSION_COOKIE)
-    .map(|cookie| cookie.value().to_owned())
-    .unwrap_or_default();
-  if logout(&mut conn, &store, &session_id).await.is_err() {
-    return AppError::server_error().into_response();
-  }
-  (jar.add(expired_session_cookie()), Redirect::to(LOGIN_PATH)).into_response()
-}
-
-/// `GET /logout`: destroy the session, expire the cookie, then render a neutral
-/// signed-out page or redirect to the flow's registered post-logout URI.
+/// `POST /logout`: verify the session-bound form token, then destroy the
+/// session and expire the cookie.
 ///
-/// No query parameter is ever honored, so the target cannot be supplied by the
-/// caller. The return URI was frozen in session meta when the authorization
-/// flow was bound, and it is used only if the owning client is still active and
-/// still registers that exact URI. If there is no session, no bound client, or
-/// the client/URI is no longer valid, the neutral page is rendered and no
-/// redirect is issued. The server session is destroyed idempotently; a missing
-/// session is a no-op.
-pub async fn get_logout(
+/// A missing or invalid token is rejected with `invalid_request` and the
+/// session is left intact, so a cross-site form post cannot sign the user
+/// out. With no session the call is idempotent: the cookie is still expired
+/// and the caller lands on login.
+pub async fn post_logout(
   State(state): State<AppState>,
   Extension(request_id): Extension<RequestId>,
   jar: CookieJar,
+  Form(form): Form<LogoutForm>,
 ) -> Response {
   let mut conn = state.redis.clone();
   let store = Store::from_config(&state.config.redis);
@@ -1493,21 +1510,33 @@ pub async fn get_logout(
     .get(SESSION_COOKIE)
     .map(|cookie| cookie.value().to_owned())
     .unwrap_or_default();
-  let target = match load_session(&mut conn, &store, &session_id)
-    .await
-    .unwrap_or(None)
-  {
-    Some(live) => match resolve_post_logout_target(&state, &live).await {
-      Ok(target) => target,
-      Err(error) => {
-        emit_pii_decrypt(&request_id, &error);
-        if logout(&mut conn, &store, &session_id).await.is_err() {
-          return AppError::server_error().into_response();
-        }
-        return (jar.add(expired_session_cookie()), error.into_response()).into_response();
+  let live = match load_session(&mut conn, &store, &session_id).await {
+    Ok(Some(live)) => live,
+    Ok(None) => {
+      return (
+        jar.add(expired_session_cookie(&state.config)),
+        Redirect::to(LOGIN_PATH),
+      )
+        .into_response();
+    }
+    Err(error) => return error.into_response(),
+  };
+  if !verify_form_token(&live.meta.csrf_secret, LOGOUT_ACTION, &form.csrf) {
+    return AppError::invalid_request("invalid form submission").into_response();
+  }
+  let target = match resolve_post_logout_target(&state, &live).await {
+    Ok(target) => target,
+    Err(error) => {
+      emit_pii_decrypt(&request_id, &error);
+      if logout(&mut conn, &store, &session_id).await.is_err() {
+        return AppError::server_error().into_response();
       }
-    },
-    None => None,
+      return (
+        jar.add(expired_session_cookie(&state.config)),
+        error.into_response(),
+      )
+        .into_response();
+    }
   };
   if logout(&mut conn, &store, &session_id).await.is_err() {
     return AppError::server_error().into_response();
@@ -1516,7 +1545,36 @@ pub async fn get_logout(
     Some(uri) => Redirect::to(&uri).into_response(),
     None => render(SignedOutTemplate),
   };
-  (jar.add(expired_session_cookie()), response).into_response()
+  (jar.add(expired_session_cookie(&state.config)), response).into_response()
+}
+
+/// `GET /logout`: read-only sign-out confirmation.
+///
+/// This handler never destroys the session and never touches cookies: it
+/// renders the confirmation form (with a fresh session-bound token) for a
+/// live session, or the neutral signed-out page when anonymous. Only
+/// `POST /logout` with a valid form token ends the session, so a forced
+/// navigation to this URI cannot sign the user out.
+///
+/// No query parameter is ever honored, so the target cannot be supplied by the
+/// caller. The post-logout return URI is resolved only by `POST /logout` from
+/// the URI frozen in session meta when the authorization flow was bound, and
+/// it is used only if the owning client is still active and still registers
+/// that exact URI.
+pub async fn get_logout(State(state): State<AppState>, jar: CookieJar) -> Response {
+  let mut conn = state.redis.clone();
+  let store = Store::from_config(&state.config.redis);
+  let session_id = jar
+    .get(SESSION_COOKIE)
+    .map(|cookie| cookie.value().to_owned())
+    .unwrap_or_default();
+  match load_session(&mut conn, &store, &session_id).await {
+    Ok(Some(live)) => render(LogoutConfirmTemplate {
+      csrf: form_token(&live.meta.csrf_secret, LOGOUT_ACTION),
+    }),
+    Ok(None) => render(SignedOutTemplate),
+    Err(error) => error.into_response(),
+  }
 }
 
 /// Resolve the session's bound post-logout target, if it is still trustworthy.
@@ -1576,13 +1634,17 @@ pub async fn get_login_success(State(state): State<AppState>, jar: CookieJar) ->
     .unwrap_or_default();
   match load_session(&mut conn, &store, &session_id).await {
     Ok(Some(live)) if !live.record.subject.is_empty() && !live.record.restricted => {
-      render(SuccessTemplate)
+      render(SuccessTemplate {
+        csrf: form_token(&live.meta.csrf_secret, LOGOUT_ACTION),
+      })
     }
     Ok(_) => Redirect::to(LOGIN_PATH).into_response(),
     Err(error) => error.into_response(),
   }
 }
 
+/// Form-token action for the sign-out confirmation form.
+const LOGOUT_ACTION: &str = "logout";
 /// Form-token action for the password-change form.
 const CHANGE_PASSWORD_ACTION: &str = "change-password";
 /// Form-token action for the TOTP-confirmation form.

+ 380 - 19
src/ui/login/tests/auth.rs

@@ -6,6 +6,7 @@ use axum::http::{StatusCode, header};
 use axum_extra::extract::CookieJar;
 use sqlx::Row as _;
 
+use super::super::LOGOUT_ACTION;
 use super::{
   Harness, LoginAttempt, TestError, TestResult, TestUser, app_state, attempt, cookie_header,
   create_user, current_code, deps_of, harness, pending_request, presession, wrong_code,
@@ -21,8 +22,8 @@ use crate::{
   store::{pending, sessions},
   ui::login::{
     CONSENT_PATH, LOCKOUT_THRESHOLD, LOCKOUT_WINDOW, LOGIN_SUCCESS_PATH, LoginForm, LoginOutcome,
-    LoginPageQuery, attempt_login, bind_pending_request, get_login, get_logout, load_session,
-    post_login, session_cookie,
+    LoginPageQuery, LogoutForm, attempt_login, bind_pending_request, expired_session_cookie,
+    form_token, get_login, get_logout, load_session, post_login, post_logout, session_cookie,
   },
 };
 
@@ -111,12 +112,19 @@ async fn login_success_sets_session_cookie_and_bound_redirect() -> TestResult {
   Ok(())
 }
 
+async fn logout_csrf(harness: &mut Harness, session_id: &str) -> Result<String, TestError> {
+  let live = load_session(&mut harness.conn, &harness.store, session_id)
+    .await?
+    .ok_or_else(|| unexpected("the session must be live"))?;
+  Ok(form_token(&live.meta.csrf_secret, LOGOUT_ACTION))
+}
+
 #[tokio::test]
-async fn get_logout_destroys_session_and_clears_cookie() -> TestResult {
+async fn post_logout_destroys_session_and_clears_cookie() -> TestResult {
   let mut h = harness().await?;
   let user = create_user(
     &mut h,
-    "logout-get@example.test",
+    "logout-post@example.test",
     "correct-horse-03",
     false,
     true,
@@ -129,17 +137,24 @@ async fn get_logout_destroys_session_and_clears_cookie() -> TestResult {
     _ => return Err(unexpected("correct credentials must authenticate")),
   };
 
+  let token = logout_csrf(&mut h, &session_id).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
 
   if response.status() != StatusCode::OK {
     return Err(unexpected(
-      "GET /logout must render a neutral page, not redirect",
+      "POST /logout must render a neutral page, not redirect",
     ));
   }
   if response.headers().get(header::LOCATION).is_some() {
     return Err(unexpected(
-      "GET /logout must never auto-redirect (no CSRF bounce)",
+      "POST /logout must never auto-redirect (no CSRF bounce)",
     ));
   }
   let set_cookie = response
@@ -148,28 +163,254 @@ async fn get_logout_destroys_session_and_clears_cookie() -> TestResult {
     .and_then(|value| value.to_str().ok())
     .unwrap_or_default();
   if !set_cookie.contains("session_id=") || !set_cookie.contains("Max-Age=0") {
-    return Err(unexpected("GET /logout must expire the session cookie"));
+    return Err(unexpected("POST /logout must expire the session cookie"));
+  }
+  if cookie_attr_names(&cookie_header(&h, &session_id)) != cookie_attr_names(set_cookie) {
+    return Err(unexpected(
+      "the clearing cookie must mirror the session cookie attributes",
+    ));
   }
   if sessions::get(&mut h.conn, &h.store, &session_id)
     .await?
     .is_some()
   {
     return Err(unexpected(
-      "GET /logout must destroy the server-side session",
+      "POST /logout must destroy the server-side session",
     ));
   }
   Ok(())
 }
 
 #[tokio::test]
-async fn get_logout_without_session_is_idempotent() -> TestResult {
+async fn post_logout_without_csrf_is_rejected_and_session_survives() -> TestResult {
+  let mut h = harness().await?;
+  let user = create_user(
+    &mut h,
+    "logout-csrf@example.test",
+    "correct-horse-31",
+    false,
+    true,
+  )
+  .await?;
+  let (pre, csrf) = presession(&mut h).await?;
+  let code = current_code(&user, &h)?;
+  let session_id = match try_login(&mut h, attempt(&pre, &csrf, &user, &code)).await? {
+    LoginOutcome::Authenticated { session_id, .. } => session_id,
+    _ => return Err(unexpected("correct credentials must authenticate")),
+  };
+  for presented in [String::new(), "forged-token".to_owned()] {
+    let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
+    let response = post_logout(
+      State(app_state(&h)),
+      Extension(RequestId::new()),
+      jar,
+      Form(LogoutForm {
+        csrf: presented.clone(),
+      }),
+    )
+    .await;
+    if response.status() != StatusCode::BAD_REQUEST {
+      return Err(unexpected(
+        "POST /logout without a valid token must be rejected",
+      ));
+    }
+    if response.headers().contains_key(header::SET_COOKIE) {
+      return Err(unexpected("a rejected sign-out must not touch cookies"));
+    }
+    if load_session(&mut h.conn, &h.store, &session_id)
+      .await?
+      .is_none()
+    {
+      return Err(unexpected(
+        "a rejected sign-out must leave the session intact",
+      ));
+    }
+  }
+  Ok(())
+}
+
+#[tokio::test]
+async fn post_logout_rejects_other_sessions_form_token() -> TestResult {
+  let mut h = harness().await?;
+  let user_a = create_user(
+    &mut h,
+    "logout-cross-a@example.test",
+    "correct-horse-32",
+    false,
+    true,
+  )
+  .await?;
+  let user_b = create_user(
+    &mut h,
+    "logout-cross-b@example.test",
+    "correct-horse-33",
+    false,
+    true,
+  )
+  .await?;
+  let (pre_a, csrf_a) = presession(&mut h).await?;
+  let code_a = current_code(&user_a, &h)?;
+  let session_a = match try_login(&mut h, attempt(&pre_a, &csrf_a, &user_a, &code_a)).await? {
+    LoginOutcome::Authenticated { session_id, .. } => session_id,
+    _ => return Err(unexpected("correct credentials must authenticate")),
+  };
+  let (pre_b, csrf_b) = presession(&mut h).await?;
+  let code_b = current_code(&user_b, &h)?;
+  let session_b = match try_login(&mut h, attempt(&pre_b, &csrf_b, &user_b, &code_b)).await? {
+    LoginOutcome::Authenticated { session_id, .. } => session_id,
+    _ => return Err(unexpected("correct credentials must authenticate")),
+  };
+  // A form token minted for session B (action-separated per-session context)
+  // must not authorize session A's sign-out.
+  let foreign = logout_csrf(&mut h, &session_b).await?;
+  let jar = CookieJar::new().add(session_cookie(&h.config, &session_a));
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: foreign }),
+  )
+  .await;
+  if response.status() != StatusCode::BAD_REQUEST {
+    return Err(unexpected(
+      "POST /logout with another session's token must be rejected",
+    ));
+  }
+  if response.headers().contains_key(header::SET_COOKIE) {
+    return Err(unexpected("a rejected sign-out must not touch cookies"));
+  }
+  if load_session(&mut h.conn, &h.store, &session_a)
+    .await?
+    .is_none()
+  {
+    return Err(unexpected(
+      "a rejected sign-out must leave the victim session intact",
+    ));
+  }
+  if load_session(&mut h.conn, &h.store, &session_b)
+    .await?
+    .is_none()
+  {
+    return Err(unexpected(
+      "a rejected sign-out must leave the other session intact",
+    ));
+  }
+  // A token for a different action on the same session must also fail: form
+  // tokens are action-separated, not interchangeable.
+  let live_a = load_session(&mut h.conn, &h.store, &session_a)
+    .await?
+    .ok_or_else(|| unexpected("the session must be live"))?;
+  let cross_action = form_token(&live_a.meta.csrf_secret, "change-password");
+  let jar = CookieJar::new().add(session_cookie(&h.config, &session_a));
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: cross_action }),
+  )
+  .await;
+  if response.status() != StatusCode::BAD_REQUEST {
+    return Err(unexpected(
+      "POST /logout with another action's token must be rejected",
+    ));
+  }
+  if load_session(&mut h.conn, &h.store, &session_a)
+    .await?
+    .is_none()
+  {
+    return Err(unexpected(
+      "a cross-action reject must leave the session intact",
+    ));
+  }
+  Ok(())
+}
+
+#[tokio::test]
+async fn post_logout_wrong_content_type_fails_closed() -> TestResult {
+  let mut h = harness().await?;
+  let user = create_user(
+    &mut h,
+    "logout-ctype@example.test",
+    "correct-horse-34",
+    false,
+    true,
+  )
+  .await?;
+  let (pre, csrf) = presession(&mut h).await?;
+  let code = current_code(&user, &h)?;
+  let session_id = match try_login(&mut h, attempt(&pre, &csrf, &user, &code)).await? {
+    LoginOutcome::Authenticated { session_id, .. } => session_id,
+    _ => return Err(unexpected("correct credentials must authenticate")),
+  };
+  let token = logout_csrf(&mut h, &session_id).await?;
+  let app = axum::Router::new()
+    .route("/logout", axum::routing::post(post_logout))
+    .with_state(app_state(&h))
+    .layer(Extension(RequestId::new()));
+  // A well-formed token sent under the wrong content type must fail closed:
+  // the Form extractor rejects before any session state is touched.
+  let body = format!("csrf={token}");
+  let request = axum::http::Request::post("/logout")
+    .header(axum::http::header::CONTENT_TYPE, "application/json")
+    .header(
+      axum::http::header::COOKIE,
+      format!("session_id={session_id}"),
+    )
+    .body(axum::body::Body::from(body))
+    .map_err(|_| unexpected("request build failed"))?;
+  let response = tower::ServiceExt::oneshot(app, request)
+    .await
+    .map_err(|_| unexpected("router failed"))?;
+  if !response.status().is_client_error() {
+    return Err(unexpected(
+      "POST /logout with the wrong content type must fail closed with 4xx",
+    ));
+  }
+  if load_session(&mut h.conn, &h.store, &session_id)
+    .await?
+    .is_none()
+  {
+    return Err(unexpected(
+      "a content-type reject must leave the session intact",
+    ));
+  }
+  Ok(())
+}
+
+#[tokio::test]
+async fn post_logout_without_session_is_idempotent() -> TestResult {
   let h = harness().await?;
-  let response = get_logout(
+  let response = post_logout(
     State(app_state(&h)),
     Extension(RequestId::new()),
     CookieJar::new(),
+    Form(LogoutForm {
+      csrf: String::new(),
+    }),
   )
   .await;
+  if response.status() != StatusCode::SEE_OTHER {
+    return Err(unexpected(
+      "POST /logout without a session must land on login",
+    ));
+  }
+  let set_cookie = response
+    .headers()
+    .get(header::SET_COOKIE)
+    .and_then(|value| value.to_str().ok())
+    .unwrap_or_default();
+  if !set_cookie.contains("session_id=") || !set_cookie.contains("Max-Age=0") {
+    return Err(unexpected(
+      "an anonymous sign-out must still expire the cookie",
+    ));
+  }
+  Ok(())
+}
+
+#[tokio::test]
+async fn get_logout_without_session_is_idempotent() -> TestResult {
+  let h = harness().await?;
+  let response = get_logout(State(app_state(&h)), CookieJar::new()).await;
   if response.status() != StatusCode::OK {
     return Err(unexpected(
       "GET /logout without a session must still succeed",
@@ -180,6 +421,9 @@ async fn get_logout_without_session_is_idempotent() -> TestResult {
       "GET /logout must not redirect even when anonymous",
     ));
   }
+  if response.headers().contains_key(header::SET_COOKIE) {
+    return Err(unexpected("GET /logout must not touch cookies"));
+  }
   Ok(())
 }
 
@@ -276,7 +520,7 @@ async fn bind_logout_flow(
 }
 
 #[tokio::test]
-async fn get_logout_redirects_to_bound_post_logout_uri() -> TestResult {
+async fn post_logout_redirects_to_bound_post_logout_uri() -> TestResult {
   let mut h = harness().await?;
   let uri = "https://client.example.test/signed-out";
   let session_id = bind_logout_flow(
@@ -288,8 +532,15 @@ async fn get_logout_redirects_to_bound_post_logout_uri() -> TestResult {
   )
   .await?;
 
+  let token = logout_csrf(&mut h, &session_id).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
 
   if response.status() != StatusCode::SEE_OTHER {
     return Err(unexpected("a bound logout must redirect"));
@@ -314,7 +565,7 @@ async fn get_logout_redirects_to_bound_post_logout_uri() -> TestResult {
 }
 
 #[tokio::test]
-async fn get_logout_ignores_disabled_post_logout_uri() -> TestResult {
+async fn post_logout_ignores_disabled_post_logout_uri() -> TestResult {
   let mut h = harness().await?;
   let uri = "https://client.example.test/disabled-out";
   let session_id = bind_logout_flow(
@@ -326,8 +577,15 @@ async fn get_logout_ignores_disabled_post_logout_uri() -> TestResult {
   )
   .await?;
 
+  let token = logout_csrf(&mut h, &session_id).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
 
   if response.status() != StatusCode::OK {
     return Err(unexpected("a disabled post-logout URI must not redirect"));
@@ -339,7 +597,7 @@ async fn get_logout_ignores_disabled_post_logout_uri() -> TestResult {
 }
 
 #[tokio::test]
-async fn get_logout_ignores_deactivated_client() -> TestResult {
+async fn post_logout_ignores_deactivated_client() -> TestResult {
   let mut h = harness().await?;
   let uri = "https://client.example.test/inactive-out";
   let session_id = bind_logout_flow(
@@ -352,8 +610,15 @@ async fn get_logout_ignores_deactivated_client() -> TestResult {
   .await?;
   client_repo::deactivate(&h.pool, "logout-inactive").await?;
 
+  let token = logout_csrf(&mut h, &session_id).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
 
   if response.status() != StatusCode::OK {
     return Err(unexpected(
@@ -848,8 +1113,15 @@ async fn logout_redirects_to_registered_uri() -> TestResult {
   if sealed.is_none() {
     return Err(unexpected("the sealed client name must be stored"));
   }
+  let token = logout_csrf(&mut h, &session_id).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
   if response.status() != StatusCode::SEE_OTHER {
     return Err(unexpected("a bound logout must redirect"));
   }
@@ -885,8 +1157,15 @@ async fn logout_redirects_to_registered_uri() -> TestResult {
     .bind(corrupt)
     .execute(&h.pool)
     .await?;
+  let token = logout_csrf(&mut h, &other).await?;
   let jar = CookieJar::new().add(session_cookie(&h.config, &other));
-  let response = get_logout(State(app_state(&h)), Extension(RequestId::new()), jar).await;
+  let response = post_logout(
+    State(app_state(&h)),
+    Extension(RequestId::new()),
+    jar,
+    Form(LogoutForm { csrf: token }),
+  )
+  .await;
   if response.status() != StatusCode::SERVICE_UNAVAILABLE {
     return Err(unexpected("a corrupt client row must fail closed with 503"));
   }
@@ -965,3 +1244,85 @@ async fn login_corrupt_totp_ciphertext_fails_closed_with_503() -> TestResult {
   }
   Ok(())
 }
+
+/// Attribute names of a `Set-Cookie` value, minus the leading pair and any
+/// expiry marker, lowercased and sorted for set comparison.
+fn cookie_attr_names(header: &str) -> Vec<String> {
+  let mut names = Vec::new();
+  for part in header.split(';').skip(1) {
+    let trimmed = part.trim();
+    if trimmed.is_empty() {
+      continue;
+    }
+    let name = match trimmed.split('=').next() {
+      Some(name) => name.trim().to_ascii_lowercase(),
+      None => continue,
+    };
+    if name == "max-age" || name == "expires" {
+      continue;
+    }
+    names.push(name);
+  }
+  names.sort();
+  names
+}
+
+#[tokio::test]
+async fn get_logout_leaves_session_intact_and_renders_confirm() -> TestResult {
+  let mut h = harness().await?;
+  let user = create_user(
+    &mut h,
+    "logout-readonly@example.test",
+    "correct-horse-30",
+    false,
+    true,
+  )
+  .await?;
+  let (pre, csrf) = presession(&mut h).await?;
+  let code = current_code(&user, &h)?;
+  let session_id = match try_login(&mut h, attempt(&pre, &csrf, &user, &code)).await? {
+    LoginOutcome::Authenticated { session_id, .. } => session_id,
+    _ => return Err(unexpected("correct credentials must authenticate")),
+  };
+  let jar = CookieJar::new().add(session_cookie(&h.config, &session_id));
+  let response = get_logout(State(app_state(&h)), jar).await;
+  if response.status() != StatusCode::OK {
+    return Err(unexpected("GET /logout must render the confirm page"));
+  }
+  if response.headers().contains_key(header::SET_COOKIE) {
+    return Err(unexpected("GET /logout must not touch cookies"));
+  }
+  let raw = http_body_util::BodyExt::collect(response.into_body())
+    .await
+    .map_err(|_| unexpected("the confirm body must read"))?
+    .to_bytes();
+  let body = String::from_utf8_lossy(&raw);
+  if !body.contains("action=\"/logout\"") || !body.contains("name=\"csrf\"") {
+    return Err(unexpected(
+      "GET /logout must render the sign-out confirm form",
+    ));
+  }
+  if load_session(&mut h.conn, &h.store, &session_id)
+    .await?
+    .is_none()
+  {
+    return Err(unexpected("GET /logout must never destroy the session"));
+  }
+  Ok(())
+}
+
+#[tokio::test]
+async fn logout_clearing_cookie_mirrors_session_cookie_attrs() -> TestResult {
+  let h = harness().await?;
+  let session = session_cookie(&h.config, "logout-attr-session").to_string();
+  let clearing = expired_session_cookie(&h.config).to_string();
+  if !clearing.contains("Max-Age=0") {
+    return Err(unexpected("the clearing cookie must expire immediately"));
+  }
+  if cookie_attr_names(&session) != cookie_attr_names(&clearing) {
+    return Err(unexpected(
+      "the clearing cookie must mirror the session cookie attributes",
+    ));
+  }
+  Ok(())
+}

+ 15 - 0
tests/proptest_rotation.rs

@@ -20,6 +20,7 @@ use d_oauth::{
   state::RedisConn,
   store::{
     Store, StoreError,
+    grants::{GrantRecord, create as create_grant},
     tokens::{
       AccessRecord, RefreshRecord, RotateOutcome, RotationInput, derive_rotation_id, put_refresh,
       rotate,
@@ -190,6 +191,20 @@ async fn rotation_apply_replay_absent_mismatch() -> TestResult {
     Duration::from_secs(600),
   )
   .await?;
+  // Production mint always writes the grant record before any rotation;
+  // the rotation fence requires it, so the harness mirrors mint here.
+  create_grant(
+    &mut conn,
+    &store,
+    &GrantRecord {
+      grant_id: grant.clone(),
+      client_id: client.clone(),
+      subject: "user-1".to_owned(),
+      scope: "read".to_owned(),
+    },
+    Duration::from_secs(600),
+  )
+  .await?;
 
   // First rotation applies.
   let input = rotation_input(

+ 13 - 0
tests/resilience.rs

@@ -44,6 +44,7 @@ use d_oauth::{
   store::{
     ErrorClass, OpClass, Store, StoreError, classify_redis,
     dcr::claim_dpop_jti,
+    grants::{GrantRecord, create as create_grant},
     tokens::{
       AccessRecord, RefreshRecord, RotateOutcome, RotationInput, get_refresh, put_refresh, rotate,
       tombstone_of,
@@ -325,6 +326,18 @@ async fn seed_refresh(
     Duration::from_secs(3600),
   )
   .await?;
+  create_grant(
+    &mut conn,
+    &store,
+    &GrantRecord {
+      grant_id: grant.to_owned(),
+      client_id: client.to_owned(),
+      subject: "user-1".to_owned(),
+      scope: "api:read".to_owned(),
+    },
+    Duration::from_secs(3600),
+  )
+  .await?;
   Ok(store)
 }