docs(phase-05): add security threat verification (26/26 closed)
This commit is contained in:
Lucas Berger
@@ -0,0 +1,103 @@
|
||||
---
|
||||
phase: 05
|
||||
slug: web-push-notifications
|
||||
status: verified
|
||||
threats_open: 0
|
||||
asvs_level: 1
|
||||
created: 2026-06-10
|
||||
---
|
||||
|
||||
# Phase 05 — Security
|
||||
|
||||
> Per-phase security contract: threat register, accepted risks, and audit trail.
|
||||
>
|
||||
> **Audit type:** Threat-mitigation verification (declared dispositions only — not a blind vulnerability scan). The register was authored at plan time across the eight `05-0N-PLAN.md` `<threat_model>` blocks. Each threat below was verified by locating its declared mitigation in the implemented code (file:line); documentation/intent was NOT accepted as evidence. Implementation files were READ-ONLY during this audit.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| developer machine → git | VAPID private key must never cross into a committed file | VAPID private key (secret) |
|
||||
| pnpm registry → repo | package installs are untrusted supply-chain input | web-push, workbox-* packages |
|
||||
| API → push service (APNs/FCM) | server signs with VAPID private key; response status is untrusted | push payload, response status |
|
||||
| browser → POST /api/push/subscription | untrusted subscription body crosses into the API | endpoint URL, p256dh/auth keys |
|
||||
| SW → push payload | push payload from the service is untrusted input parsed in the SW | notification copy |
|
||||
| SW → /callback navigation | OIDC callback must reach the server, never the SW cache | OIDC auth code |
|
||||
| list/event mutation → push audience | audience must be derived from list/calendar access, not the request | member identity, list/event metadata |
|
||||
| reminder query → push audience | reminder eligibility is decided by the SQL WHERE, not by any request | shared-calendar event metadata |
|
||||
| client permission state → UI | `Notification.permission` + localStorage drive which surface shows; no server trust | none (client-only) |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation | Status |
|
||||
|-----------|----------|-----------|-------------|------------|--------|
|
||||
| T-05-01 | Information Disclosure | VAPID_PRIVATE_KEY | mitigate | `.env` + `apps/api/.env` gitignored (`.gitignore:10-11`); only `.env.example` tracked; `.env.example:36` is a placeholder, no real key in any tracked file | closed |
|
||||
| T-05-SC | Tampering (supply chain) | npm installs (web-push, workbox-*) | mitigate | Blocking human checkpoint executed pre-install (`05-01-SUMMARY.md:50`); deps at audited versions (web-push@^3.6.7, workbox-*@^7.4.1) | closed |
|
||||
| T-05-02 | Tampering (migration) | drizzle migration on populated MariaDB | mitigate | generate+migrate only: `db/migrations/0003_same_xavin.sql` (CREATE push_subscriptions + ADD title); no `db:push` used | closed |
|
||||
| T-05-03 | Cryptography misuse | VAPID signing | mitigate | `pushDispatcher.ts:19,104` web-push only; `index.ts:120` sole `setVapidDetails`; never hand-rolled | closed |
|
||||
| T-05-04 | Denial of Service | malformed push response / per-sub crash | mitigate | `pushDispatcher.ts:108-120` per-send try/catch; never throws — one failed send never aborts the fan-out | closed |
|
||||
| T-05-05 | Information Disclosure (logs) | error logs | mitigate | `pushDispatcher.ts:118-119` logs only statusCode + err.message; never subscription keys or payload body | closed |
|
||||
| T-05-06 | Information Disclosure | list-change copy | mitigate | `listChangeDispatcher.ts:107-115` generic copy "{Actor} made {N} change(s) to {ListName}"; no item text (D-02) | closed |
|
||||
| T-05-07 | Spoofing | actor self-notification | mitigate | `pushCoalescer.ts:39-49` keys on `${listId}:${actorId}`; `listChangeDispatcher.ts:89-91` filters `uid !== actorId` (D-03) | closed |
|
||||
| T-05-08 | Denial of Service | unbounded pending map | accept | Accepted risk (see log); per-(list,actor) keys, entries self-delete on fire (`pushCoalescer.ts:60-63`) | closed |
|
||||
| T-05-09 | Spoofing | POST /subscription (user A as user B) | mitigate | `push.ts:92-106` userId from `resolveUserId(c)` (OIDC session), never the body | closed |
|
||||
| T-05-10 | Input Validation | subscription body | mitigate | `push.ts:60-66,92` zod subscribeSchema: endpoint url().max(2048), p256dh ≤512, auth ≤256 before insert | closed |
|
||||
| T-05-11 | Tampering | SW serving /callback from cache | mitigate | `sw.ts:59-67` NavigationRoute denylist `/^\/callback/, /^\/api\//, /^\/health/` | closed |
|
||||
| T-05-12 | Denial of Service | malformed push payload in SW | mitigate | `sw.ts:86-130` try/catch around `event.data.json()`; `showNotification` runs unconditionally (generic fallback) | closed |
|
||||
| T-05-13 | Access Control | DELETE /subscription | mitigate | `push.ts:131-136` scoped `WHERE userId = caller`; cannot delete another member's subscription | closed |
|
||||
| T-05-14 | Information Disclosure | list-change push to a non-member | mitigate | `listChangeDispatcher.ts:74-101` audience = owner ∪ list_shares only; never all users | closed |
|
||||
| T-05-15 | Information Disclosure | item text in payload | mitigate | `listChangeDispatcher.ts:107-115` generic copy, no item text (same as T-05-06) | closed |
|
||||
| T-05-16 | Spoofing | actor notified of own change | mitigate | `listChangeDispatcher.ts:89-91` actor excluded from audience (same as T-05-07) | closed |
|
||||
| T-05-17 | Information Disclosure | reminder leaking a personal-calendar event | mitigate | `reminderScheduler.ts:88-95` `WHERE calendars.isShared = true` in the SQL query; personal events never selected (D-05) | closed |
|
||||
| T-05-18 | Denial of Service | duplicate reminder storm at window boundary | mitigate | `reminderScheduler.ts:38,131-132,162` in-memory dedup Set `${uid}:${minuteBucket}`; per-event try/catch | closed |
|
||||
| T-05-19 | Denial of Service | one bad subscription aborting the cycle | mitigate | `reminderScheduler.ts:145-157` per-subscription try/catch; dispatchPush swallows + prunes 410/404 | closed |
|
||||
| T-05-20 | Spoofing | actor notified of own event change | mitigate | `eventChangeDispatcher.ts:142-151` `ne(userId, actorUserId)` + `.filter`; `poller.ts:70`/`outboxWorker.ts:174` supply actor | closed |
|
||||
| T-05-21 | Denial of Service | description-edit spam | mitigate | `eventChangeDispatcher.ts:31-37,64-71` `isMeaningfulChange` excludes description-only edits (D-04) | closed |
|
||||
| T-05-22 | Information Disclosure | event-change code calling Fastmail | mitigate | `eventChangeDispatcher.ts:18-21` no tsdav import; reads MariaDB cache only (D-13) | closed |
|
||||
| T-05-23 | Tampering | silent re-subscribe without permission | mitigate | `usePushSubscription.ts:153-155` health-check re-subscribes only when `Notification.permission === 'granted'` | closed |
|
||||
| T-05-24 | Information Disclosure | XSS via copy | mitigate | Zero `dangerouslySetInnerHTML={...}` usage in `apps/pwa/src`; all copy is plain-text JSX children | closed |
|
||||
| T-05-25 | Repudiation | toggle off leaves stale server subscription | mitigate | `SettingsSheet.tsx:110-112` → `usePushSubscription.ts:245-257` `unsubscribe()` issues `DELETE /api/push/subscription` | closed |
|
||||
|
||||
*Status: open · closed*
|
||||
*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)*
|
||||
|
||||
---
|
||||
|
||||
## Accepted Risks Log
|
||||
|
||||
| Risk ID | Threat Ref | Rationale | Accepted By | Date |
|
||||
|---------|------------|-----------|-------------|------|
|
||||
| AR-05-01 | T-05-08 | In-memory `pending` Map in `pushCoalescer.ts` is keyed per-(list, actor). For a two-person household (expanding to a small N-member family) the key space is small and bounded; entries self-delete when the debounce timer fires (`pushCoalescer.ts:60-63`). No unbounded growth path under normal operation. | Plan author (`05-03-PLAN.md` threat model) | 2026-06-10 |
|
||||
|
||||
*Accepted risks do not resurface in future audit runs.*
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-06-10 | 26 | 26 | 0 | gsd-security-auditor (opus) |
|
||||
|
||||
---
|
||||
|
||||
## Notes (informational — not blockers)
|
||||
|
||||
1. **`db:push` script still present.** `apps/api/package.json:13` defines `"db:push": "drizzle-kit push"`. T-05-02 concerns the migration that was *performed* (generate+migrate via `0003_same_xavin.sql`, verified); the script's mere existence is not the threat. Repo memory `drizzle-mariadb-push-unsafe` documents the prohibition. Consider guarding/removing the script in a future hardening pass.
|
||||
2. **VAPID public key served under the `/api` OIDC guard** (`push.ts:78-80`). The public key is non-secret by design; serving it only to authenticated members is acceptable for v1 (documented in the route comment). Not a registered threat.
|
||||
3. **Reminder fan-out cross-joins ALL push_subscriptions** (`reminderScheduler.ts:87`). Intentional and member-count-agnostic: a shared-calendar reminder notifies every member. T-05-17 confirms event *selection* is shared-only via the WHERE clause, so no personal-calendar event reaches the fan-out. Correct by design.
|
||||
|
||||
---
|
||||
|
||||
## Sign-Off
|
||||
|
||||
- [x] All threats have a disposition (mitigate / accept / transfer)
|
||||
- [x] Accepted risks documented in Accepted Risks Log
|
||||
- [x] `threats_open: 0` confirmed
|
||||
- [x] `status: verified` set in frontmatter
|
||||
|
||||
**Approval:** verified 2026-06-10
|
||||
Reference in New Issue
Block a user