From 7fa648dcafc3d38b6c86ce2067557381ca7058f0 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 13 Oct 2026 00:25:42 +0200 Subject: [PATCH] docs(phase-17): add security threat verification (1 open: T-17-15) --- .../phases/17-blog-companions/17-SECURITY.md | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 .planning/phases/17-blog-companions/17-SECURITY.md diff --git a/.planning/phases/17-blog-companions/17-SECURITY.md b/.planning/phases/17-blog-companions/17-SECURITY.md new file mode 100644 index 0000000..6be441e --- /dev/null +++ b/.planning/phases/17-blog-companions/17-SECURITY.md @@ -0,0 +1,148 @@ +--- +phase: "17" +slug: "blog-companions" +status: draft +# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate) +threats_open: 1 +asvs_level: 1 +block_on: high +created: "2026-10-13" +--- + +# Phase 17 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| admin browser → cabana API | authenticated but untrusted input | HTML, repeater rows, reorder sets, query values | +| plugin code → signpost registry | trusted compiled code, validated at boot | item type contracts | +| host config → signpost | operator-controlled patterns | URL patterns | +| stored HTML → admin editor | imported or saved HTML rendered in the admin | HTML | +| admin SPA → npm registry | new editor dependency | packages | +| executor → git origin | tags and gitlinks read by other repositories | releases | +| internet → /sitemap.xml, /sitemap.xsl, /_menu/api, /_faq/api | anonymous GETs | public XML and JSON | +| admin → definition, settings, menu and FAQ records | stored item text reaches public XML and JSON | URLs, HTML, text | +| Winter export files → import commands | operator-supplied JSONL and JSON | legacy rows | +| executor → PHP production server / live database | redeploy and read-only export | production rows | +| nginx → Go, Nuxt SSR → plugin APIs | forwarded headers, loopback fetches | Host, X-Forwarded-Proto | +| test process → Docker, gate script → sibling repos | throwaway containers and local checkouts | test data | + +--- + +## Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation (evidence) | Status | +|-----------|----------|-----------|----------|-------------|-----------------------|--------| +| T-17-01 | Tampering (stored XSS) | richeditor save | high | mitigate | seaglass on create/update, `cabana/crud.go:619,653`; TestCompanionsRichEditorSmoke | closed | +| T-17-02 | Tampering (mass assignment) | repeater rows | medium | mitigate | `field_repeater.go:282` decodeRepeaterRow; non-list is 422 | closed | +| T-17-03 | Tampering | dependent dropdown | medium | mitigate | `field_dependent.go:267` per-row recheck, 422 | closed | +| T-17-04 | Elevation of Privilege (IDOR) | reorder POST | high | mitigate | `reorder.go` FOR UPDATE, exact-set check, 409; smoke tests | closed | +| T-17-05 | DoS / Tampering | reorder tree validation | medium | mitigate | `checkReorderNodes` cycle, duplicate and depth checks before write | closed | +| T-17-06 | Information Disclosure | dependent-options, singleton routes | medium | mitigate | `protect()` plus token gate; anonymous 401 asserted | closed | +| T-17-07 | Spoofing (CSRF) | reorder POST | medium | mitigate | `requireAjax` plus CSRF check | closed | +| T-17-08 | Tampering | signpost boot | low | mitigate | `signpost.go` boot validation, no runtime registration | closed | +| T-17-09 | Tampering (supply chain) | bluemonday | low | mitigate | exact v1.0.27, go.sum, decision note | closed | +| T-17-10 | Tampering (XSS in admin) | RichEditorField | high | mitigate | DOMPurify display, TipTap schema; browser tests | closed | +| T-17-11 | Tampering | link insertion | medium | mitigate | `isAllowedLink` allowlist, server re-check | closed | +| T-17-12 | Tampering (silent rewrite) | untouched imported HTML | medium | mitigate | no emit until edit; byte-identical test | closed | +| T-17-13 | Elevation of Privilege | ReorderView, singleton | low | mitigate | 403/409 states, no silent retry | closed | +| T-17-14 | Tampering / Repudiation | tag and push | high | mitigate | blocking-human checkpoint; v0.1.23 verified on origin | closed | +| T-17-15 | Tampering (XML injection) | sitemap serialize + `frontend_url` rewrite | high | mitigate | serializer escaping verified (`serialize.go`); **`classes.Rewrite` inserts unvalidated `settings.FrontendURL` raw into ``, `xhtml:link href` and the stylesheet PI** | **open** | +| T-17-16 | Tampering (mass assignment) | sitemap Definition | medium | mitigate | Fillable exactly `theme, data` | closed | +| T-17-17 | Information Disclosure | updates_manifest_path | medium | mitigate | regular file, 5 MiB cap, JSON object only | closed | +| T-17-18 | DoS | /sitemap.xml generation | medium | mitigate | 50,000 cap, RE2 patterns, throttle and cache | closed | +| T-17-19 | Spoofing | backend URL rewrite | medium | mitigate | X-Forwarded-Proto only from trusted proxies | closed | +| T-17-20 | Elevation of Privilege | definitions controller, settings | medium | mitigate | `access_definitions` permission, 403 asserted | closed | +| T-17-21 | Tampering | sitemap import | medium | mitigate | refuses non-empty without --force, dry run, one transaction | closed | +| T-17-22 | Tampering | Journal additive change | low | mitigate | additive diff only (58851dd..HEAD) | closed | +| T-17-23 | Tampering (stored XSS) | FAQ HTML | high | mitigate | seaglass on API output incl. raw-SQL rows | closed | +| T-17-24 | Tampering (XSS via links) | menu entry URL | high | mitigate | AllowedURL on save, in API and in layer | closed | +| T-17-25 | Information Disclosure | unpublished/inactive entries | medium | mitigate | is_active / is_published filters, 404s | closed | +| T-17-26 | DoS | public menu and FAQ APIs | medium | mitigate | 120/min buckets, 429 asserted | closed | +| T-17-27 | Tampering (mass assignment) | FAQ, Menu, MenuEntry | medium | mitigate | explicit Fillable lists | closed | +| T-17-28 | Elevation of Privilege (IDOR) | entry reorder across menus | high | mitigate | partition on menu_id, foreign entry gets 409 | closed | +| T-17-29 | Tampering | dependent reference | medium | mitigate | provider recheck via signpost | closed | +| T-17-30 | Tampering | menu and FAQ import | medium | mitigate | --force, dry run, one transaction, SELECT-only SQL | closed | +| T-17-31 | Repudiation / Tampering | published API shape | medium | mitigate | blocking-human checkpoint recorded in 17-04-SUMMARY | closed | +| T-17-32 | Tampering (XSS) | layer v-html and links | high | mitigate | span for non-allowlisted URL, rel noopener; node tests | closed | +| T-17-33 | Tampering / Information Disclosure | export from live PHP DB | high | mitigate | process evidence in 17-05-SUMMARY (see notes) | closed | +| T-17-34 | Spoofing | sitemap base URL behind nginx | medium | mitigate | nginx forwards Host/proto, Go trusts only loopback proxy | closed | +| T-17-35 | DoS | /_menu and /_faq routes | medium | mitigate | throttle buckets plus nginx GET/HEAD only | closed | +| T-17-36 | Tampering | host CORS for new APIs | low | accept | public read APIs, no credentials | closed (accepted) | +| T-17-37 | Tampering / Repudiation | pushes, tags, gitlinks | high | mitigate | blocking-human approval; heads verified on origin | closed | +| T-17-38 | DoS | UAT redeploy | medium | mitigate | `nginx -t`, `.prev` binary, rollback in DEPLOY.md | closed | +| T-17-39 | Repudiation (false green) | check-phase17.sh | high | mitigate | fail-closed detectors, `--self-test` passes | closed | +| T-17-40 | Tampering | unsafe test fixtures | low | accept | fixtures only in test processes | closed (accepted) | +| T-17-41 | Information Disclosure | test databases, secrets | low | mitigate | testcontainers / throwaway DSNs | closed | +| T-17-SC (01) | Tampering | plan 01 installs | high | mitigate | no npm package; Go deps pinned in go.sum | closed | +| T-17-SC (02) | Tampering | TipTap installs | high | mitigate | legitimacy checkpoint, exact pins, integrity hashes, no install scripts | closed | +| T-17-SC (03) | Tampering | plan 03 installs | high | mitigate | no installs | closed | +| T-17-SC (04) | Tampering | plan 04 installs | high | mitigate | no installs | closed | +| T-17-SC (05) | Tampering | layer dependencies | high | mitigate | pinned set, lockfiles, layers pinned by tag and SHA | closed | +| T-17-SC (06) | Tampering | plan 06 installs | high | mitigate | no manifest or lockfile changes | closed | + +*Status: open · closed · open — below high threshold (non-blocking)* +*Severity: critical > high > medium > low — only open threats at or above `block_on` count toward threats_open* + +### Open blocking threat detail: T-17-15 + +The serializer escapes every `URLEntry` value. The gap is `classes.Rewrite` (`rewrite.go:19-45`, called from `routes.go:212`): +it does `strings.ReplaceAll(s, backend, frontend)` with `settings.FrontendURL`, which has no validation anywhere +(`Settings.Rules()` empty, no save hook, plain `type: text` field, no import check). The CR-01 output filter +(`generator.go:233` `httpURL(loc)`) runs before the rewrite, so it does not cover this value. The auditor reproduced it with the +shipped `Rewrite`: `frontend_url = javascript:alert(document.domain)//` yields `javascript:alert(document.domain)/blog/x`, and +`assets/sitemap.xsl` renders ``, which re-opens the CR-01 class (stored XSS on the public origin) through a second +admin-controlled field. The same settings entry requires only `golem15.sitemap.access_definitions`. Review finding IN-07 is the same code +path, filed at info severity as a malformed-XML issue and still `open`. + +Closure options (any one satisfies the declared mitigation; the first two together are cleanest): +1. Validate `frontend_url` on save and import as an absolute `http(s)://` URL with no control characters, whitespace or `"<>&`. +2. In `Rewrite`, apply EscapeText for `` and EscapeAttr for `href` and the PI, and re-apply the http(s) output filter after the rewrite. +3. Record an explicit accepted risk against T-17-15 for the settings value. + +--- + +## Accepted Risks Log + +| Risk ID | Threat Ref | Rationale | Accepted By | Date | +|---------|------------|-----------|-------------|------| +| AR-17-01 | T-17-36 | Public read APIs carry no credentials; `cors.paths` lists only `_user`, `_journal`, `_menu`, `_faq` with `supports_credentials: false`; sitemap has no CORS. | Plan 17-05 threat register (approved with the plan) | 2026-10-13 | +| AR-17-02 | T-17-40 | Unsafe HTML and URL fixtures exist only in `_test.go` files and `host/testdata/companions/`, read by test processes on throwaway databases. | Plan 17-06 threat register (approved with the plan) | 2026-10-13 | + +*Accepted risks do not resurface in future audit runs.* + +--- + +## Audit Notes (informational, not counted in threats_open) + +1. **Review fixes not shipped.** CR-01 and WR-01..WR-07 are on plugin `origin/main` (sitemap `cf5c40e`, menu `9c0f833`, journal `31d901c`) and framework `master` (`964f1a7`), but not in tag `v0.1.23` and not in the host gitlinks (`5a260db`, `33a65ad`, `06d0c9b`). The UAT binary carries the pre-fix plugins. A framework tag, plugin `require` bumps and a host gitlink bump must precede any production deploy. +2. **Dependencies outside plan 02's register.** `dompurify` 3.4.16, `@vitest/browser` 3.2.7 and `playwright` 1.60.0 came in through quick task 261012-s3g, exact-pinned with integrity hashes, legitimacy table and decision note. +3. **T-17-33 deviation.** The operator supplied a full mysqldump instead of the SELECT-only export. It was loaded into a `--network none` throwaway container and deleted afterwards (asserted in 17-05-SUMMARY; no residue found on this host). +4. **T-17-34.** With `app.url` unset (UAT), the sitemap base URL uses `r.Host` regardless of peer (PHP `url()` parity). Setting `app.url` in production removes the dependency. +5. **Open info findings** IN-01..IN-09 remain tracked in 17-REVIEW-DISPOSITION; IN-07 shares the T-17-15 code path. +6. **Protocol-relative menu URLs** (`//host/...`) are accepted as plain anchors (deferred 17-06 item 2); not script execution. +7. **Gate coverage.** The review-fix tests run but are not required by name in `check-phase17.sh`. + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-10-13 | 47 | 46 | 1 | msd-security-auditor (ASVS 1, block_on high) | + +--- + +## 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 by T-17-15