familysync/.planning/phases/12-initial-setup-wizard/12-REVIEW-FIX.md

phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status

phase	fixed_at	review_path	iteration	findings_in_scope	fixed	skipped	status
12-initial-setup-wizard	2026-06-15T16:46:00Z	.planning/phases/12-initial-setup-wizard/12-REVIEW.md	3	1	1	0	all_fixed

Phase 12: Code Review Fix Report

Fixed at: 2026-06-15T16:46:00Z Source review: .planning/phases/12-initial-setup-wizard/12-REVIEW.md Iteration: 3

Summary:

• Findings in scope: 1
• Fixed: 1
• Skipped: 0

Fixed Issues

WR-01: upsertUser inserts new OIDC users with claimed=false (schema default); the TOCTOU guard queries WHERE claimed = false without oidcIss IS NULL

Files modified: apps/api/src/auth/user.ts, apps/api/src/routes/setup.ts, apps/api/tests/auth/user.test.ts, apps/api/tests/routes/setup.test.ts Commit: 687f9dc Applied fix: Both recommended fixes applied for defense-in-depth:

1. apps/api/src/auth/user.ts — upsertUser step 5: Added claimed: true to the insert values for fresh OIDC users. An OIDC-created user is identity-bound at insert time and is never a pending wizard bootstrap user; the explicit flag prevents any future path from treating it as unclaimed. The first-login-claims path (step 2) is unaffected — it updates a pre-existing oidcIss=null row; this change only touches the brand-new OIDC insert path.

2. apps/api/src/routes/setup.ts — TOCTOU guard in POST /credential: Changed WHERE claimed = false FOR UPDATE to WHERE oidc_iss IS NULL AND claimed = false FOR UPDATE. This matches the precise semantic definition of a "pending wizard bootstrap user" and is consistent with the isSetupLocked sentinel and the claim query in upsertUser.

3. apps/api/tests/auth/user.test.ts: Added WR-01 unit test asserting that the fresh OIDC insert values include claimed: true (and that oidcIss/oidcSub are set, distinguishing it from a wizard bootstrap row).

4. apps/api/tests/routes/setup.test.ts: Added WR-01 integration test that seeds an OIDC user with claimed=false and oidcIss NOT NULL, then verifies POST /credential still returns 200 — confirming the narrowed guard ignores the OIDC row and only counts true wizard bootstrap rows.

Verification: All 402 API tests (29 files) and 253 PWA tests (21 files) pass. pnpm -r typecheck clean.

---

Fixed: 2026-06-15T16:46:00Z Fixer: Claude (gsd-code-fixer) Iteration: 3