diff --git a/.planning/notes/journal-html-sanitizing-bluemonday.md b/.planning/notes/journal-html-sanitizing-bluemonday.md index 9d1fdc4..4530981 100644 --- a/.planning/notes/journal-html-sanitizing-bluemonday.md +++ b/.planning/notes/journal-html-sanitizing-bluemonday.md @@ -29,6 +29,9 @@ Everything else that UGC refuses is refused: `script`, `style`, `iframe`, `objec - **Byte-stable when nothing is stripped.** `sanitizeHTML` returns the goldmark output unchanged when that output and bluemonday's output tokenize to the same tokens: the same type, tag name, decoded text and set of decoded attributes. Otherwise it returns bluemonday's output. bluemonday re-escapes `"` as `"` and `'` as `'`, so storing its output every time would rewrite every imported post on its next save. Returning the original bytes is safe only on that token-level equality. Any element, attribute or comment that the policy drops makes the tokens differ, and then the sanitized output is stored. - **Category content refuses.** `classes.CheckHTML` keeps its exported signature and its 422 contract. D-21 stores category content exactly as submitted, and `TestCategoriesAdminContent` asserts a 422 for unsafe content. CheckHTML now uses the same policy and comparison, and refuses whenever the policy would change the HTML. +- **Translations follow the same rule (2026-10-10, audit W-1).** `classes.FormatTranslation` runs on every locale of every API and import write: a non-blank `content` re-derives `content_html` with `FormatHTML` (a supplied `content_html` is replaced, as for the host column); a `content_html` sent alone goes through `classes.SanitizeHTML`, the same policy. A translation keyed on the default locale, which `translate.SetTranslated` writes straight into the host column, is covered too. PHP stored a client-supplied translated `content_html` as-is on the API and import paths; the port follows the PHP admin widget, which always derived it. +- **Duplicate attributes count as a change (audit W-2).** `golang.org/x/net/html` drops a repeated attribute while tokenizing, so the byte-stable comparison could not see `href="https://x" href="javascript:..."`. `htmlTokens` compares the raw attribute names of each start tag (`rawAttrNames`) with the attributes the tokenizer kept; any difference stores the sanitized output and makes `CheckHTML` refuse. + ## Rationale The regex blocklist could be bypassed once raw HTML passed through. These payloads came back unchanged with no error: `` (no space before the attribute), ``, `javascript:`, `javascript:`, `javascript:`, `javascript:`, MathML `xlink:href`, and the inert-looking but dangerous ``, `` and `
`. The `data:` pattern also matched ordinary text, so "Data: 12 maja 2024" and "metadata:" were refused (WR-01). A sanitizer that tokenizes the HTML checks attributes and URL values in the same way a browser parses them, so it does not depend on any text pattern. @@ -51,6 +54,6 @@ The regex blocklist could be bypassed once raw HTML passed through. These payloa ## Follow-ups -- Move the post 64 author-footer CSS (`.author-footer`, `.author-info`, `.author-photo`, `.author-details`) into the public theme before the post is re-saved. -- `vue-journal-layer/app/utils/htmlGate.ts` still uses the same three regexes, so it has the same bypasses and the same `data:` false positive. It gives no defence in depth. Replace it with DOMPurify (with a decision note) in that repository. +- Done: the post 64 author-footer CSS moved into vue-grzybyfunkcjonalne-app `app/assets/css/base.css` (2c46185). +- Done: `vue-journal-layer` replaced its regex gate with sanitize-html 2.18.0 mirroring this policy (5beb1c5; decision note `docs/decisions/0001-sanitize-html.md` in that repository, which explains why not DOMPurify). - If editors need `target="_blank"` links, `rel`, or `title` values with quotes or `&`, extend the policy deliberately. These are not allowed now because no imported content uses them. diff --git a/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-SECURITY.md b/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-SECURITY.md index a63e62d..c564bb1 100644 --- a/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-SECURITY.md +++ b/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-SECURITY.md @@ -43,8 +43,8 @@ created: "2026-10-10" | T-16.1-11 | Repudiation (false green) | 16.1-VALIDATION.md | medium | mitigate | Row 16.1-03-03 updated 2026-10-10 to the post-CR-01 test names; re-run gives four top-level PASS lines | closed | | T-16.1-12 | Tampering / DoS | cabana DryRun tests | low | mitigate | DryRun DBs use `DryRun: true`, `DisableAutomaticPing`, `port=1` DSN; Postgres tests use testcontainers | closed | | T-16.1-SC | Tampering (supply chain) | go.mod / package.json | high | mitigate | summercms.go adds no dependency; sm-journal-plugin adds bluemonday v1.0.27 (+ aymerick/douceur v0.2.0, gorilla/css v1.0.1), named in the decision note, hash-pinned, `go mod verify` clean; vue-journal-layer adds sanitize-html 2.18.0 with decision note docs/decisions/0001-sanitize-html.md | closed | -| T-16.1-13 (audit W-1) | Tampering (stored XSS, latent) | sm-journal-plugin translated `content_html` (API syncTranslations, import) | medium | mitigate | Fix in progress: sanitize or derive every translated `content_html` write | open — below high threshold (non-blocking) | -| T-16.1-14 (audit W-2) | Tampering | sm-journal-plugin FormatHTML byte-preserving shortcut (duplicate attributes) | low | mitigate | Fix in progress: treat duplicate attributes as a change so the sanitized output is stored and CheckHTML refuses | open — below high threshold (non-blocking) | +| T-16.1-13 (audit W-1) | Tampering (stored XSS, latent) | sm-journal-plugin translated `content_html` (API syncTranslations, import) | medium | mitigate | `classes.FormatTranslation` on every API and import locale write: derived from `content` or sanitized with the same policy; default-locale bypass covered (sm-journal-plugin 0d00ec8; harness tests TestJournalAPITranslatedContentHTML, TestJournalImportTranslatedContentHTML) | closed | +| T-16.1-14 (audit W-2) | Tampering | sm-journal-plugin FormatHTML byte-preserving shortcut (duplicate attributes) | low | mitigate | Raw attribute names compared with tokenizer output; duplicates store the sanitized output and CheckHTML refuses (sm-journal-plugin 5f47083; TestFormatHTMLDuplicateAttributes, TestRawAttrNames, TestCheckHTML cases) | 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* @@ -66,6 +66,7 @@ created: "2026-10-10" | Audit Date | Threats Total | Closed | Open | Run By | |------------|---------------|--------|------|--------| | 2026-10-10 | 13 (register) + 2 audit warnings | 13 | 2 (below high, non-blocking) | msd-security-auditor (ASVS L1, boundary checks for T-16.1-01/02/WR-03) | +| 2026-10-10 | 15 | 15 | 0 | orchestrator (T-16.1-13/14 fixed in 0d00ec8, 5f47083; tests re-run) | Audit notes: the auditor flagged W-3 (public layer gate) and W-4 (no record of the a3551f2 UAT deploy). W-3 is resolved by vue-journal-layer 5beb1c5; W-4 is recorded in T-16.1-02 above. The admin preview (`modules/cabana/field_markdown.go`) stays in goldmark safe mode.