Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
32 KiB
32 KiB
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 sincef433fef(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<x>(, 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
<x>, NUL,%41with 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 strippingcustom_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_lockreadable, 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 ofloop_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) andattrs(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:
labelcounts 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 listskipped_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
10c98cdthen 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),874a92cH2,203d14aH3,62ce99eH1,570c1e6L1,3eeb7eeM1,6cf0a56M4; 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
contentbypasses 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