Files
familysync/.planning/phases/04-shared-lists-live-sync/04-SECURITY.md
T
Lucas Berger fa71cf1a30 docs(04): re-verify phase 04 after gap-closure — 4/4 verified, 0 threats open
LIST-03 collation gap closed, T-04-05 + T-04-08 closed by plan 04-07.
Phase 04 sign-off complete.
2026-06-09 14:32:54 -04:00

8.1 KiB
Raw Blame History

phase, slug, status, threats_open, asvs_level, created, closed
phase slug status threats_open asvs_level created closed
4 shared-lists-live-sync verified 0 1 2026-06-09 2026-06-09

Phase 4 — Security

Per-phase security contract: threat register, accepted risks, and audit trail. Register authored at plan time (register_authored_at_plan_time: true); this audit VERIFIES each declared mitigation against implemented code — it does not scan for new threat classes.


Trust Boundaries

Boundary Description Data Crossing
Browser → API (/api/*) OIDC session cookie (Authelia) or dev-bypass; all list/item routes gated List names, item text, sharing state
API → MariaDB Drizzle parameterized queries (mysql2) List/item/share rows
API → SSE clients GET /api/sse/lists per-list-channel fan-out, scoped by getAccessibleListIds Minimal {type, listId, payload} event envelopes
npm registry → build New deps (react-router, dnd-kit, fractional-indexing) installed during phase Third-party source

Threat Register

Threat ID Category Component Disposition Mitigation Status
T-04-01 Tampering drizzle-kit push truncating populated tables mitigate generate+migrate only; 0001_lists_schema.sql and 0002_yielding_mattie_franklin.sql are additive (CREATE TABLE / ALTER TABLE MODIFY), no DROP/TRUNCATE closed
T-04-01b Spoofing/AuthZ unauthenticated SSE subscription mitigate resolveUserId → 401; endpoint behind OIDC middleware; client withCredentials closed
T-04-02 Information Disclosure scoped fan-out leak (D-04) — load-bearing mitigate per-list channel list:${listId} + getAccessibleListIds; GET /api/lists scoped closed
T-04-03 Information Disclosure getAccessibleListIds over-returning ids mitigate scoped to owner_id OR list_shares.userId; deduped via Set closed
T-04-04 Denial of Service EventEmitter max-listeners accept setMaxListeners(200) headroom closed (accepted)
T-04-05 Elevation of Privilege accessing/mutating another member's list via direct id mitigate checkListAccess on every list + item handler; DELETE list owner-only; 403 otherwise; isShared reconciliation now owner-gated at lists.ts:336 (plan 04-07) closed
T-04-06 Tampering XSS via list name / item text mitigate plain-text JSX children only; no dangerouslySetInnerHTML in ListCard/ItemRow closed
T-04-07 Tampering overposting on PATCH mitigate zod patchListSchema (name/isShared) + patchItemSchema exactly-one-of(checked/text/position) closed
T-04-08 Elevation of Privilege self-adding to / manipulating list_shares mitigate Owner-only guard at lists.ts:336: if (patch.isShared !== undefined && !access.isOwner) return 403. A non-owner sharee can no longer delete or insert list_shares via PATCH { isShared }. Verified by two negative tests (lists.test.ts:452, lists.test.ts:477): sharee → 403 + list_shares unchanged. (plan 04-07) closed
T-04-09 Tampering resurrecting a deleted item via in-flight edit (D-09) mitigate DELETE final; PATCH fetches row first, 404 if missing; no upsert path closed
T-04-10 Denial of Service pathological zipper inserts growing rank accept VARCHAR(255) headroom; fractional-indexing graceful degradation closed (accepted)
T-04-11 Denial of Service EventSource reconnect storm mitigate es.close() before setTimeout; bounded backoff; give up after MAX_ATTEMPTS=6 closed
T-04-12 Information Disclosure over-broad SSE event payload mitigate payload is {type, listId, payload:{id,...}} minimal; per-channel scoped closed
T-04-SC Tampering npm supply chain (react-router, dnd-kit, fractional-indexing) mitigate RESEARCH legitimacy audit + blocking human checkpoint (04-01 Task 1) before install closed

Status: open · closed Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)


Closed Threat Detail (Plan 04-07)

T-04-08 — Sharee can rewrite list_shares via PATCH isShared — CLOSED

Closed by: plan 04-07 (c0bd6d7) File: apps/api/src/routes/lists.ts:334-338

The owner-only guard was added immediately after the checkListAccess block and before any updateValues construction:

// T-04-08 / T-04-05: owner-only guard for isShared mutations.
// A sharee may rename a list (patch.name) but must never mutate list_shares.
if (patch.isShared !== undefined && !access.isOwner) {
  return c.json({ error: 'Only the list owner can change sharing settings' }, 403)
}

Test coverage (WR-04 now covered):

  • lists.test.ts:452 — sharee sends { isShared: false } → 403; list_shares row still exists (length 1). Proves the db.delete(listShares) path is unreachable for non-owners.
  • lists.test.ts:477 — sharee sends { isShared: true } → 403; share count unchanged. Proves the db.insert(listShares) path is unreachable for non-owners.
  • Pre-existing owner-toggle tests (false→true, true→false) and sharee-rename test continue to pass.

T-04-05 — isShared reconciliation runs for any allowed user — CLOSED

The shared root cause with T-04-08 (no access.isOwner guard on the reconciliation block) is resolved by the same guard. All other T-04-05 paths (GET/POST-item/PATCH-text/DELETE gated via checkListAccess) were already correct and remain so.


Audit Observations (non-blocking, from 04-REVIEW.md)

These are not declared threats in the register; recorded for traceability. They do not change any threat disposition under block_on: high.

  • CR-03 — Number(c.req.param(...)) → NaN unguarded (lists.ts:323,407,459,521,569,663). Invalid path params (/api/lists/abc) coerce to WHERE id = NaN (MariaDB → effectively 0) rather than returning 400. Behavior is benign-by-accident (empty/404 responses) and does not defeat any declared mitigation (access checks still run against a non-matching id), so it is not a BLOCKER here — but it is fragile input handling that should be hardened with an Number.isInteger guard. Does not open a new threat class.
  • CR-02 — stale SSE subscription scope (sse.ts:85-121): availability/UX gap (newly shared lists not delivered live until poll fallback), not a confidentiality leak — does not affect T-04-02 (scope is computed correctly, just not refreshed). Non-security.
  • WR-01/WR-02 — async SSE listener + cleanup-after-loop (sse.ts:96-119): listener-leak / unhandled-rejection robustness. Relevant to T-04-04 DoS posture but within the accepted setMaxListeners(200) envelope; not a register threat.
  • WR-03 — position accepts any 1..255 string (lists.ts:113): no fractional-indexing format validation. T-04-07 (overposting / field whitelist) is still satisfied — exactly-one-field refine holds. Malformed-rank robustness is an integrity hardening item, not the declared threat.

Accepted Risks Log

Risk ID Threat Ref Rationale Accepted By Date
AR-04-04 T-04-04 Single-process household app; setMaxListeners(200) (100 members × 2 devices) is generous headroom; Redis fan-out deferred (D-18) Plan (04-02) 2026-06-09
AR-04-10 T-04-10 VARCHAR(255) rank headroom; fractional-indexing degrades gracefully; rebalance via generateNKeysBetween available if ever needed (out of scope) Plan (04-05) 2026-06-09

Accepted risks do not resurface in future audit runs.


Security Audit Trail

Audit Date Threats Total Closed Open Run By
2026-06-09 14 13 1 gsd-security-auditor
2026-06-09 14 14 0 gsd-verifier (re-verification after plan 04-07)

Sign-Off

  • All threats have a disposition (mitigate / accept / transfer)
  • Accepted risks documented in Accepted Risks Log
  • threats_open: 0 confirmed
  • status: verified set in frontmatter

Approval: APPROVED — all 14 threats closed; T-04-08 and T-04-05 closed by plan 04-07 owner guard + negative tests.