From f0aa901f573acefdb224642836f4131ab0dc1e5d Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 18 Jun 2026 20:21:27 -0400 Subject: [PATCH] docs(phase-20): add security threat verification --- .../20-SECURITY.md | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 .planning/phases/20-admin-member-editor-form-declutter/20-SECURITY.md diff --git a/.planning/phases/20-admin-member-editor-form-declutter/20-SECURITY.md b/.planning/phases/20-admin-member-editor-form-declutter/20-SECURITY.md new file mode 100644 index 0000000..0b799fa --- /dev/null +++ b/.planning/phases/20-admin-member-editor-form-declutter/20-SECURITY.md @@ -0,0 +1,71 @@ +--- +phase: 20 +slug: admin-member-editor-form-declutter +status: verified +threats_open: 0 +asvs_level: 1 +created: 2026-06-18 +--- + +# Phase 20 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. +> Result: **SECURED** — 10/10 threats CLOSED. `register_authored_at_plan_time: true` (verify-only; no new-threat scan). + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| client → /api/admin | Untrusted admin-session input crosses into the admin surface; guarded by router-wide `requireAdmin` (`admin.ts:48`). No NEW boundary added by this phase. | Member profile fields, `isAdmin` toggle, app-password credential | +| PWA → /api/admin | Client fetch (`updateMemberProfile`, `saveCredential`) calls into the admin surface; server-side `requireAdmin` + last-admin guard are the real boundaries. Client toggle state is non-authoritative. | Same as above; password fields are write-only | + +--- + +## Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation | Status | +|-----------|----------|-----------|-------------|------------|--------| +| T-20-01 | Elevation of Privilege | PATCH /members/:id isAdmin toggle | mitigate | Router-wide `requireAdmin` first statement (`admin.ts:48`); PATCH adds no second/weaker guard (`:234`); 403 test (`admin.test.ts:1180-1190`) | closed | +| T-20-02 | Denial of Service (self-lockout) | last-admin demotion | mitigate | Last-admin 409 guard, count+update in txn with `FOR UPDATE` (`admin.ts:271-293`); 409 test (`:1128-1150`), self-demote-with-2nd-admin 200 test (`:1153-1177`) | closed | +| T-20-03 | Tampering | malformed :id / wrong-type body | mitigate | `parsePositiveIntParam` rejects bad ids → 400 (`admin.ts:88-93,235`); `updateMemberSchema` + `noEchoHook` reject wrong types → 400 (`:225-234,76`); test F (`:1193-1216`) | closed | +| T-20-04 | Information Disclosure | error echo on invalid input | mitigate | `noEchoHook` returns only `{ error: 'Invalid request' }` (`admin.ts:76-80`); request body never logged (`:241,297-300`) | closed | +| T-20-05 | Spoofing (stale session) | updateMemberProfile fetch | mitigate | `SessionExpiredError` on 401/opaqueredirect reuses existing re-auth flow (`client.ts:261`); `redirect:'manual'` + `credentials:'include'` (`:254-258`) | closed | +| T-20-06 | Elevation of Privilege (client trust) | last-admin sentinel | accept | Server 409 authoritative (`admin.ts:271-293`); client only surfaces `'last-admin'` sentinel (`client.ts:262`). See Accepted Risks Log. | closed | +| T-20-07 | Information Disclosure | password / app-password fields | mitigate | Fields write-only: `type=password` + `autoComplete="new-password"`, blank init (`MemberEditorSheet.tsx:188-205,639-882`); no `console.*` logging (0 grep matches) | closed | +| T-20-08 | Tampering | CalDAV credential | mitigate | App-password save routes through `saveCredential` → server-side CalDAV validation before store (`MemberEditorSheet.tsx:338`, `admin.ts:373-393`); invalid → failure copy, nothing stored (`:355`) | closed | +| T-20-09 | Elevation of Privilege (UI bypass) | admin toggle | mitigate | Toggle cosmetic `role=switch` (`MemberEditorSheet.tsx:546`); reverts + inline error on `last-admin` (`:288-295`); real enforcement is server 409 (T-20-02) | closed | +| T-20-SC | Tampering (supply chain) | npm installs | mitigate | No new packages; `lucide-react@1.17.0` already in `package.json:30`; imports `AdminPage.tsx:28`, `MemberEditorSheet.tsx:25-27` | 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-20-01 | T-20-06 | Client-side last-admin toggle state is non-authoritative by design. The demotion guard is enforced server-side (409, `admin.ts:271-293`); the client only surfaces the rejection via the `'last-admin'` sentinel (`client.ts:262`) and reverts the toggle (`MemberEditorSheet.tsx:294`). A tampered client that ignores the sentinel still cannot bypass the guard — the server rejects regardless. Matches existing pattern: "isAdmin drives nav visibility; the real boundary is server-side." Residual risk: none beyond the already-mitigated server boundary at ASVS L1. | Lucas Berger (per Plan 20-02 threat model) | 2026-06-18 | + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-06-18 | 10 | 10 | 0 | gsd-security-auditor | + +**Notable hardening beyond plan:** the last-admin guard wraps count+update in a transaction with a `FOR UPDATE` locking read (`admin.ts:249-286`) to defeat a concurrent double-demotion race — strengthens T-20-02 past the plan minimum. + +--- + +## 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-18