Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
72 lines
5.0 KiB
Markdown
72 lines
5.0 KiB
Markdown
---
|
|
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
|