fix: Task 1 review round 1 - escaped )$ in reference settings, allow map, routing rulings

I-1: reject )$ in every decoded string setting and in the re-serialized
JSON. S21: string settings cleaned and re-serialized. S22: injectable
dynamic-content allow map (name => setting => field type), loop_* only via
the map. S23: unknown content.innerContent is rich text. S24: keys below
scalar kinds are unknown, ...Url path segments are URLs. S25: isVariable
removed. M-1: real \u escapes in tests, malicious suite as editor, fixture
html() raw for unfiltered_html. M-2: S7 gate at any depth plus
css.advanced.customCss.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/Unit/Integrations/Divi/Compose/AttrWriterTest.php

@@ -6,6 +6,7 @@ namespace LIT\MCP\Tests\Unit\Integrations\Divi\Compose;
 
 use InvalidArgumentException;
 use LIT\MCP\Integrations\Divi\Compose\AttrWriter;
+use LIT\MCP\Tests\Unit\Integrations\Divi\Schema\ModuleCatalogTest;
 use PHPUnit\Framework\TestCase;
 
 final class AttrWriterTest extends TestCase
@@ -147,7 +148,8 @@ final class AttrWriterTest extends TestCase
     }
 
     /**
-     * Ruling S6: each malicious reference fails and nothing is stored.
+     * Ruling S6: each malicious reference fails and nothing is stored (as an editor without
+     * unfiltered_html).
      *
      * @return array<string, array{string, string, array<string, mixed>, array<string, mixed>}>
      */
@@ -196,12 +198,25 @@ final class AttrWriterTest extends TestCase
      */
     public function testRejectsMaliciousReferencesAndStoresNothing(string $module, string $path, array $map, array $attrs): void
     {
-        $result = ComposeFixture::attrWriter(true)->apply($module, $attrs, [$path => $map], 'attrs');
+        $result = ComposeFixture::attrWriter()->apply($module, $attrs, [$path => $map], 'attrs');
 
         self::assertArrayHasKey('attrs.' . $path, $result['errors']);
         self::assertSame($attrs, $result['attrs']);
     }
 
+    /** Ruling S21: an editor's script in before/after is cleaned out, the reference re-serialized. */
+    public function testReferenceSettingsAreCleanedForEditors(): void
+    {
+        $sent = '$variable({"type":"content","value":{"name":"post_title","settings":{"after":"","before":"<script>alert(1)</script>Titel: "}}})$';
+        $result = ComposeFixture::attrWriter()->apply('divi/text', self::TEXT, ['content.innerContent' => ['desktop' => $sent]], 'attrs');
+
+        self::assertSame([], $result['errors']);
+        self::assertSame(
+            '$variable({"type":"content","value":{"name":"post_title","settings":{"after":"","before":"Titel: "}}})$',
+            $result['attrs']['content']['innerContent']['desktop']['value']
+        );
+    }
+
     public function testEmbeddedReferenceNamesTheRule(): void
     {
         $this->expectException(InvalidArgumentException::class);
@@ -231,11 +246,46 @@ final class AttrWriterTest extends TestCase
         self::assertSame('section', $admin['attrs']['module']['advanced']['html']['desktop']['value']['elementType']);
         self::assertSame('data-x', $admin['attrs']['module']['decoration']['attributes']['desktop']['value']['attributes'][0]['name']);
 
+        // Deeper paths and the canvas portal's custom CSS are gated too (fix round 1, M-2).
+        foreach (['module.advanced.html.htmlBefore', 'module.decoration.attributes.desktop', 'css.advanced.customCss'] as $path) {
+            try {
+                ComposeFixture::attrWriter()->cleanValue('divi/canvas-portal', $path, 'x');
+                self::fail('Accepted for an editor: ' . $path);
+            } catch (InvalidArgumentException $e) {
+                self::assertStringContainsString('nur im Divi-Builder', $e->getMessage());
+            }
+        }
+        self::assertSame('x', ComposeFixture::attrWriter(true)->cleanValue('divi/canvas-portal', 'css.advanced.customCss', 'x'));
+        self::assertSame('x', ComposeFixture::attrWriter()->cleanValue('divi/text', 'module.advanced.htmlish', 'x'));
+
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('nur im Divi-Builder');
         ComposeFixture::attrWriter()->cleanValue('divi/button', 'module.decoration.attributes', ['attributes' => []]);
     }
 
+    /** Ruling S23: content.innerContent of unknown kind (defined only in Divi's JS) is rich text. */
+    public function testUnknownContentInnerContentIsRichText(): void
+    {
+        self::assertSame('unknown', ModuleCatalogTest::sampleCatalog()->pathKind('divi/blurb', 'content.innerContent'));
+        self::assertSame('<p>Hallo</p>', ComposeFixture::attrWriter()->cleanValue('divi/blurb', 'content.innerContent', '<p>Hallo</p><script>x()</script>'));
+        // Other unknown innerContent stays text.
+        self::assertSame('Los', ComposeFixture::attrWriter()->cleanValue('divi/button', 'button.innerContent', '<b>Los</b>'));
+    }
+
+    /** Ruling S24: keys below a scalar-kind path are unknown; a …Url path segment means URL. */
+    public function testKeysBelowScalarKindsAndUrlPathSegments(): void
+    {
+        $writer = ComposeFixture::attrWriter();
+
+        self::assertSame(
+            ['url' => '', 'text' => 'a'],
+            $writer->cleanValue('divi/heading', 'title.innerContent', ['url' => 'javascript:x', 'text' => '<b>a</b>'])
+        );
+        self::assertSame('', $writer->cleanValue('divi/contact-form', 'success.advanced.redirectUrl', 'javascript:alert(1)'));
+        self::assertSame('', $writer->cleanValue('divi/text', 'module.advanced.imageUrl', 'javascript:alert(1)'));
+        self::assertSame('Titel', $writer->cleanValue('divi/heading', 'title.innerContent', '<b>Titel</b>'));
+    }
+
     /** Ruling S13: keys inside value maps are plain identifiers. */
     public function testRejectsOddKeysInsideValues(): void
     {

tests/Unit/Integrations/Divi/Compose/ComposeFixture.php

@@ -30,9 +30,10 @@ final class ComposeFixture
                 return trim(strip_tags($value));
             }
 
+            /** Like WpValueSanitizer (ruling S4): raw for users with unfiltered_html. */
             public function html(string $value): string
             {
-                return (string) preg_replace('#<script\b[^>]*>.*?</script>#is', '', $value);
+                return $this->unfiltered ? $value : (string) preg_replace('#<script\b[^>]*>.*?</script>#is', '', $value);
             }
 
             public function code(string $value): string

tests/Unit/Integrations/Divi/Compose/PlaceholdersTest.php

@@ -37,10 +37,14 @@ final class PlaceholdersTest extends TestCase
     /** @dataProvider placeholders */
     public function testResolvesToDiviVariables(string $input, string $expected): void
     {
-        $resolved = ComposeFixture::placeholders()->resolve($input);
+        $placeholders = ComposeFixture::placeholders();
+        $resolved = $placeholders->resolve($input);
 
         self::assertSame($expected, $resolved);
-        self::assertSame($expected !== $input, Placeholders::isVariable($resolved));
+        if ($expected !== $input) {
+            // What placeholders produce passes the strict checker unchanged (ruling S1/S25).
+            self::assertSame($resolved, $placeholders->references()->canonical($resolved, ComposeFixture::sanitizer()));
+        }
     }
 
     /** @return array<string, array{string, string}> */
@@ -131,6 +135,5 @@ final class PlaceholdersTest extends TestCase
             ['color' => '$variable({"type":"color","value":{"name":"gcid-primary-color","settings":{}}})$', 'size' => '20px'],
             ComposeFixture::placeholders()->resolveDeep(['color' => '{{color:primary}}', 'size' => '20px'])
         );
-        self::assertFalse(Placeholders::isVariable('$variable(kaputt)$'));
     }
 }

tests/Unit/Integrations/Divi/Compose/VariableRefTest.php

@@ -9,12 +9,24 @@ use LIT\MCP\Integrations\Divi\Compose\VariableRef;
 use LIT\MCP\Tests\Unit\Integrations\Divi\Design\DesignSystemTest;
 use PHPUnit\Framework\TestCase;
 
-/** Ruling S1/S2/S12: every $variable(…)$ value is parsed, strictly checked and re-serialized. */
+/** Ruling S1/S2/S12/S21/S22: every $variable(…)$ value is parsed, strictly checked, cleaned and re-serialized. */
 final class VariableRefTest extends TestCase
 {
-    private static function ref(): VariableRef
+    /** @param array<string, array<string, string>>|null $dynamicContent */
+    private static function ref(?array $dynamicContent = null): VariableRef
     {
-        return new VariableRef(DesignSystemTest::sample());
+        return new VariableRef(DesignSystemTest::sample(), $dynamicContent);
+    }
+
+    private static function content(string $name, string $settings): string
+    {
+        return '$variable({"type":"content","value":{"name":"' . $name . '","settings":' . $settings . '}})$';
+    }
+
+    /** A JSON "\uXXXX" escape, built from bytes so no tool turns it into the character itself. */
+    private static function escape(string $hex): string
+    {
+        return chr(92) . 'u' . $hex;
     }
 
     /** @return array<string, array{string, string}> */
@@ -38,16 +50,16 @@ final class VariableRefTest extends TestCase
                 '$variable({"type":"color","value":{"name":"gcid-a1b2","settings":{"opacity":"50","hue":-10,"lightness":1.5}}})$',
             ],
             'post date with format and html before' => [
-                '$variable({"type":"content","value":{"name":"post_date","settings":{"before":"<b>Am</b> ","after":"","date_format":"custom","custom_date_format":"d.m.Y"}}})$',
-                '$variable({"type":"content","value":{"name":"post_date","settings":{"before":"<b>Am</b> ","after":"","date_format":"custom","custom_date_format":"d.m.Y"}}})$',
+                self::content('post_date', '{"before":"<b>Am</b> ","after":"","date_format":"custom","custom_date_format":"d.m.Y"}'),
+                self::content('post_date', '{"before":"<b>Am</b> ","after":"","date_format":"custom","custom_date_format":"d.m.Y"}'),
             ],
             'custom meta' => [
-                '$variable({"type":"content","value":{"name":"custom_meta_telefon-mobil","settings":{"before":"","after":""}}})$',
-                '$variable({"type":"content","value":{"name":"custom_meta_telefon-mobil","settings":{"before":"","after":""}}})$',
+                self::content('custom_meta_telefon-mobil', '{"before":"","after":""}'),
+                self::content('custom_meta_telefon-mobil', '{"before":"","after":""}'),
             ],
             'theme font' => [
-                '$variable({"type":"content","value":{"name":"--et_global_body_font","settings":{}}})$',
-                '$variable({"type":"content","value":{"name":"--et_global_body_font","settings":{}}})$',
+                self::content('--et_global_body_font', '{}'),
+                self::content('--et_global_body_font', '{}'),
             ],
         ];
     }
@@ -58,6 +70,45 @@ final class VariableRefTest extends TestCase
         self::assertSame($expected, self::ref()->canonical($input, ComposeFixture::sanitizer()));
     }
 
+    /**
+     * Ruling S21: string settings are cleaned and the cleaned value is re-serialized.
+     *
+     * @return array<string, array{string, string}>
+     */
+    public static function cleaned(): array
+    {
+        $script = self::escape('003c') . 'script' . self::escape('003e') . 'alert(1)' . self::escape('003c') . '/script'
+            . self::escape('003e');
+
+        return [
+            'script in before' => [
+                self::content('post_title', '{"before":"<b>Hi</b><script>alert(1)</script>","after":""}'),
+                self::content('post_title', '{"before":"<b>Hi</b>","after":""}'),
+            ],
+            'script in after' => [
+                self::content('custom_meta_telefon', '{"before":"","after":"<script>x()</script>!"}'),
+                self::content('custom_meta_telefon', '{"before":"","after":"!"}'),
+            ],
+            'escaped script in before' => [
+                self::content('post_title', '{"before":"' . $script . 'Hallo","after":""}'),
+                self::content('post_title', '{"before":"Hallo","after":""}'),
+            ],
+            'tags in date format' => [
+                self::content('post_date', '{"custom_date_format":"<b>d</b>.m"}'),
+                self::content('post_date', '{"custom_date_format":"d.m"}'),
+            ],
+        ];
+    }
+
+    /** @dataProvider cleaned */
+    public function testCleansStringSettings(string $input, string $expected): void
+    {
+        $canonical = self::ref()->canonical($input, ComposeFixture::sanitizer());
+
+        self::assertSame($expected, $canonical);
+        self::assertStringNotContainsString('<script', $canonical);
+    }
+
     /**
      * Hand-crafted attacks (preflight §4 "Malicious $variable tests").
      *
@@ -65,45 +116,42 @@ final class VariableRefTest extends TestCase
      */
     public static function malicious(): array
     {
-        $content = static fn (string $name, string $settings): string =>
-            '$variable({"type":"content","value":{"name":"' . $name . '","settings":' . $settings . '}})$';
+        $end = self::escape('0029') . '$';
 
         return [
-            'script in before' => [$content('post_title', '{"before":"<script>alert(1)</script>","after":""}'), 'before'],
-            'script in after' => [$content('post_title', '{"before":"","after":"<img src=x onerror=alert(1)><script>x()</script>"}'), 'after'],
-            'escaped script in before' => [
-                $content('post_title', '{"before":"<script>alert(1)</script>","after":""}'),
-                'before',
-            ],
-            'tags in date format' => [$content('post_date', '{"date_format":"<b>d</b>"}'), 'date_format'],
+            'escaped end marker in before' => [self::content('post_title', '{"before":"x' . $end . ' <b>y</b>","after":""}'), 'allein im Wert'],
+            'escaped end marker in after' => [self::content('post_title', '{"before":"","after":"' . $end . '"}'), 'allein im Wert'],
+            'escaped end marker in date format' => [self::content('post_date', '{"custom_date_format":"d' . $end . '"}'), 'allein im Wert'],
+            'escaped end marker in name' => [self::content('post_title' . $end, '{}'), 'Unbekannter Name'],
+            'select with markup' => [self::content('post_date', '{"date_format":"<b>d</b>"}'), 'date_format'],
             'type evil' => ['$variable({"type":"evil","value":{"name":"gcid-a1b2","settings":{}}})$', 'Typ'],
             'unknown gcid' => ['$variable({"type":"color","value":{"name":"gcid-nope","settings":{}}})$', 'gcid-nope'],
-            'unknown gvid' => [$content('gvid-nope', '{}'), 'gvid-nope'],
+            'unknown gvid' => [self::content('gvid-nope', '{}'), 'gvid-nope'],
             'content gvid as color' => ['$variable({"type":"color","value":{"name":"gvid-s1","settings":{}}})$', 'gvid-s1'],
-            'color gcid as content' => [$content('gcid-a1b2', '{}'), 'gcid-a1b2'],
+            'color gcid as content' => [self::content('gcid-a1b2', '{}'), 'gcid-a1b2'],
             'name with paren' => ['$variable({"type":"color","value":{"name":"gcid-a1b2)","settings":{}}})$', 'Name'],
             'name with style breakout' => [
                 '$variable({"type":"color","value":{"name":"gcid-a1b2;}</style><script>alert(1)</script>","settings":{}}})$',
                 'Name',
             ],
-            'unknown settings key' => [$content('post_title', '{"before":"","foo":"1"}'), 'foo'],
-            'enable_html' => [$content('custom_meta_telefon', '{"before":"","after":"","enable_html":"on"}'), 'enable_html'],
+            'unknown settings key' => [self::content('post_title', '{"before":"","foo":"1"}'), 'foo'],
+            'setting the option lacks' => [self::content('post_featured_image', '{"before":"x"}'), 'before'],
+            'enable_html' => [self::content('custom_meta_telefon', '{"before":"","after":"","enable_html":"on"}'), 'enable_html'],
             'color setting not numeric' => [
                 '$variable({"type":"color","value":{"name":"gcid-a1b2","settings":{"opacity":"1;}</style>"}}})$',
                 'opacity',
             ],
-            'protected meta key' => [$content('custom_meta__edit_lock', '{}'), 'custom_meta__edit_lock'],
-            'unknown dynamic content' => [$content('post_meta_key', '{}'), 'post_meta_key'],
+            'protected meta key' => [self::content('custom_meta__edit_lock', '{}'), 'custom_meta__edit_lock'],
+            'loop content outside a loop' => [self::content('loop_post_title', '{}'), 'loop_post_title'],
+            'unknown dynamic content' => [self::content('post_meta_key', '{}'), 'post_meta_key'],
             'extra top-level key' => ['$variable({"type":"color","value":{"name":"gcid-a1b2","settings":{}},"x":1})$', 'x'],
             'extra value key' => ['$variable({"type":"color","value":{"name":"gcid-a1b2","settings":{},"x":1}})$', 'x'],
             'greedy smuggling' => ['$variable({"a":"})$<script>alert(1)</script>"})$', 'allein im Wert'],
-            'nested reference in before' => [
-                $content('post_title', '{"before":"$variable(x)$","after":""}'),
-                'allein im Wert',
-            ],
+            'nested reference in before' => [self::content('post_title', '{"before":"$variable(x)$","after":""}'), 'allein im Wert'],
+            'nested marker in before' => [self::content('post_title', '{"before":"$variable(","after":""}'), 'allein im Wert'],
             'broken json' => ['$variable(kaputt)$', 'JSON'],
-            'trailing text' => [$content('post_title', '{}') . ' ', 'allein im Wert'],
-            'embedded in text' => ['<p>Hi ' . $content('post_title', '{"before":"","after":""}') . '</p>', 'allein im Wert'],
+            'trailing text' => [self::content('post_title', '{}') . ' ', 'allein im Wert'],
+            'embedded in text' => ['<p>Hi ' . self::content('post_title', '{"before":"","after":""}') . '</p>', 'allein im Wert'],
         ];
     }
 
@@ -116,6 +164,19 @@ final class VariableRefTest extends TestCase
         self::ref()->canonical($input, ComposeFixture::sanitizer());
     }
 
+    /** I-1: the escape really is in the input, and nothing with ")$" inside comes out. */
+    public function testEscapedEndMarkerIsBytesNotTheCharacter(): void
+    {
+        $input = self::content('post_title', '{"before":"x' . self::escape('0029') . '$","after":""}');
+
+        self::assertStringNotContainsString(')$ ', $input);
+        self::assertSame(1, substr_count($input, ')$'));
+        self::assertStringContainsString("\\u0029", $input);
+
+        $this->expectException(InvalidArgumentException::class);
+        self::ref()->canonical($input, ComposeFixture::sanitizer());
+    }
+
     public function testUrlFieldsTakeContentReferencesWithoutBeforeOrAfter(): void
     {
         $ref = self::ref();
@@ -125,7 +186,7 @@ final class VariableRefTest extends TestCase
 
         foreach (
             [
-                '$variable({"type":"content","value":{"name":"post_link_url","settings":{"before":"javascript:alert(1)//","after":""}}})$',
+                self::content('custom_meta_ziel', '{"before":"javascript:alert(1)//","after":""}'),
                 '$variable({"type":"color","value":{"name":"gcid-a1b2","settings":{}}})$',
             ] as $value
         ) {
@@ -138,6 +199,47 @@ final class VariableRefTest extends TestCase
         }
     }
 
+    /** Ruling S22: an injected allow map (Divi's live registry in Task 5) decides names and setting types. */
+    public function testInjectedDynamicContentMap(): void
+    {
+        $ref = self::ref([
+            'site_title' => ['before' => 'text', 'after' => 'richtext'],
+            'post_date' => ['date_format' => 'select', 'offset' => 'number'],
+            'loop_post_title' => [],
+            'custom_meta_preis' => ['before' => 'text', 'enable_html' => 'yes_no_button'],
+        ]);
+        $sanitizer = ComposeFixture::sanitizer();
+
+        self::assertSame(
+            self::content('site_title', '{"before":"Site","after":"<em>!</em>"}'),
+            $ref->canonical(self::content('site_title', '{"before":"<b>Site</b>","after":"<em>!</em>"}'), $sanitizer)
+        );
+        self::assertSame(
+            self::content('post_date', '{"date_format":"d.m.Y","offset":3}'),
+            $ref->canonical(self::content('post_date', '{"date_format":" d.m.Y","offset":3}'), $sanitizer)
+        );
+        self::assertSame(self::content('loop_post_title', '{}'), $ref->canonical(self::content('loop_post_title', '{}'), $sanitizer));
+        // Names outside the map: custom_meta_ keeps its rule, the rest is unknown.
+        self::assertSame(
+            self::content('custom_meta_telefon', '{"before":"<b>T</b>"}'),
+            $ref->canonical(self::content('custom_meta_telefon', '{"before":"<b>T</b>"}'), $sanitizer)
+        );
+
+        $rejected = [
+            'post_title' => self::content('post_title', '{}'),
+            'enable_html' => self::content('custom_meta_preis', '{"enable_html":"on"}'),
+            'offset' => self::content('post_date', '{"offset":[1]}'),
+        ];
+        foreach ($rejected as $fragment => $value) {
+            try {
+                $ref->canonical($value, $sanitizer);
+                self::fail('Accepted: ' . $value);
+            } catch (InvalidArgumentException $e) {
+                self::assertStringContainsString($fragment, $e->getMessage());
+            }
+        }
+    }
+
     public function testDetectsReferences(): void
     {
         self::assertTrue(VariableRef::contains('a $variable( b'));