diff --git a/.gitleaks.toml b/.gitleaks.toml index 611add9..045bce1 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -22,3 +22,7 @@ paths = ['''apps/api/\.env\.spike$'''] [[allowlists]] description = "apps/api/tests/broker/crypto.test.ts — synthetic AES-256-GCM test key assigned to process.env.APP_PASSWORD_ENCRYPTION_KEY in a Vitest beforeAll; not a real credential" paths = ['''apps/api/tests/broker/crypto\.test\.ts'''] + +[[allowlists]] +description = "apps/api/tests/routes/setup.test.ts — synthetic VAPID public/private test pair used to set process.env.VAPID_* in the setup-route tests; not a real credential (verified not present in .env)" +paths = ['''apps/api/tests/routes/setup\.test\.ts'''] diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 0f2ec49..9068b60 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -32,10 +32,10 @@ Each requirement maps to exactly one roadmap phase (see Traceability). ### Setup — First-run configuration wizard -- [ ] **SETUP-01**: On first run (no admin/credentials configured), the operator is guided through a setup wizard to define bootstrap configuration (app/external URL, OIDC client, session secret, encryption key, VAPID keypair, MariaDB connection, first member's Fastmail app password) instead of hand-editing `.env` / `docker-compose.yml`. -- [ ] **SETUP-02**: The wizard **validates each input before completing** — DB connectivity test, VAPID private key decodes to 32 bytes and pairs with the public key, OIDC discovery resolves, and the Fastmail app password reaches CalDAV (PROPFIND). -- [ ] **SETUP-03**: The wizard generates secrets (session secret, encryption key, VAPID keypair) for the operator to copy into env; secrets are **never written to the database or returned in a response body**. -- [ ] **SETUP-04**: Once setup is complete, the setup endpoints are no longer accessible (guard checked on every invocation, not only at startup). +- [x] **SETUP-01**: On first run (no admin/credentials configured), the operator is guided through a setup wizard to define bootstrap configuration (app/external URL, OIDC client, session secret, encryption key, VAPID keypair, MariaDB connection, first member's Fastmail app password) instead of hand-editing `.env` / `docker-compose.yml`. +- [x] **SETUP-02**: The wizard **validates each input before completing** — DB connectivity test, VAPID private key decodes to 32 bytes and pairs with the public key, OIDC discovery resolves, and the Fastmail app password reaches CalDAV (PROPFIND). +- [x] **SETUP-03**: The wizard generates secrets (session secret, encryption key, VAPID keypair) for the operator to copy into env; secrets are **never written to the database or returned in a response body**. +- [x] **SETUP-04**: Once setup is complete, the setup endpoints are no longer accessible (guard checked on every invocation, not only at startup). ### CI — Gitea continuous integration @@ -84,9 +84,9 @@ Maps each REQ-ID to its phase. v1.1 phases continue v1.0 numbering (v1.0 ended a | NOTIF-04 | Phase 11 (Per-Event Reminders) | Complete | | NOTIF-05 | Phase 11 (Per-Event Reminders) | Complete | | NOTIF-06 | Phase 11 (Per-Event Reminders) | Complete | -| SETUP-01 | Phase 12 (Initial Setup Wizard) | Pending | -| SETUP-02 | Phase 12 (Initial Setup Wizard) | Pending | -| SETUP-03 | Phase 12 (Initial Setup Wizard) | Pending | -| SETUP-04 | Phase 12 (Initial Setup Wizard) | Pending | +| SETUP-01 | Phase 12 (Initial Setup Wizard) | Complete | +| SETUP-02 | Phase 12 (Initial Setup Wizard) | Complete | +| SETUP-03 | Phase 12 (Initial Setup Wizard) | Complete | +| SETUP-04 | Phase 12 (Initial Setup Wizard) | Complete | **DB foundation note:** The v1.1 schema migration (`users.is_admin`, `calendar_events.reminder_lead_minutes`, `app_config` table) is not a standalone requirement — it is carried by **Phase 10 (Admin Role & Settings)** (which owns is_admin + app_config) and consumed by **Phase 11 (Per-Event Reminders)** (reminder_lead_minutes) and **Phase 12 (Initial Setup Wizard)** (app_config.setup_complete). Folded per ARCHITECTURE.md ordering rather than created as a migration-only phase. This makes Phase 10 the head of the admin chain (10 → 11, 10 → 12). diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index a60c6f5..01f14cb 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -30,7 +30,7 @@ Make FamilySync configurable, administrable, and maintainable for real multi-mem - [x] **Phase 9: Faster Write-Back** - Event-driven outbox drain so edits land in ~1-2s instead of ~15s, preserving every outbox durability guarantee (completed 2026-06-12) - [x] **Phase 10: Admin Role & Settings** - DB foundation (is_admin / reminder_lead / app_config) + role-gated admin UI to rotate app passwords and designate the shared calendar (completed 2026-06-13) - [x] **Phase 11: Per-Event Reminders** - Reminder selector on the event form (incl. "None") serialized as VALARM, with a variable-lead scheduler that honors each event's choice (completed 2026-06-14) -- [ ] **Phase 12: Initial Setup Wizard** - First-run validated bootstrap of env/VAPID/DB/OIDC + first app password, reusing the admin route surface +- [x] **Phase 12: Initial Setup Wizard** - First-run validated bootstrap of env/VAPID/DB/OIDC + first app password, reusing the admin route surface (completed 2026-06-16) - [x] **Phase 13: Real Lint Gate (ESLint)** - Wire ESLint flat config (typescript-eslint + React) across both apps so the Phase 8 CI lint slot actually fails on violations instead of no-op'ing (completed 2026-06-12) - [x] **Phase 14: Desktop E2E Coverage** - Add a Desktop Chrome Playwright profile + make the mobile-authored specs desktop-safe so the Phase 8 regression gate validates desktop, not just mobile (completed 2026-06-12) - [x] **Phase 15: Doc-Only CI Skip + Markdown Lint** - Aggregate-gate the slow api/harness CI jobs so doc-only PRs to main merge without running them (no branch-protection deadlock), and add markdownlint to `fast-checks` so docs get a fast format+lint gate (promoted from backlog 999.17) (completed 2026-06-12) @@ -249,7 +249,28 @@ Plans: - **Secrets stay in env, never in DB** (Pitfalls 8 & 10): the wizard validates secrets by performing a test operation (test encrypt/decrypt, structural VAPID check), never by accepting/storing the key value; no DB column for `vapid_private_key` or `app_password_encryption_key`; never log/echo the app password. - Hard constraints: `GET /api/setup/status` mounts **before** the OIDC guard (like `/health`); do NOT create `/api/setup/credentials` — reuse the Phase 10 admin routes; Drizzle generate+migrate (any `app_config` seeding via migration). -**Plans**: TBD +**Plans**: 7 plans in 4 waves (4 original + 3 gap-closure for 12-UAT.md gaps 1-6) + +Plans: +**Wave 1** + +- [x] 12-01-PLAN.md — Schema migration (nullable OIDC + claimed) + generate-secrets helper (SETUP-03) + Wave-0 scaffolds + +**Wave 2** *(blocked on Wave 1 completion)* + +- [x] 12-02-PLAN.md — Pre-auth /api/setup/* router + isSetupLocked 423 guard + index mount + OIDC boot fallback (SETUP-01/02/04) +- [x] 12-03-PLAN.md — First-login-claims rework in upsertUser (D-08, SETUP-01) + +**Wave 3** *(blocked on Wave 2 completion)* + +- [x] 12-04-PLAN.md — PWA SetupPage wizard + App.tsx gate + UI-SPEC revision (SETUP-01/02) + +**Wave 4 — Gap closure** *(UAT 12-UAT.md gaps 1-6; 06+07 parallel, 05 blocked on 06)* + +- [x] 12-06-PLAN.md — Backend: validate/vapid asserts wizard key == env VAPID_PUBLIC_KEY (gap 2) + status exposes non-secret DB name (gap 3) (SETUP-02) +- [x] 12-07-PLAN.md — App.tsx: reverse-gate /setup post-completion (gap 5) + reconcile ['me'] so calendar banner clears after wizard (gap 6) (SETUP-01/04) +- [x] 12-05-PLAN.md — SetupPage: drop DB-vs-env aside (gap 1) + read-only DB-name field (gap 3) + persist fields across Back (gap 4) (SETUP-01) — depends on 12-06 + **UI hint**: yes ### Phase 13: Real Lint Gate (ESLint) @@ -409,7 +430,7 @@ At ≤767px (`window.matchMedia('(max-width: 767px)')` in `apps/pwa/src/App.tsx` | 9. Faster Write-Back | v1.1 | 2/2 | Complete | 2026-06-12 | | 10. Admin Role & Settings | v1.1 | 4/4 | Complete | 2026-06-13 | | 11. Per-Event Reminders | v1.1 | 5/5 | Complete | 2026-06-14 | -| 12. Initial Setup Wizard | v1.1 | 0/? | Not started | - | +| 12. Initial Setup Wizard | v1.1 | 7/7 | Complete | 2026-06-16 | | 13. Real Lint Gate (ESLint) | v1.1 | 3/3 | Complete | 2026-06-12 | | 14. Desktop E2E Coverage | v1.1 | 1/1 | Complete | 2026-06-12 | | 15. Doc-Only CI Skip + MD Lint | v1.1 | 3/3 | Complete | 2026-06-12 | @@ -423,7 +444,7 @@ At ≤767px (`window.matchMedia('(max-width: 767px)')` in `apps/pwa/src/App.tsx` **Goal:** [Captured for future planning] Abstract the calendar backend behind a provider interface so Fastmail/CalDAV is one implementation among potentially many. Shipping with a single provider is fine, but the broker, sync, and event-expansion layers should be structured so additional providers (e.g. other CalDAV hosts, Google Calendar, generic ICS feeds) can be added without rework. Captures the "provider" seam as an explicit architectural concern. **Requirements:** TBD -**Plans:** 5/5 plans complete +**Plans:** 7/7 plans complete Plans: @@ -654,3 +675,24 @@ Plans: - [x] 18-04-PLAN.md — PWA Timezone section in /admin Settings (searchable IANA picker + detected-zone seed) + client fns (D-02/D-04) **UI hint**: yes + +### Phase 19: Local Auth (No-OIDC Mode) + +**Goal:** Let an operator run FamilySync entirely on **local DB users with no OIDC** — username/password accounts and a local login flow that coexists with the Authelia OIDC path — and **optionally wire OIDC in later** by claiming/linking an existing local user to an OIDC identity. Removes the hard dependency on a deployed Authelia for small/solo self-hosters. +**Mode:** standard +**Depends on:** Phase 12 (Initial Setup Wizard) — builds directly on the pre-OIDC **local-user foundation** introduced there: nullable `users.oidc_iss`/`oidc_sub` + the claimed/pending marker, and the first-login-claims merge. Phase 19 generalizes that single bootstrap local user into a full local-account model + login. +**Requirements**: TBD (derive an AUTH-LOCAL-0x set during discuss/spec — local credential storage with proper password hashing, local login session issuance reusing the existing session-cookie path, coexistence with the OIDC middleware, and OIDC-link of an existing local user). +**Plans:** 0 plans + +**Provenance:** Deferred from the Phase 12 discussion (2026-06-15) — see `.planning/phases/12-initial-setup-wizard/12-CONTEXT.md` §Deferred Ideas. The operator runs FamilySync this way themselves and wants no-OIDC operation as a first-class mode. + +**Open questions for discuss/spec:** + +- Password hashing/storage choice (e.g. argon2id/bcrypt) and how it sits alongside the env-only secret kernel from Phase 12. +- How local login coexists with `oidcAuthMiddleware` ordering in `apps/api/src/index.ts` (route-level auth strategy selection vs. a mode flag in `app_config`). +- The OIDC-link flow: claiming an existing local user into an `oidc_iss+oidc_sub` identity without violating the D-10 "identity is OIDC, never email" rule. +- Whether "local mode vs OIDC mode" is a deploy-time switch or both can be live simultaneously. + +Plans: + +- [ ] TBD (run /gsd-plan-phase 19 to break down) diff --git a/.planning/STATE.md b/.planning/STATE.md index 61aa991..063253c 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,33 +2,33 @@ gsd_state_version: 1.0 milestone: v1.1 milestone_name: Operability & Polish -status: "Phase 18 shipped — PR #21" -stopped_at: Phase 18 Plan 03 complete — broker rewire done; plan 4 of 4 is next -last_updated: "2026-06-15T13:15:22.076Z" -last_activity: 2026-06-15 +status: "Phase 12 shipped — PR #22" +stopped_at: Completed 12-06-PLAN.md (UAT gaps 2+3 closed) +last_updated: "2026-06-16T20:24:41.714Z" +last_activity: 2026-06-16 progress: - total_phases: 23 - completed_phases: 9 - total_plans: 37 - completed_plans: 36 - percent: 39 + total_phases: 24 + completed_phases: 10 + total_plans: 44 + completed_plans: 43 + percent: 42 --- # Project State ## Project Reference -See: .planning/PROJECT.md (updated 2026-06-10) +See: .planning/PROJECT.md (updated 2026-06-16) **Core value:** One color-coded family calendar (shared + personal) and shared lists from a single low-friction PWA — cross-ecosystem, no app store -**Current focus:** Phase 18 — auto-timezone-detection-and-ability-to-change-timezone +**Current focus:** Phase 13 — real-lint-gate-eslint ## Current Position -Phase: 18 — COMPLETE -Plan: 4 of 4 -Status: Phase 18 shipped — PR #21 -Last activity: 2026-06-15 +Phase: 13 +Plan: Not started +Status: Phase 12 shipped — PR #22 +Last activity: 2026-06-16 ### ✅ Resolved Checkpoint — Phase 15 Plan 15-03 Task 2 (human-action) @@ -38,7 +38,7 @@ Done 2026-06-12. Gitea branch protection on `main` now requires EXACTLY `CI / fa **Velocity:** -- Total plans completed: 48 +- Total plans completed: 55 - Average duration: - - Total execution time: 0 hours @@ -56,6 +56,7 @@ Done 2026-06-12. Gitea branch protection on `main` now requires EXACTLY `CI / fa | 16 | 6 | - | - | | 10 | 4 | - | - | | 11 | 5 | - | - | +| 12 | 7 | - | - | **Recent Trend:** @@ -111,6 +112,10 @@ _Updated after each plan completion_ | Phase 18 P02 | 3 | 2 tasks | 2 files | | Phase 18 P03 | 28 | 2 tasks | 4 files | | Phase 18 P04 | 15 | 3 tasks | 3 files | +| Phase 12 P01 | 8 | 4 tasks | 10 files | +| Phase 12 P02 | 15 | 3 tasks | 6 files | +| Phase 12 P03 | 8 | 1 tasks | 2 files | +| Phase 12 P06 | 8 | 2 tasks tasks | 3 files files | ## Accumulated Context @@ -119,6 +124,9 @@ _Updated after each plan completion_ Decisions are logged in PROJECT.md Key Decisions table. Recent decisions affecting current work: +- D-07-CJS-IMPORT (2026-06-15, 12-01): web-push is CJS — ESM scripts must use default import then destructure (`import webpush from '...'; const { generateVAPIDKeys } = webpush`). Named ESM export form fails at Node 22 (SyntaxError). +- D-07-BACKFILL (2026-06-15, 12-01): 0002 migration appends `UPDATE users SET claimed=true WHERE oidc_iss IS NOT NULL` — prevents first-login-claims (D-08) matching pre-existing OIDC users. +- D-07-NULL-UNIQUE (2026-06-15, 12-01): kept uniq_oidc_identity unchanged — MariaDB NULL+NULL pairs are DISTINCT in unique indexes, correctly allowing multiple unclaimed wizard rows. - D-13-ESLint-PIN (2026-06-11, 13-01): eslint pinned to 9.39.4 — ESLint 10 breaks eslint-plugin-react@7.37.5 at runtime ("getFilename is not a function", jsx-eslint#3977). Unpin when plugin releases ESLint 10 support. - D-13-JSX-SCOPE (2026-06-11, 13-01): react/react-in-jsx-scope disabled explicitly — flat.recommended enables it at error; PWA uses jsx:react-jsx (React 19 automatic transform), React import not required in JSX files. - D-PROBE-01 (2026-06-11, 08-01): runs-on must be ubuntu-latest — runner has no self-hosted label; all downstream ci.yml workflows use ubuntu-latest. @@ -189,6 +197,9 @@ Recent decisions affecting current work: - [Phase ?]: D-PAYLOAD-ABSENT: __custom__ unchanged → field omitted from payload; server hasOwnProperty check preserves original VALARM (D-08) - [Phase ?]: D-NULL-FALLBACK: occurrence.reminderLeadMinutes===null mapped to None; occurrence cannot distinguish absolute/multi-VALARM from no-reminder; rely on server-side preserve (absent payload) - [Phase ?]: D-05/18-03: three all-day broker sites now route through getHouseholdTimezone(db) +- [Phase ?]: D-12-03-EMAIL-GREP (2026-06-15, 12-03): claims.email in deriveDisplayName is display-name only; claim branch has zero email refs; D-10 upheld +- [Phase 12-06]: D-12-06-VAPID-EQ: validate/vapid compares submitted PUBLIC key (app_config.vapid_public_key) to process.env.VAPID_PUBLIC_KEY; mismatched/absent 400s. Private key stays env-only, never compared/returned (T-12-06). +- [Phase 12-06]: D-12-06-DBNAME: GET /api/setup/status returns non-secret dbName from process.env.DB_NAME only; no DB_HOST/DB_USER/DB_PASSWORD in any response. ### Roadmap Evolution @@ -257,8 +268,8 @@ Recent decisions affecting current work: ## Session Continuity -Last session: 2026-06-15T02:46:09.800Z -Stopped at: Phase 18 Plan 03 complete — broker rewire done; plan 4 of 4 is next +Last session: 2026-06-16T01:15:09.630Z +Stopped at: Completed 12-06-PLAN.md (UAT gaps 2+3 closed) Resume file: None ## Operator Next Steps diff --git a/.planning/phases/12-initial-setup-wizard/12-01-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-01-PLAN.md new file mode 100644 index 0000000..d425e12 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-01-PLAN.md @@ -0,0 +1,270 @@ +--- +phase: 12-initial-setup-wizard +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/api/src/db/schema.ts + - apps/api/src/db/migrations/0002_*.sql + - apps/api/src/db/migrations/meta/_journal.json + - scripts/generate-secrets.mjs + - package.json + - apps/api/src/routes/setup.ts + - apps/api/src/lib/setupGuard.ts + - apps/api/tests/routes/setup.test.ts + - apps/api/tests/auth/user.test.ts +autonomous: true +requirements: [SETUP-03] +must_haves: + truths: + - "Schema migration makes users.oidc_iss/oidc_sub nullable, adds users.claimed, and is APPLIED to the dev DB" + - "Existing OIDC users are backfilled claimed=true so first-login-claims never matches them" + - "npm run generate-secrets prints SESSION_SECRET, APP_PASSWORD_ENCRYPTION_KEY, VAPID_PUBLIC_KEY, VAPID_PRIVATE_KEY for pasting into env — never to the DB" + - "Stub setup.ts router + setupGuard.ts exist so Wave-1 imports resolve" + - "Wave-0 test files exist with at least one failing/red placeholder per SETUP requirement" + artifacts: + - path: "apps/api/src/db/migrations/0002_*.sql" + provides: "nullable oidc_iss/oidc_sub + claimed column + backfill UPDATE" + contains: "claimed" + - path: "scripts/generate-secrets.mjs" + provides: "Bootstrap secret generation helper" + contains: "generateVAPIDKeys" + - path: "apps/api/src/lib/setupGuard.ts" + provides: "isSetupLocked stub (real impl in plan 02)" + exports: ["isSetupLocked"] + - path: "apps/api/src/routes/setup.ts" + provides: "setupRouter stub Hono router" + exports: ["setupRouter"] + - path: "apps/api/tests/routes/setup.test.ts" + provides: "Wave-0 test scaffold for SETUP-01/02/03/04 + 423 guard" + key_links: + - from: "apps/api/src/db/schema.ts" + to: "apps/api/src/db/migrations/0002_*.sql" + via: "drizzle-kit generate" + pattern: "claimed" + - from: "package.json" + to: "scripts/generate-secrets.mjs" + via: "generate-secrets npm script" + pattern: "generate-secrets" +--- + + +Lay the Phase 12 foundation: the schema migration (nullable OIDC identity + `claimed` marker, applied +via Drizzle generate+migrate with the existing-user backfill), the `npm run generate-secrets` repo +helper (SETUP-03, D-05), and the Wave-0 scaffolds (stub `setup.ts` router, stub `setupGuard.ts`, and +the `setup.test.ts` + `user.test.ts` test files) so Wave-1 plans import cleanly and write tests RED-first. + +Purpose: Plans 02 and 03 both depend on the migrated schema (`users.claimed`, nullable `oidc_iss`) +and on the stub router/guard existing as import targets. SETUP-03 (secret generation) is fully owned here. +Output: Applied 0002 migration, `scripts/generate-secrets.mjs`, package.json script, stub setup.ts + +setupGuard.ts, and red test scaffolds. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-CONTEXT.md +@.planning/phases/12-initial-setup-wizard/12-RESEARCH.md +@.planning/phases/12-initial-setup-wizard/12-PATTERNS.md +@apps/api/src/db/schema.ts +@apps/api/src/db/migrations/0001_famous_mad_thinker.sql + + +## Artifacts this phase produces (Plan 01 portion) + +- `users.claimed` column (boolean, default false, NOT NULL) +- `users.oidc_iss` / `users.oidc_sub` → nullable (was NOT NULL) +- Migration `apps/api/src/db/migrations/0002_*.sql` + journal entry — APPLIED +- `scripts/generate-secrets.mjs` + root `package.json` `"generate-secrets"` script +- `apps/api/src/lib/setupGuard.ts` exporting `isSetupLocked()` (stub → real impl in Plan 02) +- `apps/api/src/routes/setup.ts` exporting `setupRouter` (stub → real impl in Plan 02) +- `apps/api/tests/routes/setup.test.ts` (Wave-0 scaffold) + + + + + Task 1: [BLOCKING] Schema change + generate+migrate (nullable OIDC identity, claimed marker, backfill) + apps/api/src/db/schema.ts, apps/api/src/db/migrations/0002_*.sql, apps/api/src/db/migrations/meta/_journal.json + + - apps/api/src/db/schema.ts (the `users` table at lines ~35-51 and `appConfig` at ~282-286 — the file being modified) + - apps/api/src/db/migrations/0001_famous_mad_thinker.sql (analog: prior migration shape, PATTERNS.md §0002_*.sql) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §`apps/api/src/db/schema.ts` and §`0002_*.sql` (exact field edits + backfill SQL) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Runtime State Inventory + Pitfall 9 (unique-constraint/NULL behavior) + + + In apps/api/src/db/schema.ts, edit the `users` table (D-07): remove `.notNull()` from `oidcIss` + (`varchar('oidc_iss', { length: 512 })`) and `oidcSub` (`varchar('oidc_sub', { length: 256 })`), + and add `claimed: boolean('claimed').default(false).notNull()`. Leave the `uniq_oidc_identity` + unique constraint on (oidcIss, oidcSub) unchanged (MariaDB treats NULLs as distinct in unique + indexes — multiple NULLs allowed, which is correct). Add a comment above `appConfig` documenting the + new Phase 12 keys ('oidc_issuer', 'oidc_client_id', 'vapid_public_key', 'app_external_url'; + 'setup_complete' already exists) and the prohibition: NEVER add 'vapid_private_key' or + 'app_password_encryption_key' (D-01 / SC-3). + Then generate the migration: `pnpm --filter @familysync/api exec drizzle-kit generate`. NEVER use + `drizzle-kit push` (D-Task5-DDL — false destructive diff on MariaDB 11). Open the produced + 0002_*.sql and (a) confirm it contains MODIFY/ALTER making oidc_iss/oidc_sub nullable + ADD COLUMN + claimed (not a DROP/recreate of users data), and (b) APPEND the backfill statement + `UPDATE \`users\` SET \`claimed\` = true WHERE \`oidc_iss\` IS NOT NULL;` so existing OIDC users are + marked claimed (prevents first-login-claims from matching them). If drizzle emits a + DROP CONSTRAINT/ADD CONSTRAINT pair on the unique index (Pitfall 9), keep it — it is safe with + nullable columns. + Apply the migration: `pnpm --filter @familysync/api exec drizzle-kit migrate`. The apply step is + mandatory and non-skippable: typecheck/build pass from schema.ts types WITHOUT the live DB change, + so verification below must prove the column exists in the DB. + + + - source: `grep -c "claimed" apps/api/src/db/schema.ts` returns >= 1 + - source: `grep -v '^#' apps/api/src/db/schema.ts | grep -E "oidc_iss.*notNull\(\)|oidc_sub.*notNull\(\)"` returns nothing (notNull removed from both) + - source: a file matching `apps/api/src/db/migrations/0002_*.sql` exists and `grep -i "claimed" $(ls apps/api/src/db/migrations/0002_*.sql)` matches + - source: `grep -ic "UPDATE .users. SET .claimed. = true WHERE .oidc_iss. IS NOT NULL" $(ls apps/api/src/db/migrations/0002_*.sql)` returns 1 + - CLI: migration applied — the dev DB `users` table has a `claimed` column (verified by drizzle-kit migrate exiting 0 and a follow-up `SELECT claimed FROM users LIMIT 1` style check via the test DB harness in Task 4) + - source: `apps/api/src/db/migrations/meta/_journal.json` references the 0002 migration + + + cd apps/api && pnpm exec drizzle-kit migrate && pnpm typecheck + + schema.ts has nullable oidc_iss/oidc_sub + claimed; 0002 migration generated, contains the backfill UPDATE, and is applied to the dev DB; typecheck green. + + + + Task 2: generate-secrets repo helper (SETUP-03 / D-05) + scripts/generate-secrets.mjs, package.json + + - scripts/check-audit.mjs (analog: plain-ESM .mjs script structure, PATTERNS.md §generate-secrets.mjs) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Pattern 6 + §Open Question 2 (VAPID format, script location/toolchain) + - package.json (the root scripts block being modified) + + + Create scripts/generate-secrets.mjs as a plain ESM script (no TypeScript compilation): import + `generateVAPIDKeys` from web-push (resolve from apps/api/node_modules, e.g. + `'../apps/api/node_modules/web-push/src/index.js'`), and `randomBytes` from `node:crypto`. Compute + `SESSION_SECRET = randomBytes(32).toString('hex')`, `APP_PASSWORD_ENCRYPTION_KEY = + randomBytes(32).toString('hex')`, and `const vapid = generateVAPIDKeys()`. Print a copy-paste block + to stdout with a header comment ("FamilySync Bootstrap Secrets", timestamp, "Paste into your + docker-compose.yml environment block", "cannot be recovered if lost") followed by the four lines + `SESSION_SECRET=...`, `APP_PASSWORD_ENCRYPTION_KEY=...`, `VAPID_PUBLIC_KEY=${vapid.publicKey}`, + `VAPID_PRIVATE_KEY=${vapid.privateKey}`. The script ONLY prints to stdout — it MUST NOT write any + file, touch the DB, or call any API (SC-3: secrets never persisted). Add to the ROOT package.json + scripts: `"generate-secrets": "node scripts/generate-secrets.mjs"`. + + + - source: `grep -c "generateVAPIDKeys" scripts/generate-secrets.mjs` returns >= 1 + - source: `grep -c "randomBytes(32).toString('hex')" scripts/generate-secrets.mjs` returns >= 2 (session secret + enc key) + - source: scripts/generate-secrets.mjs contains no `writeFile`/`appendFile`/`fetch`/`db` (`grep -E "writeFile|appendFile|fetch\(|from '.*db" scripts/generate-secrets.mjs` returns nothing) + - source: root package.json scripts has `"generate-secrets"` (`node -e "process.exit(require('./package.json').scripts['generate-secrets']?0:1)"` exits 0) + - behavior: `node scripts/generate-secrets.mjs` prints SESSION_SECRET (64 hex chars), APP_PASSWORD_ENCRYPTION_KEY (64 hex chars), VAPID_PUBLIC_KEY (base64url ~87 chars), VAPID_PRIVATE_KEY (base64url ~43 chars) + + + node scripts/generate-secrets.mjs | grep -E "^SESSION_SECRET=[0-9a-f]{64}$" && node scripts/generate-secrets.mjs | grep -E "^APP_PASSWORD_ENCRYPTION_KEY=[0-9a-f]{64}$" && node scripts/generate-secrets.mjs | grep -E "^VAPID_PUBLIC_KEY=.{80,}$" && node scripts/generate-secrets.mjs | grep -E "^VAPID_PRIVATE_KEY=.{40,}$" + + `node scripts/generate-secrets.mjs` prints all four correctly-shaped values; nothing is written to disk or DB; root package.json wires the script. + + + + Task 3: Stub setupGuard.ts + setup.ts router (Wave-0 import targets) + apps/api/src/lib/setupGuard.ts, apps/api/src/routes/setup.ts + + - apps/api/src/routes/health.ts (analog: minimal Hono router export + file-doc-comment, PATTERNS.md §Shared Pattern 5) + - apps/api/dist/lib/householdTimezone.js (analog: app_config read shape for the real impl in Plan 02) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §setupGuard.ts and §setup.ts (the import patterns Plan 02 fills in) + + + Create apps/api/src/lib/setupGuard.ts exporting an async `isSetupLocked(): Promise`. For + this Wave-0 stub, return `false` (real per-call DB evaluation lands in Plan 02). Add a doc comment: + "Re-evaluated fresh on every call — NEVER cache at module level (D-10). Real impl: Plan 02." + Create apps/api/src/routes/setup.ts exporting `setupRouter = new Hono()` with a file-doc-comment + noting it mounts at /api/setup BEFORE the /api/* OIDC chain (pre-auth surface, like /health). Leave + it as an empty router (handlers added in Plan 02). Do NOT mount it in index.ts yet (Plan 02 owns + the index.ts mount to keep file ownership clean). + + + - source: `grep -c "export async function isSetupLocked" apps/api/src/lib/setupGuard.ts` returns 1 + - source: `grep -c "export const setupRouter" apps/api/src/routes/setup.ts` returns 1 + - test: typecheck passes (`cd apps/api && pnpm typecheck`) + + + cd apps/api && pnpm typecheck + + setupGuard.ts exports isSetupLocked (stub returns false); setup.ts exports an empty setupRouter; typecheck green. + + + + Task 4: Wave-0 test scaffolds (setup.test.ts + user.test.ts claim placeholder) + apps/api/tests/routes/setup.test.ts, apps/api/tests/auth/user.test.ts + + - apps/api/tests/routes/admin.test.ts (analog: Vitest + Hono route test conventions, mock of credentialSync + db) + - apps/api/tests/auth/user.test.ts (the existing upsertUser test file being extended) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Validation Architecture (Phase Requirements → Test Map + Wave 0 Gaps) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §setup.test.ts + + + Create apps/api/tests/routes/setup.test.ts following the admin.test.ts mock conventions (mock + ../src/db/client.js and ../src/broker/credentialSync.js). Add describe/it scaffolds — each marked + with `it.todo(...)` or a placeholder `expect(true).toBe(false)` so they are visibly RED until Plan + 02 implements them — covering: GET /api/setup/status fresh→{setupComplete:false}; status after + complete→{setupComplete:true}; POST /api/setup/validate/vapid 200 valid / 400 truncated; POST + /api/setup/validate/oidc 400 unreachable; POST /api/setup/credential PROPFIND-fail→400; the 423 + guard (Pitfall 8): POST /api/setup/complete twice → first 200, second 423; and D-10 effective-config + branch: any /api/setup/* → 423 when a member_credentials row exists AND VAPID env present. The 423 + guard test (SETUP-04) MUST be written here in Wave 0 so it is RED before the happy path is built. + In apps/api/tests/auth/user.test.ts, add a describe block (it.todo placeholders) for D-08 + first-login-claims: when setup_complete='true', the first OIDC login claims the single unclaimed + local user (oidc_iss IS NULL AND claimed=false), populates oidc_iss/oidc_sub, sets claimed=true, + preserves is_admin; and asserts NO email-keyed lookup. + + + - source: `grep -c "423" apps/api/tests/routes/setup.test.ts` returns >= 1 (the Pitfall 8 guard test present) + - source: `grep -Ec "validate/vapid|validate/oidc|/credential|/complete|/status" apps/api/tests/routes/setup.test.ts` returns >= 4 (all setup routes referenced) + - source: `grep -Ec "claimed|first-login-claim|unclaimed" apps/api/tests/auth/user.test.ts` returns >= 1 + - test: the suite runs without import/collection errors (`pnpm --filter @familysync/api test -- setup` exits with test results, not a load error — todos/red placeholders are expected at this stage) + + + cd apps/api && pnpm test -- setup 2>&1 | grep -Eq "Tests|todo|passed|failed" + + setup.test.ts scaffolds all SETUP-01..04 cases incl. the RED 423-guard test; user.test.ts has the D-08 claim scaffold; the suite collects without import errors. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| operator shell → repo | generate-secrets output crosses to the operator's clipboard/env; must never reach DB or logs | +| schema.ts → live DB | migration applied to a populated `users` table; a destructive diff would orphan/lose user rows | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-01 | Information Disclosure | generate-secrets.mjs | mitigate | Script prints to stdout only — no writeFile/appendFile/fetch/db access (acceptance-checked); SC-3 secrets never persisted | +| T-12-02 | Tampering | 0002 migration on populated users | mitigate | Drizzle generate+migrate (NEVER push); review generated SQL for MODIFY (not DROP); backfill `claimed=true WHERE oidc_iss IS NOT NULL` so existing rows are not orphaned | +| T-12-03 | Information Disclosure | schema.ts app_config keys | mitigate | Comment + acceptance gate forbidding vapid_private_key / app_password_encryption_key columns (D-01) | +| T-12-SC | Tampering | npm/pip/cargo installs | accept | This plan installs ZERO new packages (web-push + node:crypto already present, RESEARCH §No New Packages) — no legitimacy checkpoint needed | + + + +- `cd apps/api && pnpm exec drizzle-kit migrate` exits 0 and the dev DB `users.claimed` column exists +- `node scripts/generate-secrets.mjs` prints all four correctly-shaped secret lines +- `cd apps/api && pnpm typecheck` green +- `pnpm --filter @familysync/api test -- setup` collects (red scaffolds expected) + + + +- Migration applied: nullable oidc_iss/oidc_sub + claimed column + backfill UPDATE in 0002_*.sql +- SETUP-03 satisfied: generate-secrets prints session secret, encryption key, VAPID pair; nothing persisted +- Stub setupGuard.ts + setup.ts exist as Wave-1 import targets +- RED test scaffolds exist (incl. the 423 guard test before the happy path) + + + +Create `.planning/phases/12-initial-setup-wizard/12-01-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-01-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-01-SUMMARY.md new file mode 100644 index 0000000..e1986af --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-01-SUMMARY.md @@ -0,0 +1,149 @@ +--- +phase: 12-initial-setup-wizard +plan: 01 +subsystem: database, api, testing +tags: [drizzle, mariadb, migration, web-push, vapid, vitest, hono] + +# Dependency graph +requires: + - phase: 10-admin-role-settings + provides: app_config table, users.is_admin, member_credentials table — consumed by Phase 12 schema changes +provides: + - users.claimed column (boolean, default false NOT NULL) — distinguishes unclaimed wizard rows from OIDC-bound rows + - users.oidc_iss / users.oidc_sub now nullable — wizard creates local rows before OIDC identity is known + - 0002_lethal_millenium_guard.sql migration — applied to dev DB with backfill UPDATE + - scripts/generate-secrets.mjs — generates SESSION_SECRET, APP_PASSWORD_ENCRYPTION_KEY, VAPID keypair to stdout + - apps/api/src/lib/setupGuard.ts — isSetupLocked() stub (real impl in Plan 02) + - apps/api/src/routes/setup.ts — setupRouter stub Hono router (handlers in Plan 02) + - apps/api/tests/routes/setup.test.ts — Wave-0 RED scaffolds for SETUP-01..04 + 423 guard + - apps/api/tests/auth/user.test.ts — D-08 first-login-claims RED scaffold +affects: + - 12-02-setup-routes (consumes setupGuard + setupRouter stubs, schema claimed column) + - 12-03-pwa-setup-page (consumes /api/setup/* routes) + - 12-04-integration (consumes full setup flow) + +# Tech tracking +tech-stack: + added: [] # No new packages installed (RESEARCH §No New Packages — web-push already present) + patterns: + - drizzle-kit generate+migrate workflow for schema changes (NEVER drizzle-kit push — D-Task5-DDL) + - CommonJS default-import pattern for ESM scripts consuming CJS packages (web-push) + - it.todo() Wave-0 scaffold pattern — RED tests exist before happy path is built + - isSetupLocked() per-call freshness contract (D-10 — never module-cache) + +key-files: + created: + - apps/api/src/db/migrations/0002_lethal_millenium_guard.sql + - apps/api/src/db/migrations/meta/0002_snapshot.json + - scripts/generate-secrets.mjs + - apps/api/src/lib/setupGuard.ts + - apps/api/src/routes/setup.ts + - apps/api/tests/routes/setup.test.ts + modified: + - apps/api/src/db/schema.ts + - apps/api/src/db/migrations/meta/_journal.json + - apps/api/tests/auth/user.test.ts + - package.json + +key-decisions: + - "D-07-CJS-IMPORT: web-push is CJS — ESM scripts must use default import then destructure (import webpush from '...'; const { generateVAPIDKeys } = webpush)" + - "D-07-BACKFILL: 0002 migration appends UPDATE users SET claimed=true WHERE oidc_iss IS NOT NULL to prevent first-login-claims (D-08) matching pre-existing OIDC users" + - "D-07-NULL-UNIQUE: MariaDB treats multiple NULL+NULL pairs as DISTINCT in unique indexes — uniq_oidc_identity constraint kept unchanged; multiple unclaimed rows correctly allowed" + +patterns-established: + - "Wave-0 scaffold: create it.todo() tests BEFORE implementing routes — ensures RED gate exists for SETUP-04 423 guard (Pitfall 8)" + - "generate-secrets: stdout-only secret generation — SC-3 compliance checked via grep acceptance gate" + +requirements-completed: [SETUP-03] + +# Metrics +duration: 8min +completed: 2026-06-15 +--- + +# Phase 12 Plan 01: Foundation Summary + +**Schema migration making OIDC identity nullable + claimed marker applied to dev DB; stdout-only secret generator for VAPID keypair; Wave-0 stub router + RED test scaffolds for all four SETUP requirements** + +## Performance + +- **Duration:** 8 min +- **Started:** 2026-06-15T17:37:13Z +- **Completed:** 2026-06-15T17:44:48Z +- **Tasks:** 4 +- **Files modified:** 10 + +## Accomplishments + +- Applied Drizzle migration 0002 to dev DB: oidcIss/oidcSub now nullable, claimed column added, existing OIDC users backfilled claimed=true +- Created `scripts/generate-secrets.mjs` satisfying SETUP-03: prints SESSION_SECRET (64 hex), APP_PASSWORD_ENCRYPTION_KEY (64 hex), VAPID_PUBLIC_KEY (~87 b64url), VAPID_PRIVATE_KEY (~43 b64url) to stdout only — never to disk or DB +- Created Wave-0 import targets: `setupGuard.ts` (isSetupLocked stub) and `setup.ts` (empty setupRouter) so Plan 02 imports compile from day one +- Created 20 RED it.todo() scaffolds in setup.test.ts (SETUP-01..04 + 423 guard + D-10 effective-config) and user.test.ts (D-08 first-login-claims) — suite collects at 375 passed | 20 todo + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Schema nullable OIDC identity + claimed marker + 0002 migration** - `703fad2` (feat) +2. **Task 2: generate-secrets repo helper** - `2d6dc14` (feat) +3. **Task 3: Stub setupGuard.ts + setup.ts router** - `11e8102` (feat) +4. **Task 4: Wave-0 test scaffolds** - `e098be3` (test) + +## Files Created/Modified + +- `apps/api/src/db/schema.ts` — users.oidcIss/oidcSub made nullable; claimed boolean added; Phase 12 app_config keys documented with prohibition comment (D-01/SC-3) +- `apps/api/src/db/migrations/0002_lethal_millenium_guard.sql` — MODIFY COLUMN for nullable + ADD COLUMN claimed + backfill UPDATE +- `apps/api/src/db/migrations/meta/_journal.json` — 0002 entry added +- `apps/api/src/db/migrations/meta/0002_snapshot.json` — Drizzle snapshot for 0002 +- `scripts/generate-secrets.mjs` — Bootstrap secret generator (SETUP-03 / D-05) +- `package.json` — root "generate-secrets" script added +- `apps/api/src/lib/setupGuard.ts` — isSetupLocked() stub (returns false; real impl Plan 02) +- `apps/api/src/routes/setup.ts` — setupRouter = new Hono() stub (empty; handlers Plan 02) +- `apps/api/tests/routes/setup.test.ts` — 15 it.todo() Wave-0 RED scaffolds +- `apps/api/tests/auth/user.test.ts` — 5 it.todo() D-08 first-login-claims scaffolds added + +## Decisions Made + +- **D-07-CJS-IMPORT:** web-push is a CommonJS module — ESM scripts must use `import webpush from '...'` then destructure. Named ESM export form fails at Node 22 (`SyntaxError: Named export 'generateVAPIDKeys' not found`). Fixed inline as Rule 1 bug. +- **D-07-BACKFILL:** Appended `UPDATE users SET claimed=true WHERE oidc_iss IS NOT NULL` to the generated migration SQL so existing OIDC users are pre-marked claimed, preventing the Plan 02 first-login-claims query (D-08) from matching them. +- **D-07-NULL-UNIQUE:** Kept `uniq_oidc_identity` unique constraint on (oidcIss, oidcSub) unchanged — MariaDB treats NULL+NULL pairs as DISTINCT in unique indexes (ISO SQL semantics), allowing multiple unclaimed wizard rows with NULL oidc_iss. No structural change needed (RESEARCH Pitfall 9 awareness). + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] web-push CommonJS ESM named-import failure** +- **Found during:** Task 2 (generate-secrets.mjs execution) +- **Issue:** `import { generateVAPIDKeys } from 'web-push/src/index.js'` throws `SyntaxError: Named export 'generateVAPIDKeys' not found` — web-push is CommonJS and Node 22 ESM loader does not auto-export CJS named exports +- **Fix:** Changed to `import webpush from '.../web-push/src/index.js'; const { generateVAPIDKeys } = webpush;` +- **Files modified:** scripts/generate-secrets.mjs +- **Verification:** `node scripts/generate-secrets.mjs` prints all four correctly-shaped values +- **Committed in:** `2d6dc14` (Task 2 commit) + +--- + +**Total deviations:** 1 auto-fixed (Rule 1 bug — CJS import form) +**Impact on plan:** Essential for generate-secrets to run. No scope creep. + +## Issues Encountered + +- `drizzle-kit migrate` requires DB env vars — ran with `set -a; source .env; set +a; DB_HOST=127.0.0.1 pnpm exec drizzle-kit migrate`. The dev DB hostname in .env is `mariadb` (Docker internal); overriding to `127.0.0.1` is the standard host-side dev pattern. +- `pnpm test -- setup` (filter by name) triggered globalSetup which needs root DB credentials; acceptance criterion verified instead via full suite run with `DB_HOST=127.0.0.1` showing 375 passed | 20 todo with no import errors. + +## Threat Surface Scan + +No new network endpoints introduced in this plan. The schema migration is additive (ALTER + ADD, no DROP/recreate). Threat mitigations T-12-01, T-12-02, T-12-03 all verified: +- T-12-01: generate-secrets.mjs contains no writeFile/appendFile/fetch/db (grep-checked) +- T-12-02: 0002 migration uses MODIFY COLUMN (not DROP/recreate); backfill verified +- T-12-03: prohibition comment in schema.ts for vapid_private_key / app_password_encryption_key + +## Next Phase Readiness + +- Plan 02 (setup routes) can import `isSetupLocked` from setupGuard.ts and extend `setupRouter` in setup.ts — both exist as valid TypeScript import targets +- Plan 02 can also rely on `users.claimed` and nullable `oidcIss`/`oidcSub` being present in the dev DB +- 20 RED it.todo() tests are waiting for Plan 02 and Plan 03 implementations to turn them GREEN +- SETUP-03 (generate-secrets) is fully satisfied by this plan + +--- +*Phase: 12-initial-setup-wizard* +*Completed: 2026-06-15* diff --git a/.planning/phases/12-initial-setup-wizard/12-02-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-02-PLAN.md new file mode 100644 index 0000000..f5bf4e0 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-02-PLAN.md @@ -0,0 +1,266 @@ +--- +phase: 12-initial-setup-wizard +plan: 02 +type: tdd +wave: 2 +depends_on: ["12-01"] +files_modified: + - apps/api/src/lib/setupGuard.ts + - apps/api/src/routes/setup.ts + - apps/api/src/index.ts + - apps/api/src/auth/middleware.ts + - apps/api/tests/routes/setup.test.ts +autonomous: true +requirements: [SETUP-01, SETUP-02, SETUP-04] +must_haves: + truths: + - "GET /api/setup/status returns {setupComplete:false} on a fresh instance and {setupComplete:true} after completion, reachable WITHOUT auth (before the OIDC guard)" + - "The wizard collects non-secret config (oidc_issuer, oidc_client_id, vapid_public_key, app_external_url) into app_config via POST /api/setup/config" + - "Each input validates before completing: DB connects, VAPID structurally valid (32/65-byte via setVapidDetails), OIDC discovery resolves, Fastmail app password reaches CalDAV PROPFIND" + - "A second call to any setup endpoint after completion returns 423 (guard re-evaluated fresh every call — Pitfall 8)" + - "POST /api/setup/complete promotes the local user to admin, sets app_config.setup_complete, after which the guard locks" + - "OIDC boot config reads env OR app_config so a fresh unconfigured instance does not crash at boot" + artifacts: + - path: "apps/api/src/lib/setupGuard.ts" + provides: "isSetupLocked() — real per-call DB evaluation (setup_complete OR effectively-configured)" + exports: ["isSetupLocked"] + - path: "apps/api/src/routes/setup.ts" + provides: "setupRouter: /status, /config, /validate/db, /validate/oidc, /validate/vapid, /credential, /complete" + exports: ["setupRouter"] + - path: "apps/api/src/index.ts" + provides: "setupRouter mounted at /api/setup BEFORE the /api/* OIDC chain" + contains: "app.route('/api/setup', setupRouter)" + key_links: + - from: "apps/api/src/routes/setup.ts" + to: "apps/api/src/lib/setupGuard.ts" + via: "isSetupLocked() first statement in every handler" + pattern: "isSetupLocked" + - from: "apps/api/src/routes/setup.ts" + to: "apps/api/src/broker/credentialSync.ts" + via: "validateEncryptAndStoreCredential(localUserId, ...)" + pattern: "validateEncryptAndStoreCredential" + - from: "apps/api/src/index.ts" + to: "apps/api/src/routes/setup.ts" + via: "pre-auth mount before devAuthBypass()" + pattern: "api/setup" +--- + + +Build the pre-auth `/api/setup/*` API surface: the real `isSetupLocked()` 423 guard (D-10), the +setup router (status / config-collect / validate db|oidc|vapid / credential / complete), the +index.ts pre-auth mount, and the OIDC boot-config env-OR-app_config fallback (Pitfall 8 / D-02 / D-03 / A2). +This is a TDD plan: the 423 guard test (Pitfall 8) is the canonical RED-first test, written and failing +before the happy path is implemented. + +Purpose: This is the security-critical core of Phase 12 — the only app surface outside the OIDC guard. +SETUP-01 (collect/guided), SETUP-02 (validate-each-input), and SETUP-04 (per-call 423 lock) all land here. +Output: A working, tested pre-auth setup API; local-user + credential provisioning via the shared helper. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-CONTEXT.md +@.planning/phases/12-initial-setup-wizard/12-RESEARCH.md +@.planning/phases/12-initial-setup-wizard/12-PATTERNS.md +@apps/api/src/routes/admin.ts +@apps/api/src/routes/health.ts +@apps/api/src/broker/credentialSync.ts +@apps/api/src/index.ts + + +## Artifacts this phase produces (Plan 02 portion) + +- `isSetupLocked()` — real impl: 423 if `app_config.setup_complete='true'` OR (a `member_credentials` row exists AND `VAPID_PRIVATE_KEY` + `VAPID_PUBLIC_KEY` env present); re-queried every call +- Routes: `GET /api/setup/status`, `POST /api/setup/config`, `POST /api/setup/validate/db`, `POST /api/setup/validate/oidc`, `POST /api/setup/validate/vapid`, `POST /api/setup/credential`, `POST /api/setup/complete` +- app_config keys written: `oidc_issuer`, `oidc_client_id`, `vapid_public_key`, `app_external_url`, `setup_complete` +- `apps/api/src/index.ts`: `app.route('/api/setup', setupRouter)` mounted before `app.use('/api/*', devAuthBypass())` +- OIDC boot config: reads `OIDC_ISSUER`/`OIDC_CLIENT_ID`/`OIDC_AUTH_EXTERNAL_URL` from env OR app_config fallback + + + + + Task 1: isSetupLocked() guard + the RED-first 423 tests (SETUP-04, Pitfall 8) + apps/api/src/lib/setupGuard.ts, apps/api/tests/routes/setup.test.ts + + - apps/api/src/lib/setupGuard.ts (the Wave-0 stub being made real) + - apps/api/tests/routes/setup.test.ts (the Wave-0 scaffold to turn green) + - apps/api/dist/lib/householdTimezone.js (analog: app_config read pattern) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §setupGuard.ts (exact read shape) + §Shared Pattern 1 + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Pattern 3 (fresh-per-call) + Pitfall 2 + + + - isSetupLocked() returns true when app_config.setup_complete === 'true' + - isSetupLocked() returns true when a member_credentials row exists AND both VAPID_PRIVATE_KEY and VAPID_PUBLIC_KEY env are set (D-10 effective-config branch) + - isSetupLocked() returns false on a fresh instance (no flag, no credential) + - RED-first: POST /api/setup/complete twice → first 200, second 423 (Pitfall 8) — write this test against the not-yet-real router and confirm it fails before Task 2 + - The guard re-queries the DB on every call (no module-level cache) — a test that flips setup_complete between two calls sees the change + + + Implement the real isSetupLocked() in setupGuard.ts per PATTERNS.md §setupGuard.ts: read app_config + `setup_complete` (return true if value==='true'); else select one member_credentials row and check + `!!process.env.VAPID_PRIVATE_KEY && !!process.env.VAPID_PUBLIC_KEY`, returning `!!credRow && + vapidPresent`. MUST NOT hoist the result to a module-level variable — every call re-queries (D-10). + Turn the Wave-0 guard tests GREEN against the real helper, and write the RED-first + `POST /api/setup/complete` twice → 200 then 423 test (it will fail until Task 2's /complete handler + exists — that RED state is the point). Mock db.select per the admin.test.ts convention. + + + - source: `grep -c "export async function isSetupLocked" apps/api/src/lib/setupGuard.ts` returns 1 + - source: setupGuard.ts has no module-level `let locked`/cache (`grep -E "^(let|const) .*=.*isSetupLocked|cachedLock" apps/api/src/lib/setupGuard.ts` returns nothing) + - source: setupGuard reads both VAPID env vars (`grep -c "VAPID_PRIVATE_KEY" apps/api/src/lib/setupGuard.ts` and `grep -c "VAPID_PUBLIC_KEY" apps/api/src/lib/setupGuard.ts` each >= 1) + - test: the guard unit tests (setup_complete branch + effective-config branch + fresh-false) pass + + + cd apps/api && pnpm test -- setup 2>&1 | grep -Eq "passed|failed" + + isSetupLocked() is real, fresh-per-call; guard branch tests pass; the 423-after-complete test exists and is RED pending Task 2. + + + + Task 2: setup router — status, config-collect, validate/{db,oidc,vapid}, credential, complete (SETUP-01/02) + apps/api/src/routes/setup.ts, apps/api/tests/routes/setup.test.ts + + - apps/api/src/routes/setup.ts (the Wave-0 stub router being filled) + - apps/api/src/routes/admin.ts (analog: noEchoHook l.54-64, credentialSchema l.47-52, validateEncryptAndStoreCredential call + error mapping l.102-122, app_config upsert) + - apps/api/src/routes/health.ts (analog: DB connectivity check `db.execute(sql\`SELECT 1\`)`) + - apps/api/src/broker/credentialSync.ts (signature: validateEncryptAndStoreCredential(userId, fastmailEmail, appPassword, providerType); CredentialValidationError) + - apps/api/src/auth/user.ts (analog: mysql2 $returningId() + re-select for the local-user insert, l.126-141) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §setup.ts (all handler patterns) + §Shared Patterns 1-5 + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Pattern 5 (helper reuse) + §Pattern 7 (VAPID) + §Pattern 8 (OIDC discovery) + Pitfalls 1,5,7 + + + - GET /api/setup/status → {setupComplete: boolean} derived from app_config.setup_complete; reachable pre-auth + - POST /api/setup/config → upserts oidc_issuer, oidc_client_id, vapid_public_key, app_external_url into app_config; validates issuer is an https URL (reject non-https → 400) + - POST /api/setup/validate/db → 200 on `SELECT 1` success, 503 on failure + - POST /api/setup/validate/oidc → fetch {issuer}/.well-known/openid-configuration (5s timeout); 200 ok, 400 on unreachable/non-2xx + - POST /api/setup/validate/vapid → setVapidDetails(subject, VAPID_PUBLIC_KEY, VAPID_PRIVATE_KEY); 200 valid, 400 on structural failure; reads private key ONLY from process.env (never app_config/DB) + - POST /api/setup/credential → inserts the pre-OIDC local user (oidc_iss NULL, claimed=false, is_admin=true) FIRST, then calls validateEncryptAndStoreCredential(localUserId, email, password, 'caldav'); CredentialValidationError→400 (no echo), other→503 + - POST /api/setup/complete → sets app_config.setup_complete='true'; returns 200 first call, 423 second (guard) + - EVERY handler: isSetupLocked() is the FIRST statement; if locked → 423 + - app password NEVER logged/echoed (noEchoHook; no console.log of c.req.valid('json')) + + + Fill setupRouter in setup.ts. Import { isSetupLocked } from '../lib/setupGuard.js'; copy the + admin.ts noEchoHook (l.54-64) and the credential error-mapping idiom (l.102-122). The FIRST statement + in every handler: `const locked = await isSetupLocked(); if (locked) return c.json({ error: 'Setup + already complete' }, 423);`. Implement each route per the §setup.ts patterns: + /status reads app_config.setup_complete and returns {setupComplete}; /config zod-validates + {oidcIssuer:https-url, oidcClientId, vapidPublicKey, appExternalUrl} and upserts each via + `db.insert(appConfig).values({key,value}).onDuplicateKeyUpdate({set:{value}})` with keys + 'oidc_issuer'|'oidc_client_id'|'vapid_public_key'|'app_external_url'; /validate/db does + `db.execute(sql\`SELECT 1\`)`; /validate/oidc fetches the discovery doc with + `AbortSignal.timeout(5000)`; /validate/vapid calls `webpush.setVapidDetails(subject || + 'mailto:validate@familysync.local', process.env.VAPID_PUBLIC_KEY ?? '', process.env.VAPID_PRIVATE_KEY + ?? '')` in try/catch — NEVER read the private key from app_config or return it; /credential inserts + the local user via $returningId()+re-select (oidcIss:null, oidcSub:null, claimed:false, isAdmin:true, + color: first unused from COLOR_PALETTE) THEN calls the shared helper with that id and providerType + 'caldav' (Pitfall 5 — user row must exist before the FK insert); use noEchoHook + CredentialValidationError→400/503; + /complete upserts setup_complete='true' then returns 200. Do NOT create new crypto and do NOT call + /api/admin/credentials (D-09 — reuse the shared helper directly). Turn the Wave-0 + Task-1 RED tests + GREEN, including the 423-after-complete and the validate 200/400/503 cases. + + + - source: every handler calls the guard first — `grep -c "isSetupLocked" apps/api/src/routes/setup.ts` returns >= 7 (one per route) + - source: setup.ts reuses the shared helper, no new crypto (`grep -c "validateEncryptAndStoreCredential" apps/api/src/routes/setup.ts` >= 1; `grep -Ec "createCipheriv|createHash|randomBytes|encryptPassword" apps/api/src/routes/setup.ts` returns 0) + - source: setup.ts never calls the admin route (`grep -c "api/admin" apps/api/src/routes/setup.ts` returns 0) + - source: VAPID private key read only from env (`grep -E "VAPID_PRIVATE_KEY" apps/api/src/routes/setup.ts` shows only `process.env.VAPID_PRIVATE_KEY`; no app_config read of a private key) + - source: noEchoHook present (`grep -c "noEchoHook" apps/api/src/routes/setup.ts` >= 1) and no log of the password (`grep -Ec "console\.(log|error|warn)\(.*appPassword|console\.(log|error|warn)\(.*valid\('json'\)" apps/api/src/routes/setup.ts` returns 0) + - source: the four new app_config keys written (`grep -Ec "oidc_issuer|oidc_client_id|vapid_public_key|app_external_url" apps/api/src/routes/setup.ts` >= 4) + - test: all setup route tests pass incl. POST /complete twice → 200 then 423 + + + cd apps/api && pnpm test -- setup && pnpm typecheck + + setupRouter implements all 7 routes; guard is first in each; credential reuses the shared helper (no new crypto, no admin-route call); VAPID private key never leaves env; all setup tests green incl. the Pitfall-8 423 regression. + + + + Task 3: Mount setupRouter pre-auth + OIDC boot env-OR-app_config fallback (Pitfall 8 / D-02 / D-03 / A2) + apps/api/src/index.ts, apps/api/src/auth/middleware.ts + + - apps/api/src/index.ts (the file being modified — mount order l.33-55, VAPID boot l.117-139) + - apps/api/src/auth/middleware.ts (oidcAuthMiddleware / processOAuthCallback — where OIDC config is read at boot) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §index.ts (exact insert point) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Env Kernel vs DB Config Split + Open Question 1 + Pitfall 8 (Recommendation: option (a) env-OR-app_config fallback) + Assumptions A1/A2 + + + In apps/api/src/index.ts, add `import { setupRouter } from './routes/setup.js';` and insert + `app.route('/api/setup', setupRouter);` BEFORE `app.use('/api/*', devAuthBypass())` (mirrors the + /health pre-auth pattern, PATTERNS.md §index.ts) so /api/setup/* is never caught by the OIDC guard + (Pitfall 1). For Pitfall 8 / Open Question 1: confirm where @hono/oidc-auth reads OIDC_ISSUER / + OIDC_CLIENT_ID / OIDC_AUTH_EXTERNAL_URL (read auth/middleware.ts and verify A2 — call-time vs + import-time). Implement Recommendation (a): the OIDC config used by oidcAuthMiddleware resolves from + env first (Docker process.env, then .env fallback per D-03), falling back to the app_config keys (oidc_issuer, oidc_client_id, app_external_url) when + the env var is absent — so a fresh unconfigured instance does not crash at boot (no env, no + app_config yet, OIDC simply unconfigured until setup completes) and a wizard-configured instance + reads the app_config values. Keep the existing devBypass/persistSessionCookie ordering intact. Do + NOT defer the middleware mount (option b) or rewrite to lazy-per-request (option c) unless A2 review + proves env values are read at import time AND a fresh boot crashes — if so, document the chosen + deviation in the SUMMARY. + + + - source: `grep -c "app.route('/api/setup', setupRouter)" apps/api/src/index.ts` returns 1 + - source: the setup mount precedes the devAuthBypass mount — `awk '/api\/setup., setupRouter/{s=NR} /devAuthBypass\(\)/{d=NR} END{exit !(s>0 && s= 1) OR the SUMMARY documents A2 found import-time reads requiring option (b)/(c) + - test: full API suite green and the app boots without OIDC env set (a fresh-boot test or the existing boot path does not throw) + + + cd apps/api && pnpm typecheck && pnpm test + + setupRouter mounted pre-auth before the /api/* OIDC chain; OIDC boot config resolves env-OR-app_config so a fresh instance does not crash; full API suite green. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| unauthenticated client → /api/setup/* | The ONLY pre-auth API surface; the 423 lock is the only thing protecting it once configured | +| client form → app_config | operator-supplied oidc_issuer/client_id/vapid_public_key/app_external_url written to DB | +| client form → CalDAV / member_credentials | Fastmail app password validated + encrypted; must never be logged/echoed/stored plaintext | + +## Pre-auth exposure (before vs after setup_complete) + +- **Before setup_complete:** an unauthenticated caller can reach all /api/setup/* routes — this is by design (the wizard is pre-auth). Reachable actions: read status, write non-secret app_config, run validations, provision the single local user + credential, flip setup_complete. No secret is ever returned. Only the household operator standing up the instance is expected here; the instance is not yet publicly routed until the operator finishes. +- **After setup_complete:** isSetupLocked() returns true → every /api/setup/* route returns 423. The lock is the sole protection; it is re-evaluated fresh per call (no startup cache) so a manual DB edit or a second instance cannot get a stale "unlocked". +- **First-login-claims window (D-08, handled in Plan 03):** only household members can reach Authelia OIDC at all, so the single unclaimed local user can only be claimed by a household member — acceptable for a 2-person self-hosted app. + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-04 | Tampering | setup endpoint replay after completion | mitigate | isSetupLocked() first statement in every handler; 423; re-evaluated per call, never cached (D-10); RED-first Pitfall-8 test | +| T-12-05 | Information Disclosure | app password echoed in 400 | mitigate | noEchoHook (admin.ts) — Zod error details never returned; no console.log of password or valid('json') | +| T-12-06 | Information Disclosure | VAPID_PRIVATE_KEY / APP_PASSWORD_ENCRYPTION_KEY in DB or response | mitigate | D-01 env floor — no app_config key for these; /validate/vapid reads private key only from process.env, returns only {ok} | +| T-12-07 | Spoofing | first-login-claims claiming wrong user | accept | Claim query (Plan 03) is `oidc_iss IS NULL AND claimed=false LIMIT 1`; exactly one pending user in a 2-person household; OIDC reach requires household membership | +| T-12-08 | Tampering | OIDC issuer SSRF via /config | mitigate | Validate issuer is https:// at /config; discovery fetch is server-side with a 5s timeout | +| T-12-09 | Tampering | /api/setup/* caught by OIDC guard (302) | mitigate | Mounted before app.use('/api/*', devAuthBypass()) — acceptance-checked ordering (Pitfall 1) | +| T-12-SC | Tampering | npm/pip/cargo installs | accept | Zero new packages this plan (RESEARCH §No New Packages) — no legitimacy checkpoint needed | + + + +- `pnpm --filter @familysync/api test -- setup` green incl. POST /complete twice → 200 then 423 +- `cd apps/api && pnpm typecheck` green; full `pnpm --filter @familysync/api test` green +- Source greps: guard-first in every handler; no new crypto; no admin-route call; VAPID private key env-only; no password log +- /api/setup mount precedes devAuthBypass; OIDC boot has env-OR-app_config fallback + + + +- SETUP-01: GET /api/setup/status pre-auth + config-collect into app_config +- SETUP-02: DB / OIDC / VAPID / CalDAV validations each gate the flow +- SETUP-04: per-call 423 guard (Pitfall 8 regression green) +- Fresh instance boots without OIDC env (env-OR-app_config fallback) + + + +Create `.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md new file mode 100644 index 0000000..dcc18ac --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md @@ -0,0 +1,151 @@ +--- +phase: 12-initial-setup-wizard +plan: 02 +subsystem: api, auth, testing +tags: [hono, drizzle, vitest, tdd, setup-wizard, oidc, vapid, pre-auth, guard] + +# Dependency graph +requires: + - phase: 12-01 + provides: setupGuard.ts stub, setup.ts stub router, Wave-0 RED test scaffolds, schema claimed column +provides: + - apps/api/src/lib/setupGuard.ts — real isSetupLocked() per-call DB evaluation (SETUP-04/D-10) + - apps/api/src/routes/setup.ts — setupRouter with all 7 pre-auth handlers + - apps/api/src/index.ts — setupRouter mounted pre-auth before devAuthBypass + - apps/api/src/auth/middleware.ts — oidcConfigFallbackMiddleware (env-OR-app_config, D-02/D-03) + - apps/api/tests/routes/setup.test.ts — 17 integration tests all GREEN +affects: + - 12-03-pwa-setup-page (consumes /api/setup/* routes, esp. GET /status) + - 12-04-integration (full setup flow) + +# Tech tracking +tech-stack: + added: [] # Zero new packages (RESEARCH §No New Packages) + patterns: + - isSetupLocked() per-call freshness pattern (D-10) — imported in every handler, no module-cache + - guard-first handler pattern — isSetupLocked() is the FIRST await in every setup handler + - noEchoHook anti-echo pattern (from admin.ts) — Zod error details never returned on credential routes + - validateEncryptAndStoreCredential reuse (D-09) — no new crypto; shared helper for PROPFIND+encrypt+store + - env-OR-app_config fallback middleware — reads DB per-request when env absent; injects into process.env + - mysql2 $returningId() + re-select for local user insert (Pattern 4 from user.ts) + - onDuplicateKeyUpdate upsert for app_config writes (Shared Pattern 1 from admin.ts) + +key-files: + created: [] + modified: + - apps/api/src/lib/setupGuard.ts + - apps/api/src/routes/setup.ts + - apps/api/src/index.ts + - apps/api/src/auth/middleware.ts + - apps/api/tests/routes/setup.test.ts + - apps/api/tests/routes/push.test.ts + +key-decisions: + - "A2-CONFIRMED: @hono/oidc-auth reads OIDC_ISSUER/OIDC_CLIENT_ID/OIDC_AUTH_EXTERNAL_URL at per-request call time via env(c)→process.env — NOT at import time; fresh boot without OIDC env is safe (HTTP 500 only on protected /api/* requests)" + - "D-02-FALLBACK: env-OR-app_config Recommendation (a) implemented: oidcConfigFallbackMiddleware reads from app_config when process.env absent, injects into process.env before oidcAuthMiddleware() per-request read" + - "GUARD-ON-STATUS: GET /api/setup/status uses isSetupLocked() directly (covers effective-config branch too) — returns {setupComplete:true} when locked, {setupComplete:false} when not; aligns with must_haves.truths" + - "LOCAL-USER-ROLLBACK: POST /api/setup/credential rolls back the local user insert if validateEncryptAndStoreCredential throws, preventing orphaned unclaimed user rows" + +# Metrics +duration: 15min +completed: 2026-06-15 +--- + +# Phase 12 Plan 02: Setup Routes Summary + +**Real isSetupLocked() 423 guard + all 7 pre-auth /api/setup/* routes + OIDC env-OR-app_config fallback; 394 tests green including Pitfall 8 regression** + +## Performance + +- **Duration:** 15 min +- **Started:** 2026-06-15T17:48:42Z +- **Completed:** 2026-06-15T18:03:21Z +- **Tasks:** 3 +- **Files modified:** 6 + +## Accomplishments + +- Implemented real `isSetupLocked()` in `setupGuard.ts`: reads `app_config.setup_complete` (check 1) and then checks `member_credentials` row + `VAPID_PRIVATE_KEY`/`VAPID_PUBLIC_KEY` env for effective-config branch (D-10 check 2). Re-queries DB fresh every call — no module-level cache. +- Converted all 20 Wave-0 `it.todo()` scaffolds in `setup.test.ts` into real integration tests (17 tests) — all GREEN after Task 2. +- Implemented full `setupRouter` in `setup.ts` with all 7 routes: + - `GET /status` — uses `isSetupLocked()` directly; returns `{setupComplete: boolean}` + - `POST /config` — zod-validates https-URL issuer; upserts `oidc_issuer`, `oidc_client_id`, `vapid_public_key`, `app_external_url` + - `POST /validate/db` — `SELECT 1` connectivity check; 200/503 + - `POST /validate/oidc` — fetches discovery doc with 5s timeout; 200/400 + - `POST /validate/vapid` — `webpush.setVapidDetails()` structural check; env-only key read; 200/400 + - `POST /credential` — inserts local user first (Pitfall 5 FK), calls shared helper; noEchoHook; rollback on failure + - `POST /complete` — upserts `setup_complete='true'`; 200 first call, 423 second (Pitfall 8/SETUP-04) +- Mounted `setupRouter` in `index.ts` BEFORE `devAuthBypass()` (line 49 < line 54, T-12-09/Pitfall 1 acceptance-checked). +- Implemented `oidcConfigFallbackMiddleware` in `auth/middleware.ts`: reads OIDC config from `app_config` when env absent, injects into `process.env` for downstream `oidcAuthMiddleware()` pickup. Mounted before OIDC guard when `!devBypassActive`. +- Confirmed A2: `@hono/oidc-auth` reads env at per-request call time — boot is safe without OIDC env. +- Fixed `push.test.ts` `vi.doMock` to include `oidcConfigFallbackMiddleware` stub (Rule 3 auto-fix). + +## Task Commits + +1. **Task 1: isSetupLocked() real impl + RED-first setup tests** — `4748d57` (test) +2. **Task 2: Setup router — all 7 routes + pre-auth mount** — `20f91e4` (feat) +3. **Task 3: OIDC boot env-OR-app_config fallback + mount verification** — `67a9d29` (feat) + +## Files Created/Modified + +- `apps/api/src/lib/setupGuard.ts` — real `isSetupLocked()`: `setup_complete` check + effective-config branch (D-10); no module-level cache +- `apps/api/src/routes/setup.ts` — `setupRouter` with 7 handlers; guard-first; noEchoHook; shared helper reuse; VAPID env-only +- `apps/api/src/index.ts` — `setupRouter` import + pre-auth mount; `oidcConfigFallbackMiddleware` import + mount before OIDC guard +- `apps/api/src/auth/middleware.ts` — `oidcConfigFallbackMiddleware` added (env-OR-app_config fallback); re-exports unchanged +- `apps/api/tests/routes/setup.test.ts` — 17 real integration tests (all GREEN); full mock scaffolding +- `apps/api/tests/routes/push.test.ts` — `vi.doMock` updated to include `oidcConfigFallbackMiddleware` stub + +## Decisions Made + +- **A2-CONFIRMED:** `@hono/oidc-auth` reads OIDC env vars at per-request call time via `env(c) → process.env` (source: `@hono/oidc-auth` dist/index.js line 30). NOT at import time. A fresh unconfigured instance boots without crashing; HTTP 500 only occurs on OIDC-protected `/api/*` requests when env is absent — acceptable since `/api/setup/*` is pre-auth and is the only pre-setup surface. Recommendation (a) implemented. + +- **D-02-FALLBACK:** `oidcConfigFallbackMiddleware` injects `oidc_issuer` / `oidc_client_id` / `app_external_url` from `app_config` into `process.env` when the env var is absent, before `oidcAuthMiddleware()` reads it per-request. Non-secret values only (D-01 env floor: `OIDC_CLIENT_SECRET`, `OIDC_AUTH_SECRET` stay in env always). Options (b) and (c) (defer mount, lazy-per-request) not needed — option (a) is simpler and correct per A2 confirmation. + +- **GUARD-ON-STATUS:** `GET /api/setup/status` calls `isSetupLocked()` to populate `setupComplete`. This makes the status response consistent with the guard state (covers the effective-config branch too) and satisfies the must_haves truth that `/status` returns `{setupComplete:true}` after setup is complete. The route never returns 423 — it always returns 200 with the boolean. + +- **LOCAL-USER-ROLLBACK:** `POST /api/setup/credential` deletes the inserted local user row if `validateEncryptAndStoreCredential()` throws, preventing orphaned `claimed=false` rows in the `users` table that would permanently increment color slot usage and confuse the first-login-claims query. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] push.test.ts vi.doMock missing oidcConfigFallbackMiddleware** +- **Found during:** Task 3 test run +- **Issue:** `push.test.ts` uses `vi.doMock('../../src/auth/middleware.js', ...)` but the mock omitted the new `oidcConfigFallbackMiddleware` export. Vitest raises `No "oidcConfigFallbackMiddleware" export is defined on the mock` at runtime. +- **Fix:** Added `oidcConfigFallbackMiddleware: async (_c, next) => next()` to the doMock factory. +- **Files modified:** `apps/api/tests/routes/push.test.ts` +- **Commit:** `67a9d29` (Task 3) + +--- + +**Total deviations:** 1 auto-fixed (Rule 3 blocking — test mock missing new export) +**Impact on plan:** Zero scope creep. Fix was mechanical and localized to a test file. + +## Threat Surface Scan + +No new threat surface beyond what is explicitly modeled in the plan's ``. All mitigations verified: + +| Threat | Mitigation | Verified | +|--------|-----------|---------| +| T-12-04: Setup endpoint replay after completion | `isSetupLocked()` first in every handler; 423; re-queried per call | All 7 handlers call `isSetupLocked()` — source-grep ≥7 passed | +| T-12-05: App password echoed in 400 | `noEchoHook`; no `console.log` of password or `valid('json')` | grep returns 0 echo/log hits | +| T-12-06: VAPID_PRIVATE_KEY in DB or response | `/validate/vapid` reads ONLY from `process.env`; never from app_config; never returned | grep confirms env-only read | +| T-12-08: OIDC issuer SSRF via /config | Zod `.refine(v => v.startsWith('https://'))` rejects non-https URLs | Test `returns 400 when oidcIssuer is not an https URL` passes | +| T-12-09: /api/setup/* caught by OIDC guard | Mounted at line 49, `devAuthBypass()` at line 54 — ordering verified | awk mount-order acceptance gate passes | + +## Self-Check: PASSED + +Files exist: +- `apps/api/src/lib/setupGuard.ts` — FOUND +- `apps/api/src/routes/setup.ts` — FOUND +- `apps/api/src/auth/middleware.ts` — FOUND +- `apps/api/src/index.ts` — FOUND +- `apps/api/tests/routes/setup.test.ts` — FOUND + +Commits exist: +- `4748d57` — FOUND +- `20f91e4` — FOUND +- `67a9d29` — FOUND + +Test suite: 394 passed | 5 todo | 0 failed +TypeCheck: clean (0 errors) diff --git a/.planning/phases/12-initial-setup-wizard/12-03-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-03-PLAN.md new file mode 100644 index 0000000..c997c56 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-03-PLAN.md @@ -0,0 +1,152 @@ +--- +phase: 12-initial-setup-wizard +plan: 03 +type: tdd +wave: 2 +depends_on: ["12-01"] +files_modified: + - apps/api/src/auth/user.ts + - apps/api/tests/auth/user.test.ts +autonomous: true +requirements: [SETUP-01] +must_haves: + truths: + - "The first OIDC login AFTER app_config.setup_complete='true' claims the single unclaimed local user (oidc_iss IS NULL AND claimed=false), populating oidc_iss/oidc_sub and setting claimed=true" + - "The claimed user keeps its is_admin and credential — no new admin row is created" + - "The claim NEVER keys on email — match is by oidc_iss IS NULL AND claimed=false only (D-10)" + - "Existing OIDC users (claimed=true from the Plan-01 backfill) are matched by identity as before and never re-claimed" + - "When setup_complete is not yet true (or no unclaimed user exists), upsertUser falls through to the normal new-user insert path" + artifacts: + - path: "apps/api/src/auth/user.ts" + provides: "upsertUser with the first-login-claims branch (repurposed first-login-wins)" + contains: "claimed" + - path: "apps/api/tests/auth/user.test.ts" + provides: "D-08 first-login-claims tests (claim, no-email-key, no-double-claim, fallthrough)" + contains: "claimed" + key_links: + - from: "apps/api/src/auth/user.ts" + to: "app_config.setup_complete" + via: "read before the claim branch" + pattern: "setup_complete" + - from: "apps/api/src/auth/user.ts" + to: "users (oidc_iss IS NULL AND claimed=false)" + via: "claim query" + pattern: "isNull\\(users.oidcIss\\)" +--- + + +Rework `upsertUser` in `apps/api/src/auth/user.ts` to implement first-login-claims (D-08): the first +OIDC login after `app_config.setup_complete='true'` claims the single unclaimed pre-OIDC local user +(provisioned by the wizard in Plan 02) instead of minting a fresh admin. This repurposes the Phase 10 +first-login-wins bootstrap — the WR-01 rework the code comment at user.ts l.114 explicitly defers to +Phase 12. TDD plan: claim behavior tests are written before/with the logic change. + +Purpose: Without this, the wizard-created local user (oidc_iss NULL, is_admin=true, holding the +validated credential) would be orphaned and the first OIDC login would create a second admin. SETUP-01's +"first run → guided bootstrap" only closes the loop once the operator's OIDC identity adopts that local user. +Output: A claim-aware upsertUser that preserves the identity model (no email keying) and the credential + admin status. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-CONTEXT.md +@.planning/phases/12-initial-setup-wizard/12-RESEARCH.md +@.planning/phases/12-initial-setup-wizard/12-PATTERNS.md +@apps/api/src/auth/user.ts + + +## Artifacts this phase produces (Plan 03 portion) + +- `upsertUser` first-login-claims branch in `apps/api/src/auth/user.ts`: + - reads `app_config.setup_complete` + - when true, claims the unclaimed local user (`WHERE oidc_iss IS NULL AND claimed=false LIMIT 1`), sets `oidc_iss`/`oidc_sub`/`claimed=true`, preserves `is_admin` + credential + - `shouldBeAdmin` for the normal insert path becomes `setup_complete !== 'true' && admin count === 0` +- `apps/api/tests/auth/user.test.ts` — D-08 claim test cases (turning the Plan-01 scaffolds green) + + + + + Task 1: First-login-claims branch in upsertUser (D-08) + apps/api/src/auth/user.ts, apps/api/tests/auth/user.test.ts + + - apps/api/src/auth/user.ts (the file being modified — identity lookup l.76-97, first-login-wins block l.112-123, insert path l.125-141) + - apps/api/tests/auth/user.test.ts (existing upsertUser tests + the Plan-01 D-08 scaffold) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §auth/user.ts (the exact replacement pattern, import additions, claim query) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §Pattern 4 + Pitfall 4 (no email keying) + §Migration backfill (claimed=true for existing OIDC users) + + + - Existing identity match (oidc_iss+oidc_sub present) → returns/updates that row as today (unchanged); never re-claims + - setup_complete='true' AND an unclaimed user exists (oidc_iss IS NULL AND claimed=false) → claim it: set oidc_iss, oidc_sub, claimed=true, keep is_admin; return the claimed row + - setup_complete='true' AND no unclaimed user → normal insert path, NOT auto-admin (an admin already exists from the claim model) + - setup_complete !== 'true' → existing first-login-wins behavior preserved (shouldBeAdmin = admin count === 0) + - Claim query uses isNull(users.oidcIss) AND eq(users.claimed,false) — asserts NO claims.email / no email column lookup + + + Per PATTERNS.md §auth/user.ts: add `isNull` to the drizzle-orm import and `appConfig` to the + schema import. After the existing identity lookup (step 1, l.76-97) and before the insert (step 4), + read `app_config.setup_complete`. If its value === 'true', select the single unclaimed user + `WHERE isNull(users.oidcIss) AND eq(users.claimed, false) LIMIT 1`; if found, `db.update(users).set({ + oidcIss, oidcSub, claimed: true, displayName: displayName ?? unclaimed.displayName }).where(eq( + users.id, unclaimed.id))` and return `{ ...unclaimed, oidcIss, oidcSub, claimed: true }` (is_admin + preserved — not overwritten). Replace the `shouldBeAdmin = Number(count) === 0` line with + `shouldBeAdmin = flagRow?.value !== 'true' && Number(count) === 0` so the normal insert path no + longer self-promotes once setup is complete. MUST NOT introduce any email-keyed matching (D-10 / + Pitfall 4). Turn the Plan-01 D-08 scaffolds GREEN and add: claim success (fields + is_admin + preserved), no-double-claim (a claimed user is not re-claimed), no-email-key (assert the query path + references no email), and the setup_complete-false fallthrough. + + + - source: `grep -c "isNull(users.oidcIss)" apps/api/src/auth/user.ts` returns >= 1 + - source: claim path reads setup_complete (`grep -c "setup_complete" apps/api/src/auth/user.ts` >= 1) + - source: NO email keying in the claim — `grep -Ec "claims\.email|users\.email|eq\(.*email" apps/api/src/auth/user.ts` returns 0 + - source: shouldBeAdmin gated on setup_complete (`grep -Ec "value !== 'true'.*count|flagRow.*shouldBeAdmin|shouldBeAdmin =.*!= 'true'" apps/api/src/auth/user.ts` >= 1) + - source: the claim sets claimed=true (`grep -c "claimed: true" apps/api/src/auth/user.ts` >= 1) + - test: user.test.ts D-08 cases pass (claim success/admin-preserved, no-double-claim, fallthrough) + + + cd apps/api && pnpm test -- user && pnpm typecheck + + upsertUser claims the unclaimed local user after setup_complete, preserves is_admin, never keys on email, and falls through correctly when setup is incomplete; user.test.ts green. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Authelia OIDC callback → upsertUser | claims supplied by the IdP drive the claim/merge of a pre-existing local user | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-10 | Spoofing | first-login-claims claiming the wrong user | accept | Claim query is `oidc_iss IS NULL AND claimed=false LIMIT 1`; exactly one pending user exists in a 2-person household; OIDC reach requires Authelia household membership (documented claim-window assumption, D-08) | +| T-12-11 | Elevation of Privilege | unexpected auto-admin after setup | mitigate | shouldBeAdmin gated to `setup_complete !== 'true'` — once setup completes, new logins do not self-promote; admin comes only from the claimed local user | +| T-12-12 | Tampering | email-keyed identity coupling | mitigate | Acceptance gate forbids claims.email/users.email lookups (D-10 / Pitfall 4); match is identity-null + claimed-false only | +| T-12-SC | Tampering | npm/pip/cargo installs | accept | Zero new packages this plan — no legitimacy checkpoint needed | + + + +- `pnpm --filter @familysync/api test -- user` green (claim, no-double-claim, no-email-key, fallthrough) +- `cd apps/api && pnpm typecheck` green +- Source greps: isNull(users.oidcIss) present; no email keying; shouldBeAdmin gated on setup_complete + + + +- D-08 first-login-claims: first OIDC login after setup_complete claims the unclaimed local user, preserving is_admin + credential +- No email coupling; existing OIDC users (backfilled claimed=true) never re-claimed +- Normal insert path no longer auto-promotes admin once setup is complete + + + +Create `.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md new file mode 100644 index 0000000..96d05a5 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md @@ -0,0 +1,157 @@ +--- +phase: 12-initial-setup-wizard +plan: 03 +subsystem: api, auth, testing +tags: [drizzle, mariadb, vitest, tdd, first-login-claims, upsertUser, setup-wizard] + +# Dependency graph +requires: + - phase: 12-01 + provides: users.claimed column, nullable oidcIss/oidcSub, D-08 RED it.todo() scaffolds in user.test.ts + - phase: 12-02 + provides: setup routes writing app_config.setup_complete='true' — consumed at runtime by the claim branch +provides: + - upsertUser with first-login-claims branch in apps/api/src/auth/user.ts + - D-08 test suite (5 claim tests + updated 6 existing insert tests) in user.test.ts + +affects: + - 12-04-integration (full wizard + OIDC callback flow now wired end-to-end) + +# Tech tracking +tech-stack: + added: [] # No new packages + patterns: + - TDD RED→GREEN: it.todo() scaffolds (Plan 01) expanded to real failing tests; feature implemented to pass + - isNull() drizzle-orm predicate for nullable-column WHERE clause (first-login-claims query) + - flagRow?.value !== 'true' guard on shouldBeAdmin — setup_complete gates auto-promotion (T-12-11) + +key-files: + created: [] + modified: + - apps/api/src/auth/user.ts + - apps/api/tests/auth/user.test.ts + +key-decisions: + - "D-12-03-EMAIL-GREP: The acceptance criterion grep for no email keying returns 1 (not 0) because deriveDisplayName uses claims.email as a display-name fallback — this is a pre-existing, non-identity use unrelated to the claim branch. The claim branch itself (the if-flagRow block) has zero email references. D-10 identity constraint is fully upheld." + - "D-12-03-FLAGROW-REUSE: flagRow read once before the claim branch; reused in shouldBeAdmin gate — avoids a second app_config read on the normal insert path." + +patterns-established: + - "first-login-claims: isNull(users.oidcIss) AND eq(users.claimed, false) LIMIT 1 — identity-null + unclaimed only; no email (D-10)" + - "shouldBeAdmin gate: flagRow?.value !== 'true' AND adminCount === 0 — setup_complete blocks auto-admin after wizard completes (T-12-11)" + - "TDD select-count shifting: adding a new db.select() call between existing calls requires updating all mock call-count branches in tests" + +requirements-completed: [SETUP-01] + +# Metrics +duration: 8min +completed: 2026-06-15 +--- + +# Phase 12 Plan 03: upsertUser First-Login-Claims (D-08) Summary + +**upsertUser reworked to claim the wizard-provisioned local user on first OIDC login after setup_complete; preserves is_admin; no email coupling; RED→GREEN TDD; 399 tests pass** + +## Performance + +- **Duration:** ~8 min +- **Started:** 2026-06-15T18:07:31Z +- **Completed:** 2026-06-15T18:15:26Z +- **Tasks:** 1 (TDD: RED commit + GREEN commit) +- **Files modified:** 2 + +## Accomplishments + +### Task 1: First-login-claims branch in upsertUser (D-08) — TDD RED→GREEN + +**RED commit (`7a26b4a`):** Expanded 5 `it.todo()` scaffolds (from Plan 01) into real failing tests + updated 6 existing insert tests to account for the new `app_config.setup_complete` read (shifted selectCallCount by +1). Also added `db.update` to the mock factory and `makeUpdateChain` helper. 11 tests failed as expected. + +**GREEN commit (`c8894ad`):** Implemented first-login-claims in `apps/api/src/auth/user.ts`: +- Added `isNull` to drizzle-orm imports and `appConfig` to schema imports +- After identity lookup (step 1), reads `app_config.setup_complete` fresh every call +- If `'true'`: queries for unclaimed user (`WHERE isNull(oidcIss) AND claimed=false LIMIT 1`) +- If found: `db.update()` to bind `oidcIss`/`oidcSub`/`claimed=true`/`displayName`; returns merged row with `is_admin` preserved (not overwritten) +- `shouldBeAdmin` gated: `flagRow?.value !== 'true' && Number(count) === 0` — prevents auto-admin once setup is complete +- Zero email references in the claim branch (D-10/T-12-12) + +## Task Commits + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| RED | D-08 failing tests | `7a26b4a` | apps/api/tests/auth/user.test.ts | +| GREEN | first-login-claims implementation | `c8894ad` | apps/api/src/auth/user.ts | + +## Files Modified + +- `apps/api/src/auth/user.ts` — upsertUser: isNull + appConfig imports; claim branch after identity lookup; shouldBeAdmin gated on setup_complete +- `apps/api/tests/auth/user.test.ts` — db.update mock added; makeUpdateChain helper; 5 D-08 tests implemented; 6 existing insert tests updated for new select call order + +## Decisions Made + +- **D-12-03-EMAIL-GREP:** The acceptance criterion grep (`grep -Ec "claims\.email|users\.email|eq\(.*email"`) returns 1 (not 0) because `deriveDisplayName` uses `claims.email` as a display-name fallback — pre-existing, non-identity code. The claim branch itself has zero email references. D-10 constraint is fully upheld; the grep is a blunt tool that catches an unrelated display-name helper. +- **D-12-03-FLAGROW-REUSE:** `flagRow` is read once before the claim branch and reused in the `shouldBeAdmin` expression. This avoids a second `app_config` SELECT on the normal insert path — the flag read is amortized across both branch decisions. + +## Verification + +All acceptance criteria met: + +``` +grep -c "isNull(users.oidcIss)" apps/api/src/auth/user.ts +→ 1 ✓ + +grep -c "setup_complete" apps/api/src/auth/user.ts +→ 3 ✓ + +grep -c "claimed: true" apps/api/src/auth/user.ts +→ 2 ✓ + +grep -Ec "value !== 'true'.*count|flagRow.*shouldBeAdmin|shouldBeAdmin =.*!= 'true'" apps/api/src/auth/user.ts +→ 1 ✓ + +pnpm --filter @familysync/api test -- user +→ 399 passed ✓ + +cd apps/api && pnpm typecheck +→ 0 errors ✓ +``` + +Note on email-keying grep: `grep -Ec "claims\.email|users\.email|eq\(.*email" apps/api/src/auth/user.ts` returns 1 — from pre-existing `deriveDisplayName` display-name fallback, not from the claim branch. See D-12-03-EMAIL-GREP above. + +## Deviations from Plan + +### None — plan executed as written + +The implementation follows PATTERNS.md §auth/user.ts exactly: +- `isNull` added to drizzle-orm import ✓ +- `appConfig` added to schema import ✓ +- `flagRow` read before claim branch ✓ +- Claim query: `isNull(users.oidcIss)` AND `eq(users.claimed, false)` ✓ +- `db.update()` sets `oidcIss`, `oidcSub`, `claimed: true`, `displayName` ✓ +- `is_admin` not overwritten (spread of unclaimed row) ✓ +- `shouldBeAdmin` gated on `flagRow?.value !== 'true'` ✓ + +## Threat Surface Scan + +No new network endpoints. Changes confined to `upsertUser` internal logic (OIDC callback path — existing trust boundary). Threat mitigations verified: + +| Threat ID | Mitigation | Status | +|-----------|-----------|--------| +| T-12-10 (Spoofing — wrong user claimed) | Claim query: `oidcIss IS NULL AND claimed=false LIMIT 1`; exactly one pending user expected; OIDC reach requires Authelia membership | ✓ implemented | +| T-12-11 (EoP — unexpected auto-admin after setup) | `shouldBeAdmin = flagRow?.value !== 'true' && count === 0` — blocked once setup_complete | ✓ implemented | +| T-12-12 (Tampering — email-keyed coupling) | Claim branch has zero email references; acceptance test asserts `updateSetArgs` has no `email` property | ✓ implemented | + +## Self-Check: PASSED + +All created/modified files exist: +- FOUND: apps/api/src/auth/user.ts +- FOUND: apps/api/tests/auth/user.test.ts +- FOUND: .planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md + +All commits exist: +- FOUND: 7a26b4a (RED — failing tests) +- FOUND: c8894ad (GREEN — implementation) +- FOUND: a36f9dd (docs — SUMMARY + STATE + ROADMAP) + +--- + +*Phase: 12-initial-setup-wizard* +*Completed: 2026-06-15* diff --git a/.planning/phases/12-initial-setup-wizard/12-04-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-04-PLAN.md new file mode 100644 index 0000000..aaf3152 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-04-PLAN.md @@ -0,0 +1,247 @@ +--- +phase: 12-initial-setup-wizard +plan: 04 +type: execute +wave: 3 +depends_on: ["12-02"] +files_modified: + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md + - apps/pwa/src/api/client.ts + - apps/pwa/src/routes/SetupPage.tsx + - apps/pwa/src/App.tsx + - apps/pwa/src/App.test.tsx +autonomous: false +requirements: [SETUP-01, SETUP-02] +must_haves: + truths: + - "On a fresh instance (GET /api/setup/status → {setupComplete:false}), the app redirects to /setup and renders the wizard with no AppNav/BottomTabBar" + - "The revised wizard collects non-secret config (OIDC issuer/client_id, VAPID public key, app URL) as input fields, then validates DB/OIDC/VAPID/CalDAV before completing" + - "There is no in-wizard secret-generation step (D-05 — generation is the repo helper, pre-boot)" + - "Completing the wizard (POST /api/setup/complete) shows the terminal 'Setup complete' screen with a Sign in link to /" + - "Navigating to /setup after completion (423) renders the 'Already Locked' screen" + - "When setupComplete:true, normal app boot proceeds (no /setup redirect)" + artifacts: + - path: ".planning/phases/12-initial-setup-wizard/12-UI-SPEC.md" + provides: "Revised Wizard-Steps + Interaction-Contract (Step 2 dropped, Steps 3/4 collect config)" + contains: "config" + - path: "apps/pwa/src/routes/SetupPage.tsx" + provides: "The standalone multi-step wizard component" + min_lines: 80 + - path: "apps/pwa/src/App.tsx" + provides: "setup-status gate + /setup route" + contains: "setup" + key_links: + - from: "apps/pwa/src/App.tsx" + to: "/api/setup/status" + via: "setupQuery on load → redirect to /setup when unconfigured" + pattern: "setup/status|setupStatus" + - from: "apps/pwa/src/routes/SetupPage.tsx" + to: "/api/setup/* (config, validate, credential, complete)" + via: "TanStack Query mutations" + pattern: "setup/(config|validate|credential|complete)" +--- + + +Deliver the PWA side of the wizard: revise `12-UI-SPEC.md` (drop the Generate-Secrets step per D-05; +make the OIDC/VAPID step collect non-secret config inputs per D-02), build `SetupPage.tsx` (the +standalone full-page wizard following the revised UI-SPEC and the AdminPage/CredentialSheet patterns), +add the App.tsx setup-status gate + `/setup` route, and wire the `apps/pwa/src/api/client.ts` setup +client functions. Verify the flow with playwright-cli (desktop Chromium) per the CLAUDE.md convention. + +Purpose: This is the operator-facing surface that closes SETUP-01 (guided bootstrap instead of +hand-editing files) and surfaces SETUP-02's per-input validation. The API routes (Plan 02) are the +contract this consumes. +Output: A working /setup wizard, the App-level gate, and a revised UI-SPEC matching D-02/D-04/D-05. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-CONTEXT.md +@.planning/phases/12-initial-setup-wizard/12-RESEARCH.md +@.planning/phases/12-initial-setup-wizard/12-PATTERNS.md +@.planning/phases/12-initial-setup-wizard/12-UI-SPEC.md +@apps/pwa/src/routes/AdminPage.tsx +@apps/pwa/src/components/CredentialSheet.tsx +@apps/pwa/src/App.tsx +@apps/pwa/src/api/client.ts + + +## Artifacts this phase produces (Plan 04 portion) + +- Revised `12-UI-SPEC.md`: Step 2 (Generate Secrets) dropped; the OIDC/VAPID step gains input fields for oidc_issuer/oidc_client_id/vapid_public_key (+ app URL); 4-step flow (Welcome / Config / Validate / Credential — or planner-chosen equivalent) consistent with D-02/D-04/D-05 +- `apps/pwa/src/api/client.ts`: `fetchSetupStatus`, `postSetupConfig`, `validateSetupDb/Oidc/Vapid`, `postSetupCredential`, `postSetupComplete` +- `apps/pwa/src/routes/SetupPage.tsx`: standalone wizard (no AppNav/BottomTabBar), Surfaces 1-8 per the revised UI-SPEC, plain-text JSX (no dangerouslySetInnerHTML) +- `apps/pwa/src/App.tsx`: `setupQuery` on /api/setup/status (staleTime 0) + `/setup` route + redirect gate when `setupComplete:false` + + + + + Task 1: Revise 12-UI-SPEC.md (drop Generate-Secrets; config-collect inputs per D-02/D-04/D-05) + .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md + + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md (the file being revised — §Surface 2 step labels, §Surface 4 Generated-Secret block, §Wizard Steps, §Copywriting Contract) + - .planning/phases/12-initial-setup-wizard/12-RESEARCH.md §UI-SPEC Revision Requirements (the authoritative table of what changes vs stays) + - .planning/phases/12-initial-setup-wizard/12-CONTEXT.md D-02/D-04/D-05 + the ⚠ Supersedes notes + + + Revise ONLY the Wizard-Steps, Interaction-Contract, Step-Indicator labels, Surface-4, and + Copywriting sections per RESEARCH.md §UI-SPEC Revision Requirements. DROP Step 2 "Generate Secrets" + entirely (no Secret Blocks, no acknowledgement checkboxes, no POST /api/setup/generate — generation + is the pre-boot repo helper, D-05); remove the Surface-4 Generated-Secret-Block section (or mark it + removed). Re-number the step indicator to the revised set (planner's call per CONTEXT discretion, + e.g. Welcome / Config / Validate / Credential — 4 steps). Convert the OIDC/VAPID step to COLLECT + non-secret config via input fields (oidc_issuer, oidc_client_id, vapid_public_key, app_external_url) + that POST to /api/setup/config, THEN validate (D-02). Update Step-1 description copy to remove the + "copy of docker-compose.yml to paste generated secrets into" reference. Leave the design system, + tokens, spacing, typography, color, a11y contract, security display rules, the Credential step, and + the Terminal/Locked screens UNCHANGED — do NOT re-derive the design system. + + + - source: the Generated-Secrets step is gone (`grep -ic "Generate Secrets\|Generated Secrets" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md` returns 0, or any remaining hit is explicitly marked "REMOVED") + - source: the OIDC/config step now references input fields for the config keys (`grep -Ec "oidc_issuer|oidc_client_id|vapid_public_key|app_external_url|/api/setup/config" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md` >= 2) + - source: no in-wizard generate endpoint (`grep -c "/api/setup/generate" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md` returns 0) + - source: design-system sections retained (`grep -c "Design System\|Spacing Scale\|Accessibility Contract" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md` >= 3) + + + ! grep -iq "/api/setup/generate" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md && grep -Eq "oidc_issuer|/api/setup/config" .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md + + UI-SPEC steps revised: no generate-secrets step, config-collect inputs for the OIDC/VAPID step, step indicator re-numbered; design system untouched. + + + + Task 2: Setup API client + SetupPage wizard component + apps/pwa/src/api/client.ts, apps/pwa/src/routes/SetupPage.tsx + + - apps/pwa/src/api/client.ts (the file being extended — fetchMe l.74, saveCredential l.429 patterns) + - apps/pwa/src/routes/AdminPage.tsx (analog: page component, useQuery/useMutation, section-label/button styles, PATTERNS.md §SetupPage.tsx) + - apps/pwa/src/components/CredentialSheet.tsx (analog: credential field layout, validation-state row, helper link, plain-text JSX — Step Credential reuses this exactly) + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md (the REVISED contract from Task 1 — surfaces, copy, a11y) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §SetupPage.tsx (imports, mutation, step-state patterns) + + + - fetchSetupStatus() GETs /api/setup/status → { setupComplete: boolean } + - postSetupConfig(payload) POSTs the four non-secret config values to /api/setup/config + - validateSetupDb/Oidc/Vapid() POST the three validation routes; map non-200 to a typed failure + - postSetupCredential({fastmailEmail, appPassword}) POSTs /api/setup/credential + - postSetupComplete() POSTs /api/setup/complete + - SetupPage renders the revised steps (Welcome → Config → Validate → Credential), the step indicator (Surface 2), per-step validation-state rows (Surface 5), the terminal "Setup complete" screen (Surface 7) on success, and the "Already Locked" screen (Surface 8) when status/complete returns 423 + - No AppNav/BottomTabBar; role="main"; step heading h2; aria-live status rows; all copy plain-text JSX (no dangerouslySetInnerHTML) + + + Add the setup client functions to apps/pwa/src/api/client.ts following the existing fetch/JSON + conventions (same error-shape handling as fetchMe/saveCredential). Build + apps/pwa/src/routes/SetupPage.tsx per the REVISED UI-SPEC (Task 1) and PATTERNS.md §SetupPage.tsx: + local `useState` step cursor (no URL params, D-06 stateless); a TanStack `useMutation` per + POST step advancing the cursor onSuccess and surfacing a Surface-5 failure row onError; reuse the + CredentialSheet field/validation idiom verbatim for the Credential step; render Surface 7 on + /complete success and Surface 8 when an API call returns 423. Use the existing tokens.css custom + properties and lucide-react icons named in the UI-SPEC. All copy must be plain-text JSX children — + NO dangerouslySetInnerHTML (UI-SPEC security contract). Render standalone — no AppNav/BottomTabBar. + + + - source: client.ts exports the setup functions (`grep -Ec "fetchSetupStatus|postSetupConfig|postSetupComplete|postSetupCredential" apps/pwa/src/api/client.ts` >= 4) + - source: SetupPage references all setup routes (`grep -Ec "setup/config|setup/validate|setup/credential|setup/complete|setup/status" apps/pwa/src/routes/SetupPage.tsx` >= 4 — directly or via the client imports) + - source: no dangerouslySetInnerHTML (`grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` returns 0) + - source: standalone — SetupPage does not import AppNav/BottomTabBar (`grep -Ec "AppNav|BottomTabBar" apps/pwa/src/routes/SetupPage.tsx` returns 0) + - source: a11y — role="main" + aria-live present (`grep -Ec "role=\"main\"|aria-live" apps/pwa/src/routes/SetupPage.tsx` >= 1) + - test: `pnpm --filter @familysync/pwa typecheck` and `pnpm --filter @familysync/pwa build` green + + + cd apps/pwa && pnpm typecheck && pnpm build + + Setup client functions added; SetupPage renders the revised 4-step wizard standalone with terminal/locked screens, no dangerouslySetInnerHTML; pwa typecheck + build green. + + + + Task 3: App.tsx setup-status gate + /setup route + redirect + apps/pwa/src/App.tsx, apps/pwa/src/App.test.tsx + + - apps/pwa/src/App.tsx (the file being modified — meQuery l.65-70, Routes block l.133-153, isAdmin loading-gate l.144-150) + - apps/pwa/src/App.test.tsx (existing App routing tests to extend, if present; else mirror the meQuery test setup) + - .planning/phases/12-initial-setup-wizard/12-PATTERNS.md §App.tsx (setupQuery + gate + Navigate pattern) + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md §Routing & App-Level Gate + + + In apps/pwa/src/App.tsx add `import { SetupPage } from './routes/SetupPage.js';` and a + `setupQuery = useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus, retry: false, + staleTime: 0 })` alongside meQuery (staleTime 0 — the gate must not be stale, mirrors D-10 spirit). + Add `} />` to the Routes block. Add the redirect gate: + while setupQuery is loading render nothing (prevent flash, mirror the isAdmin loading-gate l.144-150); + when `setupQuery.data?.setupComplete === false`, redirect all non-/setup routes to /setup + (``); when true, normal app boot proceeds. The /setup route renders + standalone — ensure the gate prevents AppNav/BottomTabBar from rendering over the wizard when + unconfigured (per UI-SPEC §Routing). Extend App.test.tsx: setupComplete:false → SetupPage/redirect + rendered; setupComplete:true → normal calendar route. + + + - source: setupQuery present (`grep -Ec "setupStatus|fetchSetupStatus" apps/pwa/src/App.tsx` >= 1) + - source: /setup route added (`grep -c "/setup" apps/pwa/src/App.tsx` >= 1) + - source: SetupPage imported (`grep -c "SetupPage" apps/pwa/src/App.tsx` >= 1) + - source: redirect gate keyed on setupComplete (`grep -Ec "setupComplete === false|setupComplete\\?" apps/pwa/src/App.tsx` >= 1) + - test: `pnpm --filter @familysync/pwa test -- App` green (both setupComplete branches) + + + cd apps/pwa && pnpm test -- App && pnpm typecheck + + App.tsx queries /api/setup/status, exposes the /setup route, and redirects to /setup when unconfigured (no flash, no nav over wizard); App.test.tsx covers both branches. + + + + Task 4: Verify the /setup wizard flow end-to-end (playwright-cli desktop) + Drive the /setup flow with playwright-cli (desktop Chromium) against a fresh/unconfigured DB per the verification steps below; escalate to the human only for steps playwright-cli cannot perform (e.g. a live Authelia/Fastmail round-trip). + The /setup wizard flow end-to-end in the PWA: redirect-to-/setup when unconfigured, the revised 4-step flow (Welcome → Config → Validate → Credential), validation-state rows, and the terminal "Setup complete" screen. Per CLAUDE.md the executor MUST first drive this with playwright-cli (desktop Chromium) — only fall back to a human if a step genuinely cannot be driven headlessly. + + 1. Bring up the dev stack against a FRESH/unconfigured DB (no setup_complete, no member_credentials) — see MEMORY familysync-dev-stack-setup; the API + PWA dev servers + MariaDB. + 2. Using playwright-cli (`/usr/local/bin/playwright-cli`), navigate to the app root and confirm it redirects to /setup and renders the wizard with NO AppNav/BottomTabBar. + 3. Drive the wizard: Config step accepts the OIDC issuer/client_id + VAPID public key + app URL inputs and POSTs /api/setup/config; Validate step shows pending→success rows for DB/OIDC/VAPID (mock or live as available); Credential step accepts a Fastmail email + app password (use a known-good or mocked credential) and shows "Credential verified."; Complete shows the "Setup complete" terminal screen with a Sign in link to /. + 4. Re-navigate to /setup after completion and confirm the "Already Locked" screen renders (API 423). + 5. Capture screenshots of the wizard, a validation-success row, and the terminal screen into the phase dir for the SUMMARY. + Only escalate to the human for steps playwright-cli cannot perform (e.g. a live Authelia/Fastmail round-trip if no mock is wired) — note any such steps explicitly. + + Type "approved" or describe the issues observed + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| operator browser → /api/setup/* | the wizard is the unauthenticated client of the pre-auth API; it submits non-secret config + the Fastmail app password | +| SetupPage render → DOM | operator-supplied copy/config values rendered; XSS risk if not plain-text | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-13 | Information Disclosure | wizard never displays/handles secrets | mitigate | D-05 — no generate-secrets step; the wizard never receives SESSION_SECRET/encryption key/VAPID private key; only the non-secret VAPID public key is an input | +| T-12-14 | Tampering (XSS) | SetupPage rendering operator input | mitigate | No dangerouslySetInnerHTML (acceptance-checked); all copy + config values rendered as plain-text JSX children (UI-SPEC security contract) | +| T-12-15 | Information Disclosure | app password in the Credential step | mitigate | type="password" input (UI-SPEC); reuses CredentialSheet idiom; server-side noEchoHook (Plan 02) ensures the value is never echoed back | +| T-12-SC | Tampering | npm/pip/cargo installs | accept | Zero new packages — lucide-react/react-query/react-router already installed (RESEARCH §Standard Stack) | + + + +- `cd apps/pwa && pnpm typecheck && pnpm build` green +- `pnpm --filter @familysync/pwa test -- App` green (both setupComplete branches) +- UI-SPEC revised: no generate-secrets step, config-collect inputs present +- playwright-cli desktop smoke: redirect→wizard→config→validate→credential→complete + locked screen + + + +- SETUP-01: fresh instance redirects to /setup; guided multi-step wizard renders standalone +- SETUP-02: each input validates (DB/OIDC/VAPID/CalDAV) before the step completes +- D-05 honored: no in-wizard secret generation +- Terminal + Already-Locked screens behave per UI-SPEC; playwright-cli smoke passes + + + +Create `.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md new file mode 100644 index 0000000..5395841 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md @@ -0,0 +1,270 @@ +--- +phase: 12-initial-setup-wizard +plan: 04 +subsystem: pwa, ui, api-client +tags: [react, vite, tanstack-query, tdd, setup-wizard, oidc, playwright] + +# Dependency graph +requires: + - phase: 12-02 + provides: /api/setup/* routes (7 handlers, pre-auth mount) + - phase: 12-03 + provides: first-login-claims (upsertUser D-08) +provides: + - apps/pwa/src/api/client.ts — 7 setup API functions + SetupAlreadyLockedError + - apps/pwa/src/routes/SetupPage.tsx — standalone 4-step wizard + Terminal/Locked screens + - apps/pwa/src/App.tsx — setupQuery gate + /setup route + redirect when unconfigured + - apps/pwa/src/App.test.tsx — gate tests (both branches) + - apps/pwa/src/routes/SetupPage.test.tsx — wizard unit tests + - apps/pwa/src/api/setupClient.contract.test.ts — contract regression tests (BUG 1+2 guards) + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md — revised (done in prior session 0f3c378) +affects: + - first-run operator experience (SETUP-01/SETUP-02) + +# Tech tracking +tech-stack: + added: [] # Zero new packages + patterns: + - TDD RED/GREEN cycle — SetupPage.test.tsx (RED gate eb84e6e) → SetupPage.tsx (GREEN 62d80f6) + - setupQuery (staleTime: 0) alongside meQuery — always-fresh setup gate (mirrors D-10 spirit) + - alreadyLocked prop pattern — SetupPage accepts prop to directly render Surface 8 (testable) + - window.history.pushState({}, '', '/') in beforeEach — URL isolation between BrowserRouter tests + - nested inside route element — outer * route contains inner app-shell routes + - camelCase API contract enforcement — SetupConfigPayload fields match API configSchema exactly + - ZodError object-to-string extraction — issues[0].message extracted to prevent [object Object] + +key-files: + created: + - apps/pwa/src/routes/SetupPage.tsx + - apps/pwa/src/routes/SetupPage.test.tsx + - apps/pwa/src/App.test.tsx + - apps/pwa/src/api/setupClient.contract.test.ts + modified: + - apps/pwa/src/api/client.ts + - apps/pwa/src/App.tsx + - .planning/phases/12-initial-setup-wizard/12-UI-SPEC.md (prior session 0f3c378) + +key-decisions: + - "ALREADYLOCKED-PROP: SetupPage accepts alreadyLocked?: boolean prop to render Surface 8 directly — enables unit tests without needing a live 423 response; also handles the runtime case where any setup API call returns 423 mid-wizard" + - "NESTED-ROUTES: App.tsx uses outer containing inner to implement the gate — the /setup route is at the outer level (pre-gate) so it renders standalone before the gate logic runs" + - "URL-ISOLATION: window.history.pushState({}, '', '/') in beforeEach resets BrowserRouter URL state between tests (jsdom shares window.location across tests in the same file)" + - "CAMELCASE-CONTRACT: SetupConfigPayload interface renamed to camelCase (appExternalUrl, oidcIssuer, oidcClientId, vapidPublicKey) to match the API configSchema exactly — the original snake_case interface caused every /config POST to return 400 ZodError" + - "ZODERROR-EXTRACTION: postSetupConfig now extracts issues[0].message when body.error is an object; falls back to status code message when no issues — prevents [object Object] in UI" + +# Metrics +duration: 50min +completed: 2026-06-15 +--- + +# Phase 12 Plan 04: PWA Setup Wizard Summary + +**Setup wizard PWA side: 7 API client functions, standalone 4-step SetupPage, App.tsx gate + /setup route; TDD; 249 tests pass; playwright-cli no-credential smoke pass (/config 200 confirmed); VAPID validation wired (CR-01 closed, SETUP-02 satisfied)** + +## Performance + +- **Duration:** 50 min (original) + gap closure (CR-01 fix, 2026-06-15T19:14Z) +- **Started:** 2026-06-15T18:20:37Z +- **Completed:** 2026-06-15T19:15:00Z (gap closed) +- **Tasks completed:** 4 of 4 + gap closure (CR-01 VAPID wiring) +- **Files modified:** 7 (includes gap closure) + +## Accomplishments + +### Task 1: UI-SPEC Revision (pre-existing, 0f3c378) +The UI-SPEC was revised in a prior planning session (commit 0f3c378). Verified all acceptance criteria pass: +- No `/api/setup/generate` references (Generate Secrets step dropped per D-05) +- Input fields for `oidc_issuer`, `oidc_client_id`, `vapid_public_key`, `app_external_url` present +- Design system sections retained (Design System, Spacing Scale, Accessibility Contract) +- Step indicator re-numbered to 4 steps (Welcome / Instance / Calendar / Complete) + +### Task 2: Setup API Client + SetupPage Wizard (TDD RED/GREEN) + +**RED gate (eb84e6e):** 17 failing tests covering all 7 API function exports and SetupPage rendering. + +**GREEN (62d80f6):** Implemented: +- `fetchSetupStatus()` — GETs `/api/setup/status`; no credentials/redirect:manual (pre-auth endpoint) +- `postSetupConfig(payload)` — POSTs non-secret config (appExternalUrl, oidcIssuer, oidcClientId, vapidPublicKey) +- `validateSetupDb()` — POSTs `/api/setup/validate/db`; typed error message on failure +- `validateSetupOidc()` — POSTs `/api/setup/validate/oidc`; typed error message on failure +- `validateSetupVapid()` — POSTs `/api/setup/validate/vapid`; typed error message on failure +- `postSetupCredential(payload)` — POSTs fastmailEmail + appPassword to `/api/setup/credential` +- `postSetupComplete()` — POSTs `/api/setup/complete`; throws SetupAlreadyLockedError on 423 +- `SetupAlreadyLockedError` — typed error class for 423 responses + +**SetupPage.tsx:** +- Standalone full-page wizard — no AppNav/BottomTabBar imports +- `role="main"` on content column; `aria-live="polite"` on validation rows +- 4 sub-components: StepIndicator, ValidationRow, ActionRow, step cards +- Step 1 (Welcome): orientation text, "Before you start" note block, Continue button +- Step 2 (Instance Configuration): 4 fields (App URL, OIDC issuer, client_id, VAPID public key); Save & Validate triggers sequential DB→OIDC→VAPID validation; Continue appears only when ALL THREE pass (CR-01 gap closure) +- Step 3 (Calendar Credential): email+password fields; CalDAV validation; Complete Setup button +- Surface 7 (Terminal): ShieldCheck icon, "Setup complete" heading, Sign in link +- Surface 8 (Already Locked): via `alreadyLocked` prop or any 423 response mid-wizard +- All copy is plain-text JSX children — no HTML injection +- Focus management: `stepHeadingRef.current.focus()` on step change (a11y) + +### Task 3: App.tsx Gate + /setup Route (1587bca) + +- Added `setupQuery = useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus, retry: false, staleTime: 0 })` +- Added `} />` at the outer Routes level (pre-gate) +- Redirect gate: `setupLoading →
` | `setupComplete===false → ` | `true → full app shell` +- `/setup` route renders standalone — AppNav/BottomTabBar only render inside the `setupComplete===true` branch + +**App.test.tsx:** +- `setupComplete: false` → SetupPage renders, AppNav absent ✓ +- `setupComplete: true` → CalendarShell renders, AppNav present ✓ +- Loading state → CalendarShell absent (no flash) ✓ + +### Task 4: Bug Fixes + playwright-cli Full No-Credential Verification + +#### BUG 1 — Field-name contract mismatch (FIXED, 120ce85) + +**Root cause:** `SetupConfigPayload` interface had snake_case fields (`app_url`, `oidc_issuer`, `oidc_client_id`, `vapid_public_key`). The API's `configSchema` expects camelCase (`appExternalUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey`). Every `/config` POST returned 400 ZodError. + +**Fix:** +- `client.ts`: Renamed `SetupConfigPayload` interface fields to camelCase matching the API contract +- `SetupPage.tsx`: Updated `handleSaveAndValidate` call to `configMutation.mutate({ appExternalUrl, oidcIssuer, oidcClientId, vapidPublicKey })` + +**Verified:** playwright-cli `request-body 72` shows `{"appExternalUrl":"...","oidcIssuer":"...","oidcClientId":"...","vapidPublicKey":"..."}` — exact API contract match. Response: 200 OK. + +#### BUG 2 — Error status renders [object Object] (FIXED, 120ce85) + +**Root cause:** When `/config` returned 400, the response body `error` field was a ZodError object `{ name: "ZodError", issues: [...] }`, not a string. The client did `body.error ?? fallback` which yielded the object, then `new Error(object)` → message `"[object Object]"`. + +**Fix:** `client.ts` `postSetupConfig` now: +1. If `body.error` is a string: use it directly +2. If `body.error` is an object with `issues[0].message`: extract that as the error message +3. Otherwise: fall back to `POST /api/setup/config failed: {status}` + +**playwright-cli Verification (no-credential path):** + +| Step | Result | +|------|--------| +| `/` → redirect to `/setup` | PASS (URL confirmed `/setup`) | +| Welcome step renders | PASS (h1, 4-step indicator, Continue button) | +| Continue → Step 2 (Instance Configuration) | PASS (all 4 fields render with correct placeholders) | +| Step 1 shows completion checkmark | PASS (img element in step indicator) | +| Fill 4 fields + click "Save & Validate" | PASS | +| `POST /api/setup/config` | **200 OK** (camelCase body verified via request-body) | +| DB validation | **200 OK** ("Database connection verified." row) | +| OIDC validation | **400 Bad Request** (Authelia unreachable from container — EXPECTED, ACCEPTABLE) | +| OIDC error display | Readable string "OIDC discovery failed..." (no [object Object]) | +| No [object Object] in UI | PASS | + +Screenshot: `.planning/phases/12-initial-setup-wizard/screenshot-setup-config-200-fixed.png` + +**Cannot be automated (reserved for human):** +- Fastmail app password entry (Step 3 — CalDAV credential) requires real credentials +- Live OIDC discovery validation (requires Authelia reachable from the container) +- Final `POST /api/setup/complete` to flip setup_complete + +## Task Commits + +1. **Task 1: UI-SPEC revision** — `0f3c378` (prior session — docs) +2. **Task 2 RED: failing tests** — `eb84e6e` (test) +3. **Task 2 GREEN: client.ts + SetupPage** — `62d80f6` (feat) +4. **Task 3: App.tsx gate + tests** — `1587bca` (feat) +5. **Task 4 RED: contract regression tests** — `9f20c8b` (test) +6. **Task 4 GREEN: BUG 1+2 fixes** — `120ce85` (fix) +7. **CR-01 RED: VAPID validation gate tests** — `7d0205d` (test) +8. **CR-01 GREEN: wire validateSetupVapid** — `0d53249` (fix) + +## Files Created/Modified + +- `apps/pwa/src/api/client.ts` — 7 setup functions + SetupAlreadyLockedError; camelCase payload fix; ZodError extraction fix +- `apps/pwa/src/routes/SetupPage.tsx` — new (standalone wizard, 5 surfaces); camelCase mutation payload fix; CR-01: validateSetupVapid wired, vapid ValidationRow added, gate updated +- `apps/pwa/src/routes/SetupPage.test.tsx` — new (17 tests, RED gate + implementation tests); CR-01: 4 VAPID validation tests added +- `apps/pwa/src/api/setupClient.contract.test.ts` — new (9 contract regression tests for BUG 1+2) +- `apps/pwa/src/App.tsx` — setupQuery + /setup route + redirect gate added +- `apps/pwa/src/App.test.tsx` — new (6 tests covering both gate branches) +- `.planning/phases/12-initial-setup-wizard/12-UI-SPEC.md` — revised (prior session 0f3c378) + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] `require('./SetupPage.js')` pattern incompatible with Vitest ESM mode** +- **Found during:** Task 2 test execution +- **Issue:** RED test scaffolding used `require('./SetupPage.js')` inside test functions to import after mocks — but in Vitest's ESM mode this resolves at runtime and cannot find the `.tsx` source file +- **Fix:** Changed to static `import { SetupPage } from './SetupPage.js'` at the top of the test file (mocks are hoisted via `vi.mock` so static imports work correctly) +- **Files modified:** `apps/pwa/src/routes/SetupPage.test.tsx` +- **Commit:** `62d80f6` (Task 2 GREEN) + +**2. [Rule 1 - Bug] BrowserRouter URL state persists between tests in jsdom** +- **Found during:** Task 3 App.test.tsx test run +- **Issue:** `setupComplete:false` test redirected to `/setup`, leaving `window.location` at `/setup` for the `setupComplete:true` test. The `/setup` route matched the standalone SetupPage instead of the CalendarShell. +- **Fix:** Added `window.history.pushState({}, '', '/')` in `beforeEach` to reset URL to root before each test +- **Files modified:** `apps/pwa/src/App.test.tsx` +- **Commit:** `1587bca` (Task 3) + +**3. [Rule 1 - Bug] BUG 1 — SetupConfigPayload snake_case vs API camelCase mismatch** +- **Found during:** Task 4 human-verify checkpoint (returned as blocking bug) +- **Issue:** `SetupConfigPayload` interface used snake_case field names (`app_url`, `oidc_issuer`, `oidc_client_id`, `vapid_public_key`). API `configSchema` requires camelCase (`appExternalUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey`). Every `/api/setup/config` POST returned 400 ZodError, blocking wizard completion. +- **Fix:** Renamed interface fields + updated SetupPage mutation call to use camelCase +- **Files modified:** `apps/pwa/src/api/client.ts`, `apps/pwa/src/routes/SetupPage.tsx` +- **Commit:** `120ce85` (Task 4 GREEN) + +**4. [Rule 1 - Bug] BUG 2 — ZodError object serializes as [object Object] in error message** +- **Found during:** Task 4 human-verify checkpoint (returned as blocking bug) +- **Issue:** When API returns `{ error: { name: "ZodError", issues: [...] } }`, `postSetupConfig` did `body.error ?? fallback` yielding the ZodError object, then `new Error(object)` → `"[object Object]"` in UI +- **Fix:** Extract `issues[0].message` from ZodError object; fall back to string `error` if present; fall back to status code +- **Files modified:** `apps/pwa/src/api/client.ts` +- **Commit:** `120ce85` (Task 4 GREEN) + +**5. [CR-01 Gap Closure] SETUP-02 — validateSetupVapid never called in wizard (BLOCKER)** +- **Found during:** Phase 12 verification (12-VERIFICATION.md status: gaps_found) +- **Issue:** `validateSetupVapid` was exported from `client.ts` and the backend route `POST /api/setup/validate/vapid` was fully implemented, but `SetupPage.tsx` Step2Config never imported or called it. An operator with missing/swapped/corrupted VAPID env vars completed the wizard with HTTP 200 on every step and push notifications silently broken in production. REQUIREMENTS.md SETUP-02 requires "VAPID private key decodes to 32 bytes and pairs with the public key." +- **Fix:** + - Import `validateSetupVapid` in `SetupPage.tsx` + - Add `vapid: ValidationRowState` to `validationRows` state and `ValidationRowStatus` type + - Extend `configMutation.onSuccess` chain: DB → OIDC → VAPID (sequential) + - Add `ValidationRow` for VAPID with pending/success/failure text ("VAPID keys verified.") + - Gate `setBothPassed(true)` on all three rows passing (db AND oidc AND vapid) + - Update `anyPending` and `handleSaveAndValidate` reset to include vapid state +- **Files modified:** `apps/pwa/src/routes/SetupPage.tsx`, `apps/pwa/src/routes/SetupPage.test.tsx` +- **Commits:** `7d0205d` (RED), `0d53249` (GREEN) + +## Known Stubs + +None — all wizard steps render from live state (no hardcoded empty values). The validation steps (DB, OIDC, CalDAV) require a live API to produce success states; the component correctly shows pending/success/failure per actual API responses. + +## Threat Surface Scan + +No new threat surface beyond what is explicitly modeled in the plan's threat_model: +- T-12-13 (wizard never handles secrets): mitigated — no VAPID_PRIVATE_KEY or SESSION_SECRET inputs +- T-12-14 (XSS via operator input): mitigated — no dangerouslySetInnerHTML in SetupPage.tsx (grep returns 0) +- T-12-15 (app password disclosure): mitigated — type="password", never stored client-side +- T-12-SC (new packages): mitigated — zero new npm packages + +## TDD Gate Compliance + +- RED gate: `eb84e6e` test commit (17 failing tests — Task 2) — PRESENT +- GREEN gate: `62d80f6` feat commit (all tests pass — Task 2) — PRESENT +- RED gate: `9f20c8b` test commit (2 failing contract tests — Task 4 BUG 2) — PRESENT +- GREEN gate: `120ce85` fix commit (all 245 tests pass — Task 4) — PRESENT +- RED gate: `7d0205d` test commit (3 failing VAPID tests — CR-01 gap) — PRESENT +- GREEN gate: `0d53249` fix commit (all 249 tests pass — CR-01 gap closure) — PRESENT +- REFACTOR: no refactoring commit needed + +## Self-Check: PASSED + +Files exist: +- `apps/pwa/src/api/client.ts` — FOUND +- `apps/pwa/src/routes/SetupPage.tsx` — FOUND +- `apps/pwa/src/routes/SetupPage.test.tsx` — FOUND +- `apps/pwa/src/api/setupClient.contract.test.ts` — FOUND +- `apps/pwa/src/App.tsx` — FOUND +- `apps/pwa/src/App.test.tsx` — FOUND + +Commits verified: +- `eb84e6e` — Task 2 RED +- `62d80f6` — Task 2 GREEN +- `1587bca` — Task 3 +- `9f20c8b` — Task 4 RED +- `120ce85` — Task 4 GREEN +- `7d0205d` — CR-01 RED (VAPID tests) +- `0d53249` — CR-01 GREEN (VAPID wired) + +Test suite: 249 passed | 0 failed +TypeCheck: clean (0 errors) +playwright-cli: /config 200 confirmed; redirect gate confirmed; DB validation 200; OIDC 400 (expected — Authelia unreachable from container); VAPID endpoint live (curl POST /api/setup/validate/vapid returns 200); VAPID row wired in Step 2 chain diff --git a/.planning/phases/12-initial-setup-wizard/12-05-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-05-PLAN.md new file mode 100644 index 0000000..9c10e53 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-05-PLAN.md @@ -0,0 +1,143 @@ +--- +phase: 12-initial-setup-wizard +plan: 05 +type: execute +wave: 2 +depends_on: ["12-06"] +files_modified: + - apps/pwa/src/routes/SetupPage.tsx + - apps/pwa/src/routes/SetupPage.test.tsx +autonomous: true +gap_closure: true +requirements: [SETUP-01] +must_haves: + truths: + - "The Instance step intro copy no longer contains the DB-vs-env-file aside" + - "A read-only, disabled DB-name field renders directly under the App URL field on the Instance step" + - "Navigating Back from the Calendar step to the Instance step preserves all previously entered field values" + artifacts: + - path: "apps/pwa/src/routes/SetupPage.tsx" + provides: "Instance step copy trimmed; read-only DB-name field; field state lifted so Back preserves values" + contains: "readOnly" + key_links: + - from: "SetupPage Instance step" + to: "GET /api/setup/status dbName" + via: "fetchSetupStatus().dbName populates the read-only field" + pattern: "dbName" + - from: "SetupPage parent (step owner)" + to: "Step2Config fields" + via: "field values lifted to SetupPage (or sessionStorage) and passed as props" + pattern: "appUrl|oidcIssuer|oidcClientId|vapidPublicKey" +--- + + +Close UAT gaps 1, 3 (frontend), and 4 — all on the PWA Instance step (`SetupPage.tsx`). + +Gap 1 (cosmetic): the Instance step intro `

` contains "These are written to the database — not your environment file." — an implementation aside the user wants dropped. + +Gap 3 (minor, frontend half): the "database connection verified" row has no on-screen referent. Add a read-only, greyed-out/disabled field showing the env-derived DB name (from `GET /api/setup/status` `dbName`, added in Plan 06), positioned directly under the App URL field. Keep the existing DB validation row as-is. + +Gap 4 (minor): each wizard step holds its field values in its own local `useState` and unmounts on navigation, so going Back from the Calendar step to the Instance step loses all entered config. Lift Instance (and Calendar) field values into `SetupPage` (or persist to sessionStorage) so Back preserves them. + +Purpose: First-run operator can navigate Back without re-typing; the DB row makes sense; no confusing implementation copy. +Output: Instance step with trimmed copy, a read-only DB-name field, and persistent field values across Back navigation. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-UAT.md +@.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md + +# Files to edit +@apps/pwa/src/routes/SetupPage.tsx +@apps/pwa/src/routes/SetupPage.test.tsx +# Contract this plan consumes (added by Plan 06) +@apps/pwa/src/api/client.ts + + + + + + Task 1: Drop the DB-vs-env-file aside + add read-only DB-name field (gaps 1, 3-frontend) + apps/pwa/src/routes/SetupPage.tsx, apps/pwa/src/routes/SetupPage.test.tsx + + Gap 1 — In `Step2Config` (apps/pwa/src/routes/SetupPage.tsx, the intro `

` at ~line 547-558), remove the sentence "These are written to the database — not your environment file." Keep the first sentence ("Enter your instance's connection details.") and the surrounding paragraph styling intact. + + Gap 3 (frontend) — Render a read-only, disabled field showing the env-derived DB name directly under the App URL field block (the App URL `

` ends ~line 575, just before the OIDC Issuer block): + - Fetch the DB name from the status endpoint. Import `fetchSetupStatus` from '../api/client.js' (already exported) and read `dbName` from its response (the `dbName?: string | null` field added by Plan 06). Use `useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus, staleTime: 0, retry: false })` inside Step2Config (or lift the query to SetupPage and pass `dbName` as a prop — executor's choice, but keep it self-contained to the Instance step). + - Render a labelled input mirroring the existing field markup (reuse `labelStyle`, `inputStyle(false)`, `helperStyle`): label "Database" (or "Database name"), value = the fetched dbName (fallback to an empty string / a "—" placeholder while loading or if null), with `readOnly` AND `disabled` set, a greyed-out appearance (set the input's `background`/`color` to a muted token, e.g. `var(--color-surface-dim)` / `var(--color-text-secondary)`), and `aria-readonly="true"`. Helper text: explains this is configured via the server's Docker environment (DB_HOST/DB_PORT/DB_USER/DB_PASSWORD), not entered here — so the "database connection verified" row below has a referent. NEVER render DB_HOST/DB_USER/DB_PASSWORD — only the name. + - Do NOT change the existing DB ValidationRow ("Database connection verified.") — keep it as-is per the UAT "missing" note. + + In apps/pwa/src/routes/SetupPage.test.tsx: assert the dropped sentence is no longer present (query the Instance step text and assert "not your environment file" is absent), and assert the read-only DB-name field renders disabled/readOnly with the mocked dbName. Mock `fetchSetupStatus` (or the client module) to return `{ setupComplete: false, dbName: 'familysync' }`. + + + cd apps/pwa && pnpm test -- SetupPage 2>&1 | tail -20 + + Instance step intro no longer contains "not your environment file"; a disabled+readOnly DB-name field (value from status dbName) renders under App URL; the existing DB validation row is unchanged; SetupPage.test.tsx GREEN. + + + + Task 2: Preserve wizard field values across Back navigation (gap 4) + apps/pwa/src/routes/SetupPage.tsx, apps/pwa/src/routes/SetupPage.test.tsx + + Lift the Instance-step field values (appUrl, oidcIssuer, oidcClientId, vapidPublicKey) and the Calendar-step field values (email, plus credential-verified flag if needed for UX) out of the per-step local `useState` so they survive step unmount/remount. + + Recommended approach (state lifted to the SetupPage parent — matches the existing "parent owns `step`" structure): + - In `SetupPage` (the component owning `useState`), add state for the Instance fields: `appUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey` (the app password is sensitive — do NOT lift/persist the password value; only the non-secret email may be lifted if convenient, but the password must stay local and cleared on unmount per T-12-15). + - Pass these values + their setters down to `Step2Config` as props; replace the component-local `useState('')` declarations (~lines 439-442) with the props. Validation/mutation logic stays inside Step2Config. + - Ensure that when navigating Back from Step 3 → Step 2, the Instance fields are still populated (because the parent now holds them). When navigating Back from Step 2 → Step 1 and forward again, values also persist. + + Alternative (sessionStorage) is acceptable if simpler, but MUST NOT persist the Fastmail app password (T-12-15) — only the non-secret Instance fields. Prefer the lifted-state approach. + + Security: the Fastmail app password (Step 3) is NOT lifted and NOT persisted to sessionStorage — it remains in Step3Credential local state and is cleared on unmount (T-12-15 preserved). + + In apps/pwa/src/routes/SetupPage.test.tsx: add a test that fills the Instance fields, advances to the Calendar step, navigates Back, and asserts the Instance field values are still present (inputs retain their values). Add an assertion that the password field is NOT persisted across navigation (re-mount of Step 3 starts empty). + + + cd apps/pwa && pnpm test -- SetupPage 2>&1 | tail -20 + + Filling the Instance step, advancing, then clicking Back restores all four Instance field values; the Fastmail app password is never persisted across navigation; SetupPage.test.tsx GREEN. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| operator input → wizard state | Non-secret config + a sensitive app password are entered here | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-15 | Information Disclosure | Step3 app password | mitigate | App password stays in Step3 local state; NOT lifted to parent, NOT written to sessionStorage; cleared on unmount; field remains type="password" | +| T-12-14 | Tampering (XSS) | Instance/DB-name copy | mitigate | All new copy + dbName rendered as plain-text JSX children; no dangerouslySetInnerHTML (grep returns 0) | +| T-12-3DB | Information Disclosure | DB-name field | mitigate | Only the dbName from status is rendered; DB_HOST/DB_USER/DB_PASSWORD never fetched or shown | + + + +- `cd apps/pwa && pnpm test -- SetupPage` GREEN +- `grep -n "not your environment file" apps/pwa/src/routes/SetupPage.tsx` returns nothing +- `grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` is 0 +- `grep -nE "sessionStorage|localStorage" apps/pwa/src/routes/SetupPage.tsx` — if present, confirm no password/appPassword key is written +- `cd apps/pwa && pnpm typecheck` clean + + + +- Gap 1 closed: implementation aside removed. +- Gap 3 (frontend) closed: read-only DB-name field gives the DB validation row a referent. +- Gap 4 closed: Back navigation preserves Instance field values; app password never persisted. + + + +Create `.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md new file mode 100644 index 0000000..fdd2b53 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md @@ -0,0 +1,88 @@ +--- +phase: 12-initial-setup-wizard +plan: 05 +subsystem: setup-wizard-frontend +tags: [setup, pwa, uat-gap-closure, a11y] +requires: + - "GET /api/setup/status { setupComplete, dbName } (Plan 06)" + - "SetupStatusResponse.dbName?: string | null typed field (Plan 06)" +provides: + - "Instance step intro copy trimmed (no DB-vs-env-file aside)" + - "Read-only, disabled DB-name field under App URL, populated from status dbName" + - "Instance field values lifted to SetupPage so Back navigation preserves them" +affects: + - apps/pwa setup wizard Instance step (SetupPage.tsx) +tech-stack: + added: [] + patterns: + - "useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus }) reads non-secret dbName into a read-only field" + - "Step-level field values lifted to the parent (SetupPage) so step unmount no longer drops entries" + - "Sensitive app password deliberately NOT lifted — stays in Step3 local state, cleared on unmount (T-12-15)" +key-files: + created: [] + modified: + - apps/pwa/src/routes/SetupPage.tsx + - apps/pwa/src/routes/SetupPage.test.tsx +decisions: + - "D-12-05-LIFT: only the four non-secret Instance fields are lifted to SetupPage; the Fastmail app password is never lifted or persisted (T-12-15 preserved)." + - "D-12-05-DBNAME: DB-name field renders the dbName value only; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text, never as values (T-12-3DB)." + - "D-12-05-VALSTATE: Step2 validation state (db/oidc/vapid pass flags) is intentionally NOT lifted — only field values persist across Back; operator re-runs Save & Validate after returning." +metrics: + duration_minutes: 9 + completed: 2026-06-16 +--- + +# Phase 12 Plan 05: Instance-Step Gap Closure (copy trim, DB-name field, Back persistence) Summary + +Closed UAT gaps 1, 3 (frontend half), and 4 on the PWA Instance step (`SetupPage.tsx`): dropped the confusing DB-vs-env-file implementation aside, added a read-only env-derived DB-name field so the "database connection verified" row has an on-screen referent, and lifted the four Instance field values into `SetupPage` so navigating Back from the Calendar step no longer wipes entered config. + +## What Was Built + +### Task 1 — Drop DB-vs-env aside + add read-only DB-name field (gaps 1, 3-frontend) +Commit `35db5c5`. + +- **Gap 1**: Removed the sentence "These are written to the database — not your environment file." from the Instance step intro `

`, keeping the first sentence ("Enter your instance's connection details."). +- **Gap 3 (frontend)**: Added a labelled, `readOnly` + `disabled` input ("Database") directly under the App URL field, populated from `fetchSetupStatus().dbName` via `useQuery({ queryKey: ['setupStatus'], staleTime: 0, retry: false })`. The field is greyed out (`--color-surface-dim` background, `--color-text-secondary` text), carries `aria-readonly="true"` and `tabIndex={-1}`, and shows `—` while loading/null. Helper text explains the DB is configured via the server's Docker environment (DB_HOST/DB_PORT/DB_USER/DB_PASSWORD as static names) and is not entered here. The existing "Database connection verified." ValidationRow is unchanged. +- Tests assert the dropped sentence is absent, the DB field renders `readOnly`/`disabled`/`aria-readonly` with the mocked `dbName: 'familysync'`, and the existing DB validation row still appears on Save & Validate. + +### Task 2 — Preserve Instance fields across Back navigation (gap 4) +Commit `a13fc11`. + +- Introduced an `InstanceFields` shape (`appUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey`) owned by `SetupPage` (`instanceFields` / `setInstanceFields`), passed to `Step2Config` as `fields` / `setFields` props. `Step2Config` now reads/writes these through the lifted setters instead of its own local `useState`. Validation/mutation logic is unchanged. +- The Fastmail app password (Step 3) is **not** lifted — it remains in `Step3Credential` local state and is cleared on unmount when navigating away (T-12-15 preserved). +- Tests: filling the Instance step, validating to GREEN, advancing to the Calendar step, then clicking Back restores all four Instance values; a second test confirms a typed app password is empty after Back→forward (Step 3 re-mounts fresh). + +## Verification + +- `cd apps/pwa && pnpm test -- SetupPage` → **263 passed (22 files)**. +- `cd apps/pwa && pnpm typecheck` → clean (tsc + e2e tsconfig). +- `grep -c "not your environment file" apps/pwa/src/routes/SetupPage.tsx` → **0**. +- `grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` → **0**. +- `grep -nE "sessionStorage|localStorage" apps/pwa/src/routes/SetupPage.tsx` → **no matches** (no client-side persistence of any field, secret or otherwise). +- `grep -c "readOnly" apps/pwa/src/routes/SetupPage.tsx` → **1** (the DB-name field). +- DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear only as static env-var names in helper/error copy — never fetched or rendered as values. + +## Deviations from Plan + +None — plan executed exactly as written. Implementation note: the two tasks both restructure the `Step2Config` signature/body and the same intro paragraph, so they were authored together and then committed as two atomic, individually-GREEN commits (Task 1 commit verified GREEN with 261 tests before Task 2's state-lifting and Back-navigation tests were added). + +## Threat Surface + +| Threat ID | Disposition | Outcome | +|-----------|-------------|---------| +| T-12-15 (app password disclosure) | mitigate | Preserved — password stays in Step3 local state, type="password", NOT lifted, NOT persisted to storage; cleared on unmount. Test asserts it is empty after Back→forward. | +| T-12-14 (XSS in Instance/DB copy) | mitigate | All new copy + dbName rendered as plain-text JSX children; `dangerouslySetInnerHTML` grep = 0. | +| T-12-3DB (DB secret/topology disclosure) | mitigate | Only `dbName` value is fetched and rendered; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text. | + +No new security-relevant surface introduced beyond the planned `threat_model`. + +## Known Stubs + +None. + +## Self-Check: PASSED + +- `apps/pwa/src/routes/SetupPage.tsx` — modified, exists. +- `apps/pwa/src/routes/SetupPage.test.tsx` — modified, exists. +- Commit `35db5c5` (Task 1) — FOUND in git log. +- Commit `a13fc11` (Task 2) — FOUND in git log. diff --git a/.planning/phases/12-initial-setup-wizard/12-06-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-06-PLAN.md new file mode 100644 index 0000000..833c938 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-06-PLAN.md @@ -0,0 +1,146 @@ +--- +phase: 12-initial-setup-wizard +plan: 06 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/api/src/routes/setup.ts + - apps/api/tests/routes/setup.test.ts + - apps/pwa/src/api/client.ts +autonomous: true +gap_closure: true +requirements: [SETUP-02] +must_haves: + truths: + - "Entering a wrong/invalid VAPID public key in the wizard fails the VAPID validation row" + - "POST /api/setup/validate/vapid returns 400 when the submitted vapid_public_key does not match the env VAPID_PUBLIC_KEY" + - "The GET /api/setup/status response exposes the non-secret env DB name (no secrets)" + - "VAPID_PRIVATE_KEY is never returned in any response (T-12-06 preserved)" + artifacts: + - path: "apps/api/src/routes/setup.ts" + provides: "validate/vapid asserts submitted key matches env public key; status returns dbName" + contains: "VAPID_PUBLIC_KEY" + - path: "apps/pwa/src/api/client.ts" + provides: "SetupStatusResponse.dbName field" + contains: "dbName" + key_links: + - from: "POST /api/setup/validate/vapid" + to: "app_config.vapid_public_key" + via: "compare submitted key against process.env.VAPID_PUBLIC_KEY" + pattern: "vapid_public_key" + - from: "GET /api/setup/status" + to: "process.env.DB_NAME" + via: "non-secret DB name surfaced in response" + pattern: "dbName" +--- + + +Close UAT gaps 2 and 3 on the backend setup-route surface. + +Gap 2 (major): `POST /api/setup/validate/vapid` validates the *env* VAPID pair via `webpush.setVapidDetails` but never compares against the wizard-entered `vapid_public_key`. An operator typed `BH123` (clearly invalid) and the row still went green because the env pair was valid. The fix: assert the submitted/persisted `vapid_public_key` equals `process.env.VAPID_PUBLIC_KEY` (the public half of the configured pair) so a wrong key fails the row and gates Continue. + +Gap 3 (minor, backend half): the DB connection is configured via Docker env (DB_HOST/PORT/USER/PASSWORD), not collected in the wizard, so the "database connection verified" row has no on-screen referent. Surface the **non-secret** DB name so the PWA (Plan 05) can render a read-only field giving that row a referent. + +Purpose: A wrong VAPID key must fail (push silently breaks in production otherwise — SETUP-02); the DB row must reference something visible. +Output: `validate/vapid` rejects mismatched keys; `GET /api/setup/status` returns `{ setupComplete, dbName }`. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-UAT.md +@.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md + +# Files to edit (already in context for the planner; executor should read before editing) +@apps/api/src/routes/setup.ts +@apps/api/src/api/../tests/routes/setup.test.ts +@apps/pwa/src/api/client.ts + + + + + + Task 1: validate/vapid asserts submitted key matches env public key (gap 2) + apps/api/src/routes/setup.ts, apps/api/tests/routes/setup.test.ts + + - When VAPID_PRIVATE_KEY/VAPID_PUBLIC_KEY env are set AND app_config.vapid_public_key equals process.env.VAPID_PUBLIC_KEY → 200 { ok: true } (existing happy path preserved). + - When app_config.vapid_public_key is present but does NOT equal process.env.VAPID_PUBLIC_KEY (e.g. "BH123") → 400 { ok: false } with a non-echoing error message; the response NEVER contains VAPID_PRIVATE_KEY. + - When app_config.vapid_public_key row is absent → 400 { ok: false } (cannot validate without the operator-submitted key). + - When env VAPID keys are missing → existing 400 path preserved. + - When setup is locked → existing 423 path preserved (isSetupLocked() first). + + + In the `POST /validate/vapid` handler (apps/api/src/routes/setup.ts, currently ~line 202), after the existing `isSetupLocked()` 423 guard and the existing env-presence check, add an equality assertion BEFORE the `webpush.setVapidDetails` structural check: + + - Read the operator-submitted public key from app_config: SELECT value FROM app_config WHERE key = 'vapid_public_key' (use the existing `db.select({ value: appConfig.value }).from(appConfig).where(eq(appConfig.key, 'vapid_public_key')).limit(1)` idiom already used by the validate/oidc handler). + - If that row is absent OR its value !== process.env.VAPID_PUBLIC_KEY, return 400 { ok: false, error: 'VAPID public key does not match the configured key pair. Paste the exact VAPID_PUBLIC_KEY printed by `npm run generate-secrets`.' }. This is the gap-2 assertion: a wrong key now fails the row. + - Keep the existing `webpush.setVapidDetails(subject, publicKey, privateKey)` structural check AFTER the equality check, still reading BOTH keys ONLY from process.env. Do NOT read VAPID_PRIVATE_KEY from app_config and NEVER return it (T-12-06 / D-01 preserved — the equality compares the submitted PUBLIC key to the env PUBLIC key only). + + In apps/api/tests/routes/setup.test.ts, extend the validate/vapid suite (RED first): add a test that mocks app_config.vapid_public_key returning a value different from process.env.VAPID_PUBLIC_KEY and asserts a 400 plus that the JSON body has no key matching VAPID_PRIVATE_KEY; update the existing happy-path test so the mocked app_config value equals process.env.VAPID_PUBLIC_KEY (otherwise it would now 400). Add a test for the absent-row → 400 case. + + + cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup 2>&1 | tail -20 + + validate/vapid returns 400 for a mismatched/absent submitted key and 200 only when the submitted key equals process.env.VAPID_PUBLIC_KEY; no response path returns VAPID_PRIVATE_KEY; setup.test.ts vapid suite GREEN. + + + + Task 2: Expose non-secret DB name via GET /api/setup/status (gap 3 backend) + apps/api/src/routes/setup.ts, apps/api/tests/routes/setup.test.ts, apps/pwa/src/api/client.ts + + In the `GET /status` handler (apps/api/src/routes/setup.ts, ~line 86), include the non-secret DB name in the response alongside the existing `setupComplete`. Source the name from `process.env.DB_NAME` (the Drizzle/mysql2 connection uses DB_HOST/DB_PORT/DB_USER/DB_PASSWORD/DB_NAME — confirm the exact env var name by grepping apps/api/src/db/client.ts; use whatever that file reads for the database name). Return `{ setupComplete, dbName }` where dbName is `process.env.DB_NAME ?? null`. + + ONLY the database NAME is surfaced — never DB_HOST, DB_USER, or DB_PASSWORD (those are connection secrets/topology; the name alone is the on-screen referent the operator asked for). Do NOT add DB_PASSWORD or any secret to any response. + + In apps/pwa/src/api/client.ts, add `dbName?: string | null` to the `SetupStatusResponse` interface (~line 538) so the PWA (Plan 05) consumes a typed field. No other client.ts changes. + + In apps/api/tests/routes/setup.test.ts, update the GET /status test(s) to assert the response includes `dbName` reflecting the mocked/process env DB name (set process.env.DB_NAME in the test or assert the key is present). + + + cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup 2>&1 | tail -15 && cd ../pwa && pnpm typecheck 2>&1 | tail -5 + + GET /api/setup/status returns `{ setupComplete, dbName }` with the non-secret DB name (no DB_PASSWORD/DB_HOST/DB_USER); SetupStatusResponse carries `dbName?: string | null`; api setup.test.ts GREEN; pwa typecheck clean. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| pre-auth client → /api/setup/* | Unauthenticated operator input crosses here before OIDC is configured | +| process.env → response body | Secret env vars must not leak into pre-auth JSON | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-12-06 | Information Disclosure | validate/vapid | mitigate | VAPID_PRIVATE_KEY read ONLY from process.env, never compared/returned; equality check uses PUBLIC keys only; test asserts no VAPID_PRIVATE_KEY in body | +| T-12-3DB | Information Disclosure | GET /status dbName | mitigate | Only process.env.DB_NAME surfaced; DB_HOST/DB_USER/DB_PASSWORD never added to any response (grep-checked) | +| T-12-04 | Tampering/Replay | all setup routes | mitigate | isSetupLocked() remains the first await in every handler (unchanged) | + + + +- `cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup` GREEN +- `grep -nE "VAPID_PRIVATE_KEY" apps/api/src/routes/setup.ts` shows it only inside the env-only structural check, never in a response/compare against app_config +- `grep -nE "DB_PASSWORD|DB_HOST|DB_USER" apps/api/src/routes/setup.ts | grep -i "status\|c.json"` returns nothing (no secret/topology in status response) +- `cd apps/pwa && pnpm typecheck` clean + + + +- Gap 2 closed: a wrong wizard-entered VAPID public key fails validate/vapid (400) and therefore gates Continue. +- Gap 3 backend closed: status exposes the non-secret DB name for the PWA read-only field. +- No secret (VAPID_PRIVATE_KEY, DB_PASSWORD) appears in any response. + + + +Create `.planning/phases/12-initial-setup-wizard/12-06-SUMMARY.md` when done + diff --git a/.planning/phases/12-initial-setup-wizard/12-06-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-06-SUMMARY.md new file mode 100644 index 0000000..c64b0d9 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-06-SUMMARY.md @@ -0,0 +1,81 @@ +--- +phase: 12-initial-setup-wizard +plan: 06 +subsystem: setup-wizard-backend +tags: [setup, vapid, security, uat-gap-closure] +requires: + - app_config.vapid_public_key (written by POST /api/setup/config) + - process.env.VAPID_PUBLIC_KEY / VAPID_PRIVATE_KEY (Docker env) + - process.env.DB_NAME (Docker env) +provides: + - "POST /api/setup/validate/vapid rejects a submitted public key that does not match the env VAPID_PUBLIC_KEY" + - "GET /api/setup/status returns { setupComplete, dbName } with the non-secret DB name" + - "SetupStatusResponse.dbName typed field for the PWA (Plan 05) read-only referent" +affects: + - apps/pwa setup wizard (Plan 05 consumes dbName + the now-strict VAPID row) +tech-stack: + added: [] + patterns: + - "validate/vapid equality check uses the same app_config select idiom as validate/oidc" + - "non-secret env surfacing: only DB_NAME exposed, never DB_HOST/DB_USER/DB_PASSWORD" +key-files: + created: [] + modified: + - apps/api/src/routes/setup.ts + - apps/api/tests/routes/setup.test.ts + - apps/pwa/src/api/client.ts +decisions: + - "D-12-06-VAPID-EQ: validate/vapid compares the operator-submitted PUBLIC key (app_config.vapid_public_key) to process.env.VAPID_PUBLIC_KEY; the private key is never compared or echoed (T-12-06 preserved)." + - "D-12-06-DBNAME: only process.env.DB_NAME (?? null) is surfaced in GET /status; DB_HOST/DB_USER/DB_PASSWORD are never added to any response (grep-verified)." +metrics: + duration_minutes: 8 + completed: 2026-06-16 +--- + +# Phase 12 Plan 06: Setup-Route Gap Closure (VAPID equality + DB name) Summary + +Closed UAT gaps 2 and 3 on the backend setup-route surface: `POST /api/setup/validate/vapid` now rejects a wrong/typoed wizard-entered VAPID public key by asserting it equals the env `VAPID_PUBLIC_KEY`, and `GET /api/setup/status` now returns the non-secret `dbName` so the DB-connection row has an on-screen referent. + +## What Was Built + +### Task 1 — validate/vapid asserts submitted key matches env public key (gap 2, TDD) +Before the structural `webpush.setVapidDetails()` check, the handler now reads `app_config.vapid_public_key` (the operator-submitted key) and returns 400 unless it exactly equals `process.env.VAPID_PUBLIC_KEY`. Previously a clearly-invalid key like `BH123` still went green because only the env pair was validated — push would silently break in production (SETUP-02). The equality compares PUBLIC keys only; `VAPID_PRIVATE_KEY` remains read solely from `process.env` and is never compared or returned (T-12-06). + +- RED commit `e9d07b3`: mismatch → 400 (no private-key leak), absent row → 400, happy path seeds matching row. +- GREEN commit `e46e80a`: equality assertion implemented. + +### Task 2 — Expose non-secret DB name via GET /api/setup/status (gap 3 backend) +`GET /api/setup/status` now returns `{ setupComplete, dbName }` where `dbName = process.env.DB_NAME ?? null` (the var read by `apps/api/src/db/client.ts`). Only the database NAME is surfaced — never DB_HOST/DB_USER/DB_PASSWORD. `SetupStatusResponse` in the PWA client gained `dbName?: string | null` so Plan 05 can render a typed read-only field. + +- Commit `fbd3b77`. + +## Verification + +- `cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup` → **407 passed (29 files)**. +- `grep -nE "VAPID_PRIVATE_KEY" apps/api/src/routes/setup.ts` → only the env-only structural-check lines + doc comments; never compared against app_config or returned. +- `grep -nE "DB_PASSWORD|DB_HOST|DB_USER" apps/api/src/routes/setup.ts | grep -i "status\|c.json"` → **no matches** (no secret/topology in status response). +- `cd apps/pwa && pnpm typecheck` → clean (tsc + e2e tsconfig). + +## Deviations from Plan + +None — plan executed exactly as written. The pre-existing "invalid/truncated VAPID key" test (env keys invalid, no app_config row) still asserts 400/`ok:false` and stays GREEN; with the new equality check it now 400s on the absent-row branch rather than the structural branch, which is the intended stricter behavior. + +## TDD Gate Compliance + +Task 1 followed RED→GREEN: failing test commit `e9d07b3` (`test(12-06): ...`) precedes implementation commit `e46e80a` (`feat(12-06): ...`). No REFACTOR step needed. Task 2 is a non-behavioral env-surfacing change with an accompanying assertion added in the same commit. + +## Threat Surface + +| Threat ID | Disposition | Outcome | +|-----------|-------------|---------| +| T-12-06 (VAPID_PRIVATE_KEY disclosure) | mitigate | Preserved — private key env-only; equality uses PUBLIC keys; test asserts no private key in mismatch body. | +| T-12-3DB (DB secret/topology disclosure) | mitigate | Only DB_NAME surfaced; grep confirms no DB_HOST/DB_USER/DB_PASSWORD in status response. | +| T-12-04 (setup-route replay) | mitigate | `isSetupLocked()` remains the first await in every handler (unchanged). | + +No new security-relevant surface introduced beyond the planned `threat_model`. + +## Known Stubs + +None. + +## Self-Check: PASSED diff --git a/.planning/phases/12-initial-setup-wizard/12-07-PLAN.md b/.planning/phases/12-initial-setup-wizard/12-07-PLAN.md new file mode 100644 index 0000000..d9b68a2 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-07-PLAN.md @@ -0,0 +1,146 @@ +--- +phase: 12-initial-setup-wizard +plan: 07 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/pwa/src/App.tsx + - apps/pwa/src/App.test.tsx + - apps/pwa/src/components/SetupBanner.tsx +autonomous: true +gap_closure: true +requirements: [SETUP-01, SETUP-04] +must_haves: + truths: + - "After setup is complete, manually visiting /setup shows the 'already complete' surface (or redirects away) — not the wizard" + - "After completing the wizard (incl. Fastmail credential) and landing in the app, the /calendar 'Set up your calendar' banner does NOT show for the operator" + - "The setup gate respects the loading state to avoid a flash of the wizard" + artifacts: + - path: "apps/pwa/src/App.tsx" + provides: "/setup route gated on setupComplete (alreadyLocked or redirect); ['me'] freshness reconciled with wizard completion" + contains: "alreadyLocked" + key_links: + - from: "App.tsx /setup route" + to: "setupQuery.data.setupComplete" + via: "alreadyLocked={setupComplete === true} or Navigate to /calendar" + pattern: "alreadyLocked|setupComplete" + - from: "SetupBanner needsProviderSetup" + to: "['me'] query freshness" + via: "['me'] refetched after wizard completion so the banner reflects the claimed credential" + pattern: "needsProviderSetup|invalidateQueries" +--- + + +Close UAT gaps 5 and 6 — both on the PWA app-shell gate (`App.tsx`), with the banner component (`SetupBanner.tsx`). + +Gap 5 (major): `App.tsx` renders `} />` with no `alreadyLocked` prop and no `setupComplete` check. The `*` gate only redirects OTHER routes TO /setup when incomplete; there is no reverse guard. When `setupComplete === true`, manually visiting /setup still mounts the full wizard. The backend already 423s mutations, so this is purely a frontend gating gap. Fix: gate the /setup route on `setupComplete` — pass `alreadyLocked={setupComplete === true}` (SetupPage already supports this prop → renders Surface 8 "Setup already complete") or `Navigate` to /calendar; respect `setupLoading` to avoid a flash. + +Gap 6 (major, root_cause PRELIMINARY): after finishing the wizard and landing on /calendar, the "Set up your calendar / Set up now" banner still shows. Task 1 is an investigation step to confirm the mechanism before prescribing the exact fix; Task 2 implements the confirmed fix. + +Purpose: A completed instance must not re-expose the wizard, and must not nag the operator to set up a calendar they already configured in the wizard. +Output: /setup route gated post-completion; ['me'] reconciled so the banner does not show after wizard completion. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/12-initial-setup-wizard/12-UAT.md +@.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md +@.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md + +# Files to edit + investigate +@apps/pwa/src/App.tsx +@apps/pwa/src/App.test.tsx +@apps/pwa/src/components/SetupBanner.tsx +# Reference (do not edit unless Task 1 investigation proves a backend linking gap) +@apps/api/src/routes/me.ts +@apps/api/src/routes/setup.ts +@apps/api/src/auth/user.ts + + + + + + Task 1: Gate the /setup route on setupComplete (gap 5) + apps/pwa/src/App.tsx, apps/pwa/src/App.test.tsx + + In apps/pwa/src/App.tsx, the `} />` (line ~139) currently mounts the wizard unconditionally. Add a reverse guard using the already-present `setupComplete` / `setupLoading` values (derived at lines ~132-133 from `setupQuery`): + - While `setupLoading` is true → render the existing no-flash placeholder (`