Files
familysync/.planning/phases/04-shared-lists-live-sync/04-SECURITY.md
T

8.5 KiB
Raw Blame History

phase, slug, status, threats_open, asvs_level, created
phase slug status threats_open asvs_level created
4 shared-lists-live-sync issues_found 1 1 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 is additive (CREATE TABLE / ADD CONSTRAINT / CREATE INDEX), 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 — but isShared reconciliation in PATCH is not owner-gated (see T-04-08) open
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 claim: "shares server-managed only; no client-writable shares endpoint" — DEFEATED: PATCH isShared toggle mutates list_shares for any sharee open
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)


Open Threat Detail

T-04-08 (BLOCKER) — Sharee can rewrite list_shares via PATCH isShared

File: apps/api/src/routes/lists.ts:319-393

The declared mitigation for T-04-08 is "shares are server-managed only — no client-writable shares endpoint." Verification of the PATCH handler shows this is defeated:

  • The handler gates only on checkListAccess (lists.ts:327-332), which returns allowed:true for an owner OR any sharee.
  • The isShared reconciliation block (lists.ts:344-369) runs unconditionally for any allowed user — there is no access.isOwner guard. The inline comment at line 344 ("owner only affects shares") asserts a guard that does not exist in code.
  • A non-owner sharee sending { isShared: false } reaches lists.ts:365-367db.delete(listShares).where(eq(listShares.listId, listId)) — deleting all share rows for the list, revoking every other member's access (an availability + integrity attack on the owner's sharing state).
  • A non-owner sharee sending { isShared: true } reaches lists.ts:348-362 → inserts a list_shares row for every other user in the DB without the owner's consent.

This is a client-writable path that mutates list_shares, contradicting the T-04-08 claim, and is an Elevation-of-Privilege gap also touching T-04-05 (a sharee performs an owner-only sharing mutation). It is independently documented as CR-01 in 04-REVIEW.md.

Required fix (implementation — not applied by this audit): add an owner-only guard before the isShared write/reconciliation, e.g. after the access check at lists.ts:327-332:

if (patch.isShared !== undefined && !access.isOwner) {
  return c.json({ error: 'Only the list owner can change sharing settings' }, 403)
}

Add the corresponding negative test (tests/routes/lists.test.ts) asserting a sharee receives 403 when toggling isShared (currently uncovered — WR-04).

T-04-05 is marked open only because of this shared root cause; the direct-id access path for GET/POST/PATCH-text/DELETE on lists and items is correctly gated and tested.


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

Sign-Off

  • All threats have a disposition (mitigate / accept / transfer)
  • Accepted risks documented in Accepted Risks Log
  • threats_open: 0 confirmed — 1 open (T-04-08 / T-04-05 root cause)
  • status: verified set in frontmatter

Approval: pending — blocked on T-04-08 (CR-01) implementation fix + negative test