janniclas.loosenandClaude Opus 5.5
395e9644ac
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
35 lines
5.7 KiB
Markdown
35 lines
5.7 KiB
Markdown
# Etappe 2a – Entscheidungen (Rulings) und offene Kleinigkeiten
|
||
|
||
Aus dem SDD-Ledger übernommen, 2026-09-26. Weiterführende Rulings für 2b stehen im 2b-Ledger bzw. später in 2026-09-26-etappe-2b-rulings.md.
|
||
|
||
## Pre-flight rulings
|
||
- R1 (blocking B1/B2): kses bypass on Divi saves is only acceptable if NO client-supplied string reaches post_content unsanitized. `$variable(...)$` values from input (placeholders AND raw values echoed back from divi-node-get) are never written as-is: parse the JSON payload, validate strictly (type ∈ {color, content}; name ∈ existing gcid-*/gvid-*/known dynamic-content source names/custom_meta_<key> with key [a-zA-Z0-9_-]; settings keys whitelisted; before/after/other string settings sanitized with wp_kses_post for users without unfiltered_html, sanitize_text_field for plain keys), then RE-SERIALIZE by the plugin. Anything else → validation error. D7 text in 2a Task 1 (spec amendment + decisions note) must describe exactly this ("werden geparst, geprüft und vom Plugin neu erzeugt"), not "alle Werte bereinigt" in general. Implementation happens in 2b (Placeholders/AttrWriter/ShortformWriter/TextReplacer) — carried into 2b pre-flight; 2b adds tests with hand-crafted malicious $variable payloads (script in before/after, unknown type, bogus name) and a DDEV check as a user without unfiltered_html.
|
||
- R2 (B3 assumption): 2b verifies in Divi source whether dynamic-content before/after are escaped on output; regardless, R1's sanitization applies (defense in depth). Record the finding in the decisions note.
|
||
- R3 (risky, URL routing): 2b routes URL sanitization by the catalog's attribute metadata (field type url/link/image/upload) when available, and by key name (url|src|link|href|image) otherwise; background-image and link hrefs covered; tests.
|
||
- R4 (cosmetic): PostPresenter → DiviModules static dependency documented in a docblock (layering exception) — fine.
|
||
- R5: security-sensitive tasks (2a Task 2 BlockTree, 2b writers) get opus reviewers.
|
||
|
||
## Progress
|
||
- Task 1: fix round 1/5 (4 addressed: §5.6 tree_rev, §10 cross-ref D7, §11.3, §14; commits 040a60f..c9b31a6) — Ruling: 4-line docs-only fix verified by controller directly against the diff instead of a re-review agent — cost if wrong: none (docs)
|
||
- Task 1: complete (commits 703fecb..c9b31a6, review clean)
|
||
- Task 2: minor (deferred, MUST fix in 2a final wave before 2b writers): Json top-level list-like object → opaque; Block::create validates block name pattern; setAttrs/insertChild/removeChild guard on opaque blocks; copy() deep-clones stdClass or doc narrowed; Json::encode invalid UTF-8 handling documented/caught
|
||
- Task 2: complete (commits c9b31a6..8222240, review clean; fuzzed 200k inputs, 0 diffs)
|
||
- Task 3: minor (deferred, 2a final wave): AttrMap set/remove reject empty path segments
|
||
- Task 3: complete (commits 8222240..08d70ab, review clean)
|
||
- Task 4: note for 2b pre-flight: only ~564/4358 attr paths get a metadata kind; unknown kind MUST default to the strictest sanitizer (sanitize_text_field; URL only by key-name match), never raw
|
||
- Task 4: fix round 1/5 (composite paths unknown, report example, flatten reuse, shortName; commits 2f4dcf9..02ae76d)
|
||
- Task 4: complete (commits 08d70ab..02ae76d, review clean)
|
||
- Task 5: minor (deferred): design-system response unbounded for sites with hundreds of presets (no filter/detail param)
|
||
- Task 5: complete (commits 02ae76d..a54a337, review clean)
|
||
- Task 6: minor (deferred; 2a final wave): outline unbounded breadth (add limit/truncated flag); node-get exposes globalModule id for global refs; node-get description notes $variable values are re-validated on write (R1); outline outputSchema required list
|
||
- Task 6: complete (commits a54a337..d27504a, review clean)
|
||
- Task 7: fix round 1/5 (Text::shorten shared, data URIs, lazy src, ob drain, globals, oEmbed note, text extraction, tests; commits 2e9acd6..b3cd0ee)
|
||
- Task 7: complete (commits d27504a..b3cd0ee, review clean)
|
||
- Task 8: minor (deferred): divi-attrs guide should mention catalog without module lists modules
|
||
- Task 8: complete (commits b3cd0ee..f433fef, review clean)
|
||
- Final review (opus): ready with fixes. Ruling F1: final fix wave = #1 per-subName path kinds (+guide), #2 opaque guards, #3 list-like top-level attrs → opaque, #4 Block::create name validation, #7 AttrMap empty segments + object-ness, #8 copy deep-clone, #9 Json::encode UTF-8 → documented/mapped, #10 Container::append @internal/touched, #11 node-get rev + type short name, #12 outline label normalize/shorten + id pattern, #13 docblock/abnahme text, node-get R1 description note, outline outputSchema required, divi-attrs guide catalog-without-module — spec §5.3 + load-bearing for 2b writers — cost if wrong: small rework
|
||
- Ruling F2: outline limit/truncated, design-system size/filter, globalModule id on node-get → moved to 2b pre-flight (write results embed outline) — cost if wrong: none, tracked
|
||
- Ruling F3: #5 (R1/R3/embedded $variable plan amendments, R2 finding: Divi kses on before/after only when author lacks unfiltered_html, OptionLoopPostMetaKey raw), #6 (real builder fixtures), #14 (ambiguous design-system labels) → blocking items for 2b pre-flight — cost if wrong: none
|
||
- Final fix wave: 11/11 addressed (commits 4137882..4356e69 after trailer normalization; 722 tests, smoke green); per-subName path kinds chosen (path_kinds values string|object); re-review running
|
||
- Re-review: 10/11 clean; residuals (Serializer docblock, /D anchors on NAME/ID patterns, number-advanced → unknown, doc wording 'ein oder mehrere') fixed by controller inline (c24b91d, 723 tests, lint ok). Parked: AttrMap::remove() doesn't traverse stdClass (harmless, no caller); children of opaque blocks editable via direct object access (unreachable: NodeIndex doesn't index below opaque)
|