familysync/.planning/milestones/v1.0-phases/04-shared-lists-live-sync/04-07-PLAN.md

phase, plan, type, wave, depends_on, files_modified, autonomous, gap_closure, requirements, must_haves

phase	plan	type	wave	depends_on	files_modified	autonomous	gap_closure	requirements	must_haves
04-shared-lists-live-sync	07	tdd	6	04-03 04-05	apps/api/src/db/schema.ts apps/api/src/db/migrations apps/api/src/routes/lists.ts apps/api/tests/routes/lists.test.ts	true	true	LIST-03	truths artifacts key_links A member can drag an active item to a new position and the order persists (reorder via drag-to-top) — closes LIST-03 gap T-04-08 closed: a non-owner sharee sending { isShared } to PATCH /api/lists/:id receives 403; list_shares is never mutated by a sharee T-04-05 closed: the isShared reconciliation block runs only for the list owner (access.isOwner === true) path provides contains apps/api/src/db/schema.ts listItems.rank column with explicit COLLATE utf8mb4_bin utf8mb4_bin path provides contains apps/api/src/routes/lists.ts owner-only guard before isShared reconciliation in PATCH /:id access.isOwner path provides apps/api/tests/routes/lists.test.ts rank-collation regression test + sharee-403 negative test from to via pattern apps/api/src/routes/lists.ts PATCH /:id list_shares reconciliation block owner-only guard returning 403 for non-owner isShared writes access.isOwner from to via pattern apps/api/src/db/schema.ts listItems.rank MariaDB list_items.rank column generate+migrate ALTER TABLE ... MODIFY rank ... COLLATE utf8mb4_bin utf8mb4_bin

Close the two open gaps blocking Phase 4 sign-off:

1. LIST-03 drag-to-top (rank collation) — list_items.rank inherited the case-insensitive DB default collation (utf8mb4_uca1400_ai_ci). fractional-indexing emits uppercase-prefixed keys (e.g. Zz) on drag-to-top, which MariaDB sorts AFTER lowercase a… ranks even though JS sorts it BEFORE. The dragged item snaps to the bottom on refetch. Fix: migrate the column to COLLATE utf8mb4_bin so DB ORDER BY rank matches JS string order.

2. T-04-08 / T-04-05 (security BLOCKER) — The PATCH /:id isShared reconciliation block runs for ANY allowed user, including sharees. A non-owner sharee can delete every share row (isShared:false) or inject shares for all users (isShared:true). Fix: add an owner-only guard returning 403 when a non-owner sends isShared.

Both gaps are TDD: known-failing behavior with a defined assertion. Each feature follows RED → GREEN.

Purpose: Achieve threats_open: 0 in 04-SECURITY.md and full LIST-03 satisfaction in 04-VERIFICATION.md. Output: One additive migration SQL file, one schema collation edit, one owner-only guard, two new test cases.

DO NOT modify or replan 04-01 through 04-06 — they are VERIFIED. This plan adds NEW behavior and tests only.

<execution_context> @$HOME/.claude/gsd-core/workflows/execute-plan.md @$HOME/.claude/gsd-core/templates/summary.md </execution_context>

@.planning/STATE.md @.planning/ROADMAP.md @.planning/REQUIREMENTS.md @.planning/phases/04-shared-lists-live-sync/04-VERIFICATION.md @.planning/phases/04-shared-lists-live-sync/04-SECURITY.md @.planning/phases/04-shared-lists-live-sync/04-CONTEXT.md @apps/api/src/db/schema.ts @apps/api/src/db/migrations/0001_lists_schema.sql @apps/api/src/routes/lists.ts @apps/api/tests/routes/lists.test.ts @apps/api/drizzle.config.ts @apps/api/package.json

<hard_constraints>

• MariaDB only. NEVER drizzle-kit push (pnpm db:push). push emits a false destructive diff that truncates populated tables. Use pnpm --filter @familysync/api db:generate to emit the migration SQL, then pnpm --filter @familysync/api db:migrate to apply it. The schema-push gate's default push task is OVERRIDDEN for this phase.
• The new migration MUST be a non-destructive ALTER TABLE ... MODIFY — NO DROP, NO TRUNCATE. Preserve varchar(255), NOT NULL, and existing default/index semantics exactly.
• API integration tests live in apps/api/tests/ (NEVER src/) and run against the real dev MariaDB. The regression test MUST exercise the real DB so it observes the column's actual collation, not JS comparison.
• Test run prelude (matches the file header at lists.test.ts:7-10): set -a; . ./apps/api/.env 2>/dev/null; set +a; export DB_HOST=127.0.0.1 DB_PORT=3306. Drizzle-kit reads the same DB_* env vars (see drizzle.config.ts).
• <action> blocks below name identifiers and behavior only — no fenced code blocks / full implementations. </hard_constraints>

Task 1: RED → GREEN — rank-collation drag-to-top regression (LIST-03) apps/api/tests/routes/lists.test.ts, apps/api/src/db/schema.ts, apps/api/src/db/migrations/ - apps/api/tests/routes/lists.test.ts:919-982 — existing reorder describe block + seed helpers (`seedUser`, `seedList`, `seedItem`, `getApp`, `jsonRequest`, `currentDevUserId`). The test at line 930-932 explicitly sidesteps this bug with the comment "avoids collation issues with uppercase ranks". - apps/api/src/db/schema.ts:222-241 — `listItems` table; `rank` is `varchar('rank', { length: 255 }).notNull()` at line 231 with no `.$type`/collation. - apps/api/src/db/migrations/0001_lists_schema.sql:26-35 — existing additive CREATE TABLE style; the new migration must follow the same `--> statement-breakpoint` format drizzle-kit emits. - apps/api/drizzle.config.ts — `out: './src/db/migrations'`, `dialect: 'mysql'`; confirms generate writes here and reads `DB_*` env. - apps/api/package.json:14-15 — `db:generate` and `db:migrate` scripts. - 04-VERIFICATION.md gap (frontmatter `gaps:` + "Measured divergence"): `SELECT ('Zz' < 'a0')` returns `0` under the current collation but `('Zz' < 'a0' COLLATE utf8mb4_bin)` returns `1`. RED — Add a regression test inside the existing `describe('PATCH /api/list-items/:id { position } — reorder ordering (LIST-03, D-13)')` block in `lists.test.ts`. Title it to name the bug (e.g. "drag-to-top: uppercase-prefixed rank sorts above lowercase ranks (LIST-03 collation regression)"). The test must: - seed an owner, set `currentDevUserId`, seed a private list; - seed two active items where the FIRST has a lowercase rank (e.g. `a0`) and a SECOND item; - simulate drag-to-top of the second item by PATCHing `/api/list-items/:id` with `{ position: 'Zz' }` (the uppercase-prefixed key `fractional-indexing`'s `generateKeyBetween(null, 'a0')` produces when prepending before the first item — assert `'Zz' < 'a0'` is `true` in JS first to document intent); - GET `/api/lists/:listId/items` and assert the dragged item (`rank: 'Zz'`) is returned FIRST (index 0), matching JS string order. Run the test BEFORE the schema change and confirm it FAILS (the item lands last) — this is the RED proof. Do not weaken the assertion to make it pass in JS; it must hit the real DB `ORDER BY rank`.

GREEN (schema) — In `schema.ts`, change the `listItems.rank` column so it carries an explicit binary collation. Preserve `varchar` length `255` and `.notNull()` exactly; add the `utf8mb4_bin` collation via drizzle's column collation option for the mysql varchar type. Do NOT touch any other column, index, or table.

GREEN (migrate — [BLOCKING], must run before the test passes) — From repo root, with the env prelude loaded, run `pnpm --filter @familysync/api db:generate`. Inspect the newly emitted SQL file under `apps/api/src/db/migrations/` (next sequential number, e.g. `0002_*.sql`): it MUST be a single non-destructive `ALTER TABLE list_items MODIFY ... rank varchar(255) ... COLLATE utf8mb4_bin NOT NULL` (or drizzle's equivalent MODIFY/CHANGE form) with NO DROP/TRUNCATE and NO change to length or nullability. If generate emits anything destructive, STOP and report — do not edit the SQL by hand to hide it. Then apply with `pnpm --filter @familysync/api db:migrate`. NEVER run `db:push`.

After migrate, re-run the regression test — it now passes because DB `ORDER BY rank` under `utf8mb4_bin` matches JS order.

set -a; . ./apps/api/.env 2>/dev/null; set +a; export DB_HOST=127.0.0.1 DB_PORT=3306; pnpm --filter @familysync/api exec vitest run tests/routes/lists.test.ts -t "collation regression" - The new test exists in the LIST-03 reorder describe block and asserts the `'Zz'`-ranked item is returned at index 0 from GET items. - A new migration file exists under `apps/api/src/db/migrations/` whose body is an `ALTER TABLE list_items` MODIFY/CHANGE statement containing `utf8mb4_bin`, with zero occurrences of `DROP` or `TRUNCATE` (verify: `grep -ciE 'drop|truncate' apps/api/src/db/migrations/0002_*.sql` returns `0`). - `apps/api/src/db/schema.ts` line for `rank` contains `utf8mb4_bin` (verify: `grep -c 'utf8mb4_bin' apps/api/src/db/schema.ts` returns `>= 1`). - Live DB confirms the fix: a query of `information_schema.columns` for `list_items.rank` reports collation `utf8mb4_bin`. - The full reorder describe block (including the pre-existing a0–a5 tests) still passes — no regression. Drag-to-top persists: an uppercase-prefixed rank now sorts above lowercase ranks in the DB, matching JS order. LIST-03 gap closed; migration is additive (generate+migrate, no push). Task 2: RED → GREEN — owner-only guard on PATCH isShared (T-04-08 / T-04-05) apps/api/tests/routes/lists.test.ts, apps/api/src/routes/lists.ts - apps/api/src/routes/lists.ts:319-393 — PATCH `/:id` handler. `checkListAccess` (line 327) returns `{ allowed: true, isOwner: boolean, listRow }` for owner OR sharee. The `isShared` reconciliation block (lines 344-369) runs unconditionally for any allowed user. The DELETE handler at line 418 already uses `if (!access.isOwner)` as the exact guard idiom to mirror. - apps/api/src/routes/lists.ts:121-152 — `checkListAccess` return shape; `isOwner` is the authoritative owner flag (true only when `listRow.ownerId === currentUserId`). - apps/api/tests/routes/lists.test.ts:364-406, 438-450 — existing isShared toggle tests (all run as OWNER) and the "sharee can rename" test. There is NO test where a sharee toggles `isShared` — that path (WR-04) is uncovered; the existing 403-patch test (397-406) uses a non-sharee, caught earlier by `checkListAccess`. - 04-SECURITY.md "Open Threat Detail" — the exact required guard and its placement (after the access check at lines 327-332, before the reconciliation). RED — Add a negative test in the PATCH describe block of `lists.test.ts`. Title it for the threat (e.g. "T-04-08: sharee sending { isShared } gets 403 and list_shares is unchanged"). It must: - seed an owner and a sharee, seed a SHARED list (`isShared: true`), `shareList(listId, shareeId)`; - set `currentDevUserId = shareeId`; - PATCH `/api/lists/:id` with `{ isShared: false }` and assert status `403`; - assert the response body error mentions owner/sharing (the guard's message); - assert `list_shares` for the list is UNCHANGED — the sharee row still exists (query `listShares` where `listId` and `userId = shareeId`, expect length `1`). This proves the destructive delete did not run. Add a second assertion path (same or sibling test): a sharee sending `{ isShared: true }` on a private-but-shared scenario likewise gets `403` and inserts no new shares. Run before the fix and confirm it FAILS (currently 200 + shares wiped) — RED proof.

Preserve the existing owner-path tests at lines 364-395: they must still pass (owner toggling isShared continues to work).

GREEN — In `lists.ts`, immediately after the access check (the `if (!access.allowed)` block ending ~line 332) and BEFORE any update/reconciliation, add an owner-only guard: when `patch.isShared !== undefined && !access.isOwner`, return `c.json({ error: 'Only the list owner can change sharing settings' }, 403)`. This blocks both the `updateValues.isShared` write and the reconciliation block for non-owners. A sharee may still PATCH `{ name }` (the rename test at 438-450 must stay green). Update the stale inline comment at line 344 ("owner only affects shares") so it reflects the now-real guard rather than asserting a guard that didn't exist.

set -a; . ./apps/api/.env 2>/dev/null; set +a; export DB_HOST=127.0.0.1 DB_PORT=3306; pnpm --filter @familysync/api exec vitest run tests/routes/lists.test.ts -t "isShared" - New test asserts a sharee PATCHing `{ isShared: false }` receives HTTP `403` AND the sharee's `list_shares` row still exists afterward (length `1`). - New test asserts a sharee PATCHing `{ isShared: true }` receives `403` and no new shares are inserted. - `apps/api/src/routes/lists.ts` PATCH handler contains a guard referencing `access.isOwner` and `patch.isShared` that returns 403 (verify: `grep -n "patch.isShared !== undefined && !access.isOwner" apps/api/src/routes/lists.ts` returns a match before line 342). - Existing owner-path isShared toggle tests (false→true, true→false) and the sharee-rename test still pass. - Full API suite green: `pnpm --filter @familysync/api exec vitest run` reports 0 failures. A non-owner sharee can no longer mutate list_shares via PATCH isShared; T-04-08 and T-04-05 are closed. The owner-only sharing-mutation invariant is enforced and regression-tested (WR-04 now covered).

<threat_model>

Trust Boundaries

Boundary	Description	Data Crossing
Browser → API (PATCH /api/lists/:id)	OIDC session cookie (Authelia) or dev-bypass; caller may be owner OR sharee	{ name, isShared } patch body
API → MariaDB	Drizzle parameterized queries (mysql2); list_shares mutated on visibility change	list_shares delete/insert rows

STRIDE Threat Register

Threat ID	Category	Component	Disposition	Mitigation Plan
T-04-08	Elevation of Privilege	PATCH /api/lists/:id isShared reconciliation (lists.ts:344-369)	mitigate	Owner-only guard after access check: if (patch.isShared !== undefined && !access.isOwner) return 403. A sharee can no longer delete/insert list_shares. Verified by negative test asserting 403 + unchanged shares.
T-04-05	Elevation of Privilege	sharee performing owner-only sharing mutation via direct id	mitigate	Same owner-only guard closes the shared root cause; sharee retains read + name-edit + item-edit access (already gated/tested), but is blocked from the owner-only sharing mutation.
T-04-SC	Tampering	npm/pnpm installs during this plan	accept	This plan installs NO new packages (schema collation + route guard + tests only). No supply-chain surface added.

</threat_model>

Phase-level checks after both tasks:

1. Full API suite (real DB): set -a; . ./apps/api/.env 2>/dev/null; set +a; export DB_HOST=127.0.0.1 DB_PORT=3306; pnpm --filter @familysync/api exec vitest run → 0 failures (was 181 passing; now 183+ with two new cases).
2. Migration is additive: grep -ciE 'drop|truncate' apps/api/src/db/migrations/0002_*.sql → 0.
3. Collation applied in DB: query information_schema.columns for list_items.rank → collation utf8mb4_bin.
4. No push used: confirm the change was applied via db:migrate (a new numbered SQL file exists in apps/api/src/db/migrations/), not db:push.
5. Typecheck/build clean: pnpm --filter @familysync/api typecheck.

<success_criteria>

• LIST-03 drag-to-top persists across refetch (uppercase-prefixed rank sorts correctly) — verified by the collation regression test against the real DB.
• T-04-08 and T-04-05 closed: a non-owner sharee receives 403 on PATCH { isShared } and list_shares is untouched — verified by the negative test.
• The rank column carries COLLATE utf8mb4_bin in both schema.ts and the live DB, applied via a non-destructive generate+migrate (no push, no DROP/TRUNCATE).
• All pre-existing Phase 4 tests still pass (181 prior API tests + new cases; no regression).
• 04-SECURITY.md can move to threats_open: 0; 04-VERIFICATION.md LIST-03 gap resolved. </success_criteria>

Artifacts this phase produces

Artifact	Type	Detail
apps/api/src/db/migrations/0002_*.sql (next sequential number)	NEW migration	ALTER TABLE list_items MODIFY rank to COLLATE utf8mb4_bin; additive, no DROP/TRUNCATE
apps/api/src/db/schema.ts — listItems.rank collation	EDIT	varchar('rank', { length: 255 }) gains explicit utf8mb4_bin collation; length/notNull preserved
apps/api/src/routes/lists.ts — owner-only isShared guard	NEW guard	if (patch.isShared !== undefined && !access.isOwner) return c.json({ error: 'Only the list owner can change sharing settings' }, 403) after access check, before reconciliation
lists.test.ts — "collation regression" test (LIST-03)	NEW test	seeds Zz rank via drag-to-top PATCH; asserts GET returns it at index 0
lists.test.ts — "T-04-08 sharee 403" test	NEW test	sharee PATCH { isShared } → 403; list_shares unchanged (false→ and true→ paths)

Create `.planning/phases/04-shared-lists-live-sync/04-07-SUMMARY.md` when done.