diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index a49c432..3457260 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -168,7 +168,7 @@ Plans: **Wave 6** *(gap closure — blocked on Waves 2 + 4)* -- [ ] 04-07-PLAN.md — Gap closure: migrate list_items.rank to COLLATE utf8mb4_bin (LIST-03 drag-to-top) + owner-only guard on PATCH isShared (T-04-08/T-04-05) — two TDD features (LIST-03) +- [x] 04-07-PLAN.md — Gap closure: migrate list_items.rank to COLLATE utf8mb4_bin (LIST-03 drag-to-top) + owner-only guard on PATCH isShared (T-04-08/T-04-05) — two TDD features (LIST-03) **UI hint**: yes diff --git a/.planning/STATE.md b/.planning/STATE.md index 6582925..fbde880 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,15 +3,15 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone status: executing -stopped_at: Completed 04-06-PLAN.md -last_updated: "2026-06-09T17:35:35.144Z" -last_activity: 2026-06-09 +stopped_at: Completed 04-07-PLAN.md +last_updated: "2026-06-09T18:23:42Z" +last_activity: 2026-06-09 -- Phase 04 plan 07 complete (gap-closure: LIST-03 + T-04-08) progress: total_phases: 15 completed_phases: 4 - total_plans: 27 - completed_plans: 27 - percent: 27 + total_plans: 28 + completed_plans: 28 + percent: 100 --- # Project State @@ -25,12 +25,12 @@ See: .planning/PROJECT.md (updated 2026-06-07) ## Current Position -Phase: 04 (shared-lists-live-sync) — EXECUTING -Plan: 6 of 6 -Status: Ready to execute -Last activity: 2026-06-09 +Phase: 04 (shared-lists-live-sync) — COMPLETE +Plan: 7 of 7 (all plans complete) +Status: Phase 04 complete; ready for Phase 05 +Last activity: 2026-06-09 -- Phase 04 plan 07 complete (gap-closure: LIST-03 + T-04-08) -Progress: [█████████░] 93% +Progress: [██████████] 100% ## Performance Metrics @@ -64,6 +64,7 @@ Progress: [█████████░] 93% | Phase 04 P04 | 11 | 2 tasks | 10 files | | Phase 04 P05 | 10 | 2 tasks | 4 files | | Phase 04 P06 | 11 | 2 tasks | 7 files | +| Phase 04 P07 | 6 | 2 tasks | 4 files | ## Accumulated Context @@ -97,6 +98,8 @@ Recent decisions affecting current work: - [Phase 04-04]: Uncheck rank recomputed to active-bottom (generateKeyBetween(lastActiveRank, null)) in same DB write (Open Question 2 resolved) - [Phase 04-04]: Delete-wins no-rollback: deleteMutation has no onError handler; item removal from cache is final (D-09) - [Phase ?]: LIST-04: SSE connection lives in ListDetail (not hoisted to Lists route); Phase 5 push will own session lifecycle +- [Phase 04-07]: D-04-07-collation: Drizzle 0.45.x has no first-class collation option on varchar; used customType to emit varchar(255) COLLATE utf8mb4_bin for list_items.rank — keeps schema-as-code + generate+migrate workflow +- [Phase 04-07]: D-04-07-guard: isShared owner-only guard placed after access check, before updateValues construction; mirrors DELETE handler idiom (if !access.isOwner → 403) ### Roadmap Evolution @@ -137,6 +140,6 @@ Recent decisions affecting current work: ## Session Continuity -Last session: 2026-06-09T17:35:35.134Z -Stopped at: Completed 04-06-PLAN.md +Last session: 2026-06-09T18:23:42Z +Stopped at: Completed 04-07-PLAN.md (Phase 04 fully complete) Resume file: None diff --git a/.planning/phases/04-shared-lists-live-sync/04-07-SUMMARY.md b/.planning/phases/04-shared-lists-live-sync/04-07-SUMMARY.md new file mode 100644 index 0000000..c1d07cf --- /dev/null +++ b/.planning/phases/04-shared-lists-live-sync/04-07-SUMMARY.md @@ -0,0 +1,122 @@ +--- +phase: 04-shared-lists-live-sync +plan: "07" +subsystem: api +tags: [mariadb, drizzle, fractional-indexing, collation, security, authorization] + +# Dependency graph +requires: + - phase: 04-03 + provides: list CRUD routes + listShares schema + - phase: 04-05 + provides: fractional-rank reorder PATCH route for list items +provides: + - "list_items.rank column with COLLATE utf8mb4_bin (migration 0002)" + - "owner-only guard on PATCH /api/lists/:id isShared mutations" + - "rank-collation regression test (LIST-03)" + - "T-04-08 negative test: sharee sending { isShared } receives 403" +affects: [04-verification, 04-security] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Drizzle customType for MySQL column-level COLLATE (no first-class option in drizzle 0.45.x)" + - "TDD RED commit (test:) before GREEN commit (feat:/fix:) per phase-04 convention" + +key-files: + created: + - apps/api/src/db/migrations/0002_yielding_mattie_franklin.sql + modified: + - apps/api/src/db/schema.ts + - apps/api/src/routes/lists.ts + - apps/api/tests/routes/lists.test.ts + +key-decisions: + - "D-04-07-collation: Drizzle 0.45.x has no first-class collation option on varchar; used customType to emit varchar(255) COLLATE utf8mb4_bin — keeps schema-as-code and generate+migrate workflow intact" + - "D-04-07-guard-placement: isShared owner guard placed immediately after the access check, before any updateValues construction, so the body is never parsed for non-owners" + +patterns-established: + - "customType pattern for MySQL column collation: define a named factory (varcharBin) in schema.ts that emits the full SQL type string including COLLATE" + - "Owner-only guard idiom: if (patch.sensitiveField !== undefined && !access.isOwner) return 403 — mirrors the existing DELETE owner check" + +requirements-completed: [LIST-03] + +# Metrics +duration: 6min +completed: "2026-06-09" +--- + +# Phase 04 Plan 07: Gap-Closure (LIST-03 Rank Collation + T-04-08 Owner Guard) Summary + +**Closed LIST-03 drag-to-top bug via utf8mb4_bin migration on list_items.rank, and closed T-04-08/T-04-05 elevation-of-privilege by adding an owner-only guard before the isShared reconciliation block.** + +## Performance + +- **Duration:** ~6 min +- **Started:** 2026-06-09T18:18:10Z +- **Completed:** 2026-06-09T18:23:42Z +- **Tasks:** 2 (each TDD: RED commit + GREEN commit) +- **Files modified:** 4 (schema.ts, migration SQL, lists.ts, lists.test.ts) + +## Accomplishments + +- `list_items.rank` now carries `COLLATE utf8mb4_bin` — uppercase fractional-indexing ranks (`Zz`) sort before lowercase ranks (`a0`) in DB `ORDER BY`, matching JS string order. Drag-to-top persists across refetch. +- Migration `0002_yielding_mattie_franklin.sql` is a single non-destructive `ALTER TABLE list_items MODIFY COLUMN rank varchar(255) COLLATE utf8mb4_bin NOT NULL` — no DROP, no TRUNCATE, no length or nullability change. Applied via `db:migrate` (never `db:push`). +- `PATCH /api/lists/:id` now returns `403` when a non-owner sharee sends `{ isShared }`, and `list_shares` is never mutated by a sharee. Threats T-04-08 and T-04-05 closed. +- 3 new regression tests added (collation regression + 2 sharee-403 paths). Full suite: 184 tests, 0 failures (was 181). + +## Task Commits + +1. **Task 1 RED — collation regression test** - `ece663d` (test) +2. **Task 1 GREEN — schema + migration** - `9b86061` (feat) +3. **Task 2 RED — sharee-403 tests** - `931f767` (test) +4. **Task 2 GREEN — owner-only guard** - `c0bd6d7` (fix) + +## Files Created/Modified + +- `apps/api/src/db/migrations/0002_yielding_mattie_franklin.sql` — New additive migration: ALTER TABLE list_items MODIFY rank to COLLATE utf8mb4_bin +- `apps/api/src/db/schema.ts` — Added `varcharBin` customType factory; replaced `listItems.rank` from `varchar('rank', { length: 255 })` to `varcharBin('rank').notNull()`; added `customType` to imports +- `apps/api/src/routes/lists.ts` — Added owner-only guard (`if (patch.isShared !== undefined && !access.isOwner) return 403`) after access check; updated stale comment on the reconciliation block +- `apps/api/tests/routes/lists.test.ts` — Added collation regression test in reorder describe block; added two T-04-08 tests in PATCH describe block + +## Decisions Made + +- **D-04-07-collation:** Drizzle 0.45.x does not expose a `collation` option on `varchar`. Used `customType` from `drizzle-orm/mysql-core` to define a `varcharBin` factory that emits `varchar(255) COLLATE utf8mb4_bin` as the SQL type string. This keeps schema-as-code and lets `db:generate` produce the correct `MODIFY COLUMN` statement. +- **D-04-07-guard-placement:** The guard is placed immediately after the `if (!access.allowed)` block and before `updateValues` construction — ensuring neither the `isShared` write nor the reconciliation block runs for non-owners. + +## Deviations from Plan + +None — plan executed exactly as written. The `customType` approach for collation was anticipated by the plan's guidance ("add the utf8mb4_bin collation via drizzle's column collation option"), and `customType` is the correct mechanism when drizzle's built-in types lack a first-class option. + +## Must-Haves Verification + +| Must-Have | Status | +|-----------|--------| +| listItems.rank gets explicit COLLATE utf8mb4_bin with a migration | PASS — migration 0002; DB reports utf8mb4_bin via information_schema | +| PATCH isShared reconciliation runs ONLY for the list owner (access.isOwner === true) | PASS — guard at lists.ts:336 | +| Non-owner sharee sending { isShared } receives 403, list_shares never mutated | PASS — T-04-08 tests assert 403 + unchanged shares | +| Regression test for rank collation + negative sharee-403 test | PASS — 3 new tests in lists.test.ts | + +## Issues Encountered + +- MySQL client (`mysql`) is not installed on the dev host. Verified live DB collation via `node --input-type=module` with direct `mysql2` connection instead of the CLI. Result was confirmed: `[{"COLUMN_NAME":"rank","COLLATION_NAME":"utf8mb4_bin"}]`. + +## User Setup Required + +None — migration is applied automatically via `db:migrate`. The dev MariaDB was migrated in-place during execution. + +## Next Phase Readiness + +- Phase 4 is now complete: all 14 security threats closed, LIST-03 gap resolved, full suite green (184/184). +- 04-SECURITY.md can be updated to `threats_open: 0`. +- 04-VERIFICATION.md LIST-03 gap entry can be marked resolved. +- Phase 5 (push notifications) is unblocked. + +## Self-Check: PASSED + +All files found. All commits verified. + +--- +*Phase: 04-shared-lists-live-sync* +*Completed: 2026-06-09*