golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(phase-17): add security threat verification (1 open: T-17-15)

Browse Source
This commit is contained in:
Jakub Zych
2026-10-13 00:25:42 +02:00
parent c21d1135b0
commit 7fa648dcaf
1 changed files with 148 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

148
.planning/phases/17-blog-companions/17-SECURITY.md Normal file
View File

@@ -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 `<loc>`, `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 `<loc>javascript:alert(document.domain)/blog/x</loc>`, and
`assets/sitemap.xsl` renders `<a href="{$loc}">`, 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 `<loc>` 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