diff --git a/backend/app/Services/Infrastructure/HtmlPurifier/HtmlPurifierService.php b/backend/app/Services/Infrastructure/HtmlPurifier/HtmlPurifierService.php index 2e9ac91429..48ef14a1f7 100644 --- a/backend/app/Services/Infrastructure/HtmlPurifier/HtmlPurifierService.php +++ b/backend/app/Services/Infrastructure/HtmlPurifier/HtmlPurifierService.php @@ -8,6 +8,14 @@ class HtmlPurifierService { + private const ENCODED_LIQUID_TOKEN = '/%7B%7B((?:%20|%7C|[A-Za-z0-9_.\-])*)%7D%7D/i'; + + private const DECODABLE = [ + '%20' => ' ', + '%7C' => '|', + '%7c' => '|', + ]; + private HTMLPurifier_Config $config; public function __construct(private readonly HTMLPurifier $htmlPurifier) @@ -28,6 +36,10 @@ public function purify(?string $html): ?string return null; } - return $this->htmlPurifier->purify($html, $this->config); + return preg_replace_callback( + self::ENCODED_LIQUID_TOKEN, + static fn (array $matches): string => '{{'.strtr($matches[1], self::DECODABLE).'}}', + $this->htmlPurifier->purify($html, $this->config), + ); } } diff --git a/backend/tests/Feature/Services/Application/Handlers/EmailTemplate/EmailTemplateBodyPurificationTest.php b/backend/tests/Feature/Services/Application/Handlers/EmailTemplate/EmailTemplateBodyPurificationTest.php new file mode 100644 index 0000000000..86a93027f0 --- /dev/null +++ b/backend/tests/Feature/Services/Application/Handlers/EmailTemplate/EmailTemplateBodyPurificationTest.php @@ -0,0 +1,97 @@ +Hi {{ order.first_name }}

' + .'

View your order

' + .'

{{ \'\' }}

'; + + private int $accountId; + + protected function setUp(): void + { + parent::setUp(); + + $user = User::factory()->withAccount()->create(); + $this->accountId = $user->accounts()->first()->id; + } + + public function test_creating_a_template_keeps_liquid_tokens_in_links(): void + { + $template = $this->app->make(CreateEmailTemplateHandler::class)->handle($this->dto()); + + $stored = DB::table('email_templates')->where('id', $template->getId())->value('body'); + + $this->assertStringContainsString('href="https://example.com/orders?ref={{ order.number }}"', $stored); + $this->assertStringNotContainsString('%7B%7B', $stored); + } + + public function test_creating_a_template_still_strips_markup_smuggled_through_a_token(): void + { + $template = $this->app->make(CreateEmailTemplateHandler::class)->handle($this->dto()); + + $stored = DB::table('email_templates')->where('id', $template->getId())->value('body'); + + $this->assertStringNotContainsString('onerror', $stored); + } + + public function test_updating_a_template_keeps_liquid_tokens_in_links(): void + { + $created = $this->app->make(CreateEmailTemplateHandler::class)->handle($this->dto()); + + $updated = $this->app->make(UpdateEmailTemplateHandler::class)->handle($this->dto($created->getId())); + + $stored = DB::table('email_templates')->where('id', $updated->getId())->value('body'); + + $this->assertStringContainsString('href="https://example.com/orders?ref={{ order.number }}"', $stored); + $this->assertStringNotContainsString('onerror', $stored); + } + + public function test_a_stored_token_resolves_to_a_real_value_when_rendered(): void + { + $template = $this->app->make(CreateEmailTemplateHandler::class)->handle($this->dto()); + + $stored = (string) DB::table('email_templates')->where('id', $template->getId())->value('body'); + + $rendered = $this->app->make(EmailTemplateService::class)->previewTemplate( + 'Your order', + $stored, + EmailTemplateType::ORDER_CONFIRMATION, + )['body']; + + $this->assertMatchesRegularExpression( + '/href="https:\/\/example\.com\/orders\?ref=[^"{%]+"/', + $rendered, + ); + $this->assertStringNotContainsString('{{', $rendered); + $this->assertStringNotContainsString('onerror', $rendered); + } + + private function dto(?int $id = null): UpsertEmailTemplateDTO + { + return new UpsertEmailTemplateDTO( + account_id: $this->accountId, + template_type: EmailTemplateType::ORDER_CONFIRMATION, + subject: 'Your order', + body: self::BODY, + id: $id, + cta: ['label' => 'View order', 'url_token' => 'order.url'], + ); + } +} diff --git a/backend/tests/Unit/Services/Infrastructure/HtmlPurifier/HtmlPurifierServiceTest.php b/backend/tests/Unit/Services/Infrastructure/HtmlPurifier/HtmlPurifierServiceTest.php index c30c2aa34b..143bdeb471 100644 --- a/backend/tests/Unit/Services/Infrastructure/HtmlPurifier/HtmlPurifierServiceTest.php +++ b/backend/tests/Unit/Services/Infrastructure/HtmlPurifier/HtmlPurifierServiceTest.php @@ -5,6 +5,7 @@ namespace Tests\Unit\Services\Infrastructure\HtmlPurifier; use HiEvents\Services\Infrastructure\HtmlPurifier\HtmlPurifierService; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class HtmlPurifierServiceTest extends TestCase @@ -39,6 +40,132 @@ public function test_scripts_are_still_removed(): void $this->assertStringNotContainsString('alert', (string) $this->service->purify('Hi')); } + #[DataProvider('liquidTokenInUriProvider')] + public function test_liquid_tokens_survive_purification_inside_uris(string $html, string $expectedHref): void + { + $purified = (string) $this->service->purify($html); + + $this->assertStringContainsString($expectedHref, $purified); + $this->assertStringNotContainsString('%7B%7B', $purified); + } + + public static function liquidTokenInUriProvider(): array + { + return [ + 'query string' => [ + 'View', + 'href="https://example.com/orders?ref={{ order.number }}"', + ], + 'path segment' => [ + 'View', + 'href="https://example.com/{{ event.slug }}/tickets"', + ], + 'fragment' => [ + 'View', + 'href="https://example.com/orders#{{ order.id }}"', + ], + 'entire href' => [ + 'View', + 'href="{{ order.url }}"', + ], + 'token with filter' => [ + 'View', + 'href="https://example.com/{{ event.slug | downcase }}"', + ], + 'multiple tokens' => [ + 'View', + 'href="https://example.com/o?ref={{ order.number }}&t={{ order.id }}"', + ], + 'token without surrounding spaces' => [ + 'View', + 'href="https://example.com/o?ref={{order.number}}"', + ], + ]; + } + + public function test_liquid_tokens_in_text_nodes_are_untouched(): void + { + $html = '

Hi {{ order.first_name }}

{% if order.is_payment_required %}Due{% endif %}

'; + + $this->assertSame($html, $this->service->purify($html)); + } + + public function test_liquid_href_still_gets_nofollow_and_target_blank(): void + { + $purified = (string) $this->service->purify('x'); + + $this->assertStringContainsString('{{ order.number }}', $purified); + $this->assertStringContainsString('rel="nofollow', $purified); + $this->assertStringContainsString('target="_blank"', $purified); + } + + #[DataProvider('hostileLiquidTokenProvider')] + public function test_liquid_tokens_cannot_smuggle_markup_past_the_purifier(string $html, string $mustNotContain): void + { + $purified = (string) $this->service->purify($html); + + $this->assertStringNotContainsString($mustNotContain, $purified); + } + + public static function hostileLiquidTokenProvider(): array + { + return [ + 'script tag in a string literal' => [ + '

{{ \'\' }}

', + ' [ + '

{{ "" }}

', + 'onerror', + ], + 'attribute breakout' => [ + 'hi', + 'onmouseover', + ], + 'quote smuggled inside a token' => [ + 'x', + 'onmouseover', + ], + 'javascript scheme inside a token' => [ + 'x', + 'javascript:', + ], + ]; + } + + #[DataProvider('unrelatedMarkupProvider')] + public function test_purification_of_non_liquid_markup_is_unchanged(string $html, string $expected): void + { + $this->assertSame($expected, $this->service->purify($html)); + } + + public static function unrelatedMarkupProvider(): array + { + return [ + 'relative link' => ['L', 'L'], + 'mailto' => ['M', 'M'], + 'image' => ['x', 'x'], + 'javascript scheme is stripped' => ['x', 'x'], + 'data uri is stripped' => ['x', ''], + 'event handler is stripped' => ['x', 'x'], + ]; + } + + public function test_percent_encoded_braces_typed_by_hand_also_decode(): void + { + $this->assertSame('

{{foo}}

', $this->service->purify('

%7B%7Bfoo%7D%7D

')); + } + + public function test_decoding_can_only_emit_characters_that_are_inert_in_markup(): void + { + $purified = (string) $this->service->purify( + 'x' + ); + + $this->assertStringNotContainsString('assertStringNotContainsString('onerror=', $purified); + } + public function test_null_is_preserved(): void { $this->assertNull($this->service->purify(null));