From 16c6d59725d332a015def643c6664ae75e4cd501 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sun, 27 Sep 2026 01:58:47 +0200 Subject: [PATCH] fix: StoredNodeGuard keeps plain relative links copyable (P18) A link without scheme and without whitespace, quotes, <, >, backslashes or control characters passes although esc_url_raw() would prefix http://; every value with a scheme, and protocol-relative //host, must survive url() unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Divi/Compose/StoredNodeGuard.php | 21 +++- .../Divi/Compose/StoredNodeGuardTest.php | 105 ++++++++++++++++++ 2 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/Integrations/Divi/Compose/StoredNodeGuard.php b/src/Integrations/Divi/Compose/StoredNodeGuard.php index 1700748..0909f11 100644 --- a/src/Integrations/Divi/Compose/StoredNodeGuard.php +++ b/src/Integrations/Divi/Compose/StoredNodeGuard.php @@ -29,8 +29,8 @@ defined('ABSPATH') || exit; * also when the builder-only key sits below the state value, * - a string with control characters (S32), rich text or code that the sanitizer would change * (not kses-stable), a plain-text string with markup that is not kses-stable, a URL the - * sanitizer would change, or a key inside a value that is no plain identifier (S13) – routed - * exactly like ValueCleaner (kindAt(), VALUE_KEY), + * sanitizer would change (plain relative links excepted, ruling P18), or a key inside a value + * that is no plain identifier (S13) – routed exactly like ValueCleaner (kindAt(), VALUE_KEY), * - a $variable(…)$ reference VariableRef::canonical() refuses for the target post, or whose * settings canonical() would clean (e.g. before/after with markup such a user may not write). * Users with unfiltered_html may move anything (the same line as the S7 gate). Pure PHP. @@ -42,6 +42,9 @@ final class StoredNodeGuard /** An instance's globalModule: the library post ID, digits only (GlobalInstance::of()). */ private const LIBRARY_ID = '/^[1-9][0-9]*$/D'; + /** P18: no scheme before the first "/", "?" or "#"; no whitespace, quotes, <, >, backslash or controls. */ + private const RELATIVE_URL = '~^[^:/?#\s"\'<>\\\\\x00-\x1F\x7F]*(?:[/?#][^\s"\'<>\\\\\x00-\x1F\x7F]*)?$~D'; + private const BUILDER_ONLY = 'nur im Divi-Builder übertragbar (oder mit dem Recht unfiltered_html)'; public function __construct( @@ -247,13 +250,25 @@ final class StoredNodeGuard ValueKind::Html => $this->sanitizer->html($value) === $value, ValueKind::Code => $this->sanitizer->code($value) === $value, ValueKind::Text => ! str_contains($value, '<') || $this->sanitizer->html($value) === $value, - ValueKind::Url => $this->sanitizer->url($value) === trim($value), + ValueKind::Url => $this->sanitizer->url($value) === trim($value) || self::isPlainRelativeUrl($value), }; return $stable ? null : 'Inhalt, den dieses Konto so nicht schreiben darf (HTML, Skript oder Link) – ' . self::BUILDER_ONLY . '.'; } + /** + * Ruling P18: a link without scheme – nothing before the first "/", "?" or "#" contains ":" – and + * without whitespace, quotes, "<", ">", backslashes or control characters ("kontakt/", "/pfad", + * "#anker", "?q=1"). esc_url_raw() would prefix "http://" to some of them, yet they cannot carry + * javascript: or data:. Protocol-relative "//host" (and "\\host", which browsers read the same) + * does not count: it must survive url() unchanged. + */ + private static function isPlainRelativeUrl(string $value): bool + { + return preg_match(self::RELATIVE_URL, $value) === 1 && ! str_starts_with($value, '//'); + } + /** Same check as a client reference (VariableRef::canonical()); refused when cleaning would change it. */ private function referenceProblem(string $value, bool $url): ?string { diff --git a/tests/Unit/Integrations/Divi/Compose/StoredNodeGuardTest.php b/tests/Unit/Integrations/Divi/Compose/StoredNodeGuardTest.php index 3951f5f..c3867d2 100644 --- a/tests/Unit/Integrations/Divi/Compose/StoredNodeGuardTest.php +++ b/tests/Unit/Integrations/Divi/Compose/StoredNodeGuardTest.php @@ -10,6 +10,7 @@ use LIT\MCP\Integrations\Divi\BlockTree\Parser; use LIT\MCP\Integrations\Divi\BlockTree\Serializer; use LIT\MCP\Integrations\Divi\Compose\StoredNodeGuard; use LIT\MCP\Integrations\Divi\Compose\ValueCleaner; +use LIT\MCP\Integrations\Divi\Compose\ValueSanitizer; use LIT\MCP\Integrations\Divi\Compose\VariableRef; use LIT\MCP\Integrations\Divi\Pages\DynamicContentMap; use LIT\MCP\Tests\Support\ComposeFixture; @@ -322,4 +323,108 @@ final class StoredNodeGuardTest extends TestCase self::assertCount(2, $problems); self::assertSame($markup, Serializer::block($block)); } + + /** + * Like esc_url_raw() for an editor: only allowed protocols, characters outside its list go, and a + * value without scheme that does not start with "/", "#" or "?" gets "http://" (so url() changes + * relative links like "kontakt/"). $keepProtocolRelative decides what "//host" becomes. + */ + private static function wpLikeGuard(bool $keepProtocolRelative = true): StoredNodeGuard + { + $inner = ComposeFixture::sanitizer(); + $sanitizer = new class ($inner, $keepProtocolRelative) implements ValueSanitizer { + public function __construct(private ValueSanitizer $inner, private bool $keepProtocolRelative) + { + } + + public function text(string $value): string + { + return $this->inner->text($value); + } + + public function html(string $value): string + { + return $this->inner->html($value); + } + + public function code(string $value): string + { + return $this->inner->code($value); + } + + public function url(string $value): string + { + $allowed = '|[^a-z0-9-~+_.?#=!&;,/:%@$\|*\'()\[\]\x80-\xff]|i'; + $value = (string) preg_replace($allowed, '', trim($value)); + if (str_starts_with($value, '//')) { + return $this->keepProtocolRelative ? $value : 'http:' . $value; + } + if (preg_match('/^([a-z][a-z0-9+.-]*):/i', $value, $scheme) === 1) { + return in_array(strtolower($scheme[1]), ['http', 'https', 'mailto', 'tel'], true) ? $value : ''; + } + + return preg_match('/^[\/#?]/', $value) === 1 ? $value : 'http://' . $value; + } + + public function allowsUnfilteredHtml(): bool + { + return false; + } + + public function canReadPost(int $id): bool + { + return $this->inner->canReadPost($id); + } + }; + $references = new VariableRef( + DiviFixtures::designSystem(), + DynamicContentMap::fromOptions(DiviFixtures::dynamicContentRegistry()) + ); + + $cleaner = new ValueCleaner(DiviFixtures::catalog(), $references, $sanitizer); + + return new StoredNodeGuard($cleaner, $references, $sanitizer); + } + + private static function link(string $url): Block + { + return self::block(self::node('divi/button', ['button' => ['innerContent' => self::desktop([ + 'text' => 'Kontakt', + 'linkUrl' => $url, + ])]])); + } + + /** Ruling P18: relative links stay copyable although esc_url_raw() would change them. */ + public function testRelativeLinksWithoutSchemePass(): void + { + $accepted = ['kontakt/', '/pfad', '#anker', '?q=1', 'https://kunde.test/a?b=1#c', 'mailto:info@kunde.test']; + foreach ($accepted as $url) { + self::assertSame([], self::wpLikeGuard()->problems(self::link($url)), $url); + } + } + + /** Ruling P18: every value with a scheme must survive url() unchanged; odd relative values are refused. */ + public function testLinksWithASchemeOrOddCharactersAreRefused(): void + { + $refused = [ + 'javascript:alert(1)', + 'JaVaScRiPt:alert(1)', + 'java' . chr(9) . 'script:alert(1)', + 'data:text/html,probe', + 'kontakt/"data-probe="1', + "kontakt/'x", + 'kontakt/ x', + 'kontakt/', + '\\evil.example', + '/\\evil.example', + ]; + foreach ($refused as $url) { + self::assertFalse(self::link($url)->isOpaque(), $url); + self::assertNotSame([], self::wpLikeGuard()->problems(self::link($url)), $url); + } + + // Protocol-relative: only when url() leaves it as it is. + self::assertSame([], self::wpLikeGuard()->problems(self::link('//evil.example'))); + self::assertNotSame([], self::wpLikeGuard(false)->problems(self::link('//evil.example'))); + } }