diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6440010..c1be477 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,9 @@ jobs: composer require symfony/yaml:^${{ matrix.symfony }} --dev --no-interaction --no-update composer update --with-all-dependencies --no-interaction --prefer-dist --no-progress + - name: Composer audit (Critical/High) + run: composer audit --locked + - name: Run tests run: | echo "🧪 Running tests on PHP ${{ matrix.php }} with Symfony ${{ matrix.symfony }}..." diff --git a/.symfony/recipe/nowo-tech/page-layout-kit-bundle/1.0/config/packages/nowo_page_layout_kit.yaml b/.symfony/recipe/nowo-tech/page-layout-kit-bundle/1.0/config/packages/nowo_page_layout_kit.yaml index 61cf6c3..37f9530 100644 --- a/.symfony/recipe/nowo-tech/page-layout-kit-bundle/1.0/config/packages/nowo_page_layout_kit.yaml +++ b/.symfony/recipe/nowo-tech/page-layout-kit-bundle/1.0/config/packages/nowo_page_layout_kit.yaml @@ -8,5 +8,14 @@ nowo_page_layout_kit: web_ui: layout_template: '@NowoPageLayoutKitBundle/admin/layout.html.twig' css_framework: tailwind + html: + sanitize: + strategy: none doctrine: table_prefix: '' + +when@prod: + nowo_page_layout_kit: + html: + sanitize: + strategy: allowlist diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index d14210f..530a9e8 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -29,8 +29,23 @@ nowo_page_layout_kit: doctrine: table_prefix: '' connection: default + html: + sanitize: + strategy: none # none | strip | allowlist | service + service: null ``` +Production (Flex recipe): `when@prod` sets `html.sanitize.strategy: allowlist`. + +## html.sanitize + +| Key | Default | Description | +| --- | --- | --- | +| `strategy` | `none` | `none` (trusted editors), `strip`, `allowlist`, or `service` | +| `service` | `null` | Host service implementing `PageLayoutHtmlSanitizerInterface` when `strategy: service` | + +Sanitization runs on Doctrine persist/update for text/compare/cta block translations and again when serving public layouts via `PageBlockProvider`. + ## Top-level options | Key | Type | Default | Description | diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 0194648..a657fb8 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -61,11 +61,23 @@ Recommendations: ## Rich text rendering -Several default public block templates render editor-authored HTML with `|raw`, especially for long-form text and compare content. That is intentional for CMS-managed rich text, but it means: +Several default public block templates render editor-authored HTML with `|raw`, especially for long-form text and compare content. That is intentional for CMS-managed rich text. -- Only trusted editors should be able to update those fields. -- Hosts should sanitize content before storage or before rendering if untrusted HTML is possible. -- Template overrides should keep escaping behavior explicit and reviewed. +**HTML sanitization (2026-08-19):** configure `nowo_page_layout_kit.html.sanitize.strategy`: + +| Strategy | Behaviour | +| -------- | --------- | +| `none` (default) | Trusted editors only; HTML stored/rendered as-is | +| `allowlist` | DOM allowlist on persist + public render (recipe `when@prod`) | +| `strip` | Remove all tags | +| `service` | Host `PageLayoutHtmlSanitizerInterface` | + +Flex recipe sets `when@prod: strategy: allowlist`. Demo/dev may keep `none`. + +Additional guidance: + +- Only trusted editors should be able to update rich-text fields. +- Template overrides should keep escaping behaviour explicit and reviewed. ## Operational guidance diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 822f8a4..7bf8e6a 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -4,6 +4,7 @@ namespace Nowo\PageLayoutKitBundle\DependencyInjection; +use Nowo\PageLayoutKitBundle\Enum\HtmlSanitizeStrategy; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -74,6 +75,25 @@ public function getConfigTreeBuilder(): TreeBuilder ->scalarNode('connection')->defaultValue('default')->end() ->end() ->end() + ->arrayNode('html') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('sanitize') + ->addDefaultsIfNotSet() + ->info('Sanitize block HTML on persist and public render. Default none keeps trusted-editor |raw.') + ->children() + ->enumNode('strategy') + ->values(HtmlSanitizeStrategy::values()) + ->defaultValue(HtmlSanitizeStrategy::None->value) + ->end() + ->scalarNode('service') + ->defaultNull() + ->info('Service id implementing PageLayoutHtmlSanitizerInterface when strategy=service.') + ->end() + ->end() + ->end() + ->end() + ->end() ->end(); return $treeBuilder; diff --git a/src/DependencyInjection/NowoPageLayoutKitExtension.php b/src/DependencyInjection/NowoPageLayoutKitExtension.php index 7654183..5f62e3e 100644 --- a/src/DependencyInjection/NowoPageLayoutKitExtension.php +++ b/src/DependencyInjection/NowoPageLayoutKitExtension.php @@ -7,10 +7,13 @@ use Doctrine\ORM\Events; use LogicException; use Nowo\PageLayoutKitBundle\DependencyInjection\Configuration as BundleConfiguration; +use Nowo\PageLayoutKitBundle\Enum\HtmlSanitizeStrategy; use Nowo\PageLayoutKitBundle\Locale\PageLocales; use Nowo\PageLayoutKitBundle\Security\AllowAllPageLayoutKitAccessChecker; use Nowo\PageLayoutKitBundle\Security\ConfigurablePageLayoutKitAccessChecker; use Nowo\PageLayoutKitBundle\Security\PageLayoutKitAccessCheckerInterface; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtection; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtectionConfig; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -92,6 +95,7 @@ public function load(array $configs, ContainerBuilder $container): void } $this->registerAccessChecker($container, $config['security']); + $this->registerPageLayoutProtection($container, $config); $tablePrefix = (string) $config['doctrine']['table_prefix']; if ($tablePrefix !== '') { @@ -134,6 +138,42 @@ private function registerAccessChecker(ContainerBuilder $container, array $secur $container->setAlias(PageLayoutKitAccessCheckerInterface::class, $id); } + /** + * @param array $config + */ + private function registerPageLayoutProtection(ContainerBuilder $container, array $config): void + { + /** @var array $html */ + $html = $config['html']['sanitize']; + + $container->register(PageLayoutProtectionConfig::class) + ->setAutowired(false) + ->setAutoconfigured(false) + ->setArguments([ + HtmlSanitizeStrategy::from((string) $html['strategy']), + $this->optionalServiceId($html['service'] ?? null), + ]); + + $customSanitizer = $this->optionalServiceId($html['service'] ?? null); + + $container->register(PageLayoutProtection::class) + ->setAutowired(false) + ->setAutoconfigured(false) + ->setArguments([ + new Reference(PageLayoutProtectionConfig::class), + $customSanitizer !== null ? new Reference($customSanitizer) : null, + ]); + } + + private function optionalServiceId(mixed $serviceId): ?string + { + if (!is_string($serviceId) || $serviceId === '') { + return null; + } + + return $serviceId; + } + private function isSecurityBundleAvailable(ContainerBuilder $container): bool { if ($container->hasExtension('security')) { diff --git a/src/Enum/HtmlSanitizeStrategy.php b/src/Enum/HtmlSanitizeStrategy.php new file mode 100644 index 0000000..e748dbd --- /dev/null +++ b/src/Enum/HtmlSanitizeStrategy.php @@ -0,0 +1,24 @@ + + */ + public static function values(): array + { + return array_map(static fn (self $case): string => $case->value, self::cases()); + } +} diff --git a/src/EventSubscriber/PageBlockHtmlSanitizeSubscriber.php b/src/EventSubscriber/PageBlockHtmlSanitizeSubscriber.php new file mode 100644 index 0000000..56716c3 --- /dev/null +++ b/src/EventSubscriber/PageBlockHtmlSanitizeSubscriber.php @@ -0,0 +1,72 @@ + $args + */ + public function prePersist(LifecycleEventArgs $args): void + { + $this->sanitize($args->getObject()); + } + + /** + * @param LifecycleEventArgs $args + */ + public function preUpdate(LifecycleEventArgs $args): void + { + $this->sanitize($args->getObject()); + } + + private function sanitize(object $entity): void + { + $sanitizer = $this->protection->htmlSanitizer(); + + if ($entity instanceof PageTextBlockTranslation) { + $body = $entity->getBody(); + if ($body !== '') { + $entity->setBody($sanitizer->sanitize($body)); + } + + return; + } + + if ($entity instanceof PageCompareBlockTranslation) { + if ($entity->getBeforeText() !== '') { + $entity->setBeforeText($sanitizer->sanitize($entity->getBeforeText())); + } + + if ($entity->getAfterText() !== '') { + $entity->setAfterText($sanitizer->sanitize($entity->getAfterText())); + } + + return; + } + + if ($entity instanceof PageCtaBlockTranslation) { + $body = $entity->getBody(); + if ($body !== '') { + $entity->setBody($sanitizer->sanitize($body)); + } + } + } +} diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index fd4495e..64dc012 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -11,6 +11,9 @@ services: - '../../Controller/' - '../../NowoPageLayoutKitBundle.php' - '../../Resources/' + - '../../Security/PageLayoutProtection.php' + - '../../Security/PageLayoutProtectionConfig.php' + - '../../Security/Html/' Nowo\PageLayoutKitBundle\Controller\: resource: '../../Controller/' @@ -37,6 +40,12 @@ services: Nowo\PageLayoutKitBundle\Service\PageBlockProvider: arguments: $legacyContentProvider: '@?Nowo\PageLayoutKitBundle\Legacy\LegacyPageContentProviderInterface' + $protection: '@Nowo\PageLayoutKitBundle\Security\PageLayoutProtection' + + Nowo\PageLayoutKitBundle\EventSubscriber\PageBlockHtmlSanitizeSubscriber: + tags: + - { name: doctrine.event_listener, event: prePersist } + - { name: doctrine.event_listener, event: preUpdate } Nowo\PageLayoutKitBundle\Service\PageBlockMigrator: arguments: diff --git a/src/Security/Html/AllowlistPageLayoutHtmlSanitizer.php b/src/Security/Html/AllowlistPageLayoutHtmlSanitizer.php new file mode 100644 index 0000000..7f6cfee --- /dev/null +++ b/src/Security/Html/AllowlistPageLayoutHtmlSanitizer.php @@ -0,0 +1,200 @@ +> */ + private const ALLOWED_ELEMENTS = [ + 'p' => ['class'], + 'br' => [], + 'strong' => ['class'], + 'b' => ['class'], + 'em' => ['class'], + 'i' => ['class'], + 'u' => ['class'], + 's' => ['class'], + 'del' => ['class'], + 'h2' => ['class'], + 'h3' => ['class'], + 'h4' => ['class'], + 'ul' => ['class'], + 'ol' => ['class'], + 'li' => ['class'], + 'blockquote' => ['class'], + 'code' => ['class'], + 'pre' => ['class'], + 'a' => ['href', 'title', 'target', 'rel', 'class'], + 'img' => ['src', 'alt', 'title', 'width', 'height', 'loading', 'class'], + 'table' => ['class'], + 'thead' => ['class'], + 'tbody' => ['class'], + 'tr' => ['class'], + 'th' => ['class', 'colspan', 'rowspan'], + 'td' => ['class', 'colspan', 'rowspan'], + 'hr' => ['class'], + 'span' => ['class'], + 'div' => ['class'], + 'figure' => ['class'], + 'figcaption' => ['class'], + 'iframe' => ['src', 'title', 'allow', 'allowfullscreen', 'frameborder', 'width', 'height', 'class'], + ]; + + /** @var list */ + private const ALLOWED_EMBED_HOSTS = [ + 'www.youtube.com', + 'youtube.com', + 'www.youtube-nocookie.com', + 'player.vimeo.com', + ]; + + public function sanitize(string $html): string + { + $html = trim($html); + + if ($html === '') { + return ''; + } + + $document = new DOMDocument(); + $internalErrors = libxml_use_internal_errors(true); + + $document->loadHTML( + sprintf('
%s
', $html), + LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD, + ); + + libxml_clear_errors(); + libxml_use_internal_errors($internalErrors); + + $container = $document->getElementsByTagName('div')->item(0); + + if (!$container instanceof DOMElement) { + return htmlspecialchars($html, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); // @codeCoverageIgnore + } + + $this->sanitizeNode($container); + + $result = ''; + + foreach ($container->childNodes as $child) { + $result .= $document->saveHTML($child) ?: ''; + } + + return $result; + } + + private function sanitizeNode(DOMElement $node): void + { + $child = $node->firstChild; + + while ($child instanceof DOMNode) { + $next = $child->nextSibling; + + if ($child instanceof DOMElement) { + $tag = strtolower($child->nodeName); + + if (!isset(self::ALLOWED_ELEMENTS[$tag])) { + while ($child->firstChild instanceof DOMNode) { + $node->insertBefore($child->firstChild, $child); + } + + $node->removeChild($child); + } else { + $this->sanitizeAttributes($child, $tag); + $this->sanitizeNode($child); + + if ($tag === 'iframe' && !$this->isAllowedIframe($child)) { + $node->removeChild($child); + } + } + } + + $child = $next; + } + } + + private function sanitizeAttributes(DOMElement $element, string $tag): void + { + $allowed = self::ALLOWED_ELEMENTS[$tag]; + + if ($element->hasAttributes()) { + /** @var DOMAttr $attribute */ + foreach (iterator_to_array($element->attributes) as $attribute) { + if (!in_array(strtolower($attribute->name), $allowed, true)) { + $element->removeAttribute($attribute->name); + } + } + } + + if ($tag === 'a') { + $href = trim($element->getAttribute('href')); + if (!$this->isAllowedHref($href)) { + $element->removeAttribute('href'); + } else { + $element->setAttribute('rel', 'noopener noreferrer'); + } + } + + if ($tag === 'img') { + $src = trim($element->getAttribute('src')); + if (!$this->isAllowedSrc($src)) { + $element->removeAttribute('src'); + } + } + } + + private function isAllowedIframe(DOMElement $element): bool + { + $src = trim($element->getAttribute('src')); + $host = parse_url($src, PHP_URL_HOST); + + return is_string($host) && in_array($host, self::ALLOWED_EMBED_HOSTS, true); + } + + private function isAllowedHref(string $href): bool + { + if ($href === '') { + return false; + } + + if (str_starts_with($href, '/')) { + return !str_starts_with($href, '//'); + } + + return (bool) preg_match('#^(https?:|mailto:)#i', $href); + } + + private function isAllowedSrc(string $src): bool + { + if ($src === '' || str_starts_with($src, '//')) { + return false; + } + + if (str_starts_with($src, '/')) { + return true; + } + + return (bool) preg_match('#^https?://#i', $src); + } +} diff --git a/src/Security/Html/NullPageLayoutHtmlSanitizer.php b/src/Security/Html/NullPageLayoutHtmlSanitizer.php new file mode 100644 index 0000000..7b69a4e --- /dev/null +++ b/src/Security/Html/NullPageLayoutHtmlSanitizer.php @@ -0,0 +1,16 @@ +config->htmlSanitizeStrategy) { + HtmlSanitizeStrategy::None => new NullPageLayoutHtmlSanitizer(), + HtmlSanitizeStrategy::Strip => new StripPageLayoutHtmlSanitizer(), + HtmlSanitizeStrategy::Allowlist => new AllowlistPageLayoutHtmlSanitizer(), + HtmlSanitizeStrategy::Service => $this->customSanitizer ?? new NullPageLayoutHtmlSanitizer(), + }; + } +} diff --git a/src/Security/PageLayoutProtectionConfig.php b/src/Security/PageLayoutProtectionConfig.php new file mode 100644 index 0000000..9317096 --- /dev/null +++ b/src/Security/PageLayoutProtectionConfig.php @@ -0,0 +1,19 @@ +sanitizeBlockData($data); + $sectionKey = $data['sectionKey'] ?? null; if (!is_string($sectionKey) || $sectionKey === '') { @@ -123,15 +127,23 @@ private function legacyLayout(string $pageKey, string $locale): array { $content = $this->legacyContent($pageKey, $locale); - if ($pageKey === 'home') { - return $this->legacyHomeLayout($content); - } - - if ($pageKey === 'contact') { - return $this->legacyContactLayout($content); - } - - return []; + $views = match ($pageKey) { + 'home' => $this->legacyHomeLayout($content), + 'contact' => $this->legacyContactLayout($content), + default => [], + }; + + return array_map( + fn (PageBlockView $view): PageBlockView => new PageBlockView( + layoutId: $view->layoutId, + pageKey: $view->pageKey, + type: $view->type, + blockId: $view->blockId, + sectionKey: $view->sectionKey, + data: $this->sanitizeBlockData($view->data), + ), + $views, + ); } /** @param array $content @@ -269,4 +281,38 @@ private function currentLocale(): string return $request?->getLocale() ?? $this->pageLocales->getDefault(); } + + /** + * @param array $data + * + * @return array + */ + private function sanitizeBlockData(array $data): array + { + $sanitizer = $this->protection->htmlSanitizer(); + + if (isset($data['body']) && is_string($data['body']) && $data['body'] !== '') { + $data['body'] = $sanitizer->sanitize($data['body']); + } + + if (isset($data['beforeText']) && is_string($data['beforeText']) && $data['beforeText'] !== '') { + $data['beforeText'] = $sanitizer->sanitize($data['beforeText']); + } + + if (isset($data['afterText']) && is_string($data['afterText']) && $data['afterText'] !== '') { + $data['afterText'] = $sanitizer->sanitize($data['afterText']); + } + + if (isset($data['items']) && is_array($data['items'])) { + foreach ($data['items'] as $index => $item) { + if (!is_array($item) || !isset($item['body']) || !is_string($item['body']) || $item['body'] === '') { + continue; + } + + $data['items'][$index]['body'] = $sanitizer->sanitize($item['body']); + } + } + + return $data; + } } diff --git a/tests/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php index 74877d4..a5edbd2 100644 --- a/tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/tests/Unit/DependencyInjection/ConfigurationTest.php @@ -49,5 +49,21 @@ public function testConfigurationAppliesDefaultsAndCustomValues(): void self::assertSame('custom', $config['web_ui']['css_framework']); self::assertSame('acme_', $config['doctrine']['table_prefix']); self::assertSame('reporting', $config['doctrine']['connection']); + self::assertSame('none', $config['html']['sanitize']['strategy']); + self::assertNull($config['html']['sanitize']['service']); + } + + public function testHtmlSanitizeStrategyAllowlist(): void + { + $processor = new Processor(); + $config = $processor->processConfiguration(new Configuration(), [[ + 'html' => [ + 'sanitize' => [ + 'strategy' => 'allowlist', + ], + ], + ]]); + + self::assertSame('allowlist', $config['html']['sanitize']['strategy']); } } diff --git a/tests/Unit/DependencyInjection/NowoPageLayoutKitExtensionTest.php b/tests/Unit/DependencyInjection/NowoPageLayoutKitExtensionTest.php index 9b4e9de..68fbe7f 100644 --- a/tests/Unit/DependencyInjection/NowoPageLayoutKitExtensionTest.php +++ b/tests/Unit/DependencyInjection/NowoPageLayoutKitExtensionTest.php @@ -11,7 +11,10 @@ use Nowo\PageLayoutKitBundle\Locale\PageLocales; use Nowo\PageLayoutKitBundle\Security\AllowAllPageLayoutKitAccessChecker; use Nowo\PageLayoutKitBundle\Security\ConfigurablePageLayoutKitAccessChecker; +use Nowo\PageLayoutKitBundle\Security\Html\NullPageLayoutHtmlSanitizer; use Nowo\PageLayoutKitBundle\Security\PageLayoutKitAccessCheckerInterface; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtection; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtectionConfig; use PHPUnit\Framework\TestCase; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -143,6 +146,9 @@ public function testLoadPublishesParametersAndRegistersAllowAllAccessChecker(): self::assertSame([ ['event' => Events::loadClassMetadata], ], $listener->getTag('doctrine.event_listener')); + + self::assertTrue($container->hasDefinition(PageLayoutProtectionConfig::class)); + self::assertTrue($container->hasDefinition(PageLayoutProtection::class)); } public function testLoadUsesCustomAccessCheckerAliasWhenConfigured(): void @@ -238,6 +244,32 @@ public function testLoadRequiresSecurityBundleWhenAnonymousAccessIsDisabled(): v ], new ContainerBuilder()); } + public function testLoadRegistersHtmlSanitizerServiceWhenConfigured(): void + { + $container = new ContainerBuilder(); + $container->register('app.page_layout_html_sanitizer', NullPageLayoutHtmlSanitizer::class); + + (new NowoPageLayoutKitExtension())->load([ + [ + 'security' => [ + 'allow_unauthenticated' => true, + ], + 'html' => [ + 'sanitize' => [ + 'strategy' => 'service', + 'service' => 'app.page_layout_html_sanitizer', + ], + ], + ], + ], $container); + + $definition = $container->getDefinition(PageLayoutProtection::class); + self::assertEquals( + new Reference('app.page_layout_html_sanitizer'), + $definition->getArgument(1), + ); + } + public function testAliasMatchesBundleConfigurationAlias(): void { self::assertSame('nowo_page_layout_kit', (new NowoPageLayoutKitExtension())->getAlias()); diff --git a/tests/Unit/EventSubscriber/PageBlockHtmlSanitizeSubscriberTest.php b/tests/Unit/EventSubscriber/PageBlockHtmlSanitizeSubscriberTest.php new file mode 100644 index 0000000..d986722 --- /dev/null +++ b/tests/Unit/EventSubscriber/PageBlockHtmlSanitizeSubscriberTest.php @@ -0,0 +1,116 @@ +setTranslatable(new PageTextBlock()) + ->setBody('

Hi

'); + + $this->runPersist($translation); + + self::assertStringContainsString('

Hi

', $translation->getBody()); + self::assertStringNotContainsString('script', $translation->getBody()); + } + + public function testPreUpdateSanitizesCompareBlockTranslationFields(): void + { + $translation = (new PageCompareBlockTranslation()) + ->setTranslatable(new PageCompareBlock()) + ->setBeforeText('Before') + ->setAfterText('After'); + + $this->runUpdate($translation); + + self::assertStringContainsString('Before', $translation->getBeforeText()); + self::assertStringNotContainsString('script', $translation->getBeforeText()); + self::assertStringContainsString('After', $translation->getAfterText()); + self::assertStringNotContainsString('script', $translation->getAfterText()); + } + + public function testPrePersistSanitizesCtaBlockTranslationBody(): void + { + $translation = (new PageCtaBlockTranslation()) + ->setTranslatable((new PageCtaBlock())->setSectionKey('cta')) + ->setBody('

CTA

'); + + $this->runPersist($translation); + + self::assertStringContainsString('

CTA

', $translation->getBody()); + self::assertStringNotContainsString('script', $translation->getBody()); + } + + public function testSkipsEmptyBodies(): void + { + $translation = (new PageTextBlockTranslation()) + ->setTranslatable(new PageTextBlock()) + ->setBody(''); + + $this->runPersist($translation); + + self::assertSame('', $translation->getBody()); + } + + public function testIgnoresUnsupportedEntities(): void + { + $entity = new stdClass(); + + $this->runPersist($entity); + + self::assertInstanceOf(stdClass::class, $entity); + } + + private function runPersist(object $entity): void + { + $subscriber = $this->createSubscriber(); + $args = $this->createLifecycleArgs($entity); + + $subscriber->prePersist($args); + } + + private function runUpdate(object $entity): void + { + $subscriber = $this->createSubscriber(); + $args = $this->createLifecycleArgs($entity); + + $subscriber->preUpdate($args); + } + + private function createSubscriber(): PageBlockHtmlSanitizeSubscriber + { + return new PageBlockHtmlSanitizeSubscriber( + new PageLayoutProtection(new PageLayoutProtectionConfig(HtmlSanitizeStrategy::Allowlist, null)), + ); + } + + /** + * @return LifecycleEventArgs + */ + private function createLifecycleArgs(object $entity): LifecycleEventArgs + { + $args = $this->createMock(LifecycleEventArgs::class); + $args->method('getObject')->willReturn($entity); + + return $args; + } +} diff --git a/tests/Unit/Repository/PageTranslationRepositoriesTest.php b/tests/Unit/Repository/PageTranslationRepositoriesTest.php index f79c15e..21801de 100644 --- a/tests/Unit/Repository/PageTranslationRepositoriesTest.php +++ b/tests/Unit/Repository/PageTranslationRepositoriesTest.php @@ -11,6 +11,7 @@ use Nowo\PageLayoutKitBundle\Repository\PageCtaBlockTranslationRepository; use Nowo\PageLayoutKitBundle\Repository\PageHeroBlockTranslationRepository; use Nowo\PageLayoutKitBundle\Repository\PageListBlockTranslationRepository; +use Nowo\PageLayoutKitBundle\Repository\PageListItemRepository; use Nowo\PageLayoutKitBundle\Repository\PageListItemTranslationRepository; use Nowo\PageLayoutKitBundle\Repository\PageTextBlockTranslationRepository; use PHPUnit\Framework\TestCase; @@ -28,6 +29,7 @@ public function testTranslationRepositoriesCanBeInstantiated(): void self::assertInstanceOf(PageHeroBlockTranslationRepository::class, new PageHeroBlockTranslationRepository($registry)); self::assertInstanceOf(PageListBlockTranslationRepository::class, new PageListBlockTranslationRepository($registry)); self::assertInstanceOf(PageListItemTranslationRepository::class, new PageListItemTranslationRepository($registry)); + self::assertInstanceOf(PageListItemRepository::class, new PageListItemRepository($registry)); self::assertInstanceOf(PageTextBlockTranslationRepository::class, new PageTextBlockTranslationRepository($registry)); } } diff --git a/tests/Unit/Security/AllowlistPageLayoutHtmlSanitizerTest.php b/tests/Unit/Security/AllowlistPageLayoutHtmlSanitizerTest.php new file mode 100644 index 0000000..e772486 --- /dev/null +++ b/tests/Unit/Security/AllowlistPageLayoutHtmlSanitizerTest.php @@ -0,0 +1,92 @@ +sanitize('

Hello

'); + + self::assertStringContainsString('

Hello

', $result); + self::assertStringNotContainsString('script', $result); + } + + public function testAllowsSafeLinks(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize('Link'); + + self::assertStringContainsString('href="https://example.com"', $result); + self::assertStringContainsString('rel="noopener noreferrer"', $result); + } + + public function testReturnsEmptyStringForBlankInput(): void + { + self::assertSame('', (new AllowlistPageLayoutHtmlSanitizer())->sanitize(' ')); + } + + public function testRemovesDisallowedTagsAndAttributes(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize('

Text

'); + + self::assertStringContainsString('

Text

', $result); + self::assertStringNotContainsString('onclick', $result); + self::assertStringNotContainsString('object', $result); + } + + public function testRejectsUnsafeHrefAndSrc(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize( + 'Xxok', + ); + + self::assertStringNotContainsString('javascript:', $result); + self::assertStringNotContainsString('evil.test', $result); + self::assertStringContainsString('src="/local.png"', $result); + } + + public function testAllowsRelativePathsAndMailtoLinks(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize('ContactMail'); + + self::assertStringContainsString('href="/contact"', $result); + self::assertStringContainsString('href="mailto:hi@example.com"', $result); + } + + public function testAllowsExternalHttpsImageSrc(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize('logo'); + + self::assertStringContainsString('https://cdn.example.com/logo.png', $result); + } + + public function testRemovesEmptyHrefFromLinks(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $result = $sanitizer->sanitize('Empty'); + + self::assertStringNotContainsString('href=', $result); + self::assertStringContainsString('Empty', $result); + } + + public function testAllowsYouTubeIframeAndRemovesUnknownEmbeds(): void + { + $sanitizer = new AllowlistPageLayoutHtmlSanitizer(); + $allowed = $sanitizer->sanitize(''); + $blocked = $sanitizer->sanitize(''); + + self::assertStringContainsString('youtube.com/embed/abc', $allowed); + self::assertStringNotContainsString('iframe', $blocked); + } +} diff --git a/tests/Unit/Security/PageLayoutProtectionTest.php b/tests/Unit/Security/PageLayoutProtectionTest.php new file mode 100644 index 0000000..9c6f40a --- /dev/null +++ b/tests/Unit/Security/PageLayoutProtectionTest.php @@ -0,0 +1,64 @@ +htmlSanitizer()); + } + + public function testResolvesStripStrategy(): void + { + $protection = new PageLayoutProtection(new PageLayoutProtectionConfig(HtmlSanitizeStrategy::Strip, null)); + + self::assertInstanceOf(StripPageLayoutHtmlSanitizer::class, $protection->htmlSanitizer()); + } + + public function testResolvesAllowlistStrategy(): void + { + $protection = new PageLayoutProtection(new PageLayoutProtectionConfig(HtmlSanitizeStrategy::Allowlist, null)); + + self::assertInstanceOf(AllowlistPageLayoutHtmlSanitizer::class, $protection->htmlSanitizer()); + } + + public function testServiceStrategyUsesCustomSanitizerWhenProvided(): void + { + $custom = new class implements PageLayoutHtmlSanitizerInterface { + public function sanitize(string $html): string + { + return 'custom:' . $html; + } + }; + + $protection = new PageLayoutProtection( + new PageLayoutProtectionConfig(HtmlSanitizeStrategy::Service, 'app.sanitizer'), + $custom, + ); + + self::assertSame('custom:

x

', $protection->htmlSanitizer()->sanitize('

x

')); + } + + public function testServiceStrategyFallsBackToNullSanitizerWithoutCustom(): void + { + $protection = new PageLayoutProtection( + new PageLayoutProtectionConfig(HtmlSanitizeStrategy::Service, 'app.sanitizer'), + ); + + self::assertInstanceOf(NullPageLayoutHtmlSanitizer::class, $protection->htmlSanitizer()); + } +} diff --git a/tests/Unit/Security/StripPageLayoutHtmlSanitizerTest.php b/tests/Unit/Security/StripPageLayoutHtmlSanitizerTest.php new file mode 100644 index 0000000..5f5401f --- /dev/null +++ b/tests/Unit/Security/StripPageLayoutHtmlSanitizerTest.php @@ -0,0 +1,18 @@ +sanitize(' Hello bold & ')); + } +} diff --git a/tests/Unit/Service/PageBlockProviderTest.php b/tests/Unit/Service/PageBlockProviderTest.php index bb1d84a..b345e97 100644 --- a/tests/Unit/Service/PageBlockProviderTest.php +++ b/tests/Unit/Service/PageBlockProviderTest.php @@ -11,11 +11,14 @@ use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; use Nowo\PageLayoutKitBundle\Entity\PageLayoutEntry; +use Nowo\PageLayoutKitBundle\Enum\HtmlSanitizeStrategy; use Nowo\PageLayoutKitBundle\Enum\PageBlockType; use Nowo\PageLayoutKitBundle\Legacy\LegacyPageContentProviderInterface; use Nowo\PageLayoutKitBundle\Locale\PageLocales; use Nowo\PageLayoutKitBundle\Repository\PageBlockSqlRepository; use Nowo\PageLayoutKitBundle\Repository\PageLayoutEntryRepository; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtection; +use Nowo\PageLayoutKitBundle\Security\PageLayoutProtectionConfig; use Nowo\PageLayoutKitBundle\Service\PageBlockProvider; use PHPUnit\Framework\TestCase; use ReflectionObject; @@ -40,6 +43,7 @@ public function testReturnsLegacyLayoutsAndMetaWhenNoStoredLayoutExists(): void $this->createPageBlockSqlRepository([], new stdClass()), $this->createRequestStack('es'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), new FakeLegacyPageContentProvider([ 'home:es' => [ 'page_title' => 'Inicio', @@ -116,6 +120,7 @@ public function testPageMetaFallsBackToLegacyWhenStoredLayoutHasNoSeoBlock(): vo ], $state), $this->createRequestStack('en'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), new FakeLegacyPageContentProvider([ 'landing:en' => [ 'page_title' => 'Landing title', @@ -139,6 +144,7 @@ public function testUnknownLegacyPageKeysProduceAnEmptyLayout(): void $this->createPageBlockSqlRepository([], new stdClass()), $this->createRequestStack('es'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), new FakeLegacyPageContentProvider([]), ); @@ -154,6 +160,7 @@ public function testPageMetaFallsBackToLegacyDefaultLocaleAndHandlesMissingProvi $this->createPageBlockSqlRepository([], new stdClass()), $this->createRequestStack('en'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), new FakeLegacyPageContentProvider([ 'home:es' => [ 'page_title' => 'Inicio', @@ -175,6 +182,7 @@ public function testPageMetaFallsBackToLegacyDefaultLocaleAndHandlesMissingProvi $this->createPageBlockSqlRepository([], new stdClass()), $this->createRequestStack('es'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), ); self::assertSame( @@ -209,6 +217,7 @@ public function testBuildsStoredLayoutViewsCachesAndSkipsEntriesWithoutLoadedDat ], $state), $this->createRequestStack('en'), new PageLocales('es', ['es', 'en']), + $this->createProtection(), new FakeLegacyPageContentProvider([]), ); @@ -231,6 +240,71 @@ public function testBuildsStoredLayoutViewsCachesAndSkipsEntriesWithoutLoadedDat self::assertSame(4, $state->queries); } + public function testAllowlistSanitizerStripsScriptFromStoredBlockBody(): void + { + $textEntry = $this->createLayoutEntry('home', PageBlockType::Text, 11, 0, 502); + $state = new stdClass(); + $state->queries = 0; + + $provider = new PageBlockProvider( + $this->createPageLayoutEntryRepository([ + 'home' => [$textEntry], + ]), + $this->createPageBlockSqlRepository([ + 'FROM content_page_text_block b' => [[ + 'block_id' => 11, + 'title' => 'Title', + 'body' => '

Safe

', + ]], + ], $state), + $this->createRequestStack('es'), + new PageLocales('es', ['es', 'en']), + $this->createProtection(HtmlSanitizeStrategy::Allowlist), + ); + + $layout = $provider->getLayout('home', 'es'); + self::assertCount(1, $layout); + self::assertStringContainsString('

Safe

', $layout[0]->data['body']); + self::assertStringNotContainsString('script', $layout[0]->data['body']); + } + + public function testAllowlistSanitizerStripsScriptFromCardItemBodies(): void + { + $cardsEntry = $this->createLayoutEntry('home', PageBlockType::Cards, 12, 0, 503); + $state = new stdClass(); + $state->queries = 0; + + $provider = new PageBlockProvider( + $this->createPageLayoutEntryRepository([ + 'home' => [$cardsEntry], + ]), + $this->createPageBlockSqlRepository([ + 'FROM content_page_cards_block b' => [[ + 'block_id' => 12, + 'title' => 'Cards', + ]], + 'FROM content_page_card_item i' => [[ + 'block_id' => 12, + 'position' => 0, + 'title' => 'Card', + 'body' => '

Card

', + ]], + ], $state), + $this->createRequestStack('es'), + new PageLocales('es', ['es', 'en']), + $this->createProtection(HtmlSanitizeStrategy::Allowlist), + ); + + $layout = $provider->getLayout('home', 'es'); + self::assertStringContainsString('

Card

', $layout[0]->data['items'][0]['body']); + self::assertStringNotContainsString('script', $layout[0]->data['items'][0]['body']); + } + + private function createProtection(HtmlSanitizeStrategy $strategy = HtmlSanitizeStrategy::None): PageLayoutProtection + { + return new PageLayoutProtection(new PageLayoutProtectionConfig($strategy, null)); + } + private function createLayoutEntry( string $pageKey, PageBlockType $type,