diff --git a/.planning/phases/20-demo-instance-with-every-shared-plugin/20-07-SUMMARY.md b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-07-SUMMARY.md new file mode 100644 index 00000000..9cfc02a5 --- /dev/null +++ b/.planning/phases/20-demo-instance-with-every-shared-plugin/20-07-SUMMARY.md @@ -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 | +|------|--------|-------|--------------| +| `viagra`, soft hyphen, ZWJ, word joiner, BOM | clean | word viagra | TestMatchBypassWords | +| `buyfollowers` | clean | word buy followers | TestMatchBypassWords | +| fullwidth `ｖｉａｇｒａ` | clean | word viagra | TestMatchBypassWords | +| `viagra`, `viagra` | 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.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 `viagra`, `bit\.ly`, reference link with an entity, `` | clean (raw) | flagged by MatchHTML | TestMatchHTML | +| clean: `spam_bot`, `_spam`, `spamming`, `notbit.ly`, `/about`, `/x/promo`, `bit.lyx`, `

via

gra

` | 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**`, `viagra`, `_viagra_`, `[x](https://bit.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)