feat: reconcile the nest-auth keyspace and ship five security items - #95
Merged
Conversation
…ation Refresh rotation used a 30s grace window but never detected the replay of an already-consumed token: an in-grace replay forked a parallel session and a post-grace replay was rejected as a plain invalid, so a stolen refresh token stayed usable and theft was never surfaced (OWASP rotation with automatic reuse detection was not implemented). Track a refresh-token family (login lineage) minted at login and inherited unchanged across every rotation. On rotation the store now plants a consumed-token marker (`cf:`) that outlives the grace pointer and moves the hash in a family index (`fam:`). Presenting a consumed token past its grace window is therefore caught as a reuse: the store returns RotateOutcome::Reused carrying the compromised family, and the token manager revokes the entire family (every live descendant) before rejecting, forcing re-authentication. The atomic-rotation + grace-window concurrency semantics are preserved: an in-grace replay still recovers, and only a post-grace replay trips reuse. Legacy records with no family are handled gracefully (no marker, no index, never a reuse target). - SessionRecord gains `family_id` (serde-default, omitted when empty) - RotateOutcome gains `Reused(family_id)`; SessionStore gains `revoke_family` - refresh_rotate.lua plants the consumed marker + family index and detects reuse; new revoke_family.lua deletes a lineage atomically - in-memory test store mirrors the semantics for the core unit tier Tests prove: post-grace reuse revokes the live descendant (dashboard, platform, in-memory and Redis tiers); an in-grace concurrent rotation still succeeds; and a legacy no-family session never trips reuse. Coverage stays 100% on every changed file; clippy, cargo-deny, and the security-invariant gate pass.
…er-user epoch A password reset revoked the refresh sessions but left already-issued, stateless access JWTs valid for the remainder of their (up to 15-minute) lifetime: the jti-blacklist used on logout can only revoke a token you hold, and a reset does not possess the user's active access-token jtis — there is no per-user registry of them. Add a per-user token **epoch** (generation counter): stamped into the Dashboard/Platform claims at issuance (and rotation), read on every verification, and bumped on a password reset. A token stamped below the user's current epoch was issued before the reset and is now rejected on its next request, so a reset takes effect immediately instead of lingering for the access-token lifetime. The refresh sessions are revoked as before. Backward-safe: a legacy token with no epoch claim and a user with no stored epoch both read as 0, so the mechanism is inert (0 < 0 is false) until a first bump — no existing session is locked out. The epoch is server-internal (never surfaced in AuthResult); the edge WASM verifier carries the claim but does not consult it (no store), exactly like the jti-blacklist. - DashboardClaims/PlatformClaims gain `epoch` (serde-default); TS bindings regenerated - SessionStore gains current_epoch/bump_epoch; keys ep/pep; redis + in-memory impls - the dashboard password-reset flow bumps the epoch after revoking sessions Scope note: MFA-state changes and session revoke-all keep their existing "revoke other refresh sessions, current session continues" semantics and do not bump the epoch; the platform epoch mechanism is complete and symmetric, awaiting a platform credential-reset flow to trigger it. Tests prove a pre-reset access token is rejected after the bump (dashboard and platform), that a post-bump token still verifies, that the reset flow advances the epoch, and that a legacy no-epoch token stays valid. Coverage stays 100% on every changed file; clippy, cargo-deny, ts-export staleness, and the security-invariant gate pass.
Rotation minted access claims with `mfa_enabled: false` hardcoded, and the session record had no field to carry the real value. The MFA gate refuses a token only when `mfa_enabled && !mfa_verified`, so one routine refresh — every ~15 minutes for a normal client — produced a token that cleared every MFA-gated route without the holder ever completing a challenge. The bypass applied to the dashboard and platform planes alike, including the WebSocket ticket endpoint. - add `mfaEnabled` to SessionRecord, written at issuance from the account and inherited unchanged by `identity_record` / `platform_identity_record` so it survives every rotation. Field is `serde(default)`, so sessions written before it existed read back as `false` instead of failing the whole record and logging the live user base out. - read the flag in `rotated_claims` / `rotated_platform_claims` instead of hardcoding it. `mfa_verified` still resets to `false` by design: the second factor must be re-acquired through the challenge, only the enrollment state carries over. - populate the flag on the hook/eviction projections so a consumer's after-session-created payload reports the account's real MFA state. Wire parity: `mfaEnabled` is emitted unconditionally and sits last in the record, matching the field order nest-auth already writes to the shared Redis. Regression tests pin the invariant on both planes, plus the unenrolled mirror (so the flag is read, never hardcoded true) and a legacy-payload read.
…gate The proxy returned NextResponse.next() for any request it classified as a framework background fetch — on the unauthenticated path and, via redirectToLogin, on the blocked-status and RBAC-forbidden paths too. But the signals that classify a request as background (RSC, Next-Router-Prefetch, Next-Router-State-Tree, Purpose, Sec-Purpose) are plain request headers and therefore client-forgeable. Sending `RSC: 1` was enough to walk past the middleware's auth, account-status, and RBAC checks on any protected route and have the page's server components execute for an unauthenticated caller. - an unauthenticated background request now gets a bare 401 with `Cache-Control: no-store, no-cache`, matching nest-auth byte for byte. The redirect is still avoided (it would poison the client router cache), but the request is refused rather than admitted. - redirectToLogin no longer short-circuits on background requests, so blocked and forbidden callers get their refusal whatever headers they send. - detect Next-Router-State-Tree as a background signal, which nest-auth already treats as one. It carries a serialised tree, so presence is the signal. The classifier's doc now states the result is a hint about response shape only, never grounds to admit a request. Tests pin each path, including the headline case: a forged RSC header on a protected route with no session receives 401, not next(). Reverting the fix fails exactly these tests.
The brute-force lockout identifier is an HMAC of the email, and login, register, password reset, email verification, and platform login all derived it from the caller's raw input. Each casing of one address therefore had its own failure budget while resolving the same account, so rotating the spelling — user@x.com, USER@X.COM, User@X.Com — handed an attacker an unbounded supply of attempts and the lockout never fired. The same split let a single account own several concurrent OTP, cooldown, and reset records. Add a normalize_email helper (trim, then lowercase) and apply it at the engine boundary of every flow that accepts an address, before any identifier, lookup, or stored record is derived from it. Full Unicode lowercasing, not ASCII-only: nest-auth canonicalizes with JavaScript's Unicode-aware toLowerCase() and the two backends share one Redis, so an ASCII-only fold would make a non-ASCII address canonicalize differently per backend and split its keyspace. The invitation flow, which already normalized inline with to_ascii_lowercase, now routes through the same helper so that divergence cannot reappear. Password reset normalizes on both the initiate and confirm legs: the confirm step compares the supplied address against the one stored in the reset context, so canonicalizing only one side would break every reset. Tests spend the whole lockout budget across five different casings and assert the next attempt is already locked, and pin that an account still authenticates when the caller types it in a different case.
The two backends HMAC the same Redis identifiers with a key each derives from the JWT secret, but they derived it differently, and either difference alone was enough to make the keys disagree: - this side hashed `label || secret` with no separator, leaving the preimage ambiguous, while nest-auth hashes `label + ":" + secret`. - this side keyed the HMAC with the raw 32-byte digest, while nest-auth keys it with the 64-character hex TEXT of that digest. The result was that every keyed identifier landed in a different Redis slot per backend: brute-force lockout, OTP, resend cooldown, MFA setup, and the anti-replay record. On a shared Redis a lockout accrued through one backend was invisible to the other, so an attacker could halve the effective attempt budget by alternating which backend they hit. Adopt nest-auth's derivation verbatim. It is the published one (npm v1.0.11), so changing it there instead would have briefly cleared every in-flight lockout on live deployments — a worse trade for no cryptographic gain, since a hex-text key and a raw-byte key are equally sound. The separator is kept because the domain separation it provides is real. The key is written straight into a fixed-size buffer, so no heap copy of the key material outlives the derivation; the secret buffer and the intermediate digest are both zeroized. Both repos now carry the same known-answer vectors — a fixed secret, its derived key, and one identifier — so a drift on either side turns exactly one suite red instead of surfacing later as sessions and lockouts that silently miss each other.
The MFA temp token outlives the login-time status gate by its whole TTL, so an account blocked in that window could still clear the second factor and receive a full session. Revoking access must not depend on how far through the login the holder already was. Re-check the status once the account is loaded, before the code is verified, on both the dashboard and platform challenge paths. Ordering matters twice: a blocked account must be refused whatever it submits, and the recovery-code path costs one key derivation per stored code, so gating first also denies a revoked account that work. Extract the status gate into a shared status_gate module. There were already two copies of the status-to-error mapping (dashboard login and platform login) and this would have been a third; one implementation means the three planes cannot drift into different notions of "blocked". MfaService now carries the configured blocked statuses, wired from the resolved config by the builder.
Four Redis-contract divergences, one of which was a security bug rather than a
parity gap.
Session-index members were the bare token hash; nest-auth stores full key
suffixes (`rt:{hash}`, `prt:{hash}`) and also indexes the rotation grace
pointers (`rp:`/`prp:`). A bare hash cannot say which keyspace it belongs to, so
revoke_all was structurally unable to delete grace pointers: a refresh token
that had just been rotated away survived a logout-all for its entire grace
window and kept recovering sessions from it. Members now carry their own
prefix, the Lua deletes `{ns}:{member}` directly, and list_sessions filters to
the live prefix and strips it before building the detail key. The separate
`psess:`/`psd:` platform keyspace stays — only the member format converged.
`sd:`/`psd:` timestamps were RFC 3339 strings; nest-auth writes Unix
milliseconds and its listing rejects any record whose timestamps are not
numbers — then SREMs the member. So the divergence was not merely unreadable,
it was destructive: nest-auth evicted sessions this backend had written. Added
a `unix_millis` serde adapter for those two fields. `SessionRecord.created_at`
deliberately keeps RFC 3339, which is what nest-auth writes there.
Password-reset prefixes `pr:`/`prv:` renamed to `pw_reset:`/`pw_vtok:`, so a
reset link emailed by one backend resolves against the other.
The stored invitation gained `createdAt`, which nest-auth writes and validates.
Consumption is a single-use GETDEL, so nest-auth read an invitation written
here, failed validation on the missing field, and the token was already gone —
the invitation was destroyed instead of accepted. Encoding verified against
nest-auth rather than assumed: it writes an ISO string there, not the epoch
milliseconds the session detail uses.
The spec already described the prefixed-member format at §12.5; the
implementation had drifted from its own specification, not only from nest-auth.
…ts legacy tokens
Two cross-backend credential formats that did not line up.
The TOTP secret was encrypted as raw bytes; nest-auth encrypts the Base32 text.
Both backends read the same `mfaSecret` column and the same `mfa_setup:` record,
so decrypting a nest-written secret here handed Base32 ASCII to HMAC-SHA-1 as
the key and rejected every code the user's authenticator produced — and the
reverse broke the same way. Encrypting the presentation form is marginally
redundant, but the published side already stores it that way and re-encrypting
live MFA credentials to save twelve bytes is not a trade worth making. A
`decrypt_secret` helper now owns the decrypt-then-decode step so no call site
can reintroduce the mismatch, and every failure still collapses to one opaque
error rather than a format oracle.
The refresh-token shape guard accepted only the 256-bit hex form. nest-auth
minted UUID v4 before both sides converged, and those tokens live in the shared
Redis for a full refresh lifetime — seven days by default — so refusing the
shape refused to rotate sessions that were still valid with their `rt:{sha256}`
record right there. The legacy shape is accepted alongside the current one; it
grants nothing by itself, since the token still has to hash to a stored session.
Tests pin that the allowance is one exact shape and not "anything with dashes".
Everything this branch converged — key prefixes, index member shapes, record encodings, credential formats, the derived identifier key — is a contract between two implementations that can back the same deployment over one Redis. Nothing enforced it, which is how the divergences reached this point: each side was internally consistent and green. Add `conformance/wire-contract.json`, held byte-identical in both repos, and assert this implementation against it. The prefix catalog is checked against the `Prefix` enum, and the identifier-key known-answer vectors now come from the contract instead of being repeated in the test, so there is one copy of the truth rather than two that drift apart quietly. The contract records the parts that are easy to get wrong from either side: the two identity planes must never share an index prefix; the timestamp encoding is deliberately NOT uniform (the session detail is epoch milliseconds because the reader guards on a numeric type and evicts a member whose detail fails to parse, while everything else is an ISO string); `mfaEnabled` must ride on the refresh session or a rotation silently disables the second factor; and the invitation needs `createdAt` because a single-use GETDEL destroys a record the reader rejects. Changing a value there is a breaking change to the shared keyspace and has to land in both repos together.
Seven response-shape divergences from the published sibling, whose client
contract is already fixed. Two of them were more than cosmetic.
- logout accepted the refresh token only from a cookie. In a bearer deployment
there is no cookie, so the refresh session survived logout entirely — the
access token was blacklisted while the credential that mints new ones stayed
live. It now reads a body `refreshToken` as nest-auth does, cookie as fallback.
- POST /auth/mfa/challenge required the temp token in the body with no cookie
fallback, while this crate's own OAuth callback plants that token as an
HttpOnly cookie. The browser OAuth+MFA flow was therefore a dead end: the SPA
cannot read the cookie and the body was empty. The field is now optional with
the cookie behind it, a request carrying neither fails with the MFA temp-token
error rather than a generic validation 400, and the cookie is cleared on the
same policy nest-auth uses.
The rest are shapes a shared client would break on: GET /auth/me and
/auth/platform/me return the safe account as the top-level body instead of
wrapping it in `{ user }`; GET /auth/sessions returns the bare array instead of
`{ sessions }`; the platform auth body names the account `admin` and is
unconditionally bearer; both refresh endpoints echo the account alongside the
tokens; and both MFA setup routes answer 201.
The platform field name and the always-bearer delivery were already what this
crate's own specification called for at §7.11.1 — the implementation had drifted
from its spec, not only from nest-auth.
Also fixes the namespaced-key catalog test, which enumerates every allowed
prefix and so still listed the pre-rename `pr:`/`prv:` reset keys. It only
surfaced once a Docker daemon was available to run the Redis integration tier.
Two round-trip tests unwrapped `serde_json::from_str` with `?` on a multi-line call, which puts the operator's error arm on a line of its own. The literal being parsed always succeeds, so that arm is unreachable and the CI gate — which fails under 100% line coverage — tripped on it. The gate only surfaced this once the Docker-backed Redis tier was actually executing, since before that the run aborted earlier. Assert on the `Result` with `matches!` instead, which is the idiom the rest of this codebase already uses for exactly this reason and keeps the same assertions. Line coverage is back to 100.00% (16971/16971) with functions at 100%; `cargo llvm-cov --fail-under-lines 100` exits clean.
…eyspace Brings PR #71 (fix/refresh-reuse-detection) into the parity work and reconciles it with the keyspace this branch rewrote: - revoke_family prunes the PREFIXED session-index member (`rt:{hash}` / `prt:{hash}`), not the bare hash. The index format changed under the branch; pruning a bare hash would leave every revoked session listed until the index itself expired. - The e2e catalog keeps `pw_reset:`/`pw_vtok:` (the branch still carried the old `pr:`/`prv:`) and gains `cf`/`fam`/`pcf`/`pfam`/`ep`/`pep`. - SessionRecord carries both `mfaEnabled` (this branch) and `familyId` (the merged branch); both are omitted-or-defaulted for legacy records. Replaying a consumed token after a revoke-all is now reported as Reused rather than Invalid: the `cf:` marker deliberately outlives both the grace pointer and the revoke-all, so a replayed consumed token stays a theft signal even after the user logged everything out. It still never mints a session.
Reuse detection deletes the family's live sessions, but a grace pointer planted by an EARLIER rotation of the same lineage can still be inside its window when the reuse fires: detection only proves the REPLAYED token's own pointer expired, which says nothing about a younger sibling's. Recovering from that pointer minted a fresh session carrying the revoked family id and handed the thief back the lineage the revocation had just killed. The rotation script now decodes the recovered record and refuses the recovery when its family index is gone, so a recovery is valid only while its lineage is alive. A legacy record with no family recovers as before. The e2e test builds the three-token lineage and fails without the guard (verified by reverting it). Also closes the two uncovered lines the merge left in the failing-revoke test double: llvm-cov is back to 100% lines and 100% functions.
… the grace window Three changes that converge the rotation semantics with nest-auth: - The scripts no longer decode a stored record. The grace record's family and the family owner's id are parsed by the caller instead, with a real parser. nest-auth drives its end-to-end tier against an in-memory Redis whose Lua VM has no `cjson`, so a script that decodes JSON is one the shared contract cannot be exercised against on that side. - The grace window is now single-shot: the pointer is consumed on use. It used to be served on every request inside the window, so one captured consumed token could mint a fresh session over and over for the whole window. nest-auth already consumed it; this closes the divergence in the stricter direction. - The family-alive check on a grace recovery moves from the script into the store, where it reads the same. The revocation is monotonic, so checking it next to the recovery is no weaker than checking it inside the script — the session is created after the script returns either way.
Mirror of the nest-auth change: the shared contract gains the six new prefixes, the bare-hash family member shape, `familyId` on the session record, the `epoch` claim, and the rotation semantics both sides implement. The catalog test now asserts every prefix in the enum rather than a subset, so adding a keyspace on one side without the other turns this side red. Spec sections 7.3.3 / 12.4 / 12.5 are brought in line with the code: the rotation flow as it now runs (single atomic script, single-shot grace, reuse detection, family revocation), the new catalog rows, and the renumbered Lua subsections.
The three route handlers that bridge the browser's cookie session to the backend had no tests at all — 0% of `handlers.ts`, in the one module of this package that writes `Set-Cookie` back to a browser. Nine tests pin the contract that matters there: the rotated cookies are relayed verbatim, a failed refresh clears the session rather than leaving the browser holding cookies the backend no longer honors, a backend that is down fails the same way as a rejected refresh, and an attacker-supplied `redirectTo` cannot become the `Location`. The package now runs under a coverage ratchet — thresholds pinned just under what the suite reaches (86/78/90/88), so this layer can only go up. It is deliberately not 100%: the Rust crates are, this layer is the laggard, and a threshold that lies about where it is would be worse than one that admits it. CI runs `test:cov` instead of `test`. Coverage on the package: 68.7% -> 86.06% statements, and `handlers.ts` from 0% to 100% lines.
The parity half of the nest-auth change. `SameSite=None` is the one posture where the browser attaches the session cookie to a cross-site request, and it is the one this adapter had no second line of defense for. A tower layer, innermost so it sees the request exactly as the handler would, decides on headers a page cannot forge: safe methods pass, a request carrying no auth cookie passes (a bearer client has no ambient credential to spend), `Sec-Fetch-Site: same-origin`/`none` passes, and an `Origin` must appear in `cookies.trusted_origins`. Neither header at all is a non-browser caller and passes — a page cannot make a browser omit `Origin` cross-site, so the absence is evidence, not evasion. The request's own origin is never rebuilt from `Host`. Config validation refuses either half without the other, and refuses an entry that is not a bare absolute origin: a trailing slash, a path, a naked hostname or embedded userinfo can never equal an `Origin` header, so the origin the operator meant to allow would be silently blocked instead. The origin shape is checked with the crate's existing hand-rolled URL helpers rather than pulling in `url` — the dependency budget is a feature here. New `AuthError::UntrustedOrigin` / `auth.untrusted_origin`, 403, byte-identical to the nest-auth code.
The parity half of the nest-auth change: `PasswordBreachChecker` is consulted at the three places a password is set — register, reset, invitation acceptance — and never at login, where refusing a breached password someone already has would lock them out of the account they need to get into to change it. A seam, not a dependency. The default `AllowAllBreachChecker` approves everything and touches no network, so a crate upgrade never starts contacting a third party. The bundled `HibpBreachChecker` (feature `breach`) runs over the crate's existing `HttpClient` seam, so enabling the check pulls in no HTTP stack of its own — the deployment supplies the transport it already has. Only SHA-1 is added, behind the same feature, and it is the corpus's index rather than a security primitive: storage is still the configured KDF. The trait returns `bool`, not `Result`, because fail-open is the contract and not a policy the caller gets to choose: a corpus that is down, rate-limiting or answering garbage must approve the password. There is no error the engine would be right to act on. Also covers three lines the origin-check work left uncovered: a legacy record still recovers through its grace window (there is no family to check), and the family-owner walk skips a member whose record is gone, unreadable, or names no owner at all — an empty owner would build an index key every ownerless family would share. New `AuthError::PasswordCompromised` / `auth.password_compromised` (400), byte-identical to nest-auth. Coverage back to 100% lines and functions.
This adapter already enforces its limits — a governor layer per route, not a recommendation — so there was nothing to add. What was missing is the agreement: 21 numbers duplicated across two repos with nothing checking they matched. The contract now carries the table and both sides assert against it, so a limit changed on one side turns that side red rather than surfacing as the same client being throttled at different points depending on which backend answered. The storage difference is written down as deliberate and outside the contract: nest-auth counts in Redis and therefore shares the limit across instances, this adapter counts in process memory and therefore does not. Under horizontal scale the Redis one is the stricter of the two.
The parity half of the nest-auth change. `refresh_expires_in_days` bounds a single refresh token, not a session: a client rotating every fifteen minutes renews that lifetime indefinitely, so a session established once never has to be established again. `family_created_at` is stamped at login and carried unchanged through every rotation — deliberately not `created_at`, which is this session's own and resets each time; measuring from that would make the cap unreachable while looking like it worked. The rotation is refused once `jwt.absolute_session_lifetime_days` has passed, before the script runs, so nothing is consumed on the holder's behalf, and refused as a plain invalid refresh: the remedy is the same as any other, and a distinct code would only tell whoever holds the token how old the session is. Off by default, matching nest-auth: switching it on ends sessions already older than the cap, which is a deployment's decision rather than an upgrade's. A record written before the field carries no birth time and is not capped. The serde adapter is `Option`-aware so a legacy record round-trips as `None` instead of failing the whole record — but a present-and-malformed value is still an error, because a birth time that cannot be read is a cap that cannot be judged, and dropping it quietly would uncap the session.
… profile `create_with_oauth` was called with `email_verified: Some(true)` unconditionally, and `OAuthProfile` had no field for a provider to say otherwise. No bug today — the only bundled provider is Google's, which refuses an unverified profile before building one — but the first third-party provider written against that contract would have created a verified account from an address nobody proved they owned. That account belongs to whoever controls the OAuth account, not to whoever controls the mailbox, and marking it verified makes the consumer's "this email is proven" invariant false from the first login — which is how an account is taken over by registering with someone else's address at a provider that does not check. The profile now carries `email_verified` and the engine passes it through. Google's provider reports `true` unconditionally, which is honest precisely because its own guard already refused everything else. `MockOAuthProvider` gains an `unverified` constructor so the path can be driven end to end — the bundled provider structurally cannot produce it. Parity with the nest-auth change; the field cannot be defaulted to `true` without reintroducing exactly the assumption this removes.
Reuse detection, the token epoch, the absolute session lifetime, the cross-site check, the breach checker and the contract-pinned rate limits — all of which have a nest-auth counterpart, which is the point.
The previous commit swept in a live `cargo mutants --in-place` mutation: the token source had been replaced with `None`, which would have made every cookie-and-bearer read return nothing. Restores the real match. Lesson recorded in the run itself — `--in-place` mutates the working tree, so nothing may be staged while it runs.
Six survivors from `cargo mutants`, each a real gap rather than an equivalent: - `clear_session` could be replaced by a no-op: the logout test asserted no cookie carried a value, which an absent `Set-Cookie` satisfies just as well. It now asserts each cookie is expired back on the Path it was set with — a clearing header on the wrong path leaves a ghost cookie the browser keeps sending. - `clamp_secs` and `refresh_max_age_secs` could return any constant: no test read a cookie's Max-Age. A session cookie instead would log every user out when they closed the tab. - The refresh-name comparison in `carries_auth_cookie` could be inverted unnoticed: every cross-site test carried the access cookie, so the second clause was never decisive. Covered now from both sides — the refresh cookie alone is a credential, the session-signal cookie alone is not. - `forgot_password` and `resend_otp` could be replaced by an empty 200: the anti- enumeration test read only the status, and the two-step flow *skipped* itself when no OTP was minted. The uniform body is asserted, and a missing OTP now fails. `DEFAULT_MAX_BODY_BYTES` was asserted against itself, which accepts whatever the expression produces; it is pinned to the literal.
…uivalents
The v4 layout test drew a single UUID, so a broken mask left the version and variant
nibbles correct often enough to pass by luck — two of the four bit-twiddling mutants
survived on a lucky draw. It draws 64 now, which makes the assertion deterministic.
`AuthEngine::platform_auth` could return `None` unnoticed: every platform test recovers
the service with `let Some(..) else { return }`, so the whole tier skipped itself instead
of failing. It is asserted from an engine built the same way the disabled-domain test
builds one, where nothing can skip.
Three mutants are genuinely equivalent and are recorded in `mutants.toml` with the reason
each cannot be killed — the XOR/OR pair on already-masked bits, the builder whose
`Default` calls the body being replaced, and the `"inactive"` arm the wildcard already
answers identically. The patterns stay narrow so the killable mutants of the same
functions are still generated.
The fake answered `Ok(())` and the test asserted only that — so a repository that stored nothing would have let every platform test built on it pass while asserting nothing. Each update is now read back through `find_by_id`.
`token_key` could be replaced by a constant without failing anything: every test stored and consumed one token at a time, so a colliding key round-trips perfectly. Two live tokens are now asserted not to collide. The fake's `remaining_lockout_secs` guarded on a counter being positive, which it always is — `record_failure` inserts and increments under one lock and `reset` removes the entry — so the entry's existence was already the whole condition.
The enrolment gate is built on exactly those two operations — the first writer wins the setup slot, the completion reads it away so only one caller can finish — and neither was asserted against the double. A double that swallowed the value or always reported "already there" would make that gate untestable while every test stayed green.
The secret-entropy rule was only exercised well away from its floor, so tightening the comparison to reject a secret *at* 3.5 bits/char would have gone unnoticed — the boundary secret is built to land on the constant exactly, with no rounding. `mfa_encryption_key` could return a fixed or zeroed key without failing anything: the MFA tests only ever round-trip through the same resolver, so every deployment's TOTP secrets sealed under one shared key would still decrypt. It is asserted byte for byte, and absent when no MFA is configured.
The two features shipped with a CHANGELOG entry and nothing else: the README's route table, the specification's route-group index, its handler table and its rate-limit table all predate them, and `jwt.issuer`/`jwt.audience` appeared in no prose at all — a consumer reading the docs would not know either exists. The binding gets its own note next to the secret-rotation one, because the two things a deployer has to know before switching it on are not obvious from the field names: both backends must carry the same pair, and enabling it invalidates the access tokens already in flight.
Four defects the blind adversarial audit converged on, all of them reachable
from an unauthenticated or already-signed-in caller, and all of them present in
nest-auth too.
**Both login doors answered before proving the password.** The dashboard and
platform logins ran the status and email-verification gates ahead of
`passwords.verify`, so a caller who never held the credential learned an
account's moderation state from the error code alone — and learned it in
single-digit milliseconds, because the KDF was skipped. The gates now run after
the comparison: a wrong password answers `InvalidCredentials` whatever the
account's state is, and only the password holder is told why they still cannot
sign in.
**A tenant literally named `platform` shared the platform lockout key.** The
brute-force identifier was `hashed_identifier`, whose preimage is
`{tenantId}:{email}` — while the platform door builds `platform:{email}`.
Nothing stopped a tenant from being named so that the two collided, letting
either door consume the other's budget. The login path now derives its
identifier through `lockout_identifier`, which namespaces it with `dashboard:`.
`hashed_identifier` is left alone: it also keys the OTP records, whose keyspace
is shared byte-for-byte with nest-auth and is already purpose-scoped.
**Platform rotation never re-read the admin.** `refresh` worked entirely from
the session record, so blocking an administrator closed only the login door:
they kept minting access tokens for the refresh token's whole lifetime (ASVS v5
§7.4.2 requires disabling an account to end its sessions). Rotation now re-reads
the admin, and a blocked or deleted one loses every platform session, including
the one just minted.
**Rotation froze the role and the tenant.** Claims were built from the session
record written at login and inherited unchanged through every later rotation, so
demoting an ADMIN to MEMBER, or moving a user between tenants, had no effect on
a live session for up to the refresh token's lifetime — while every role check
reads that claim. The dashboard refresh already re-read the account for the
gates above; the current authority was sitting there, unused. It is now
re-stamped onto the rotated token, and only when it actually differs, so
ordinary rotation costs nothing extra.
The wire contract gains the four Redis prefixes it was missing and a new
`lockoutIdentifiers` section pinning the three preimages, so the split above is
contract-enforced rather than convention.
Eight defects from the same audit round, each of them a place where this library disagreed with itself or with nest-auth. **Every OTP failure now answers `otp_invalid`.** `forgot_password` answers the same whether or not the address exists — but it only writes an OTP record when it does, so `otp_expired` for an absent record and `otp_invalid` for a wrong code turned that uniform answer definitive after one extra request. `otp_max_attempts` said the same thing more slowly, since only a record that exists can reach a ceiling. Both collapse through `to_wire`, the treatment the three token sentinels already get, and `AuthError::http_status` now reads the WIRE code — otherwise the oracle survived as 429-vs-401. **The KDF ran unbounded.** `spawn_blocking` bounds nothing useful: Tokio's blocking pool defaults to 512 threads and each derivation holds ~16-19 MiB for its whole run, so a few hundred concurrent logins reach gigabytes of resident memory — on a route an unauthenticated caller drives, because the derivation runs before the password is known to be right and the absent-user path runs one deliberately. A semaphore admits one per core, which is the useful ceiling for CPU- and memory-bound work; past it, requests queue. nest-auth reaches the same bound through libuv's four-thread pool. **The edge verifier ignored `iss`/`aud`.** The binding was checked by the engine after `verify` returned, so the `wasm32` build — a Worker validating a session cookie, with no engine behind it — accepted a token minted for a different service. With HS256 the verifier can also sign, so "a different service that trusts the same secret" is the realistic attacker. The check moves into `bymax_auth_jwt::verify` behind `VerifyOptions`, leaving one implementation of the rule; the engine passes its configured binding in, and the WASM entry point now takes the pair too. A token carrying no such claim is refused as firmly as one carrying the wrong value. **The invitee index hashed the address with bare SHA-256.** An address is low-entropy and a bare digest of one is reversible by dictionary, which is why every other identifier in both libraries is an HMAC. `invidx` is now keyed the same way, and the store trait takes the derived identifier rather than the raw address. **`revoke_invitation` was an enumeration oracle.** The caller names an address and nothing else, so `InsufficientRole` said "there is a pending invitation here, at a role above yours" while `Ok(false)` said "there is none" — which any member could walk an address list through, and which is exactly what hashing the address into the index exists to prevent. An outranked revoker is now answered identically to one asking about an address with nothing pending. The revoker's own standing is a fact about the caller, so it still refuses out loud, and now does so before any lookup. **A rejected new password burned the reset proof.** The breach screen ran inside `apply_password_reset`, after the token or OTP had been consumed atomically, so the caller was told their password was unacceptable and, in the same breath, that the credential they needed to fix it was gone. **Request bounds are now held identical to nest-auth's** and pinned by the new `requestFieldBounds` section of the shared contract, which both conformance tiers drive the real validators against. The address, the tenant, the display name, the three reset proofs, the invitation token and five accepted-but-unused OAuth query fields were unbounded here; the email-verification OTP accepted 4-8 digits when six is the only length either backend issues; and the login password gains an explicit floor of 1, the bound `ChangePasswordDto` already reasons for. **Every identifier preimage is pinned to the contract**, which had described them without either side asserting them. `lockoutIdentifiers` becomes `identifierPreimages` — it had always carried the OTP record, which is not a lockout, and now carries the invitee index too.
Five error codes were in the catalog and reachable from nothing. `SessionExpired` and `SessionLimitReached` describe behaviours neither library has — rotation answers `RefreshTokenInvalid`, and the session cap evicts rather than refuses. `RecoveryCodeInvalid` is unreachable on purpose: a wrong recovery code answers `MfaInvalidCode`, so a caller cannot learn which kind of credential they guessed wrong. `PasswordTooWeak` is the request DTO's job, and `PasswordResetTokenExpired` was already documented as unreachable by design. A code nothing can emit is a client branch that never fires; all five are gone from both libraries, and from the generated TypeScript catalog with them. The Next.js proxy answered a failed rotation with `auth.session_expired` — a code no backend sends. A client branching on it there and on the real code everywhere else was branching on one this proxy alone invented; it now answers `auth.refresh_token_invalid`, which is what the backend actually returns. The `iss`/`aud` binding now reaches the edge end to end. `verify_jwt_hs256` takes the pair, `verifyJwtToken` accepts it as a `TokenBinding`, and `createAuthProxy` carries it from `expectedIssuer` / `expectedAudience` — the proxy is where most consumers meet the edge, so a binding it could not pass through was a binding most deployments would not have had. `NoOpEmailProvider` masks the recipient: a debug level is not a private one, and a provider that only runs when none is configured is the one likeliest to be running with verbose logging turned on. A build without the `mfa` feature now refuses an MFA challenge instead of signing one nobody can redeem — an account whose stored `mfa_enabled` is true got a token with nowhere to spend it and a "challenge issued" line in the log, so the user could not sign in and the log said the flow was working. `cookies.trusted_origins` is accepted under `Lax`/`Strict` when a cookie-domain resolver is configured. Those withhold the cookie cross-site, not cross-origin, so a deployment serving two subdomains from one shared cookie is same-site and the browser does send it — while `Sec-Fetch-Site: same-site` is not proof the request came from the app itself. Refusing the list there left that deployment with no configuration at all. The contract gains an `errorCatalog` section naming all 36 codes and the five that are internal-only, asserted on both sides. Writing it down immediately found that this crate's own catalog test was missing three codes it does emit.
Three CI jobs failed on one root cause. Refusing the MFA challenge in a build without the `mfa` feature left `build_mfa_temp_token` and `MFA_TEMP_TOKEN_TTL_SECONDS` with no caller there, and `-D warnings` makes dead code an error — so the feature matrix, the Playwright pre-build and the coverage job all stopped at the same two lines. Both are now gated with the only thing that mints a challenge. The wasm-target smoke tests still called `verify_jwt_hs256` at its old arity. They only build for `wasm32`, so nothing on the host had compiled them since the binding gained its `iss`/`aud` pair. And `mount_optional_groups` carried an `expect(unused_variables, unused_mut)` for a bare adapter, which stopped being true when the email-change group landed without a Cargo feature: it ships with the adapter and is switched on at runtime, so the parameters are read in every build. Under `-D warnings` an unfulfilled expectation is an error, which is the right outcome — the compiler saying the annotation stopped describing the code. The whole matrix (`cargo hack --each-feature` plus the hasher combinations) now runs clean.
The 100% line gate had been red on this branch since before the audit round —
74 uncovered lines, most of them from the address-change flow, which shipped
with six production branches no test reached.
Those six are covered now: an engine with no single-use store refuses both ends
rather than half-completing, an account with no local password answers as a
wrong password does, an undeliverable verification fails the request while an
undeliverable notice does not undo a change the user proved, and a token is
accepted both when it carries a matching fingerprint and when it carries none.
The rotation gains the two arms the audit's own fixes opened: a deleted platform
admin loses every session, and a store that cannot report a lockout costs the
hook and nothing else — the double gained a skip-aware failure switch, because
one login performs two of those reads and they are answered very differently.
The reuse hook failing is swallowed by design, and now proven swallowed.
The rest were not gaps in the tests but in how they were written. A multi-line
`let ... else { return }` puts its `return` on a line of its own — a line no run
ever reaches, and one llvm-cov counts — while the one-line form the suite uses
everywhere else does not. Twenty-odd of them are back on one line, four repeated
call shapes behind helpers. Five `map_err` closures in the invitation store move
behind `_inner` methods, the shape every other store in that crate uses, so the
redis failure converts through `?` instead of through a region only a broken
connection could reach. And the platform refresh handler's two error arms become
one: split, the second could only fire if the admin vanished between the
rotation and the re-read a microsecond later, which the rotation itself now
refuses.
Also fixes the two wasm-target smoke tests, which only compile for `wasm32` and
had fallen behind two changes to the surface they call — the claims literal
missing `iss`/`aud`, and the arity `verify_jwt_hs256` gained with them.
The gate checks lines AND functions, and it compares the exact percentage: one
line in 24,511 still reads `100.00%` in the report and still fails. It also
exits 1 writing nothing to either stream, so CI showed an exit code and no
reason — the whole diagnosis had to be reproduced locally.
Three shapes were keeping it under. A `map_err` closure is a function of its
own, and `acquire_kdf_permit`'s ran only if the semaphore were closed; the
closure is gone, and a test closes the semaphore to prove the refusal is a
refusal rather than a fall-through to deriving unbounded. `revoke_invitation`
refuses when no invitation store is wired — reachable only with invitations
disabled, since the builder requires the store whenever they are on, which is
why the arm had never been taken. And the DTO-bounds helpers were generic:
instantiated once per DTO, so nine functions where one would do, and one
instantiation short of the whole gate. They take an erased `&dyn Fn` now.
Along the way the invitation test first written for the store guard passed
while proving nothing — `let Ok(engine) = built else { return }` swallowed a
`MissingInvitationStore` and returned early. Asserting before binding is what
turned a green vacuous test into a red one, which is the third time in this
series that a `let-else` over the operation under test has hidden a failure.
Three store operations existed only as sequences of round trips the engine performed itself, and each sequence had a window. `create_recovered_session` writes the session a grace recovery produced in one step, gated on the per-user session index still existing. `refresh_rotate.lua` moved the index bookkeeping inside itself precisely to close the window in which "log out everywhere" could miss a session a rotation had just minted — the GRACE arm was left outside it. The script returned the recovered record and the engine wrote the `rt:`, `sess:`, `sd:` and `fam:` keys several round trips later, so a `revoke_all` landing in between swept an index that did not yet contain the session. The session survived a revocation the user was told had happened, and its access token, signed afterwards, carried the POST-bump epoch and verified. An attacker holding a stolen token gets one grace-eligible token per rotation, so they can keep a continuous stream of these in flight for as long as the victim's reset takes. The witness is the session index: `invalidate_user_sessions.lua` deletes that set once it has removed every member, so its absence is exactly "a revoke-all has run", and the successor the grace pointer named is itself indexed, so a legitimate recovery always finds it present. `sweep_grace_pointers` removes every `rp:` pointer an account holds. `list_sessions` filters them out by construction and `revoke_session` touches only the primary refresh keys, so `revoke_all_except_current` left them behind. For a revoked session that is harmless — the grace branch needs the successor's `rt:` key and it is gone. The gap is the session deliberately KEPT: its predecessor's pointer names a hash that is still alive, so whoever holds that predecessor token could take the grace branch and mint a brand-new full-lifetime session for the rest of the window, right after the user asked to sign out their other devices. `acquire_mfa_lock` / `release_mfa_lock` back the per-account MFA transition lock the engine needs to stop concurrent MFA writes undoing each other. `claim_recovery_code` does not serve: it is keyed on the code, so it serializes two attempts at the SAME code and nothing else. The in-memory ws-ticket double now mints the shape the real store mints — 64 lower-case hex rather than `wst-0`. A double whose output the engine would refuse cannot exercise the path it stands in for.
…y owner Every MFA transition rewrites one repository record carrying `mfa_enabled`, the encrypted secret and the recovery-code digests together, and `update_mfa` replaces all three wholesale — the repository is the consumer's and offers no compare-and-set, so the engine cannot add one. Read-modify-write over that with no serialization is last-write-wins, and three things fell out of it: two challenges spending different recovery codes each wrote the full list minus their own, resurrecting the loser's code; a challenge that read the list before `regenerate_recovery_codes` and spliced after it restored the whole replaced set, unspending codes the user rotated because they had leaked; and a challenge that spliced after `disable` completed wrote `mfa_enabled: true` back with the pre-disable secret. `claim_recovery_code` covers none of them — it is keyed on the code, so it serializes two attempts at the same code and nothing else. The transition now takes a per-account lock and re-reads the record inside it, so a mutation always sees the record as it stands rather than the copy its caller read. A caller that cannot take the lock is refused with `MFA_STATE_CONFLICT` rather than made to wait: concurrent state changes on one account are pathological, and "try again" is the honest answer. The lock carries a per-call token and is released by compare-and-delete. A fixed value would not survive its own TTL: ten seconds is short, the transition calls into the consumer's repository twice, and a run that overruns has already lost the lock when it releases — an unconditional delete then removes whichever transition holds it now, letting a third caller in beside the second. `GET` then `DEL` cannot express the check either, since the key can be retaken between the two round trips, so the comparison and the delete are one script.
…ups to the tenant A refresh rotation re-issued the access token from the session record, which carries the identity but not the authorization. Only some claims were re-read from the account, so a role change, a tenant move or an MFA enrolment landed in the new token for one of them and not the others — and the claim that was not re-read kept its login-time value for as long as the session lived. The condition now covers every claim the guards actually decide on, so a rotation either describes the account as it stands or it re-stamps. The tenant a repository answers with must be the tenant that was asked for. The lookup passes it and the contract says to scope by it, but the repository is the consumer's and a trait can only ask — a single-tenant host writing `find_by_email` that ignores its second argument is the shape nobody notices. Under one, every distinct tenant in the request body resolves the same account while deriving a different lockout counter, so rotating the field yields an unlimited supply of fresh attempt budgets and the lockout never engages. The mismatch is refused, folded into the same branch as the not-found case so the refusal stays indistinguishable.
…ate, cap upstream reads The WebSocket ticket authorizes a socket for its whole lifetime — there is no per-request gate behind it — so its snapshot is the last chance to describe the account correctly. It was copied from the token, whose status claim rotation stamps empty by construction, so every ticket minted from a rotated token carried a blank authorization field for as long as the socket stayed open. It is now read from the account. The email-change confirmation trusted the address recorded when the change was requested. Between the request and the confirmation the account can be banned or the address can be claimed by someone else, and neither was re-checked at the moment the write happened. The HTTP provider read the upstream response without a bound. A body that never ends is a memory exhaustion the consumer cannot see: nothing in the type says the read is unbounded, and the failure arrives as an allocator abort rather than an error the caller can handle.
…er keyspace The adapter picked how the per-route limit is keyed. Neither choice can be a default: reading the socket address behind any proxy gives the proxy's address for every client, so all of them share one bucket and a single caller sending a handful of logins locks out the whole user base with no credential; reading the forwarded address on a directly exposed service gives whatever the caller wrote, and a limiter whose key the attacker picks enforces nothing. Both look like a working limiter at runtime and nothing detects the mismatch, so the deployment states which shape it is or the adapter refuses to build. The limiter's key map also grew without bound: an entry per distinct client address, retained for the process's life, with nothing to remove the ones whose window had long expired. A background sweep now reclaims them.
…t is not one The edge helper fell back to a decode when no secret was configured, and the two branches returned the same shape with the same success flag — so a caller reading it, the natural reading of a function named `verify`, admitted a token an attacker minted the moment the secret went missing. An unset environment variable was enough to arrange that. It now refuses, and the decode-only read stays available under its own correctly-named entry point. `redirectToPath` also states its same-origin invariant explicitly rather than relying on `URL` resolution to imply it. The open redirect was already closed here — resolving against our own origin and keeping only the path drops any authority the input carried — but it closed it by substituting a different destination for the one the caller asked for, and reported nothing. Of the shapes now refused, two were reachable before: a bare relative reference, which the browser resolves against the current directory, and a control character, which ends the header and lets what follows be read as another one. The `Location` itself stays absolute, which is a constraint and not a preference. Next hands the header this middleware sets straight to `new NextURL(location, ...)` with no base, so a relative `Location` throws `ERR_INVALID_URL` there and every unauthenticated request 500s. Naming the request's own origin is safe: Next compares the two hosts immediately afterwards and rewrites `Location` down to a path whenever they match, which here they always do. A redirect to a genuinely different origin is the case Next leaves alone — and the one the path validation above refuses.
Thirteen Dependabot updates landed on main while this branch was in review. The only textual conflict is in the published package's manifest, where main bumped eslint and this branch added `@vitest/coverage-v8`; both are kept. The lockfile was regenerated from the resolved manifest rather than merged by hand. Verified against the bumped set (eslint 10.8, jsdom 30, prettier 3.9.6, @types/node 26.1.2): typecheck, lint and the vitest suite all pass.
…only those that carry one The layer skipped straight to the next handler for a request carrying none of the module's auth cookies, on the reasoning that there was no ambient credential to abuse. The reasoning missed the requests that mint one. This middleware wraps the whole router, `POST /auth/login` and `/auth/register` included — they carry no cookie and answer with a session — so under `SameSite=None` an attacker's page could log a victim's browser into the ATTACKER's account and then read back whatever the victim did there believing it was their own. A non-browser client is unaffected. It sends neither `Origin` nor `Sec-Fetch-Site`, and that shape is still admitted: no page can make a browser OMIT `Origin` on a cross-site request, so the absence is evidence there is no browser involved rather than a way around the check. An empty `trusted_origins` takes the skip's place, and closes the opposite failure. `Origin` is sent on a SAME-origin POST too, and with no `Sec-Fetch-Site` the two cannot be told apart, so the check assumed cross-site — correct where a cross-site cookie can arrive, wrong where it cannot. Under `Lax`/`Strict` with no shared cookie domain the browser withholds the cookie itself and validation REQUIRES the list to be empty, yet every same-origin POST from a browser that omits `Sec-Fetch-Site` was answered 403. Gating on `same_site == None` instead would reopen a real hole: a shared cookie domain makes sibling origins same-site under `Lax`, the browser does send the cookie on a POST between them, and `same-site` is deliberately not a safe fetch site. `Origin` is also compared case-insensitively now. Scheme and host are case-insensitive (RFC 6454 §4), so two origins differing only in case are the same origin and have to compare equal. ASCII folding specifically: Unicode folding can map distinct hosts onto one another, which in an allowlist is a way in rather than a convenience. The allowlist is read where it lives rather than cloned per request, which copied the whole `Vec` on every state-changing call. `resolved_config_with` no longer sets `trusted_origins` under `Lax`/`Strict`. Validation refuses that combination (`ConfigError::TrustedOriginsUnused`), so the fixture was building a config no deployment can hold, and a unit test over an unreachable config says nothing about the reachable ones.
`handleUnauthenticated` refuses a background (RSC / prefetch / state-tree) request with a bare 401 rather than redirecting, and says why: the client router caches the response against the route it asked for, so answering a prefetch of `/dashboard` with a redirect to `/login` leaves a login document cached for `/dashboard`, and the next genuine navigation there renders it. The blocked-account and RBAC branches never got that treatment. They authenticate first and then fail a gate, and they redirected whatever the request shape — so the module poisoned the cache through the two paths its own guarantee did not cover, while documenting that it never does. They now refuse with 403. Both halves of that matter: not a pass-through, because the headers marking a request as background are client-forgeable and `NextResponse.next()` would let a forged `RSC: 1` render the guarded page; and not a redirect, for the caching reason above. 403 rather than 401 because the credential is genuine and re-authenticating will not help — a 401 asks the client to refresh a token that is already valid, and for a blocked account that loop never terminates.
…olation The check piped a recursive `grep` into `grep -qvE`. The `-q` exits at the first match and closes the pipe, which kills the upstream `grep` with SIGPIPE — exit 141 — and under `set -o pipefail` the pipeline then evaluates as failure, so the `if` takes the ELSE branch and the gate prints `ok`. A token read from a query string, the one thing this invariant exists to catch, would have been announced as absent. It needs enough input for the upstream to still be writing when the downstream exits, so it does not reproduce on a small tree; at 400 files it is reliable, which is how a gate that could not fail went unnoticed. The pipeline collects into a variable now — `grep -v` without `-q` drains stdin and lets the upstream finish — and prints the offending lines when it does fail, which the boolean form could not. The comment claimed comment lines are stripped; only LINE comments are. Block comments stay in scope deliberately, and the reason is now written down: recognising them line by line means skipping continuation lines, which start with `*` — and so does a Rust deref. `*query_token = access_token;` is exactly what this gate is for and would be the first thing such a filter waved through. A `/* */` comment that trips the pattern is a false positive to reword; a skipped deref is a miss.
… ship a map that does not `AUTH_ERROR_CODES` is appended to the ts-rs output from `all_error_codes`, a list maintained by hand, while the `AuthErrorCode` union above it is generated from the enum. Nothing tied the two together and the list fell three variants behind: `auth.email_change_token_invalid`, `auth.password_compromised` and `auth.untrusted_origin` shipped in the exported type and not in the exported constant. A TypeScript consumer reading `AUTH_ERROR_CODES.PASSWORD_COMPROMISED` got `undefined`, while the same expression against nest-auth returned the code — a parity break in a published artifact, and one no type checker can see, because the union it would check against is still correct. The generator now refuses to write a map that does not cover the union, reading both out of the file being generated so it compares what actually ships rather than what the generator meant to ship. The next variant added to the enum fails the export test until it is listed, which is the only way a hand-maintained list beside a derived one stays honest. The review named two of the three; the guard found the third.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The rust half of a two-repo change.
nest-authand this crate can back the same deployment overone Redis, so anything touching keys, stored record shapes, JWT claims or Lua scripts is a
contract between them — and
conformance/wire-contract.jsonis held byte-identical in both, witha test on each side that reads it.
Companion PR: bymaxone/nest-auth#44.
Part 1 — parity and the credential path
Family-lineage reuse detection lands (PR #71's branch, reconciled with the keyspace this
branch rewrote).
revoke_familyprunes the prefixed index member (rt:{hash}), not the barehash — the index format changed underneath it, and pruning a bare hash would have left every
revoked session listed until the index itself expired.
Three findings, each red-checked (the test was confirmed to fail without the fix):
replayed token's own pointer expired; a pointer planted by an earlier rotation of the same
lineage can still be live. Recovering from it minted a session carrying the revoked family id.
A recovery now requires its family index to still exist.
window, so one captured consumed token could mint a session repeatedly. Single-shot now,
matching
nest-auth.Reusedrather thanInvalid:the
cf:marker deliberately outlives both the pointer and the revoke-all, so a replay stays atheft signal. It still never mints a session.
The rotation scripts no longer decode stored records.
nest-authdrives its end-to-end tieragainst an in-memory Redis whose Lua VM has no
cjson, so a script that decodes JSON is one theshared contract cannot be exercised against on that side. The grace record's family and the family
owner's id are parsed by the caller instead, with a real parser.
Part 2 — the five security items
SameSitecovers this forLax/Strict; it does not forNone.trusted_originsrequired withSameSite::NonePasswordBreachChecker, with a bundled HIBP implementation over the crate's existingHttpClientseam (featurebreach). Fails open by contract.AllowAllBreachCheckerrefresh_expires_in_daysbounds a token, not a session.email_verifiedon the OAuth profile.create_with_oauthwas called withSome(true)unconditionally.Item 2 adds no HTTP stack: the checker runs over the same seam the OAuth flows use, so a
deployment supplies the transport it already has. Only SHA-1 comes in, behind the
breachfeature, and it is the corpus's index rather than a security primitive — storage is still the
configured KDF. The trait returns
bool, notResult, because fail-open is the contract and nota policy the caller gets to choose.
Item 5 has no bug today — the bundled Google provider refuses an unverified profile before
building one — but the first third-party provider written against the old contract would have
created a verified account from an address nobody proved they owned.
The rate-limit storage difference is recorded in the contract as deliberate and outside it:
nest-authcounts in Redis and shares the limit across instances, this adapter counts in processmemory and does not. Under horizontal scale the Redis one is the stricter of the two.
The mutation sweep
The gate's configuration was never being read. cargo-mutants loads
.cargo/mutants.tomland nothing else; the file sat at the repository root, where it is ignored in silence. So
examine_globsscoped nothing,bindings/,examples/andfuzz/were being mutated despitethe excludes, and CI had been running the same way. Moved, with the requirement written into its
header and a one-line check (
cargo mutants --listmust print onlycrates/paths). Onefollow-on: with the file finally read,
additional_cargo_argssupplies--all-features, andcargo refuses the flag twice — the copy on CI's command line failed the baseline build, so it is
gone.
With the correct scope the sweep surfaced a class of test rather than a class of bug: tests that
cannot see their own subject.
clear_session -> ()Set-Cookiesatisfies that tooclamp_secs/refresh_max_age_secs-> constantMax-Age; a session cookie would log every user out when they closed the tabplatform_auth -> Nonelet Some(..) else { return }, so the whole tier skipped itselftoken_key,setup_key,challenge_bf_id,disable_bf_id,state_key-> a constantmfa_encryption_key -> [0; 32]assert_user_active,revoke_other_user_sessions->Ok(())refresh_ttl_secsarithmeticThe five key-derivation survivors are the ones worth reading twice, because each collapse is an
attack rather than a cosmetic gap: one shared MFA setup slot hands the second caller the winner's
record — their authenticator enrols against someone else's account, and they learn its TOTP secret
and recovery codes; one shared challenge or management counter lets anyone freeze any account's
MFA by failing their own attempts five times; and one shared OAuth
statekey means any forgedstatefinds the stored PKCE verifier, which is the entire CSRF protection. Each is now pinnedwith two users in play.
Three mutants are genuinely equivalent and are recorded in
.cargo/mutants.tomlwith the reasoneach cannot be killed — XOR over already-masked bits, a
Defaultthat calls the body beingreplaced, and an
"inactive"arm the wildcard answers identically. Two more are recorded asuntestable under this gate rather than equivalent: they live behind
cfg(not(feature = …))and the sweep builds
--all-features, so those lines are never compiled into the suite measuringthem. The patterns stay narrow so the killable mutants of the same functions are still generated;
where two
cfgtwins share a mutant name the entry is anchored to the line, and a shift makes itreappear as a survivor rather than disappear silently.
Also in here
handlers.tsinpackages/rust-authhad no tests at all — the one module that writesSet-Cookieback to a browser. Now at 100% lines, and the package runs under a coverage ratchetin CI (68.7% → 86.06% statements) so it can only go up.
cargo mutants --in-placemutation that agit add -Aswept into adocs commit.
--in-placeedits the working tree; nothing may be staged while it runs.Gates
cargo test --workspace --all-featuresgreen across 30 suites, 100% lines and functionsunder
cargo llvm-cov --fail-under-lines 100cargo fmt --checkandclippy --all-targets -D warningscleanredis:8containerRedis store, ~2 min per mutant here); every survivor it reported is closed — killed by a
test or recorded with its reason. Re-running
--iterateover the survivors is what caughtfour of my own fixes asserting the wrong thing, so each of those is red-checked by hand: the
test fails with the mutation applied and passes without it. CI runs the full sweep post-merge.
After the audit
mainlanded the per-user token epoch (#71) while this branch was rebuilding the refresh-tokenlineage on the shared keyspace, so the two touched the same 15 files. The branch carries the
later revision of that shared work — family lineage on nest-auth's
fam:/cf:prefixes,cjson-free Lua so the scripts run on any EVAL-capable backend, the resurrection guard moved tothe host where the recovered record already names its family — so those hunks keep the branch's
version, and what
mainadded on top is ported:TOKEN_EPOCH_RETENTION_SECSwith its startuprule, and the optional
epochin the generated TypeScript.A line-by-line audit of the two implementations against each other then found five behavioural
divergences the conformance tier could not see — it proves both sides write the same bytes, not
that both sides refuse, clean up, or report the same things. Three land here:
sanitized map goes to host-supplied hooks, so a host wiring one audit sink behind both backends
received
x-api-key,proxy-authorizationand every forwarded-identity header from this sideand not the other. nest-auth's regex is reproduced as a suffix test — no regex dependency —
matching it exactly, down to declining a bare
x-token.rejected MFA codes, refresh-token reuse, a completed password reset, and every best-effort
cleanup that failed now emit, with
mask_emailreproducing nest-auth's masking.SessionNotFoundon the logout path stays silent: it is the ordinary outcome for a sessionalready rotated, and logging it would bury the outage it exists to surface.
recordEncodingsandaccessTokenClaimswere never asserted — the two contract sectionsthat decide whether a record written by one backend is readable by the other.
Making the swallowed failures visible put their branches under the 100% line gate for the first
time, which they could not meet against a double that always succeeds — hence
InMemoryStores::fail_next_cleanup_writes, which turns "swallowed and presumed handled" into"swallowed and asserted". The in-memory store's grace window turned out to be weaker than the
Redis one it stands in for, too: neither single-shot nor lineage-checked, in the very double the
conformance tier and nest-auth's end-to-end tier both run against.
The remaining pair (the epoch-retention bound, which was missing on both sides, plus the logout
grace pointer, the OAuth 409 and the
server-onlyguard) lands in the companion PR.