pneumo

M2: Setup, auth, devices and pairing (ENG-250) #14

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-250-m2-setup-auth-devices-and-pairing

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-250-m2-setup-auth-devices-and-pairing

Linear: https://linear.app/engineereddev/issue/ENG-250

What's in this PR:

instance/: loads meta/owner.json at boot (health.initialized); the setup token is PNEUMO_SETUP_TOKEN, or one is generated and the setup URL logged once while uninitialized; an in-process setup mutex; conditional create with ifNoneMatch: "*".
auth/: POST /api/setup (scrypt verifier, first device + token), GET /api/auth/params, POST /api/auth/login with per-IP and global rate limits (TRUST_PROXY-aware, Retry-After), bearer middleware over SHA-256 token hashes.
devices/: a registry loaded from devices/*.json; GET /api/me; GET/PATCH/DELETE /api/devices[/:id]; PUT/DELETE /api/devices/self/webpush (stores the subscription only). DTOs never include tokenHash or webPush.
pairing/: POST /api/pair (16-byte single-use code, 10-minute expiry) and POST /api/pair/claim (410 when expired or used).
events.ts: a typed bus for device_added/device_updated/device_removed, plus a revoke signal for the WebSocket hub.
webpush/vapid.ts: loads meta/vapid.json, or generates and persists a P-256 VAPID keypair on first boot; /api/me returns the public key.
Log redaction for the new sensitive fields (Web Push keys, the VAPID private key), with assertions.

Verify (lead, rebased on 3c116f5):

pnpm install --frozen-lockfile → exit 0
mise run check:server → exit 0 (Tests 138 passed (138))
pnpm --filter @pneumo/server test:e2e → exit 0 (2 files, 9 tests on MinIO). The e2e:server mise task arrives with PR #13.
No pneumo-e2e-* containers left.

Deviations:

Lead decision: the VAPID keypair bootstrap (ENG-255's first bullet) moved into this issue. The frozen MeResponse requires a 65-byte vapidPublicKey, and ENG-255 is blocked by ENG-251. A null would break the Android and web clients on /api/me until then. ENG-255 keeps the sending.
The built-server startup test moved from check:server into the MinIO e2e suite, because boot now reads the bucket.

🤖 Generated with Claude Code

Commits 2

M2: Bootstrap VAPID keys for the me contract (ENG-250)

+285 -18

M2: Add setup, auth, devices and pairing (ENG-250)

+2235 -98

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, APPROVE #

PR #14 (head c2ed392) implements the issue and the lead's round 2: setup, auth params, login with the rate limiter, bearer tokens, the device registry, Web Push subscription storage, pairing, the typed event bus and the VAPID bootstrap. Every in-scope §8 row returns the table's status codes, with a route test each. packages/ is unchanged, and GET /api/me passes the full MeResponse with a 65-byte key. No FIX findings.

Findings #

FILE (often 2 × bad 2 = 4), auth/routes.ts:41-44: with TRUST_PROXY on, the limiter keys on the leftmost X-Forwarded-For entry. If the edge proxy appends to a client-sent header, a client can rotate it and bypass the per-IP limit of 5 (probe: 12 straight 401s, no 429). The global cap (30 per 15 min) still holds. Check how Railway sets XFF; if it appends, use the rightmost entry or X-Real-IP.
FILE (often 3 × bad 1 = 3), webpush/vapid.ts:26-30: the subject is persisted on first boot from VAPID_SUBJECT → PUBLIC_URL. With http://localhost:8080, web-push 3.6.7 will reject it at send time (it needs https: or mailto:), and a later VAPID_SUBJECT has no effect until meta/vapid.json is deleted. This follows the spec. Heads-up for ENG-255: read the subject from config at send time, or validate it.
FILE (often 1 × bad 2 = 2), instance/state.ts:49-53 and webpush/vapid.ts:21-25: NoSuchBucket maps to NotFound (with bucketMissing: true), and both loaders read it as "object absent". A missing bucket logs a fresh setup URL, then boot fails on the VAPID write. Fix: check bucketMissing.

Test gaps (not bugs): PATCH/DELETE of an unknown id → 404 isn't tested (a probe confirms 404); the global-limit 429 route test doesn't assert Retry-After (the unit test does).

Spec checks #

§7.1 (constant-time compare; the generated token is logged once while uninitialized, never when configured, never after setup), §7.2 (meta/owner.json shape, If-None-Match: * under the mutex), §7.3, §7.4, §7.5 (single use; a concurrent double claim → 201 + 410), §7.6, §6.3, §6.4 (N=16384, r=8, p=1, 16-byte salt), §8 rows, §5.3, §10 first bullet, §12.4: all OK. Contract freeze respected.

DoD checks #

Unit tests with MemoryStorage + fake clock; route tests for every route, including 401/403/409/413/non-JSON 400; MinIO e2e (owner written once, device persist/reload/delete, VAPID reused across restart, bundled-runtime restart and SIGTERM); redaction assertions for p256dh, auth and the VAPID private key: all present. The README edit corrects two statements this PR made false; no hot-file collision.

Deviation accepted: the bundled-boot test moved to the MinIO suite, because boot now needs a bucket. Caveat: Tangled CI runs only check:server, so until e2e:server joins CI (ENG-278), "the bundle boots" is proven locally only.

Tests run by the reviewer: mise run check:server exit 0 (138 tests); test:e2e exit 0 (9 tests); no leftover containers. Pipeline for c2ed392: 3/3 green.

Versions

pneumo-reviewer: round 1, APPROVE #

Findings #

Spec checks #

DoD checks #