docs(phase-10): add security threat verification
This commit is contained in:
149
.planning/phases/10-admin-vue-spa/10-SECURITY.md
Normal file
149
.planning/phases/10-admin-vue-spa/10-SECURITY.md
Normal file
@@ -0,0 +1,149 @@
|
||||
---
|
||||
phase: "10"
|
||||
slug: "admin-vue-spa"
|
||||
status: draft
|
||||
# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate)
|
||||
threats_open: 2
|
||||
asvs_level: 1
|
||||
created: "2026-10-12"
|
||||
---
|
||||
|
||||
# Phase 10 — Security
|
||||
|
||||
> Per-phase security contract: threat register, accepted risks, and audit trail.
|
||||
|
||||
The full evidence of the first review (tests, gate stages, removal checks) is in `10-SECURITY-REVIEW.md` (2026-09-27). This file records the threat state on master as of 2026-10-12, after Phases 12.1–17 changed the admin surface.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| Browser → `{backend.uri}/api/v1` | Untrusted requests carrying the admin cookie or a Bearer token and user-controlled headers | Session credentials, record writes |
|
||||
| Browser → `{backend.uri}/...` static | Untrusted paths resolved against the embedded dist tree | Request paths |
|
||||
| Plugin routes → admin prefix | Other compiled plugins could register paths that shadow the admin surface | Route ownership |
|
||||
| npm registry → admin/ build | Third-party packages become code in the committed dist | Build-time code |
|
||||
| SPA → options/filters endpoints | Untrusted search, paging and field/scope names select related rows | Related record labels |
|
||||
| SPA → record save | Untrusted relation ids and keys become foreign keys and pivot rows | Foreign keys, pivot rows |
|
||||
| Anonymous browser → /lang | Unauthenticated callers read the string bundle | Public UI strings |
|
||||
| Plugin YAML/override FS → boot | Plugin-supplied copy and toolbar declarations shape every schema | Configuration |
|
||||
| Admin API responses → DOM | Plugin labels, messages and record values (including stored HTML) render in the admin origin | Untrusted markup/text |
|
||||
| URL query → list requests | User-editable query parameters become list filters and sorting | Search terms |
|
||||
| Winter redirect strings → router | Plugin YAML strings choose navigation targets | URLs |
|
||||
| Browser storage | Anything persisted survives the session on a shared machine | Sidebar flag, list queries |
|
||||
| Test and gate results → phase acceptance | Gate completeness decides whether the admin surface counts as done | Acceptance evidence |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status |
|
||||
|-----------|----------|-----------|----------|-------------|------------|--------|
|
||||
| T-10-01 | Information Disclosure | cabana login/refresh cookie transport | high | mitigate | `cabana/auth.go` `cookieLoginData` (token only in HttpOnly cookie); `TestPhase10CookieAuth` | closed |
|
||||
| T-10-02 | Tampering | cookie-authenticated unsafe admin routes (CSRF) | high | mitigate | `cabana/csrf.go` `requireAjax`, SameSite=Strict; `TestPhase10CSRF` | closed |
|
||||
| T-10-03 | Information Disclosure | boardwalk static serving | medium | mitigate | `boardwalk/boardwalk.go` embedded fs + `path.Clean`; `TestTraversalIsCleaned` | closed |
|
||||
| T-10-04 | Tampering | admin HTML responses (clickjacking, sniffing, indexing) | medium | mitigate | `boardwalk/boardwalk.go:171` security headers, CSP `frame-ancestors 'none'`, X-Robots-Tag | closed |
|
||||
| T-10-05 | Spoofing | admin session cookie attributes and logout | high | mitigate | `cabana/auth.go` `sessionCookie`, `adminCookieSecure` refuses opt-out in production, `bouncer.RefreshAudienceFor` | closed |
|
||||
| T-10-06 | Elevation of Privilege | prefix and controller ID collisions | medium | mitigate | `cabana` `checkReservedSegments`; `surf/router.go:581` `checkAdminPrefix` | closed |
|
||||
| T-10-07 | Denial of Service | concurrent cookie refresh from two tabs | low | mitigate | `admin/src/api/client.ts` single-flight refresh; `blacklist_grace` | closed |
|
||||
| T-10-08 | Tampering | committed dist and generated types drift | medium | mitigate | `scripts/check-admin-dist.sh`, `scripts/check-admin-openapi.sh --check` | closed |
|
||||
| T-10-09 | Elevation of Privilege | cabana relation save (IDOR via relation ids) | high | mitigate | `cabana/relation_field.go` `checkRelationScope`; `TestPhase10RelationForgedID` | closed |
|
||||
| T-10-10 | Tampering | belongs-to mapping of protected foreign keys | high | mitigate | `assignBelongsTo` / read-only protected FK. Since 12.1-02 a field may opt in via `WritableForeignKey`; the default stays read-only | closed |
|
||||
| T-10-11 | Information Disclosure | fields/{field}/options enumeration | medium | mitigate | `protect()` before SQL, per_page 1–100, 404 for non-relation/read-only | closed |
|
||||
| T-10-12 | Information Disclosure | public /lang bundle | low | mitigate | `cabana/lang.go` serves only `backend::lang.` | closed |
|
||||
| T-10-13 | Tampering | messages and toolbar YAML | low | mitigate | Strict decoding, unknown-key rejection | closed |
|
||||
| T-10-14 | Tampering | OpenAPI document versus handler output | medium | mitigate | `TestPhase10OpenAPIConformance`, `check-admin-openapi.sh --check` | closed |
|
||||
| T-10-15 | Elevation of Privilege | filters/{scope}/options | medium | mitigate | `protect()` + scope allow-list; `TestPhase10FilterOptions` | closed |
|
||||
| T-10-16 | Tampering | SPA rendering of plugin labels, messages and record values (XSS) | high | mitigate | Planned: no raw-HTML sink in admin/src. Phase 17-02 added `v-html="display"` (`RichEditorField.vue:305`) fed by `displayHtml` (`richeditor/html.ts`), whose `clean()` can be bypassed by DOM clobbering in Chromium (see audit 2026-10-12) | open |
|
||||
| T-10-17 | Tampering | login redirect parameter (open redirect) | medium | mitigate | `safeRedirect` (exactly one leading slash) | closed |
|
||||
| T-10-18 | Elevation of Privilege | client-side hiding of actions and fields | low | accept | Server enforces permissions; see Accepted Risks | closed |
|
||||
| T-10-19 | Information Disclosure | list state in the URL | low | accept | Admin-only, Referrer-Policy same-origin; see Accepted Risks | closed |
|
||||
| T-10-20 | Tampering | Winter redirect and recordUrl strings | low | mitigate | `mapWinterUrl` maps to current-controller routes only | closed |
|
||||
| T-10-21 | Elevation of Privilege | relation link of candidates outside scope | medium | transfer | Phase 9 server contract (RelationExtendManageQuery, ExcludedRelatedIDs) | closed |
|
||||
| T-10-22 | Information Disclosure | localStorage | low | mitigate | `useSidebar.ts` flag only; `listState.ts` per-admin list queries added under T-16.1-03 and allow-listed in hygiene | closed |
|
||||
| T-10-23 | Spoofing | logout on a shared browser | medium | mitigate | `useAuth.logout` POSTs `/auth/logout` and clears state even on failure | closed |
|
||||
| T-10-24 | Repudiation | Phase 10 acceptance evidence | high | mitigate | `scripts/check-phase10.sh`. On 2026-10-12 `--hygiene` and `--self-test` exit 1 (self-test rejects the clean scratch copy), so `--all` cannot pass and the 2026-09-27 review cannot be reproduced | open |
|
||||
| T-10-25 | Tampering | framework/app boundary and hand-maintained API types | low | mitigate | `--hygiene` boundary rules pass; the stage is red only on drift (literal origin strings in dist, reorder modules tested via the router but not imported by path, and the T-10-16 sink) | open — below high threshold (non-blocking) |
|
||||
| T-10-SC | Tampering | npm/Go dependencies | high | mitigate | Exact pins + committed lockfile + `npm ci`. TipTap (`@tiptap/*` 3.31.4) was added later in 17-02 under its own plan | closed |
|
||||
|
||||
*Status: open · closed · open — below high threshold (non-blocking)*
|
||||
*Severity: critical > high > medium > low — only open threats at or above workflow.security_block_on count toward threats_open*
|
||||
*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)*
|
||||
|
||||
---
|
||||
|
||||
## Open Threat Details (audit 2026-10-12)
|
||||
|
||||
### T-10-16 — sanitizer bypass by DOM clobbering
|
||||
|
||||
- **Sink:** `admin/src/components/form/fields/RichEditorField.vue:305` `v-html="display"`, `display = displayHtml(stored)` in read-only and preview mode (`PreviewField.vue` forces `readOnly`).
|
||||
- **Defect:** `clean()` (`admin/src/components/form/richeditor/html.ts:60-88`) reads `node.nodeType`, `element.localName`, `.childNodes` and `.attributes` and calls `.remove()`/`.replaceWith()` directly on untrusted nodes. A `<form>` with a control named `nodeType` overrides the property, the form is skipped and its subtree is kept unchanged.
|
||||
- **Reproduced:** headless Chromium (scratch probe) returns `<form><input name="nodeType"><img src=x onerror=…></form>` as-is, plus `<a href="javascript:…">`, `<iframe>`, `<meta http-equiv=refresh>`, `<style>` and an off-origin `<form action method=post>` with a password input. The same works with `<img name=nodeType>`, `<button name=nodeType>` and nested in `<div>`. `<input name="localName">` throws a TypeError and breaks the read-only view. `holder.append(fragment)` (html.ts:102) adopts nodes into the live document, so image loads start inside `displayHtml`.
|
||||
- **What limits it:** the admin CSP `script-src 'self'` (`boardwalk/boardwalk.go:32`) blocks inline handlers and `javascript:` URLs. It has no `form-action`, `frame-src` or `default-src`, so phishing forms, iframe overlays, meta-refresh redirects and CSS redress still work. The server sanitizer (seaglass) strips these on admin save, but stored values are not sanitized on read (17-02 deviation 1), and legacy or imported values are exactly the case this check exists for.
|
||||
- **Neutralised (36 vectors):** svg/math namespace confusion, noscript/xmp/noembed/noframes/plaintext/textarea/title, comment/CDATA, table foster-parenting, entity/whitespace/NUL/U+2028-obfuscated `javascript:`, `data:`, `vbscript:`, event handlers, style/id/name attributes.
|
||||
- **Test gap:** `admin/tests/form/RichEditorField.test.ts:154-168` fails if `displayHtml` is bypassed outright, but happy-dom does not implement form named properties, so no current test can catch the clobbering vector.
|
||||
- **Fix:**
|
||||
1. Make `clean()` safe against clobbering. Either:
|
||||
- switch to DOMPurify (`ALLOWED_TAGS` = `EDITOR_TAGS`, `ALLOWED_ATTR` = `['href']`, a hook calling `isAllowedLink`). This needs a decision note and the dependency checkpoint; or
|
||||
- remove `HTMLFormElement` nodes first and read every property through `Node.prototype`/`Element.prototype` getters, never calling methods on the untrusted node.
|
||||
2. Add a regression test with the clobbering vectors that runs in a real browser engine.
|
||||
3. Optional: add `form-action 'self'; frame-src 'none'` to the admin CSP.
|
||||
4. Re-check T-17-10, which shares this sanitizer.
|
||||
|
||||
### T-10-24 — Phase 10 gate red since Phase 17
|
||||
|
||||
- **`scripts/check-phase10.sh --hygiene`:** exit 1. It reports:
|
||||
- raw-HTML hits at `RichEditorField.vue:305`, `richeditor/html.ts:103` and `richeditor/lossy.ts:33`;
|
||||
- dist JS naming `https://admin.invalid` and `https://prosemirror.net`;
|
||||
- `components/reorder/context.ts`, `ReorderBranch.vue` and `views/ReorderView.vue` as imported by no test.
|
||||
- **`--self-test`:** exit 1, "self-test hygiene rejected the clean scratch copy". It stops before any detector plant runs, and `--all` runs it first.
|
||||
- **Cause:** Phase 17 (cb2987c, 872fd65) added the sink, the TipTap literals and the reorder views. `check-phase17.sh` does not run `check-phase10.sh`. Earlier phases did reconcile with this gate (MarkdownField, `files.ts` XHR, `listState.ts`).
|
||||
- **Fix:**
|
||||
1. Fix T-10-16.
|
||||
2. Add line-exact, reviewed exceptions naming T-17-10 for the three raw-HTML lines.
|
||||
3. Clear the T-10-25 drift.
|
||||
4. Re-run `--self-test` and `--all` to green and refresh `10-SECURITY-REVIEW.md`.
|
||||
5. Have later phase gates, or a standing check, run `check-phase10.sh --hygiene`.
|
||||
|
||||
### T-10-25 — hygiene drift (non-blocking)
|
||||
|
||||
- **Foreign origins:** both are literal strings with no network load. `https://prosemirror.net` is in a ProseMirror error message. `https://admin.invalid/` is the `new URL` base in `isAllowedLink` (html.ts:32).
|
||||
- **Reorder modules:** they are exercised by `admin/tests/views/ReorderView.test.ts` through the router (11 tests pass), but no test imports them by path.
|
||||
- **Fix:** allow-list the two strings in the dist-origin rule (or use `document.baseURI` as the parse base), and add direct path imports of the three reorder modules to a test.
|
||||
|
||||
---
|
||||
|
||||
## Accepted Risks Log
|
||||
|
||||
| Risk ID | Threat Ref | Rationale | Accepted By | Date |
|
||||
|---------|------------|-----------|-------------|------|
|
||||
| AR-10-01 | T-10-18 | UI hiding is cosmetic; the server enforces permissions, toolbar actions, writable fields and relation scope (`TestPhase10AssembledAcceptance`, `TestPhase09PermissionMatrix`) | Plan 10-03 | 2026-09-27 |
|
||||
| AR-10-02 | T-10-19 | Admin-only, same-origin, Referrer-Policy same-origin and noindex; search terms are not secrets and stay in browser history | Plan 10-03 | 2026-09-27 |
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-09-27 | 24 | 24 | 0 | Phase 10 security review (`10-SECURITY-REVIEW.md`) |
|
||||
| 2026-10-12 | 24 | 21 | 3 (2 blocking: T-10-16, T-10-24; 1 below threshold: T-10-25) | /msd-secure-phase 10: orchestrator L1 grep + msd-security-auditor (Chromium probe) |
|
||||
|
||||
## Security Audit 2026-10-12
|
||||
| Metric | Count |
|
||||
|--------|-------|
|
||||
| Threats found | 24 |
|
||||
| Closed | 21 |
|
||||
| Open | 3 (2 blocking) |
|
||||
|
||||
---
|
||||
|
||||
## Sign-Off
|
||||
|
||||
- [x] All threats have a disposition (mitigate / accept / transfer)
|
||||
- [x] Accepted risks documented in Accepted Risks Log
|
||||
- [ ] `threats_open: 0` confirmed
|
||||
- [ ] `status: verified` set in frontmatter
|
||||
|
||||
**Approval:** pending — blocked on T-10-16 and T-10-24
|
||||
Reference in New Issue
Block a user