vinterliste/CLAUDE.md
Ole-Morten Duesund add76be486 Close the recovery lockout-DoS hole on /auth/recovery-complete
The original spec stored only `kek_salt`, `wrapped_dek_pw`+nonce,
`rec_salt`, and `wrapped_dek_rec`+nonce. Under that model, anyone who
knew a user's email could POST to /auth/recovery-complete with junk
material and overwrite the password-side wrap, locking the legitimate
user out. The data stayed safe (the attacker couldn't decrypt
anything) but the account was effectively DoS'd until the user dug up
their recovery code.

Fix: add a recovery-side verifier mirroring the password-side one.

Storage: two new columns on `users`:
  - rec_auth_salt           BLOB NOT NULL — independent of rec_salt
  - rec_auth_verifier_hash  TEXT NOT NULL — Bun.password.hash output

The migration adds them via ensureColumn() for forward-compat with
scaffold DBs that pre-date this commit; new tables get them via the
CREATE TABLE statement.

Wire protocol:
  - SignupRequest gains rec_auth_salt + rec_auth_verifier
  - RecoveryChallengeResponse gains rec_auth_salt
  - RecoveryCompleteRequest gains rec_auth_verifier

Server (server/auth.ts):
  - signup hashes the recovery verifier alongside the auth verifier
    and stores both
  - recovery-challenge returns rec_auth_salt so the client can derive
    the verifier; refuses with 409 for pre-fix accounts that have a
    NULL rec_auth_salt
  - recovery-complete calls Bun.password.verify against the stored
    hash BEFORE touching any state. Always runs verify even for
    unknown emails (against a dummy hash) so timing doesn't leak
    existence — same pattern we already used for /auth/login.

Client (frontend/src/lib/auth.ts):
  - signup() generates a fourth salt and derives the recovery
    verifier from the recovery code
  - recover() fetches the new rec_auth_salt and submits the derived
    verifier as part of recovery-complete

Recovery.svelte distinguishes the new 401 ("Feil gjenopprettingskode")
and 409 ("Denne kontoen mangler gjenopprettingsverifikator") cases.

Regression test (tests/auth.test.ts) asserts the gate is real:
  - junk recovery verifier → 401, no state changes
  - unknown email → 401 (constant-time)
  - challenge response includes rec_auth_salt
  - correctly-derived verifier passes the gate

SECURITY.md is updated to describe four salts instead of three, the
new key-model storage, and the closed lockout DoS. CLAUDE.md flags
the rec_auth_* columns as load-bearing — removing them re-opens the
hole.

This is the only deviation from the spec's stated storage model;
documented as such in both SECURITY.md and CLAUDE.md.
2026-05-25 12:28:26 +02:00

6.4 KiB

CLAUDE.md — Vinterliste

Project-specific guidance. The global rules in ~/.claude/CLAUDE.md still apply; this file adds what's specific to this codebase.

What this app is

Vinterliste is an end-to-end-encrypted "things to do this winter" list. Activities are private, semi, or public. The original spec is in winter-list-claude-code-prompt.md. The cryptographic model is in SECURITY.mdread it before touching shared/crypto.ts, the auth flow, or anything that handles passwords, recovery codes, or DEKs.

Non-negotiable invariants

Before changing code that touches auth, encryption, or the activity row shape, re-check that all of these still hold:

  1. The server never sees the user's raw password, the recovery code, or the unwrapped DEK. Adding a new endpoint that takes one of those is a bug, not a feature.
  2. Private activities have ciphertext + nonce populated and title, tags, loc_*, scheduled_at all NULL. No row in activity_tags for a private activity.
  3. Semi activities never serialize owner_id (or any creator-identifying field). The column is still set so the owner can edit/delete, but serialize() in server/activities.ts strips it.
  4. Public activities do serialize owner_id.
  5. auth_salt ≠ kek_salt. Generated independently on signup. If you ever need to "reuse" a salt to save a round trip, don't — re-read SECURITY.md.
  6. Argon2id parameters in shared/crypto.ts are stable. If you raise them, you have to store per-user parameters next to salts so old accounts unlock.
  7. Bun.password is for the auth verifier only. Never use it to derive a KEK — the KEK needs raw key bytes from crypto_pwhash.

Crypto module is the trust root

shared/crypto.ts is pure (no I/O, no globals beyond a memoised ready() promise). It runs in both Bun (tests) and the browser (Vite). If you find yourself adding a network call or platform-specific code in there, push it out to the caller instead.

Tests in tests/crypto.test.ts are the regression net. If you change any primitive, the round-trip and tamper tests must still pass.

Stack choices that are locked

These are pinned by the spec, not just by preference. Don't substitute:

  • Bun runtime + bun:sqlite + Bun.password.
  • Hono for HTTP. Bun.serve is OK if it's simpler, but prefer Hono.
  • libsodium-wrappers-sumo for client crypto (Argon2id + XChaCha20-Poly1305-IETF). The SUMO build is required — the standard libsodium-wrappers package doesn't ship crypto_pwhash. The import goes through createRequire because both packages have a broken ESM entry that references a .mjs file they don't actually publish; see the comment at the top of shared/crypto.ts.
  • Svelte 5 + Vite for the frontend. (Svelte 5 runes — $state, $derived, $effect, $props, $bindable — are the reactivity model. Don't import writable stores from svelte/store unless there's a real reason.)
  • IndexedDB for the private tag index. Not localStorage.

Things that aren't pinned and can change: CSS approach, exact error-display patterns, the choice of how the TagInput merges suggestion sources (currently labelled merge — see frontend/src/components/TagInput.svelte).

Visibility transitions

private → semi/public and back is an update PATCH, not a server toggle. The client decrypts locally (or re-encrypts), then sends the appropriate shape. server/activities.ts:patchActivity wipes the columns from the old visibility and populates the new ones — keep this logic centralised there.

Sessions

Sessions are opaque tokens stored in the sessions table; the cookie is vl_session, httpOnly, SameSite=Lax, secure-when-https. Don't switch to JWT — revocation matters more than statelessness for this app.

recovery-complete deletes all sessions for the affected user. That's the right behaviour: it kicks out any logged-in session that may have been hijacked, and the user has to re-login with the new password.

Tag input merging — design decision

Server tags and IndexedDB tags are merged in one dropdown, each row labelled with its source ("offentlig", "privat", "kun din"). For a public/semi activity, suggestions from the private index are shown but clearly marked "kun din" so the user understands accepting them will publish that tag.

If we ever want to keep them strictly separate, the change goes in TagInput.svelte — the rest of the app passes suggestions through unchanged.

What's deferred (documented in SECURITY.md)

  • Server-side rate limiting on auth/recovery endpoints. The recovery verifier closes the lockout-DoS; rate limiting reduces the online brute-force surface on top of that.
  • CSP / SRI for the SPA.

Recovery verifier — deviation from the spec

The original spec stored only kek_salt, wrapped_dek_pw+nonce, rec_salt, and wrapped_dek_rec+nonce. We additionally store rec_auth_salt and rec_auth_verifier_hash so the server can verify the caller knows the recovery code before /auth/recovery-complete writes anything. This is the only deviation from the spec's stated storage model — documented in SECURITY.md.

If you find yourself "simplifying away" the rec_auth_* columns or the verifier check, stop: that re-opens the lockout DoS. See the test in tests/auth.test.ts for the regression case.

Run / test / typecheck

  • bun install
  • bun run dev:server (API on :3000)
  • bun run dev:frontend (SPA on :5173, proxies /api)
  • bun test — must pass. Crypto tests are the regression net.
  • bun run typecheck — server and frontend TS.

Build / deploy

  • bun run build:frontend produces frontend/dist.
  • NODE_ENV=production bun run start serves API + static frontend.
  • Containerfile builds a single podman-ready image; one volume mounts /app/data for the SQLite file. README has the podman build / podman run snippet. Build args BUILD_DATE and GIT_REVISION propagate to OCI labels and /etc/build-info.

Language

UI is in Norwegian Bokmål (lang="nb" on <html>). Default text for any new UI strings should be Bokmål too. Don't add Nynorsk unless the user explicitly asks for it.

When stuck, re-read

  1. winter-list-claude-code-prompt.md — the original spec.
  2. SECURITY.md — the cryptographic model.
  3. Then look at the code.

If the spec, SECURITY.md, and the code disagree, the spec wins and the code needs fixing. If the spec and SECURITY.md disagree, flag it before changing anything.