Wrap the last-admin check and UPDATE in a db.transaction with a
SELECT...FOR UPDATE locking read so concurrent PATCH demotions
serialise and cannot both pass the guard, eliminating the TOCTOU
race (CR-01).
Add a .refine() to updateMemberSchema requiring at least one field,
returning 400 via noEchoHook instead of crashing Drizzle with an
empty SET clause (WR-06).
Add Test H asserting empty {} -> 400 { error: 'Invalid request' }.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add isAdmin field to GET /members select and mapped member object (D-02)
- Add updateMemberSchema (displayName optional string, isAdmin optional boolean)
- Register adminRouter.patch('/members/:id') with noEchoHook and requireAdmin (inherited)
- Handler: parsePositiveIntParam id validation (400), existence check (404),
D-03 last-admin guard via COUNT(*) query (409), partial set() update (200)
- Fix Test D: switch to adminId2 for GET after self-demotion (adminId1 no longer admin)
- All 44 tests green including 7 new PATCH/isAdmin tests
- Test A: PATCH displayName happy path → 200, GET reflects change
- Test B: PATCH isAdmin promote → 200, GET shows isAdmin true
- Test C: last-admin guard → 409 when only admin demotes self
- Test D: self-demotion → 200 when second admin exists
- Test E: non-admin PATCH → 403 (requireAdmin boundary)
- Test F: wrong-type body → 400 { error: "Invalid request" }; malformed :id → 400
- Test G: non-existent member id → 404
- Test H: GET /members includes boolean isAdmin per member
All 7 new tests fail RED for the right reasons (route 404 / isAdmin missing)
Lint (eslint --max-warnings 0):
- index.ts: disable no-unsafe-argument on the type-only Context mismatch when
delegating to the OIDC handler inside the local-session skip wrapper
- localAuth.ts: handleLogout is sync (no await) — drop async (require-await)
- devBypass.ts: disable detect-possible-timing-attacks on the public well-known
dev-placeholder string compare (not a secret comparison)
- remove dead code / unused bindings flagged by no-unused-vars: makeTestApp
(localSession.test), makeUnauthContext + BrowserContext import (login.spec),
unused memberId (admin.test), unused txSelectCount counter (me.test)
- localAuthMiddleware.test / me.test: fix unused + reflow-detached
eslint-disable directives
Format: prettier --write across the 20 Phase-19 files that were never formatted.
Secret scan (gitleaks): allowlist two false positives — the synthetic >=32-char
TEST_SECRET in localSession.test.ts, and .planning/ design prose (a generic-api-key
regex hit on "credential atomically, 409-equivalent"). Neither is a real secret.
Verified locally: format:check, lint, typecheck, md:lint, gitleaks (no leaks),
PWA 266/266, API 452/452.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RED phase for Task 1:
- Test 1: POST /api/admin/members creates users row + local_credentials, hash verifies
- Test 2: duplicate username returns 409, transaction rolled back (no orphaned user row)
- Test 3: admin reset password updates hash, old password no longer verifies
- Test 4: non-admin gets 403 on both POST /members and POST /members/:id/password
- Test 5: GET /api/admin/members returns hasLocalCredential:true/false per local cred existence
The seed handler computed seeded from a pre-flight SELECT then returned
seeded:!alreadySet. Under a genuine concurrent race both requests can
SELECT the empty table, both enter the insert branch, and both return
seeded:true though only one row was actually written. Replace the
SELECT + conditional onDuplicateKeyUpdate with a single INSERT IGNORE
and derive seeded from affectedRows (1 = inserted, 0 = ignored/existing
row preserved, D-03). On MariaDB onDuplicateKeyUpdate(value=value)
reports affectedRows 1 for both insert and no-op, so it cannot
distinguish them; INSERT IGNORE can. timezone is bound via a
parameterized sql template and is already IANA-validated by zod. Adds a
test asserting seeded:false for a directly-pre-inserted row.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Import sql from drizzle-orm in admin.ts
- Add onDuplicateKeyUpdate({ set: { value: sql\`value\` } }) to the
conditional INSERT in POST /config/timezone/seed so a concurrent seed
(or seed racing a PUT) cannot 500 on the app_config.key PK constraint
- Existing value is preserved per D-03 no-overwrite (no-op ODKU)
- seeded flag still reflects the pre-flight SELECT (winner: true, loser: false)
- Add tests: 403 access control, seeded:true on first seed, seeded:false
on second seed without throw (WR-02 idempotent race)
- describe('admin timezone config') covers 8 cases:
- GET and PUT 403 for non-admin authenticated user (T-18-03)
- GET with no stored row returns 200 with isExplicitlySet: false
- PUT America/Chicago then GET round-trip with isExplicitlySet: true
- PUT UTC returns 200 (Pitfall 2)
- PUT Not/AZone returns 400 and does not write to app_config (T-18-04)
- POST seed when unset stores the value (D-02)
- POST seed when already set does NOT overwrite (D-03)
- appConfig imported from db/schema for per-test cleanup
- afterEach removes household_timezone row to prevent test bleed
- 6 new cases FAIL (404 — endpoints not yet implemented); 19 existing pass
PUT /api/admin/calendars/:id/shared cleared the current shared calendar then
set the target in two non-transactional UPDATEs without checking the target
exists — a bad/stale id wiped the family shared lane and still returned ok.
Verify the target inside a transaction; return 404 when absent. Adds a
regression test (RED→GREEN).
RED phase: all admin.test.ts tests fail (404 — routes/mounts not yet created).
Tests cover:
- T-10-08 Pitfall 9: 403 for non-admin on every /api/admin/* route
- T-10-09 Pitfall 7: 400 with no echoed password for all credential failure modes
(PROPFIND/auth failure, createFastmailClient throw, network error, schema mismatch)
- T-10-11: valid credential stores encrypted (AES-256-GCM), not plaintext
- ADMIN-02: PUT /api/admin/calendars/:id/shared — exclusive is_shared=1
- T-10-12 Pitfall 6: POST /api/me/credential ignores body userId, writes to session user
- D-07: non-admin member can POST /api/me/credential (no requireAdmin on self-service)
