From dff3b209777ee302f890381be5d1eb90ecd147fe Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 22:18:23 +0200 Subject: [PATCH] docs: Etappe 2b rulings, security audit and trial feedback Co-Authored-By: Claude Opus 5.5 (1M context) --- .../notes/2026-09-26-etappe-2b-praxistest.md | 176 +++++++++ .../notes/2026-09-26-etappe-2b-rulings.md | 159 +++++++++ .../2026-09-26-etappe-2b-security-audit.md | 336 ++++++++++++++++++ 3 files changed, 671 insertions(+) create mode 100644 docs/superpowers/notes/2026-09-26-etappe-2b-praxistest.md create mode 100644 docs/superpowers/notes/2026-09-26-etappe-2b-rulings.md create mode 100644 docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md diff --git a/docs/superpowers/notes/2026-09-26-etappe-2b-praxistest.md b/docs/superpowers/notes/2026-09-26-etappe-2b-praxistest.md new file mode 100644 index 0000000..b19bbaf --- /dev/null +++ b/docs/superpowers/notes/2026-09-26-etappe-2b-praxistest.md @@ -0,0 +1,176 @@ +# WP-LIT-MCP Trial: „Tischlerei Holzwerk Berger“ + +Hands-on test of the LIT MCP server as a real MCP client would use it (`initialize` → `tools/list` → +`lit-guide` → build), against the local DDEV site `https://wordpress.ddev.site:8443` (Divi 5.2.1, +WordPress 7.1.2, Pods inactive). Site was "Breva Map Dev" before this trial. All calls went through +`tests/Integration/lib.sh`'s `connect`/`tool`/`ability` helpers, run inside the DDEV web container via +throwaway scripts under `.superpowers/trial/` (deleted at the end; git-excluded). Nothing published. + +## What was built + +All titled with the `[Test] ` prefix, all left as drafts, as requested. + +| Type | ID | Title | Notes | +|---|---|---|---| +| Page | 839 | [Test] Startseite | Hero (heading+text+button+image), 3-Leistungen-Übersicht (blurbs), Kontakt-CTA | +| Page | 834 | [Test] Leistungen | Hero, 3 service blurbs, CTA section | +| Page | 837 | [Test] Über uns | Hero, Geschichte (image+text), 3 Werte (icon-lists; one duplicated to a 4th value) | +| Page | 831 | [Test] Kontakt | Hero, Adresse+Kontaktformular, Öffnungszeiten (added afterwards via `lit-divi-insert`) | +| Post | 854 | [Test] Drei Tipps zur Pflege von Massivholzmöbeln | | +| Post | 856 | [Test] Einblick in unsere Werkstatt | text+image | +| Post | 858 | [Test] Nachhaltigkeit beginnt beim Holz | | +| Menu | 22 | [Test] Hauptmenü | assigned to `primary-menu` location in the same `lit/menu-create` call; 4 items (Startseite, Leistungen, Über uns, Kontakt) via `lit/menu-set-items` with `allow_unpublished: true` | +| Media | 828, 829, 830 | Werkstatt / Team / Massivholzmöbel (Platzhalter) | imported via `lit-media-upload-from-url` from `picsum.photos` (public hotlink-friendly placeholder service — network egress from the container worked fine, no fallback to base64 needed) | +| Site settings | – | `blogname` → "Tischlerei Holzwerk Berger", `blogdescription` → "Massivholzmöbel, Innenausbau und Restaurierung aus Berlin" | previous values (`Breva Map Dev` / empty) are in the tool's own `previous` response field if this needs reverting | + +Front page: **not set**. `lit/wp-settings-update` correctly refused `page_on_front: 839` with +`"839 ist keine veröffentlichte Seite."` — consistent with the server's own "Neue Website aufsetzen" +workflow guide, which says to publish the homepage first. That step is fundamentally at odds with +"don't publish anything," so I left `show_on_front` on `posts` and flagged it here instead of guessing +which instruction should win. + +Edits demonstrated with the Divi write tools (all on top of the pages above): a heading text change +(`lit-divi-update`/`set`), a whole new section inserted (`lit-divi-insert`), a module moved across +columns (`lit-divi-move`), a node duplicated and re-labelled (`lit/divi-duplicate-node`), and a +site-wide text fix via `lit/divi-replace-text` with a dry run first, then a confirmed run (see friction +log for what it was replacing). + +## Chronological friction log + +1. **Guide reading is thorough but not consolidated.** `lit-guide` overview lists 8 further topics + (`content-types`, `divi-attrs`, `divi-design-system`, `divi-placeholders`, `divi-shortform`, + `divi-structure`, `media`, `workflows`); getting the full picture before writing anything took 8 + separate `lit-guide` calls. The content itself is excellent (dense, example-driven, in German + matching the server's own error/hint language) — no complaints about quality, only that there's no + single "give me everything" call for a first-time client that wants to front-load context once + rather than discover topics interactively. + +2. **Contact-form field `type` collision (real friction, not just reading fatigue).** The + `divi-shortform` field table lists `contact-field`'s shortform fields as `title, id, type, required` + — but a paragraph *below* the table says `type` can't actually be set this way at insert time because + it collides with the node-schema key `type` (the module type). I read the table, used `type: "email"` + in a contact-field item, and got: + `"unbekanntes Modul \"divi/input\""` / `"divi/contact-form erlaubt nur divi/contact-field als Kind"`. + The error is technically correct (it really did try to parse `type` as a child module name) but it + doesn't say *why* — a reader has no way to connect "unknown module divi/input" back to the + already-read caveat about reserved field names. Fixed by omitting `type` on insert and setting it + afterwards with `lit-divi-update`'s `set` (which *is* documented and worked first try). **Suggestion:** + detect this specific case (a `contact-field`/`social-media-follow-network` item with a `type`/`label` + key that isn't a valid child module name) and return a dedicated hint, e.g. "`type` on a contact + field can't be set at insert — omit it here and set it afterwards with lit-divi-update, or use attrs + (fieldItem.advanced.type)." + +3. **A section can't mix `rows` and `modules`.** Tried a section with a top-level `modules` (for a + heading) *and* `rows` (for the 3-column blurbs) — got `"divi[1]": "entweder rows oder modules + angeben"`. Fully actionable, fixed in one edit by wrapping the heading in its own single-column row. + Minor cost (one extra round trip), good error. + +4. **No shortform way to link one page to another by ID.** Draft pages get an **empty** `post_name` + until published (confirmed via direct DB read — this is normal WordPress behaviour for + draft/pending posts, not a plugin bug), so there's no stable pretty permalink to put in a button's + `url` field yet. The placeholder table (`divi-placeholders`) only offers `{{post:url}}` etc. for the + *currently displayed* post, not a way to point at an arbitrary other post/page by ID. I fell back to + raw `/?page_id=` query links for the CTA/blurb/menu targets. That works for an admin/preview + session but is fragile (changes once published) and dead for anonymous visitors before publish. + **Suggestion:** a placeholder like `{{link:}}` (or reusing the `related`/attachment-ID idiom + already used for images) that resolves through `get_permalink()` at render/save time — draft-safe now, + automatically correct after publishing. This would remove the single biggest gap between the + shortform and how a real multi-page site actually gets built. + +5. **`contact-form`'s own title has no shortform heading-level control.** After building the Kontakt + page, `lit-page-render-summary` flagged `"2 Überschriften der Ebene h1 im Inhalt."` — the + contact-form module renders its `title` as a hard-coded `h1`, and unlike `heading`/`blurb` (which + both expose a `level` shortform field), `contact-form` has none. Fixed by falling back to the full + form: `lit-divi-catalog` with `module: "divi/contact-form"` to find the path, then + `attrs: {"title.decoration.font.font": {"desktop": {"value": {"headingLevel": "h2"}}}}` — which + worked and cleared the warning. This was the one case where the shortform genuinely wasn't + expressive enough for a normal homepage pattern (a contact form title next to a page's own h1). + **Suggestion:** add `level` to `contact-form`'s shortform fields (and check other title-bearing + modules — e.g. `cta`, `map`, `slider` — for the same gap) so this doesn't require discovering the + full-form path by trial and error. + +6. **`preview_url` uses `http://`, `edit_url` uses `https://`, in the same response — bug.** Every + mutating tool response on this (https-only, port-8443) site shows this mismatch, e.g. the Kontakt + page creation returned + `"preview_url":"http://wordpress.ddev.site:8443/?page_id=831&preview=true"` next to + `"edit_url":"https://wordpress.ddev.site:8443/wp-admin/post.php?post=831&action=edit"`. See Bugs + below for repro. + +7. **Positive: error messages throughout were excellent.** `validation_failed` always came with a + `fields` map keyed by the exact dotted input path (e.g. `divi[1].rows[0].modules[1][0].items[0].type`); + `rev_mismatch`, `confirm_required`, and `unsupported` (tried to overwrite a Divi page's `content` via + `lit-content-update`) all had a `hint` that named the exact next tool to call, and following it + worked first try every time — no case where the hint was misleading or where I had to guess. + +8. **Positive: `tree_rev` chaining removed round trips.** Every Divi write response carries the fresh + `tree_rev`, so a sequence of edits on the same page (change heading → move a module → insert a + section) never needed an extra `lit-divi-outline` read in between. This is the detail I'd worry the + most about regressing — it's what makes multi-step page building on Divi tolerable turn-count-wise. + +9. **`lit/menu-create` accepting `location` in the same call was a nice shortcut** (no separate + `lit/menu-assign-location` round trip needed for a fresh menu). The one remaining unavoidable extra + round trip in the menu flow was calling `lit/menu-get` immediately after `lit/menu-create` just to + obtain `menu_rev` for the following `lit/menu-set-items` — `lit/menu-create`'s own response could + include the initial `menu_rev` and save that call. + +10. **`lit-divi-catalog`'s result envelope (`{modules: [...], notes: [...]}`) isn't shown anywhere** in + the tool description or the `divi-attrs`/`divi-shortform` guides (which do embed inline JSON + examples for almost everything else). Self-inflicted (my first `jq` filter assumed a bare array and + errored), but a one-line example would remove the ambiguity the same way the other guides do. + +## Prioritized improvement suggestions + +1. **(High)** Add an internal-link placeholder for Divi shortform/full-form URL fields that resolves an + arbitrary post/page ID to its current permalink (draft-safe), e.g. `{{link:}}`. This is the + most concrete gap: right now there is no supported way to cross-link draft pages that survives + publishing, and `/?page_id=N` is the only fallback. +2. **(High)** Give the `contact-form` module's shortform a `level` field (matching `heading`/`blurb`), + and audit other title-bearing modules (`cta`, `map`, `slider`, …) for the same missing control — + this is exactly the kind of heading-hierarchy mistake `lit-page-render-summary` is built to catch, + but the shortform currently can't fix it without discovering a full-form attrs path. +3. **(Medium)** Fix the `http://` vs `https://` scheme mismatch between `preview_url` and `edit_url` in + write-tool responses (see Bugs). +4. **(Medium)** When a shortform field name collides with a node-schema key (`type` on contact-field + items, `label` on social-media-follow-network items — both already called out in prose in the + `divi-shortform` guide), have the resulting `validation_failed` error say so directly instead of + surfacing as "unknown module divi/". The current message is technically accurate but + doesn't point back to the documented workaround. +5. **(Low)** Let `lit/menu-create`'s response include the new menu's initial `menu_rev`, so a + first-time `lit/menu-set-items` doesn't need a preceding `lit/menu-get` purely to fetch it. +6. **(Low)** Show a one-line example of `lit-divi-catalog`'s result shape (module list vs. single-module + detail) in its tool description or the `divi-attrs` guide, consistent with how other tools' guides + already embed inline JSON examples. +7. **(Nice-to-have)** Consider a `lit-guide` mode that returns all topics concatenated in one call for + first-connection priming, alongside the existing per-topic calls (which remain useful for targeted + lookups later in a session). + +## Bugs (with repro) + +**Bug: `preview_url` scheme mismatch (`http://` vs. the site's actual `https://`).** +- Repro: on this DDEV site (`https://wordpress.ddev.site:8443`, HTTPS-only), call any tool that returns + both URLs for a draft, e.g. `lit-content-create` for a page, or any `lit-divi-*` write. Example + (Kontakt page, ID 831): response includes + `"preview_url":"http://wordpress.ddev.site:8443/?page_id=831&preview=true"` while the same response's + `"edit_url":"https://wordpress.ddev.site:8443/wp-admin/post.php?post=831&action=edit"` is correctly + `https`. +- Impact: a client that opens `preview_url` as given may hit a mixed-content warning, an unwanted + http→https redirect, or fail outright depending on the site's HSTS/redirect configuration. + +## Divi shortform expressiveness verdict + +For a typical small-business homepage (hero, services grid, testimonials/values, contact form, +CTA, blog teaser text) the shortform covered essentially everything needed — `section`/`row`/`column` +nesting, `heading`, `text`, `button`, `image`, `blurb`, `cta`, `icon-list`, `contact-form`, placeholders +for the design system's colors — with only two genuine full-form fallbacks in this whole build: the +contact-form title heading level (§5 above), and (not really a fallback, just a two-step shortform +sequence) setting a contact-field's `type` after insert instead of at insert time. Column-width +changes, presets, and `$variable` placeholders were never needed for this content but are documented +clearly enough that I'd expect them to work the same way. + +## Cleanup performed + +- All 36 throwaway `lit-mcp-integration-test` application passwords created by repeated `connect()` + calls during this trial were removed via `cleanup_app_passwords_for` (verified count 36 → 0). +- `.superpowers/trial/` (the scratch scripts used to drive this session) was deleted from the container. +- No plugin source code was modified. No existing content (the 4 pre-existing pages, 1 existing post) was + touched. Nothing was published. diff --git a/docs/superpowers/notes/2026-09-26-etappe-2b-rulings.md b/docs/superpowers/notes/2026-09-26-etappe-2b-rulings.md new file mode 100644 index 0000000..2dce5f3 --- /dev/null +++ b/docs/superpowers/notes/2026-09-26-etappe-2b-rulings.md @@ -0,0 +1,159 @@ +# Etappe 2b – Entscheidungen (Rulings), Befunde und offene Punkte + +Aus dem SDD-Ledger übernommen, 2026-09-26. Security-Audit und Praxistest-Feedback liegen als eigene Notizen daneben (2026-09-26-etappe-2b-security-audit.md, 2026-09-26-etappe-2b-praxistest.md). + +## Offen für spätere Etappen +- Etappe 3: Link-Platzhalter {{link:ID}} für interne Links (Praxistest), Guide „alle Themen“, S19 (Designsystem-Größe/Filter, globalModule-ID in node-get). +- Etappe U: DNS-Rebinding-Pinning beim URL-Import, Race zwischen tree_rev-Prüfung und Schreiben (L3), Build-Hygiene (Dev-Abhängigkeiten, Test-Namespace im Autoloader, installed.json), „alle Verbindungen trennen“, Sitzungen nach Deaktivierung. +- Manuell (Benutzer): Abnahme laut 2026-09-26-etappe-2b-abnahme.md inkl. Visual-Builder-Prüfung und Builder-Fixtures (S20); Testdaten der Versuchswebsite zurücksetzen („Vorbereitung“). + +## Pre-flight rulings (adopted; override the plan text; full wording in preflight.md §6) +- Ruling S1–S17 adopted as proposed in preflight.md §6 — spec §5.6/§10/D7 + 2a R1–R3 binding; PoCs show stored XSS against plan code — cost if wrong: stricter input than needed (visible validation_failed, fixable) +- Ruling S3a: S3 routing must use the per-subName path kinds from the 2a final fix wave (composite innerContent: e.g. buttonOne.innerContent linkUrl → url, text → text; image alt → text). If the fix wave marked composite paths `unknown`, route sub-keys by key-name URL fallback, else text — 2a final review #1 — cost if wrong: javascript: in link sub-fields +- Ruling S13a: design-system label placeholders: ambiguous (duplicate) labels → validation_failed listing the ids; ids (gcid/gvid) accepted directly — 2a final review #14 — cost if wrong: wrong color silently applied +- Ruling S18: outline breadth limit/truncated (moved from 2a F2) goes into Task 4 (write results embed the outline) — cost if wrong: large outputs on big pages +- Ruling S19: design-system size/filter and globalModule id on node-get → Etappe 3/4 (library/design-system work) — cost if wrong: none +- Ruling S20: real builder fixtures (2a abnahme §1, globalModule) stay a manual user step before 2b ships; E2E gates on synthetic fixtures plus S7 fixture — cost if wrong: behaviour on real global modules only verified at acceptance + +## Progress +- Note S3a: 2a fix wave chose per-subName maps — ModuleCatalog path_kinds values are string|object(subName→kind); AttrWriter routing must branch on that +- Ruling S15a: 2a final wave merged on branch (HEAD 395e964). DiviIntegration.php unchanged since f433fef (T6 whole-file replace OK). T10 must EDIT (not replace) DiviOutlineAbility.php, DiviCatalogAbility.php, docs/guides/divi-attrs.md (changed by fix wave). Plan code must respect new 2a guards: Block::create name pattern, LogicException on opaque setAttrs/insertChild/removeChild, AttrMap rejects empty segments, node-get type is short name + rev — cost if wrong: failing tests surface it in task +- Task 1: dispatched (opus implementer, BASE 395e964) +- Task 1: review (opus): spec ✅ except I-1; changes requested. Rulings: +- Ruling S21 (concern a): before/after/date_format settings are CLEANED and the sanitized value is re-serialized (not rejected when html()/text() changes them); enable_html, unknown keys, embedded refs and `)$` still rejected — D7 says "String-Settings bereinigt"; every other path cleans — cost if wrong: none security-wise (output is re-serialized from sanitized strings) +- Ruling S22 (concern b): VariableRef takes an injectable allow-map name → allowed settings (key → field type); Task 1 keeps the current fixed list as default; Task 5 wires Divi's live registry DynamicContentOptions::get_options($post_id,'edit') (filter divi_module_dynamic_content_options) into it, routing settings by field type; custom_meta_ rule, enable_html rejection, loop_* only inside loops stay — S1's fixed list breaks round-trips of valid Divi pages — cost if wrong: broader accepted settings, each still sanitized per type +- Ruling S23 (M-4): unknown kind on an attribute named `content` (`content.innerContent`) → richtext (html(), kses for editors) instead of text — Divi defines those fields only in JS settings; silent loss of formatting otherwise; html() is still kses-safe — cost if wrong: an editor could store kses-allowed tags in a field Divi renders as text (escaped/harmless) +- Ruling S24 (c): string keys below a scalar-kind path are treated as unknown so the URL key-name fallback applies; key-name fallback also tests the last path segment (…Url/…url suffix, e.g. redirectUrl) — don't rely on Divi output escaping — cost if wrong: a few legit URL-ish texts get esc_url_raw +- Ruling S25 (M-3): Placeholders::isVariable removed from src (or delegates to VariableRef); Task 2 must use VariableRef — cost if wrong: none +- Note for Task 8 (forward risk): replace-text must never run cleanValue on existing unchanged $variable strings; only changed plain-text segments are re-cleaned +- Task 1: fix round 1/5 dispatched: I-1, M-1, M-2 (anchor (\.|$) + css.advanced.customCss under S7 gate), S21–S25 +- Task 1: fix round 1/5 (I-1, M-1, M-2, S21–S25 addressed; commit 7180690; 831 tests) +- Ruling S26: all free-form custom CSS paths (`*.css.*`, i.e. every module's css.advanced.* / customCss) go behind the S7 gate ("nur im Divi-Builder") for users without unfiltered_html — core requires edit_css (normally with unfiltered_html) for custom CSS; plain CSS injection (body{display:none}) otherwise possible — cost if wrong: editors can't set per-module custom CSS via MCP +- Ruling S27: global-variable and font references keep before/after only if Divi's registry/usage supports it; otherwise reject settings on gvid/font refs — implementer to verify in Divi source (DynamicData / GlobalData) — cost if wrong: harmless extra text around a variable +- Task 1: fix round 2/5 (S26, S27; commit eb99b36; 836 tests) +- Task 1: re-review (opus): all round-1/2 items ✅; NEW Critical-1 (sanitizer re-creates `$variable(` marker after the check: `$variable(`, NUL, `%41` → unvalidated ref stored, stored XSS on admin-authored pages), Important-1 (S22 map bypasses custom_meta_ rule; post_meta_key meta_key/select_meta_key allow protected meta), Minor-1 (settings re-check after cleaning), Minor-2 (malformed map → TypeError) +- Ruling S28: invariant "checks run on the FINAL string": every sanitized output (cleanValue results and every cleaned setting) is re-checked; if it contains the $variable marker (and it did not come out of VariableRef::canonical as the whole value) → validation_failed. Test rows ``, NUL, `%41` with a fixture sanitizer that strips tags/NUL/octets like WP — cost if wrong: none (reject is safe) +- Ruling S29: allow-map names starting `custom_meta_` obey the META_KEY rule; map values for meta-key settings (`meta_key`, `select_meta_key`, loop meta keys) are validated with the META_KEY rule (no leading `_`, after stripping `custom_meta_` prefix); map shape validated in constructor (InvalidArgumentException, never TypeError) — S12 must not widen via S22 — cost if wrong: none +- Task 1: fix round 3/5 dispatched (Critical-1, Important-1, Minor-1, Minor-2 via S28/S29) +- Task 1: fix round 3/5 (S28, S29, Minor-2; commit 665e603; 854 tests) +- Task 1: re-review round 3: Critical-1/Minor-1/Important-1/Minor-2 ✅; invariant held against esc_url_raw/nesting/keys/entities/multibyte; NEW Important-A (doubled custom_meta_ prefix → Divi str_replace strips all → `_edit_lock` readable, default map), Important-B (loop_*_meta_key_ prefixes not stripped), Minor-A (stale docblock VariableRef.php:88) +- Ruling S30: meta-key check applies Divi's own derivation before the META_KEY rule: str_replace of every `custom_meta_` occurrence (DynamicContentOptionPostMetaKey.php:232,275) and of `loop_post_meta_key_`/`loop_user_meta_key_`/`loop_term_meta_key_` (DynamicContentOptionLoopPostMetaKey.php:366-373); additionally reject if a prefix still occurs after one strip (defensive) — mirror Divi exactly — cost if wrong: none +- Task 1: fix round 4/5 dispatched (fresh opus implementer) +- Task 1: fix round 4/5 (S30; commit 4832ec1; 870 tests) +- Task 1: re-review round 4: clean. Note for Task 5 handoff + Divi upgrades: S29/S30 match meta-key settings by suffix `meta_key` — re-check on every Divi version bump +- Task 1: complete (commits 395e964..4832ec1, review clean after 4 fix rounds) +- Task 2: dispatched (opus implementer, BASE 4832ec1) +- Task 2: review (opus): spec ✅, quality Approved; 59 PoCs live, none stored script/handler/javascript:/CSS breakout +- Ruling S31 (T2 Important-1): plan text for T3 (ComposeFixture::shortform anchor) and T5 (DiviComposition ShortformWriter wiring) is stale → dispatches carry: ShortformWriter(Placeholders, MediaResolver, AttrWriter, ?array $modules=null); ONE Placeholders instance (with S22 live map) shared by AttrWriter and ShortformWriter; fixture shortform(bool $unfiltered=false, ?array $modules=null) — cost if wrong: TypeError at wiring (caught by tests) +- Ruling S32 (T2 Minor-1): control characters (C0 except \t\n\r, and DEL) are stripped in AttrWriter's final-string path BEFORE the marker check (single choke point), and the fixture text() mirrors WP (keeps NUL) so the test is honest — carried into Task 3 as carry-in fix — cost if wrong: none +- Ruling S33 (T2 Minor-2/3/5): carry-in to Task 3: cleanValue kind override may only tighten (reject weaker override than routed kind); attachment alt containing the marker is dropped (treated as empty alt), never canonicalized; color-reference check via a VariableRef type accessor instead of string prefix — cost if wrong: none +- Task 2: minor (parked): $modules test seam in ShortformWriter constructor (trusted wiring only); {{post:title}} accepted in URL fields (Task 1 canonical url mode) — reconsider in T10 docs +- Task 2: complete (commits 4832ec1..f86cd5b, review clean) +- Task 3: dispatched (opus implementer, BASE f86cd5b, carry-ins S32/S33) +- Task 3: implementer done (commit 4b72c2b; 1060 tests); note for T10 docs: short-form fields type (contact-field) and label (social network) clash with node keys → only via set/full form +- Task 3: review (opus): Critical-1 (S32 strip placed AFTER sanitizer: `<\x0Bimg onerror>` passes sanitize_text_field, strip then creates tag → stored XSS confirmed live); minors 1–5 +- Ruling S32a: control-char strip runs on the RAW input BEFORE the sanitizer and before the raw marker check (cleanNode + VariableRef::setting); S28 final-string marker check stays after the sanitizer; nothing may transform the string after the sanitizer except rejection — cost if wrong: none +- Ruling S34 (T3 minors): VALUE_KEY gets /D; column widths checked with exact fraction arithmetic; StructureRules: allow divi/row inside divi/column (Divi 5 row nestable:true) but refuse plain modules in specialty columns (specialtyColumns set or row-inner children); preset ambiguity rule moved into DesignSystem::modulePresetId (shared) and Composer uses it; C1/LS/PS no change — cost if wrong: small +- Task 3: fix round 1/5 dispatched +- Task 3: fix round 1/5 (S32a, S34; commit 8688a02; 1070 tests); specialty columns refuse every child (conservative: specialty treated read-only) +- Task 3: re-review clean (Critical-1 fixed live; S34 ✅) +- Ruling S35 (T3 Minor-A): carry into Task 5: strip control chars once at entry of AttrWriter::apply and ShortformWriter::apply (before placeholder resolution / trim) so `{\0{post:title}}` can't bypass placeholder checks — cost if wrong: none +- Task 3: minor (parked, T10 cosmetic): nested row auto-label equals outer row label +- Task 3: complete (commits f86cd5b..8688a02, review clean) +- Task 4: dispatched (sonnet implementer, BASE 8688a02; S9, S14, S18) +- Task 4: minor (deferred, final wave): placeholder check duplicated as string literal (TreeEditor:68,146, NodeIndex:39,105) → Block::isPlaceholder(); move() StructureRules violation lacks a dedicated test +- Task 4: complete (commits 8688a02..ea1fd60, review clean) +- Task 5: dispatched (opus implementer, BASE ea1fd60; S4, S5, S7, S11, S22, S31, S35) +- Task 5: implementer done (commit 7762b76; 1114 tests; DDEV byte-identity as editor verified, cleanup done) +- Ruling S36 (T5 concerns): (1) loop_* options never accepted (stricter than Divi; tools can't know loop context) — accepted; (2) before/after cleaned as html (kses for editors) instead of text, to keep meaningful spaces — accepted, marker/meta checks still apply; (3) {{post:categories/tags}} filled into the map for pages although Divi's page registry omits them — accepted (S22 requires placeholders keep working) — cost if wrong: minor UX +- Note: Tasks 6–9 must obtain DiviComposition via DiviWriter::composition(DiviPage) (per-post live map) +- Task 5: review (opus): spec ✅, changes requested. I-1: RevisionPoint::capture runs before the bypass → restore-point revision kses-filtered for editors (live: iframe stripped, re-serialized) → undo path loses admin embeds +- Ruling S37: restore-point capture runs inside the same kses-off window (revision copies stored content, not client data); DDEV assertion revision === original; M-1 map field type url → url(), unknown/other types (not text/richtext/html/select/yes_no_button/select_post/number) rejected; M-3 select_post post_id numeric + current_user_can('read_post'); M-4 log getMessage(); M-5 comment documenting bypass window + hooks (no MCP input reaches them); M-7 DiviFlags also sets _et_pb_show_page_creation=off if Divi's own save does (verify); M-2 documented (suffix note exists); M-6 accepted (1a pattern) — cost if wrong: none +- Ruling S38 (for Task 6): a single update touching the same attribute path via both `set` (short form) and `attrs` (full form) is rejected with validation_failed (composite values are replaced whole per breakpoint; combining silently loses sub-keys) — cost if wrong: client must split into two changes +- Task 5: fix round 1/5 dispatched +- Task 5: fix round 1/5 (S37; commit b8cb018; 1117 tests; ValueSanitizer::canReadPost added) +- Task 5: re-review clean (I-1 live byte-identical revision; 162 core option/settings combos accepted, 0 refused; flags on/on/off) +- Task 5: minor (parked): select_post slugs no longer accepted (stricter than Divi; untouched refs unaffected); partial_write reported conservatively when rollback after capture failure fails +- Task 5: complete (commits ea1fd60..b8cb018, review clean) +- Ruling S39 (T6–T9 abilities): all-or-nothing is enforced at ability level (any validation error → nothing saved, all errors returned); abilities never call setAttrs/insertChild with client values except through Composer/AttrWriter/ShortformWriter/TreeEditor; composition only via DiviWriter::composition(DiviPage) — Task 5 review ⚠️ — cost if wrong: none +- Task 6: dispatched (opus implementer, BASE b8cb018; S7, S9, S14, S38, S39) +- Task 6: implementer done (commit 110beb7; 1128 tests) +- Ruling S40: S38 overlap rule also applies to insert/compose (Composer::module/compose nodes with set + attrs on the same path → error) — same silent-loss risk — cost if wrong: none +- Note T10: guide topic "divi-shortform" referenced by lit/divi-insert description must exist after T10 (check GuideRepository test) +- Task 6: S40 addendum (commit 22bb152; 1130 tests) +- Task 6: review (opus): spec ✅; live E2E ok (placeholder wrap, byte-exact update, conflict, perms); no structure bypass on live catalog. Important-1: admin label lost when attrs sets module.meta.adminLabel (auto label/label overwrites) +- Ruling S41: `label` counts as short-form path module.meta.adminLabel in overlaps() (compose + update); auto label skipped when attrs sets it; heading default level merged into font value instead of replaced; structure deny-list in AttrWriter (section module.advanced.type, column module.advanced.specialtyColumns) regardless of catalog; column flexType on update checked with Composer::widths rules — cost if wrong: none +- Task 6: fix round 1/5 dispatched +- Ruling S42 (user request 2026-09-26): after Task 10, before the final review: (1) clean-code review of the whole plugin (opus: file size/responsibilities, cross-stage duplication, readability of security logic, test helpers) → refactor wave (behaviour-preserving, suite green, security code only restructured); (2) THEN a dedicated whole-plugin security audit (opus, adversarial: OAuth/role gate/RequestContext, capabilities per object, kses bypass + all value paths, media import/SSRF, change log/admin UI (nonces, escaping), info leaks, uninstall) on the final code, with live DDEV PoCs; its findings go into the final fix wave together with the final review — user: "security check wäre am ende wichtig" — cost if wrong: extra review time +- Task 6: fix round 1/5 (S41; commit 909ac82; 1136 tests). Controller commit (user request, outside plan): settings page logo +- Task 6: re-review round 1: Important-1/Minor-1/Minor-2 ✅; NEW Important-A: row-width sum check refuses legit builder edits (none columns, flexWrap wrap, omitted flexType default 24_24) +- Ruling S43: on update check only the FORMAT of each new desktop flexType (the 32 builder values from field-library.js incl. `none`, or null removal); sum check only if the row's current widths already pass rowWidthsError (no none/omitted/desktop flexWrap wrap), otherwise accept; Minor-A: unknown-path error wins over overlap message for the same key; Minor-B: separate structure error text for columns; Minor-C (flexColumnStructure stale after width update) → Task 10 guide note — cost if wrong: a VB row can end up with odd widths (visible, fixable in builder) +- Task 6: fix round 2/5 dispatched +- Task 6: fix round 2/5 (S43 + grid rows skip sum; commit 0fddece; 1140 tests) +- Task 6: re-review round 2: clean +- Task 6: minor (deferred, final wave): drop 4_5 from insert SPECIAL_WIDTHS (builder list parity); format-check tablet/phone flexType via Composer::isColumnClass; T10 guide/hint: replacing desktop module.decoration.sizing drops flexType (send flexType along); flexColumnStructure stale after width update +- Task 6: complete (commits b8cb018..0fddece, review clean) +- Task 7: dispatched (sonnet implementer, BASE 0fddece) +- Task 7: implementer done (commit amended for trailer; only 1 new test for 3 abilities → test addendum requested before review) +- Task 7: test addendum (commit 1740254; 1153 tests; fixed DiviWriter::outline dropping outline for delete-only changes) +- Task 7: review (sonnet): spec ✅; live E2E byte-exact; Important-1 duplicate label built from NodeSummary::label (truncated/whitespace-collapsed display value) → use raw stored adminLabel; fix round 1/5 dispatched +- Task 7: fix round 1/5 (commit ede0221; 1155 tests) — Ruling: one-leaf fix verified by controller against the diff instead of a re-review agent — cost if wrong: none +- Task 7: complete (commits 0fddece..ede0221, review clean) +- Task 8: dispatched (opus implementer, BASE ede0221; S2, S6, S8, S10, S28, forward-risk no re-clean of unchanged) +- Ruling S44 (user request 2026-09-26, parallel): tool groups + inheritance + 'nur lesen' per group built in worktree .superpowers/worktrees/tools-gruppen (branch tools-gruppen, base ede0221) by a separate implementer; must not touch DiviIntegration/overview.md/readme; merged into etappe-2 after its review, before T10 (T10 adds the docs text); parallel implementers justified by user request + worktree isolation — cost if wrong: merge conflict in Plugin.php +- Task 8: implementer done (commit d67cd0e; 1222 tests); deviations: clean callback gets the replacement only (not whole string) + structural-equality guard (tags/entities/refs of result = original + inserted), AttrWriter::kindOf added, url/code fields never touched +- Env (user-authorized exception to plugin-dir rule, 2026-09-26): .ddev/nginx/well-known.conf added (^~ /.well-known/ → index.php), ddev restarted; discovery reachable, dotfiles still 403. Finding: WP redirect_canonical 301s /.well-known/oauth-* to trailing slash → plugin fix planned (redirect_canonical false for mcp_oauth_discovery) +- Task 8: review (opus, partial live): Critical: S8 split `<[^>]*>` fooled by `>` inside quoted attribute (title="A > B") → following attrs treated as text → replacement with quote+event handler stored & rendered (live stored XSS as editor); also rewrites href/src (D10). Important: opaque/global modules silently skipped (report claimed otherwise) → user thinks text gone site-wide +- Ruling S45: TextReplacer tokenizer must be quote-aware and cover comments: tag token `<(?:[^>"']|"[^"]*"|'[^']*')*>`, comments `` (s), unterminated `<…`/unbalanced quotes → string with hits refused; the structural guard compares FULL tag tokens (not just positions), so any change inside a tag is refused; independent final context assertion: every hit position lies in text context per a second scan (e.g. WP_HTML_Tag_Processor if available, else own state machine); malicious rows both quote styles + comment + unterminated tag, with mutation check; dry run + result list `skipped_nodes` [{post_id,id,reason:"globales Modul – in der Bibliothek bearbeiten"}] for opaque/global nodes containing the search; pages/blocks without effective change are not saved (no revision/log); tool description documents trimming of replacement in text fields — cost if wrong: none +- Task 8: fix round 1/5 dispatched +- Controller commit (user-requested OAuth local test): DiscoveryCanonical (no trailing-slash 301 on /.well-known/oauth-*), smoke /.well-known now a hard check (200, no redirect); local OAuth via Claude Code (CIMD) → lit-local ✔ Connected (user-scope MCP entry) +- Tool groups branch: review clean (sonnet); minors: comment why ToolsTab re-applies withGroups; ExposurePolicy::$groups docblock null vs [] → final wave. Merge into etappe-2 after Task 8 fix round commits +- Tool groups merged into etappe-2 (merge commit 3efcfaa; 1321 tests, lint ok); worktree + branch removed +- Task 8: fix round 1/5 (S45; commit 93d2bde; HtmlContext state machine; unquoted '<' ends tag candidate) +- Task 8: re-review agent stopped live testing (incomplete; read-only assessment positive); fresh opus re-reviewer dispatched with explicit defensive-test framing +- Task 8: re-review agents stopped live payload testing twice (classifier); Ruling S46: controller verified S45 directly — read-only DDEV script with real kses as editor, benign probe marker (data-probe attr): quoted '>' dq/sq, href-only hit, unquoted attr, comment, bare '<', textarea → all hits in text only, no attribute change; unterminated tag refused; 20k adversarial input refused in 1 ms; remaining cases covered by named unit tests (141 green: control chars/empty search, no-op no save, S10, cursor/limit, byte identity next to unknown dynamic content, skipped global nodes) — cost if wrong: residual edge case found in final security audit (S42) +- Task 8: complete (commits ede0221..93d2bde, review clean via S46) +- Parallel (user request): admin UI/UX pass dispatched (opus, brief .superpowers/sdd/admin-ux/brief.md; only src/Admin, assets, tests/Unit/Admin) +- Task 9: dispatched (opus implementer, BASE 3efcfaa; S9, S11, D7 single save path, S40/S41) +- Task 9: implementer done (commit 5965e32; 1339 tests; live DDEV ok); deviations: check/write instead of compose/afterInsert, DiviSaver::saveNew, combined errors; draft deleted if page map refuses +- Task 9: review (opus): spec ✅, Approved; live DDEV: valid create byte-exact, flags on/on/off, no script; refusal path leaves 0 rows +- Task 9: minor (deferred, final wave): discard() reports REASON_EXTRAS when the layout write fails (pass reason); DiviContentComposer::layout duplicates DiviInsertAbility compose+insert error mapping → shared helper; import order WordPressIntegrationTest +- Task 9: complete (commits 3efcfaa..5965e32, review clean) +- Task 10: waits for the admin UI agent (both touch Plugin.php/readme) +- Admin UI/UX pass committed (843f45a; 1364 tests); ConnectionUsage (last-use tracking via WP app-password record_application_password_usage, daily) added; review pending +- Task 10: dispatched (opus implementer, BASE 843f45a; S6 benign probes, S7 fixture+E2E §8, S15a, S16, divi-shortform guide, collected T10 notes, version 0.3.0, abnahme) +- Admin UI review (sonnet): spec ✅, changes requested: Art column maps destructive→"löscht" (wrong for replace/restore/menus/replace-text/trash); ConnectionUsage gates on is_wp_error but real 401s are WP_REST_Response +- Ruling S47: Art labels: explicit deletes (lit/media-delete, lit/divi-delete) → „löscht“; lit/post-trash → „Papierkorb“; any other destructive → „überschreibt“; non-readonly → „schreibt“; readonly → „liest“ (icons accordingly); ConnectionUsage records only when response status < 400 (plus existing sub===user check), test with WP_REST_Response(401); LogListTable default result → translated fallback — cost if wrong: none +- Admin UI fix round 1 (S47; b044cac; 1380 tests) verified by controller against the diff — admin UI pass complete +- Task 10: implementer committed 10c98cd then hit session limit during final checks; controller re-ran suite (1380) + lint ok; D11 superseding note added (e0ab6f5); smokes running +- Task 10: all five DDEV smokes pass (divi-write, mcp, wp-basis, media-admin, divi-read); review running +- Task 10: review (sonnet): spec ✅; Important: S7 exception text overstated (only pages not yet in Divi's serialize_blocks form) → fixed by controller in guides×3, readme, spec §1.3/§5.2, decisions note (e005d83) +- Task 10: complete (commits 843f45a..e005d83, review clean) +- S42 step 1: whole-plugin clean-code review dispatched (opus) +- S42 step 1 done: cleancode-plan.md (22 items + T1–T5). Ruling S48: refactor in 3 sequential waves following the plan's commit order — W1 foundation (M6, L4, H6, M5, M7, T1, T2, T3), W2 security restructure (H2, H3, H1, L1, M1, M4 + VariableRef META_KEY/NUMBER /D fix as a deliberate behaviour fix with tests), W3 rest (H5, H4, M3, M2, T5, M8, M9, L2, L3, L5 + all ledger "deferred, final wave" minors); behaviour-preserving except named fixes; every wave: full suite + lint + all 5 DDEV smokes; W2 gets an opus review of the check order diff — cost if wrong: refactor regressions (caught by 1380 tests + smokes) +- Ruling S49 (ledger items needing a decision): drop 4_5 from insert widths (builder parity) and format-check tablet/phone flexType with the builder list — both in W3 — cost if wrong: a client can't use 4_5 (not offered by the builder anyway) +- W1 dispatched (opus) +- W1 done: b6984d3..60290c8 (M6, L4, H6, M5, M7, T1, T2, T3); 1409 tests/6838 assertions; 5 smokes pass; report refactor-w1-report.md +- Ruling S50: W1 findings → W3: MenuWriter::fail guard the new-item delete (rollback continues, partial_write instead of internal_error; test); content-create layout-write failure passes its own reason to NewDraft::discard (T9 minor) — cost if wrong: none +- W2 dispatched (opus) +- W2 done: e420145 (/D fix: VariableRef META_KEY/NUMBER/WHOLE, Cursor, GuideRepository, ExposurePolicy×2, LogTab×2), 874a92c H2, 203d14a H3, 62ce99e H1, 570c1e6 L1, 3eeb7ee M1, 6cf0a56 M4; 1421 tests; malicious providers byte-identical (sha256); 5 smokes pass; ValueKind enum phpcs compat sniff disabled locally +- W2 opus review dispatched (check-order diff); W3 dispatched in parallel (disjoint scope; W2 review fixes applied afterwards) +- W2 review (opus): Approved, no Critical/Important. Differential harness old vs new tree: 4,383 probes over 6 paths; 4,266 byte-identical, 117 differ only by trailing "\n" (accept→reject or earlier reject); 0 reject→accept +- W2 minors → final wave: Composer gets the cleaner from AttrWriter or assertSame(cleaner) in DiviCompositionTest (S31 identity); DynamicContentRules↔VariableRef static error factories to a small shared place; HtmlTokens RAW_OPEN built from RAW_ELEMENTS; DiviSaver docblock (in W3) +- W3 done: 9da236b..3a5f658 (16 commits; H5, H4, M3, M2, T5, M8, M9, L2, L3, L5, T4 + S49/S50 behaviour fixes + ledger minors); 1449 tests; T5 partial_write wording kept (not misleading) +- Controller: smoke guard (finish/guard_exit — exit 0 without reaching the end fails) after W3 reported a silent mcp-smoke stop with exit 0; all 5 smokes pass (314b3d0) +- S42 step 2: whole-plugin security audit dispatched (opus) +- Parallel (user request): trial-website agent (sonnet) builds '[Test] …' drafts via the MCP endpoint (lib.sh JSON-RPC) and collects interface feedback → trial-feedback.md; its findings feed the final fix wave or a follow-up plan +- Trial site (sonnet via MCP): 4 Divi pages (839/834/837/831), 3 posts, menu 22 on primary-menu, 3 images; site title/tagline changed to „Tischlerei Holzwerk Berger“ (user informed). Feedback trial-feedback.md: High: internal-link placeholder by post ID ({{link:ID}}); contact-form title level in short form; Medium: preview_url http vs edit_url https (bug); shortform key collision → clearer error; Low: menu-create returns menu_rev; catalog result shape example; guide all-topics mode. → final fix wave (bug + error hint + level + menu_rev + docs); link placeholder = small feature (ruling at fix wave) +- Controller: Tools tab without code-module note, counter only as warning > 25 (22c4cc1) +- S42 step 2 done: security audit (opus) — no Critical/High; Medium: M1 raw `content` bypasses S7/S26 builder-only gates (content-create, content-update on non-Divi page; live-verified); M2 'nur lesen'/disabled Divi group doesn't stop content-create divi; M3 URL import SSRF residue (DNS rebinding, error oracle, WP 6.9 ranges); M4 Pods field restrictions ignored via PHP API; Low: L1 any logged-in user completes OAuth, L2 build hygiene (dev deps loaded, tests namespace in autoload, path disclosure, installed.json), L3 check-then-write race, L4 sessions survive deactivate; Info items → security-audit.md +- Final whole-branch review (opus) dispatched: 395e964..HEAD +- Protokoll redesign committed (0f23192; 1464 tests): columns Änderung|Objekt|Wer|Zeit, non-ok rows marked, grouped tool filter, screen options; controller removed 2 audit-probe log rows (users 66/67) +- Final review (opus): ready with fixes; §5 invariants hold end to end; must: audit M1, M2, I-1 (content-create edit_url → builder), I-2 (abnahme note); triage written to final-review.md +- Ruling S51: one final fix wave per final-fix-brief.md (must 1–4 + cheap 5–15); deferred list per triage (link placeholder → Etappe 3, DNS pinning/L3/build hygiene → Etappe U) — cost if wrong: none +- Final fix wave done: 7d0837b..63093be (14 commits; 1522 tests; 5 smokes incl. new §9 execute-ability path); deviations: item 6 only contact-field type, item 9 not reproducible over https (artifact), item 12 AuthorizeGate before mcp-oauth handler, m-9 deferred; scoped re-review (opus) dispatched +- Final fix re-review (opus): 14/15 ✅ (M2, M3 IP tricks 25 probes, M4, AuthorizeGate live, ABSPATH 182 files); ❌ M1 NUL-byte bypass (check before kses' wp_kses_no_null; live-stored divi/text as Author); minors: MediaImporter logs full URL incl. userinfo/query; uninstall docblock; media-update description can hold Divi block markup +- Ruling S52: M1 check runs on the content normalized exactly like storage will see it — strip NUL/C0 controls (wp_kses_no_null + ControlChars) BEFORE the Divi-markup check, and refuse if the normalized form contains Divi markup; same guard for every raw-content path incl. lit-media-update description; tests with \0 and other control variants inside the delimiter and shortcode; MediaImporter logs URL without userinfo/query; uninstall docblock — cost if wrong: none +- Controller: admin UI neutral wording (no claude.ai branding; 'KI-Assistent'/MCP-Server (Connector)) per user request — docs/readme still mention claude.ai as the primary client +- S52 fix committed by implementer before session limit (99eda25: DiviMarkup shared check raw+normalized, media-update description, MediaImporter log redaction, uninstall docblock); controller: 1597 tests, lint ok, smokes running +- S52 verified by controller live (NUL/VT/backslash-0/shortcode variants refused, plain block markup allowed); 1597 tests; 5 smokes pass — Etappe 2b final wave complete diff --git a/docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md b/docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md new file mode 100644 index 0000000..e62b20f --- /dev/null +++ b/docs/superpowers/notes/2026-09-26-etappe-2b-security-audit.md @@ -0,0 +1,336 @@ +# WP-LIT-MCP: whole-plugin security audit (S42 step 2) + +- **Scope:** branch etappe-2, HEAD 314b3d0. +- **Environment:** WordPress 7.1.2, Divi 5.2.1 on DDEV. +- **Method:** + - Read all of `src/`, plus the relevant parts of `vendor/wp-media/mcp-oauth` and `vendor/wordpress/mcp-adapter`. + - Ran read-only DDEV `wp eval-file` probes. + - Ran one live, self-cleaning run of the real `lit/content-create` ability as a temporary Author: + - user 67 and post 861 were created, then deleted; + - the deletion was verified afterwards: no `lit_audit_*` user and no probe post remain. + - No code was changed. The option `lit_mcp_allowed_roles` was widened in-process only, through the filter, never in the database. + +No Critical or High findings. The Divi value pipeline itself held. +The two most relevant problems are ways *around* it: +- the raw `content` field; +- the tool-group switch. + +--- + +## Findings (most severe first) + +### M1 – Medium: raw `content` lets users without unfiltered_html write the "builder-only" Divi values (the S7/S26 gates don't apply) + +**Where** +- `src/Integrations/WordPress/Content/PostWriter.php:28` (content → post_content) and `:58`. +- `ContentCreateAbility.php:127-133`. +- `ContentUpdateAbility.php:124` refuses `content` only when the post *already* uses Divi. Nothing refuses content that *introduces* Divi markup. +- Compare `src/Integrations/Divi/Compose/ValueCleaner.php:57` (`BUILDER_ONLY`). + +**Scenario** +1. An Author, or an Editor on multisite or with `DISALLOW_UNFILTERED_HTML`, calls `lit/content-create` (or `lit/content-update` on a non-Divi post). +2. The `content` is Divi 5 block markup that sets: + - a module's `css.desktop.value.freeForm` (e.g. `body{…}`); + - `module.decoration.attributes` (custom HTML attributes). +3. kses (`filter_block_content`) and Divi's own `wp_insert_post_data` security filters keep both. Only `on*` attributes are stripped, by Divi. +4. The page is now a Divi layout. From then on every `lit-divi-*` tool works on it inside the kses window. + +The same values are refused with "nur im Divi-Builder" (only in the Divi builder) when sent through `lit-divi-insert`/`-update` (rulings S7/S26). + +A prompt-injected Claude session can therefore do CSS injection or attribute injection on the live site. This is exactly what S26 was meant to prevent. + +**Evidence** +- Live run of `wp_get_ability('lit/content-create')->execute()` as a temporary Author (unfiltered_html=false): + - `freeForm CSS stored: true`; + - `data-probe stored: true`; + - rendered `
`. +- Read-only kses probe (`kses6.php`): `onmouseover` is removed by Divi and never rendered. No script execution found. Divi renders a `javascript:` `linkUrl` as `href=""`. + +**Parity note** +The same user can store the same markup through core REST (`/wp/v2/posts`) or the code editor. So this is no privilege escalation beyond WordPress core. It is a bypass of the plugin's own, explicitly decided gate. The pipeline guarantee "every path from ability input to post_content goes through ValueCleaner/Composer/TreeEditor" does not hold for `content`. + +**Fix proposal** +- In `PostWriter::prepare()` (create and update), refuse a `content` that contains ``). + +### I3 – Info: smaller observations + +1. **Multisite super admin bypasses the role gate.** A super admin always has `lit_mcp_use` (`WP_User::has_cap`), even if "Zugriff" allows no role. `AccessTab::currentUserLosesAccess` knows this; the UI text doesn't say it. +2. **Media usage lists unreadable posts.** `MediaUsageFinder.php:455-461` returns post ID and post type (not the title) for posts the user can't read. This lets a user learn that private posts exist. It is intentional, to block deletes. +3. **All post types with a UI are exposed.** Every `show_ui` post type, including non-public ones (on DDEV: `path`), is exposed; only core, Divi internals and `lit_mcp_excluded_post_types` are excluded. Consider documenting the filter for data-protection purposes (form submissions, personal data as CPTs). +4. **Nested REST requests can block later calls.** A `rest_do_request()` to another route during a tool call makes `RequestContext` false for the rest of that HTTP request (`RequestContext.php:281`). This fails closed, so later JSON-RPC calls in the same batch get `forbidden`. Functional only. +5. **`confirm: true` is not a human confirmation.** Claude sets it itself; under prompt injection it is no safeguard. Keep the destructive tools indirect by default. + +--- + +## Checked and found sound + +- **RequestContext:** recomputed on every matched REST request, cleared after our route, and required by every ability. WP-CLI/STDIO and other routes fail closed. +- **Default mcp-adapter server:** disabled; the only MCP route is `/mcp/mcp-oauth-server` (verified via `rest_get_server()->get_routes()`). +- **Core Abilities REST:** our abilities have `show_in_rest=false` (only the 3 `core/*` abilities are in REST). +- **Transport and JWT:** `OAuthHttpTransport` requires a Bearer JWT (HS256 pinned, exp, aud = our route, iss = `home_url()`, app password must still exist). The current user is set only from the JWT; cookie or Basic auth can't reach the tools. +- **Revocation:** + - AccessTab revoke deletes the app password, which the next request's check honours; + - refresh-token rotation and reuse detection live in mcp-oauth; + - revoke is limited to `edit_user`-able users and, on multisite, to site members. +- **ConnectionUsage:** records only after a <400 response on our route, and only for `sub === current user`. +- **`lit_mcp_use` gate:** dynamic cap via `user_has_cap` (no role DB changes); the option is sanitized to existing roles; the filter output is type-checked; the meta-tools' capability filters are set to `lit_mcp_use`. +- **Exposure:** + - disabled and "nur lesen" abilities are never registered, so `execute-ability` answers "not found"; + - DIRECT_ONLY abilities are `mcp.public=false`, so `execute-ability` refuses them; + - `lit/guide` is always direct. +- **Annotations:** every tool annotated `readonly` really doesn't write (render-summary runs `the_content` side effects only). +- **Per-object checks:** + - `PostGuard::editable` (`edit_post`, managed type, trash rules) on every content, status, revision, render and Divi ability, in both permission and execute; + - revisions are checked via the parent and `post_type === 'revision'`; + - attachments via `MediaGuard` (`edit_post`/`delete_post`, type check); + - menu items via item ID membership in the menu. +- **Nested IDs:** + - featured image, parent and relation fields: `read_post` plus type checks; + - menu `object_id`: `read_post`, no private, unpublished only with a flag, taxonomy must be viewable; + - Divi media/gallery and `select_post`: `read_post`; + - terms: `assign_terms`/`edit_terms`; + - meta: `edit_post_meta`/`add_post_meta` per key. +- **Query and search:** + - `content-query`/`media-search`/`replace-text` limit the query to the user's own posts without `edit_others_posts` and gate statuses; + - hidden fields can't be used as a query filter. +- **Settings:** + - `manage_options`; + - strict allow-list; + - `admin_email`/`users_can_register`/`default_role`/`siteurl`/`home` explicitly blocked; + - per-key validation (installed languages, valid timezones, published pages, image attachment). +- **Meta writes:** + - `update_post_meta` double-serializes a serialized string, so there is no object injection; + - protected keys are refused through the meta caps; + - clone and replace copy raw stored meta via `$wpdb` (no unserialize). +- **Menu URLs:** only http/https/mailto/tel-style schemes, `/path` (not `//` or `/\`) or `#`; control characters and spaces are refused; `esc_url_raw` is applied. +- **Media files:** + - MIME allow-list intersected with the core-allowed types for the user, checked against the real content (`wp_check_filetype_and_ext`); + - no SVG by default; + - `sanitize_file_name` (no traversal, double extensions defused); + - streamed size cap; + - Base64 size checked before decoding; + - temp files removed on every path; + - download and preview only read files `realpath`-inside uploads; + - caption and description go through `wp_kses_post`. +- **Error handling:** + - Throwables are mapped to `internal_error`, details only go to the PHP log; + - the sideload error is hidden; + - no SQL or path text reaches the client (apart from the M3 download messages). +- **SQL:** every query is `$wpdb->prepare()`d. The only interpolations are table names and the constant status list; the log filters are whitelisted. +- **Change log:** holds titles and references only, no content. Admin list output is escaped; revision links are gated by `edit_post` on the parent. +- **Admin UI:** + - every admin-post action has `manage_options` plus `check_admin_referer` (revoke nonce per UUID, clear-log); + - settings go through `options.php` with `settings_fields` nonces and sanitize callbacks; + - `closedpostboxes` is core AJAX with its nonce; + - notices and tabs come from fixed maps; + - everything is escaped (`esc_js`+`esc_attr` for confirm handlers). +- **Site Health and admin notices:** static text only, escaped; the loopback check is cached and only runs on demand. +- **Guides:** topic slug regex plus the glob'd file list (no traversal); filtered guides are site-owned. +- **Divi pipeline integration:** + - the kses window covers only `RevisionPoint::capture` plus the one `wp_update_post` and is restored in `finally`, only when kses was active; + - Divi's own `wp_insert_post_data` security filters stay (verified: they strip `on*` attributes); + - the insert, update, move, delete and duplicate-node tools only write through Composer/TreeEditor; + - duplicate-node adds only the stored label plus a constant; + - replace-text re-checks targets per post; + - DynamicContentMap drops `loop_*` options and types come from the registry; the fixed rules (enable_html, meta keys, markers) remain. +- **Uninstall and deactivation:** both remove their own options, transient, cron and table on every site of a network, never posts. +- **Limits and DoS:** + - cursor offset capped at 9 digits, page size ≤100; + - batch caps (media-update 50, divi-update 50, insert/delete 20, replace-text 20 targets / dry run ≤100 pages, content-create `divi` 30 sections); + - render summary strings bounded; + - text search limited to 200 characters.