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

docs(20-07): filter hardening summary

Browse Source
This commit is contained in:
Jakub Zych
2026-11-02 18:50:35 +01:00
parent 762f0ae9aa
commit d1be8aa54d
1 changed files with 185 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

185
.planning/phases/20-demo-instance-with-every-shared-plugin/20-07-SUMMARY.md Normal file
View File

@@ -0,0 +1,185 @@
---
phase: 20-demo-instance-with-every-shared-plugin
plan: 07
subsystem: moderation filter (sm-moderation-plugin) and forum pre-save seam (sm-forum-plugin)
status: complete
gap_closure: true
review_findings: [CR-02, CR-03]
tags: [moderation, normalisation, idna, nfkc, forum, seam, security]
requires: [20-06]
provides:
- "classes.Match / Filter.Check normalise text and links before matching (CR-02, CR-03)"
- "classes.MatchHTML / Filter.CheckHTML read rendered HTML as members see it"
- "models.CanonicalHost; models.LinkEntry returns the IDNA ASCII host"
- "forum events.PostSaving.HTML (the content_html that is stored)"
affects: [20-09 (dependency bumps, demo listener, tags)]
tech-stack:
added: []
patterns:
- "normalise both sides: entries and text through the same fold, entry reported as listed"
- "two passes per text (as typed, without inline markup), words before links"
- "link candidates canonicalised like the WHATWG URL parser (percent-decoded authority, last @, port dropped, IDNA ASCII, special-scheme slashes, cleaned path)"
key-files:
created:
- ../sm-moderation-plugin/classes/normalize.go
- ../sm-moderation-plugin/classes/html.go
- ../sm-moderation-plugin/classes/bypass_test.go
- ../sm-moderation-plugin/classes/html_test.go
modified:
- ../sm-moderation-plugin/classes/filter.go
- ../sm-moderation-plugin/classes/filter_test.go
- ../sm-moderation-plugin/models/settings.go
- ../sm-moderation-plugin/models/settings_test.go
- ../sm-moderation-plugin/plugin_test.go
- ../sm-moderation-plugin/go.mod
- ../sm-moderation-plugin/README.md
- ../sm-forum-plugin/events/events.go
- ../sm-forum-plugin/classes/forum_service.go
- ../sm-forum-plugin/classes/lifecycle.go
- ../sm-forum-plugin/saving_test.go
- ../sm-forum-plugin/README.md
decisions:
- "Hosts go through a WHATWG-like UTS #46 profile (lookup mapping, bidi and joiner rules, CheckHyphens and STD3 off) rather than idna.Lookup, which refuses names browsers open (ab--c.example)"
- "The authority is split by hand (last @, last :) instead of net/url, which errors on escapes and odd ports and would drop the candidate"
- "The old host regexp stays as a second candidate source next to tokenising, so every host the old matcher found (url=bad.example) is still found"
- "The markup pass also drops a backslash before ASCII punctuation, so the raw check refuses bit\\.ly too"
- "Combining marks Mn and Me are dropped when folding; Cc controls that are not spaces are dropped with Cf"
metrics:
duration: 20min
started: 2026-11-02T17:29:01Z
completed: 2026-11-02T17:49:03Z
tasks: 3
files: 16
actuals:
tokens: 17100
tasks: 3
commits: 4
plan_head_before: "sm-moderation-plugin 313d117, sm-forum-plugin 13aa642 (summercms.go: no code commits)"
plan_head_after: "sm-moderation-plugin 334697b, sm-forum-plugin ec1a82c"
---
# Phase 20 Plan 07: Filter hardening for CR-02 and CR-03 Summary
The moderation filter now matches text and links the way a member reads and a browser opens them: NFKC, dropped format characters, Unicode spaces, folded diacritics, entities and inline markup for words; percent-decoding, IDNA ASCII, mapped dots, port and userinfo, special-scheme slashes and cleaned paths for links. Rendered forum HTML can be checked with `CheckHTML`, and forum's `PostSaving` now carries that HTML.
## Commits
The plan's code lives in the two sibling repos. Nothing was pushed or tagged. The `commits: 4` above was counted with `git rev-list --count` from each repo's base. The summercms.go repo has no code commits; this SUMMARY is left uncommitted for the orchestrator.
| Repo | Hash | Subject |
|------|------|---------|
| sm-moderation-plugin | 266fc60 | fix: normalise text and link hosts before matching banned entries |
| sm-moderation-plugin | 9715ce7 | fix: match banned words and links in the forms browsers show |
| sm-moderation-plugin | 334697b | feat: MatchHTML and Filter.CheckHTML for rendered content |
| sm-forum-plugin | ec1a82c | feat: PostSaving carries the rendered post HTML |
## Probe table
Lists: words `buy followers`, `casino bonus`, `free crypto`, `viagra`; links `bit.ly`, `example.com/promo`. "Before" is v0.1.0 (313d117); every row was also checked with the reviewer's scratchpad probe, which now prints no `flagged=false` line.
| Form | Before | After | Where pinned |
|------|--------|-------|--------------|
| `via<U+200B>gra`, soft hyphen, ZWJ, word joiner, BOM | clean | word viagra | TestMatchBypassWords |
| `buy<NBSP/U+2009/U+202F/U+3000>followers` | clean | word buy followers | TestMatchBypassWords |
| fullwidth `ｖｉａｇｒａ` | clean | word viagra | TestMatchBypassWords |
| `vi&#97;gra`, `vi&#x61;gra` | clean | word viagra | TestMatchBypassWords, TestMatchHTML |
| `via**gra**`, ``via`gra` ``, `via~~gra~~`, `_viagra_` | clean | word viagra | TestMatchBypassWords, TestMatchHTML |
| NFD `żółw`, folded `zolw` (entry żółw) | clean | word żółw (as listed) | TestMatchBypassWords |
| `https://bit%2Ely/x`, `https://bit.l%79/x`, `bit.l%79/x` | clean | link bit.ly | TestMatchBypassLinks, TestFilterCheckNormalises |
| `https://ｂｉｔ.ｌｙ/x`, `bit。ly`, `bit．ly`, `bit｡ly` | clean | link bit.ly | TestMatchBypassLinks |
| `https://user@bit.ly/x`, `https://example.com@bit.ly/` | flagged (the regexp found bit.ly after the @) | link bit.ly | TestMatchBypassLinks |
| `[x](https://bit&#46;ly/a)` | clean | link bit.ly | TestMatchBypassLinks, TestMatchHTML |
| `https:bit.ly`, `https:/\bit.ly/x`, `https:\\bit.ly` | flagged (regexp found the bare host) | link bit.ly | TestMatchBypassLinks |
| `https://bit.ly./x` | flagged | link bit.ly | TestMatchBypassLinks |
| `example.com:443/promo`, `/%70romo`, `//promo`, `/./promo`, `/x/../promo`, `example.com./promo` | clean | link example.com/promo | TestMatchBypassLinks |
| `https://sub.example.com/promo` | clean | link example.com/promo | TestMatchBypassLinks |
| punycode against a Unicode entry, and the reverse | clean | link (entry as listed) | TestMatchBypassLinks |
| rendered `via<!-- x -->gra`, `bit\.ly`, reference link with an entity, `<https://bit.ly/x>` | clean (raw) | flagged by MatchHTML | TestMatchHTML |
| clean: `spam_bot`, `_spam`, `spamming`, `notbit.ly`, `/about`, `/x/promo`, `bit.lyx`, `<p>via</p><p>gra</p>` | clean | clean | bypass and HTML tests |
## The one changed expectation
`TestMatchLinks`: `a.shop.example/promo` against the entry `https://shop.example/promo` was clean and now flags (KindLink, entry as listed). Path rules cover subdomains, as host rules do (CR-02). `git diff --stat 313d117 -- classes/filter_test.go` shows 2 insertions (the comment and the case) and 1 deletion. No other existing case changed.
`TestLinkEntry`: the `żółw.example` case now expects `xn--w-uga1v8h.example`. New cases cover fullwidth letters, U+3002 and U+FF61 dots, an ASCII punycode entry, `ab--c.example` (accepted) and the invalid punycode `xn--zz.example` (refused).
## Dependency note
`golang.org/x/text v0.42.0` (unicode/norm) and `golang.org/x/net v0.58.0` (idna, html) moved from indirect to direct requires of sm-moderation-plugin. The framework already requires both directly at these versions. `go mod tidy` ran with `GOPROXY=off`, go.sum did not change, and no new module entered the build. The README's Dependencies section names both. `go list -deps ./...` shows no sm-chat-plugin or sm-forum-plugin import.
## Listener finding for 20-09
`../sm-demo-app/plugins/demo/moderation.go` (read only, not edited):
- `checkMessage` needs no change: `Check` now normalises the chat text itself.
- `checkPost` should call `f.CheckHTML(ctx, ev.HTML)` after the title and body checks, and reject under field `body` when it flags. This only compiles against the new tags (sm-forum-plugin v0.1.2 for `PostSaving.HTML`, sm-moderation-plugin v0.1.1 for `CheckHTML`). The demo's go.work uses the submodule checkouts at `plugins/golem15/{forum,moderation}`, currently at v0.1.1 and v0.1.0. So the edit belongs right after 20-09 bumps those submodules.
- Even without that edit, the raw `Check` on the body now refuses `via**gra**`, `vi&#97;gra`, `_viagra_`, `[x](https://bit&#46;ly/a)`, `[x](https:bit.ly)` and `bit\.ly`. Only the forms the renderer alone resolves, such as a comment inside a word or a reference-link destination with an entity, need `CheckHTML`.
## Deviations from Plan
### Auto-fixed Issues
**1. [Rule 1 - Bug] TestPostSavingNoListener had to gain the HTML field**
- **Found during:** Task 3
- **Issue:** The plan said the existing seam tests pass unedited. But `TestPostSavingNoListener` compares whole `PostSaving` values with `slices.Equal`, so the new field made it fail.
- **Fix:** The three expected events now carry `HTML: breakwater.Render(body)`. Nothing else in the test changed. `TestPostSavingRejects`, `TestPostSavingTracer`, the golden and method-map tests are unedited and pass.
- **Files modified:** ../sm-forum-plugin/saving_test.go
- **Commit:** ec1a82c
**2. [Rule 1 - Bug] WHATWG-like IDNA profile instead of `idna.Lookup`**
- **Found during:** Task 2
- **Issue:** `idna.Lookup` checks hyphens. It refuses `ab--c.example`, a name browsers open. Used in `LinkEntry`, that would make the admin form reject a valid entry.
- **Fix:** `models.CanonicalHost` uses `idna.New(MapForLookup, BidiRule, Transitional(false), CheckHyphens(false), StrictDomainName(false), CheckJoiners(true))`, which is UTS #46 as the URL standard applies it. Punycode results are the same (`żółw.example` → `xn--w-uga1v8h.example`). It is one exported function used by both `LinkEntry` and the matcher, so entries and candidates are always read alike.
- **Files modified:** ../sm-moderation-plugin/models/settings.go
- **Commit:** 9715ce7
**3. [Rule 1 - Bug] Authority split by hand, not with net/url**
- **Issue:** `url.Parse` errors on some decoded hosts and on non-numeric ports. Each such error would silently drop a candidate.
- **Fix:** `canonicalLink` takes the text after the last `@`, drops a `:port` and passes the host to CanonicalHost. This gives the same result as `Hostname()` for valid URLs and never drops a candidate.
- **Commit:** 266fc60
**4. [Rule 2 - Missing functionality] The host regexp stays next to tokenising**
- **Issue:** The plan said to replace the regexp with tokenising. Tokens alone miss a host glued to other text (`url=bad.example`, `a,bad.example`), which the old matcher found.
- **Fix:** `matchLinks` checks the tokens first, then every `hostPattern` match (widened for percent-escapes). The union can only add matches, and every must-stay-clean case still passes.
- **Commit:** 9715ce7
**5. [Rule 2 - Missing functionality] Small additions to the normaliser**
- The markup pass also drops a backslash before ASCII punctuation (a Markdown escape), so the raw check refuses `bit\.ly`.
- Controls (Cc) that are not spaces are dropped along with Cf.
- Diacritic folding drops enclosing marks (Me) along with Mn.
- **Commit:** 266fc60, 9715ce7
**6. [Rule 2 - Test coverage] Extra tests**
- `TestFilterCheckNormalises` also covers `CheckHTML` through the stored lists: word, link with the pl message, clean, disabled.
- `TestMatchHTMLWalk` pins the tokenizer walk: inline joins, block/br/td separation, href, src, alt and title, comments and scripts.
- **Commit:** 334697b
**7. [Rule 1 - Bug] Invisible characters escaped in test sources**
- The first bypass test was committed with a literal U+200B in its source (266fc60). In 9715ce7 every invisible or combining character in the test sources became a `\uXXXX` escape, so the cases can be read and reviewed.
### TDD notes
- Task 1: RED was seen before any code changed. Both tracer tests failed with "got no violation".
- Task 2: RED was seen before the code changed: 19 "got no violation" failures across TestMatchBypassWords, TestMatchBypassLinks and the flipped TestMatchLinks case.
- Task 3: the HTML tests were written after `html.go`, and both passed on their first run. RED was not observed separately.
## Test results
- sm-moderation-plugin: `go vet ./...` is clean. `go test -short ./...` and `go test ./...` (Docker Postgres) pass. That covers TestMatchBypassWords, TestMatchBypassLinks, TestMatchHTML, TestMatchHTMLWalk, TestLinkEntry and TestFilterCheckNormalises.
- sm-forum-plugin: `go vet ./...` is clean. `go test -short ./...` and `go test ./...` (Docker) pass, including TestPostSavingTracer, TestPostSavingNoListener, TestPostSavingRejects and TestPostSavingHTML.
- Reviewer probe (scratchpad modtest): all 20 cases print `flagged=true`.
- Acceptance greps: go.mod has 2 direct x/text and x/net lines. There are 0 chat/forum deps. `HTML: html` appears 2 times in forum_service.go and once in lifecycle.go. Each README has one changelog heading (`### v0.1.1` moderation, `### v0.1.2` forum). MatchHTML/CheckHTML appear 11 times in the moderation README.
## Known limitations (not stubs)
- Path rules are prefixes, as before: `example.com/promo` also matches `/promotional`. This behaviour is older than this plan and was not changed.
- IPv4 forms a browser normalises (`0x7f.1`, dotless decimals) and IPv6 literals are not canonicalised. No listed entry in the review relies on them.
## Threat Flags
None. No new endpoint, auth path, file access or schema change. PostSaving gains a read-only field.
## Self-Check: PASSED
- FOUND: classes/normalize.go, classes/html.go, classes/bypass_test.go, classes/html_test.go (sm-moderation-plugin)
- FOUND commits: 266fc60, 9715ce7, 334697b (sm-moderation-plugin), ec1a82c (sm-forum-plugin)