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

docs(phase-17): update security threat verification (T-17-15 closed, 0 open)

Browse Source
This commit is contained in:
Jakub Zych
2026-10-13 01:23:04 +02:00
parent faa749f5a9
commit 79b1f85564
1 changed files with 26 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
.planning/phases/17-blog-companions/17-SECURITY.md
View File

@@ -1,9 +1,9 @@
---
phase: "17"
slug: "blog-companions"
status: draft
status: verified
# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate)
threats_open: 1
threats_open: 0
asvs_level: 1
block_on: high
created: "2026-10-13"
@@ -52,7 +52,7 @@ created: "2026-10-13"
| 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-15 | Tampering (XML injection) | sitemap serialize + `frontend_url` rewrite | high | mitigate | serializer escaping (`serialize.go`); `models.AllowedFrontendURL` (`settings.go:92-112`) enforced in `classes.Rewrite` (`rewrite.go:37-39`, escape `:45`, http(s) re-filter `:49,57`), on save (`settings.go:77-82`) and on import (`import_winter.go:466-468`); TestRewriteUnsafeFrontendURL | closed |
| 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 |
@@ -89,21 +89,22 @@ created: "2026-10-13"
*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
### Closure 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`.
Open in the first audit: `classes.Rewrite` inserted the unvalidated `settings.FrontendURL` raw into `<loc>`, `xhtml:link href` and the
stylesheet PI, after the CR-01 http(s) output filter had run (`frontend_url = javascript:alert(document.domain)//` reached `<a href>` through
`assets/sitemap.xsl`). Quick task 261013-0wa implemented closure options 1 and 2; re-audited at `sm-sitemap-plugin` `1bb89ce` (`origin/main`).
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.
| Path | Control | Evidence |
|------|---------|----------|
| value already stored | `Rewrite` leaves the document unchanged unless `models.AllowedFrontendURL` accepts the value; inserted value goes through `EscapeText` / `EscapeAttr`; a rewritten loc or href that is not http(s) keeps its old value | `classes/rewrite.go:37-39`, `:45`, `:48-60`; TestRewriteUnsafeFrontendURL, TestSitemapImportSmoke |
| admin settings save | `Settings.BeforeValidate` returns a 422 on `frontend_url`, row unchanged | `models/settings.go:77-82`, hook call in `modules/cabana/settings.go:183-187`; TestSettingsBeforeValidate, TestSitemapAdminSmoke |
| `sitemap:import-winter` | `readWinterSettings` refuses the value before any write | `console/import_winter.go:466-468`; TestWinterSettingsFrontendURL, TestImportWinter/settings_upsert |
No other writer of `frontend_url` exists. The auditor probed the shipped `Rewrite` through `go test -overlay` (198 hand-picked values across
10 backend roots, 500,000 random compositions, every Unicode scalar value) and ran the results through `xmllint` and `xsltproc` with the
shipped stylesheet: no scheme, markup or attribute injection, every `<loc>` and `href` http(s). Each named test fails when its guard is
removed. `check-phase17.sh` requires the tests by name (`6e5a8e7`).
---
@@ -120,13 +121,15 @@ Closure options (any one satisfies the declared mitigation; the first two togeth
## 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.
1. **Review fixes and the T-17-15 fix are not in the host.** They are on plugin `origin/main` (sitemap `1bb89ce`, menu `9c0f833`, journal `31d901c`) and in framework tag `v0.1.24`, but the host gitlinks are unchanged (sitemap `5a260db`, menu `33a65ad`, journal `06d0c9b`) and the host still requires framework `v0.1.23`. A host build today, and the UAT binary, carry the pre-fix sitemap plugin and are still exposed to CR-01 and T-17-15 (limited by: needs `golem15.sitemap.access_definitions`; `frontend_url` is null on the migrated site). A host gitlink bump, rebuild and redeploy 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.
5. **Open info findings** IN-01..IN-09 remain tracked in 17-REVIEW-DISPOSITION; IN-07 shares the T-17-15 code path and is addressed by the escaping at `rewrite.go:45`, its disposition entry is not yet updated.
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`.
7. **Gate coverage.** Resolved by `6e5a8e7`: `check-phase17.sh` requires the sitemap review-fix and T-17-15 tests by name.
8. **T-17-15 residual, low (not injection).** `U+FFFE` and `U+FFFF` pass `AllowedFrontendURL` and `AllowedItemURL` but are not XML 1.0 characters; such a value makes `/sitemap.xml` not well-formed. No markup, attribute or scheme can be introduced, and it needs the same permission that can already empty the sitemap. Follow-up: refuse anything outside XML `Char` in both predicates.
9. **T-17-15 residual, informational.** The `<loc>` and stylesheet-href replacements are position-blind; only reachable if a compiled plugin emits a comment through `URLEntry.Extra`. With `--definitions`, `sitemap:import-winter` may read reference options before the settings file is refused (no write precedes the refusal).
---
@@ -135,6 +138,7 @@ Closure options (any one satisfies the declared mitigation; the first two togeth
| Audit Date | Threats Total | Closed | Open | Run By |
|------------|---------------|--------|------|--------|
| 2026-10-13 | 47 | 46 | 1 | msd-security-auditor (ASVS 1, block_on high) |
| 2026-10-13 | 47 | 47 | 0 | msd-security-auditor (ASVS 1, block_on high), re-audit of T-17-15 at sm-sitemap-plugin `1bb89ce` |
---
@@ -142,7 +146,7 @@ Closure options (any one satisfies the declared mitigation; the first two togeth
- [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
- [x] `threats_open: 0` confirmed
- [x] `status: verified` set in frontmatter
**Approval:** pending — blocked by T-17-15
**Approval:** verified 2026-10-13