From e300246b2762bf1910d601bbdf9a1ba00d7633c6 Mon Sep 17 00:00:00 2001 From: Luan Trindade Date: Sun, 20 Sep 2026 00:32:30 -0300 Subject: [PATCH 1/3] fix(markdown): sanitize content_html server-side to close XSS gap (F-009) GithubFlavoredMarkdownConverter passed raw HTML and javascript:/data: links through untouched, and content_html was returned by the API exactly as stored. Any consumer other than the sanitizing Angular viewer was exposed to stored XSS, including rows already persisted before this fix. - Harden the converter config (html_input=escape, allow_unsafe_links=false) as a first layer at parse time. - Add HtmlSanitizer, a dependency-free DOM-based allowlist filter for the tags/attributes Markdown legitimately produces (headings, code, tables, GFM task lists, links, images), and apply it to freshly parsed content. - Add a content_html accessor on Document that runs the same sanitizer on every read, so API responses are safe even for documents whose HTML was written to the database before this fix (no data migration performed). Co-Authored-By: Claude Sonnet 5 --- backend/app/Models/Document.php | 18 ++ .../app/Services/Markdown/HtmlSanitizer.php | 190 ++++++++++++++++++ .../app/Services/Markdown/MarkdownParser.php | 18 +- 3 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 backend/app/Services/Markdown/HtmlSanitizer.php diff --git a/backend/app/Models/Document.php b/backend/app/Models/Document.php index eb20772..b93f050 100644 --- a/backend/app/Models/Document.php +++ b/backend/app/Models/Document.php @@ -2,7 +2,9 @@ namespace App\Models; +use App\Services\Markdown\HtmlSanitizer; use Database\Factories\DocumentFactory; +use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -37,6 +39,22 @@ protected function casts(): array ]; } + /** + * Sanitizes `content_html` on every read so the API never returns + * script-capable markup, regardless of how the stored value was written + * (ingestion pipeline, factory, seed, or a row inserted before the + * sanitizer existed). The stored value itself is never mutated by this + * accessor. + * + * @return Attribute + */ + protected function contentHtml(): Attribute + { + return Attribute::make( + get: fn (?string $value): string => HtmlSanitizer::sanitize($value), + ); + } + /** * @return BelongsTo */ diff --git a/backend/app/Services/Markdown/HtmlSanitizer.php b/backend/app/Services/Markdown/HtmlSanitizer.php new file mode 100644 index 0000000..6c887c8 --- /dev/null +++ b/backend/app/Services/Markdown/HtmlSanitizer.php @@ -0,0 +1,190 @@ +> + */ + private const ALLOWED_TAGS = [ + 'h1' => [], 'h2' => [], 'h3' => [], 'h4' => [], 'h5' => [], 'h6' => [], + 'p' => [], 'br' => [], 'hr' => [], + 'strong' => [], 'em' => [], 'b' => [], 'i' => [], 's' => [], 'del' => [], + 'blockquote' => [], + 'pre' => [], + 'code' => ['class'], + 'ul' => [], 'ol' => [], 'li' => [], + 'table' => [], 'thead' => [], 'tbody' => [], 'tfoot' => [], + 'tr' => [], + 'th' => ['align'], + 'td' => ['align'], + 'a' => ['href', 'title'], + 'img' => ['src', 'alt', 'title'], + 'input' => ['type', 'checked', 'disabled'], + ]; + + /** + * URL schemes allowed for `href`/`src`. Everything else (javascript:, data:, + * vbscript:, file:, etc.) is stripped. Schemeless values (relative paths, + * fragments, query strings) are always allowed. + * + * @var list + */ + private const ALLOWED_URL_SCHEMES = ['http', 'https', 'mailto']; + + /** + * @var list + */ + private const ALLOWED_ALIGN_VALUES = ['left', 'center', 'right', 'justify', 'char']; + + private const ROOT_ID = '__command_sphere_html_sanitizer_root__'; + + /** + * Sanitizes an HTML fragment down to the allowlisted subset. + */ + public static function sanitize(?string $html): string + { + $html = (string) $html; + + if (trim($html) === '') { + return ''; + } + + $dom = new DOMDocument; + $previousErrorSetting = libxml_use_internal_errors(true); + + $loaded = $dom->loadHTML( + '
'.$html.'
', + LIBXML_NOERROR | LIBXML_NOWARNING + ); + + libxml_clear_errors(); + libxml_use_internal_errors($previousErrorSetting); + + if (! $loaded) { + return ''; + } + + $root = $dom->getElementById(self::ROOT_ID); + + if (! $root instanceof DOMElement) { + return ''; + } + + self::cleanChildren($root, $dom); + + $output = ''; + + foreach (iterator_to_array($root->childNodes) as $child) { + $rendered = $dom->saveHTML($child); + $output .= $rendered === false ? '' : $rendered; + } + + return $output; + } + + private static function cleanChildren(DOMNode $parent, DOMDocument $dom): void + { + foreach (iterator_to_array($parent->childNodes) as $node) { + if ($node instanceof DOMText) { + continue; + } + + if (! $node instanceof DOMElement) { + $parent->removeChild($node); + + continue; + } + + $tag = strtolower($node->tagName); + + if (! array_key_exists($tag, self::ALLOWED_TAGS)) { + $parent->removeChild($node); + + continue; + } + + self::sanitizeAttributes($node, $tag); + self::cleanChildren($node, $dom); + } + } + + private static function sanitizeAttributes(DOMElement $node, string $tag): void + { + $allowedAttributes = self::ALLOWED_TAGS[$tag]; + $attributeNames = []; + + foreach ($node->attributes ?? [] as $attribute) { + $attributeNames[] = $attribute->name; + } + + foreach ($attributeNames as $attributeName) { + $name = strtolower($attributeName); + $value = $node->getAttribute($attributeName); + + if (! in_array($name, $allowedAttributes, true)) { + $node->removeAttribute($attributeName); + + continue; + } + + if (in_array($name, ['href', 'src'], true) && ! self::isSafeUrl($value)) { + $node->removeAttribute($attributeName); + + continue; + } + + if ($name === 'align' && ! in_array(strtolower(trim($value)), self::ALLOWED_ALIGN_VALUES, true)) { + $node->removeAttribute($attributeName); + + continue; + } + + if ($name === 'class' && preg_match('/^language-[A-Za-z0-9_-]+$/', trim($value)) !== 1) { + $node->removeAttribute($attributeName); + } + } + + if ($tag === 'input') { + // Only the GFM task-list checkbox rendering is legitimate for a bare . + $node->setAttribute('type', 'checkbox'); + } + } + + private static function isSafeUrl(string $value): bool + { + // Strip control/whitespace characters browsers ignore while resolving a + // URL scheme; this defeats the classic `java\tscript:`/newline bypass. + $normalized = preg_replace('/[\x00-\x20\x7F]+/', '', $value) ?? ''; + + if ($normalized === '') { + return true; + } + + if (preg_match('/^([a-zA-Z][a-zA-Z0-9+.\-]*):/', $normalized, $matches) !== 1) { + return true; + } + + return in_array(strtolower($matches[1]), self::ALLOWED_URL_SCHEMES, true); + } +} diff --git a/backend/app/Services/Markdown/MarkdownParser.php b/backend/app/Services/Markdown/MarkdownParser.php index 5241076..c688759 100644 --- a/backend/app/Services/Markdown/MarkdownParser.php +++ b/backend/app/Services/Markdown/MarkdownParser.php @@ -13,8 +13,22 @@ class MarkdownParser { + /** + * Converter configuration hardened against script injection: raw HTML is + * escaped rather than passed through, and links/images using unsafe URL + * schemes (javascript:, data:, vbscript:, etc.) are stripped. This is the + * first sanitization layer; App\Services\Markdown\HtmlSanitizer is the + * second, tag/attribute-allowlist layer applied to the converted output. + * + * @var array + */ + private const SAFE_CONVERTER_CONFIG = [ + 'html_input' => 'escape', + 'allow_unsafe_links' => false, + ]; + public function __construct( - private readonly GithubFlavoredMarkdownConverter $converter = new GithubFlavoredMarkdownConverter, + private readonly GithubFlavoredMarkdownConverter $converter = new GithubFlavoredMarkdownConverter(self::SAFE_CONVERTER_CONFIG), ) {} public function parse(GitHubMarkdownFile $file): ParsedDocument @@ -35,7 +49,7 @@ public function parse(GitHubMarkdownFile $file): ParsedDocument title: $title, frontmatter: $frontmatter, contentRaw: $raw, - contentHtml: (string) $this->converter->convert($body), + contentHtml: HtmlSanitizer::sanitize((string) $this->converter->convert($body)), commands: $commands, warnings: $warnings, ); From 254999814aa2aa41a323977f7c22bcd02945c52c Mon Sep 17 00:00:00 2001 From: Luan Trindade Date: Sun, 20 Sep 2026 10:03:02 -0300 Subject: [PATCH 2/3] test(markdown): cover HtmlSanitizer and content_html output against XSS payloads Unit-tests HtmlSanitizer directly against the required payload matrix (script, on* handlers, iframe, svg onload, object/embed, style, form, javascript:/data:text/html links, encoded entities, case variation, control-byte scheme bypass) plus the legitimate rendering set. Feature-tests the full MarkdownParser pipeline against the same malicious Markdown/HTML mix, and proves a document whose content_html was written directly to the database (bypassing ingestion entirely) is served sanitized through GET /api/v1/documents/{document}. Co-Authored-By: Claude Sonnet 5 --- .../Feature/MarkdownSanitizationTest.php | 148 ++++++++++++++++++ .../Services/Markdown/HtmlSanitizerTest.php | 110 +++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 backend/tests/Feature/MarkdownSanitizationTest.php create mode 100644 backend/tests/Unit/Services/Markdown/HtmlSanitizerTest.php diff --git a/backend/tests/Feature/MarkdownSanitizationTest.php b/backend/tests/Feature/MarkdownSanitizationTest.php new file mode 100644 index 0000000..8056d26 --- /dev/null +++ b/backend/tests/Feature/MarkdownSanitizationTest.php @@ -0,0 +1,148 @@ +" as visible "<script>" text) is not a false + * positive. + */ +function assertNoExecutableHtml(string $html): void +{ + $dom = new DOMDocument; + $previous = libxml_use_internal_errors(true); + $dom->loadHTML('
'.$html.'
', LIBXML_NOERROR | LIBXML_NOWARNING); + libxml_clear_errors(); + libxml_use_internal_errors($previous); + + $dangerousTags = ['script', 'iframe', 'object', 'embed', 'style', 'form', 'svg', 'math', 'link', 'meta', 'base']; + + foreach ($dangerousTags as $tag) { + expect($dom->getElementsByTagName($tag)->length)->toBe(0); + } + + foreach ($dom->getElementsByTagName('*') as $element) { + foreach (iterator_to_array($element->attributes ?? []) as $attribute) { + $name = strtolower($attribute->name); + + expect($name)->not->toMatch('/^on/'); + + if (in_array($name, ['href', 'src'], true)) { + $normalized = strtolower(preg_replace('/[\x00-\x20\x7F]+/', '', $attribute->value) ?? ''); + + expect($normalized)->not->toMatch('/^javascript:/') + ->and($normalized)->not->toMatch('/^data:text\/html/') + ->and($normalized)->not->toMatch('/^vbscript:/'); + } + } + } +} + +it('sanitizes malicious raw HTML and links embedded in ingested Markdown while keeping legitimate rendering', function (): void { + $markdown = <<<'MD' +# Command Reference + +## /ban-player + +```yaml +syntax: /ban +description: Bans a player. +``` + +Some *legit* text with a [http link](https://example.com), a [relative link](./other.md) +and an ![image](/img/a.png "alt"). + +- [ ] pending task +- [x] done task + +| Left | Right | +|:-----|------:| +| a | b | + +```php +hi"; ?> +``` + + + + + + + + + + + + + + +
+ +[js link](javascript:alert(1)) + +[data html link](data:text/html,) + +encoded + +case variant +MD; + + $parsed = app(MarkdownParser::class)->parse(new GitHubMarkdownFile( + path: 'docs/security.md', + content: $markdown, + )); + + $html = $parsed->contentHtml; + + // Legitimate rendering is preserved. + expect($html)->toContain('

Command Reference

') + ->toContain('href="https://example.com"') + ->toContain('href="./other.md"') + ->toContain('src="/img/a.png"') + ->toContain('type="checkbox"') + ->toContain('toContain('<?php echo "<b>hi</b>"; ?>'); + + // Nothing executable survives as real markup, regardless of vector. Raw HTML + // that Markdown escaped into inert display text (e.g. "<script>") is not + // a vulnerability, so we assert against the *parsed* output rather than doing + // naive substring checks that would also flag harmless escaped text. + assertNoExecutableHtml($html); +}); + +it('serves a document whose content_html was written directly to the database, already sanitized', function (): void { + $community = Community::factory()->create(); + $plugin = Plugin::factory()->create(['community_id' => $community->id]); + $pluginVersion = PluginVersion::factory()->create(['plugin_id' => $plugin->id]); + + $document = Document::factory()->create([ + 'plugin_version_id' => $pluginVersion->id, + 'content_html' => '

Legacy Doc

' + .'' + .'' + .'click' + .'encoded', + ]); + + // The stored column keeps the untouched legacy payload: no data migration was performed. + expect($document->getRawOriginal('content_html'))->toContain('

after

')) + ->toBe('

before

after

'); +}); + +it('strips on* event handler attributes even on an otherwise allowed tag', function (): void { + expect(HtmlSanitizer::sanitize('')) + ->toBe('') + ->and(HtmlSanitizer::sanitize('

hi

')) + ->toBe('

hi

'); +}); + +it('strips an iframe tag entirely', function (): void { + expect(HtmlSanitizer::sanitize(''))->toBe(''); +}); + +it('strips an svg with an onload handler entirely', function (): void { + expect(HtmlSanitizer::sanitize(''))->toBe(''); +}); + +it('strips object and embed tags entirely', function (): void { + expect(HtmlSanitizer::sanitize(''))->toBe('') + ->and(HtmlSanitizer::sanitize(''))->toBe(''); +}); + +it('strips a style tag entirely', function (): void { + expect(HtmlSanitizer::sanitize(''))->toBe(''); +}); + +it('strips a form tag entirely', function (): void { + expect(HtmlSanitizer::sanitize('
'))->toBe(''); +}); + +it('drops javascript: link hrefs written as raw HTML', function (): void { + expect(HtmlSanitizer::sanitize('click'))->toBe('click'); +}); + +it('drops data:text/html link hrefs carrying an embedded script', function (): void { + expect(HtmlSanitizer::sanitize('click')) + ->toBe('click'); +}); + +it('drops javascript: hrefs hidden behind decimal/hex html entities', function (): void { + expect(HtmlSanitizer::sanitize('click'))->toBe('click') + ->and(HtmlSanitizer::sanitize('click'))->toBe('click'); +}); + +it('drops javascript: hrefs regardless of case', function (): void { + expect(HtmlSanitizer::sanitize('click'))->toBe('click'); +}); + +it('drops javascript: hrefs hidden behind embedded control bytes', function (): void { + expect(HtmlSanitizer::sanitize("click"))->toBe('click') + ->and(HtmlSanitizer::sanitize("click"))->toBe('click'); +}); + +it('keeps headings unchanged', function (): void { + expect(HtmlSanitizer::sanitize('

Title

Subtitle

')) + ->toBe('

Title

Subtitle

'); +}); + +it('keeps escaped code blocks safe from tag injection while preserving visible text', function (): void { + $html = '
<?php echo "hi"; ?>
'; + + // Quotes inside text content do not need re-escaping to render as literal + // quote characters; the security-relevant `<`/`>` escaping is preserved. + expect(HtmlSanitizer::sanitize($html)) + ->toBe('
<?php echo "hi"; ?>
'); +}); + +it('keeps tables with column alignment unchanged', function (): void { + $html = '' + .'
A
1
'; + + expect(HtmlSanitizer::sanitize($html))->toBe($html); +}); + +it('keeps GFM task list checkboxes unchanged', function (): void { + $html = '
  • todo
    • ' + .'
  • done
'; + + expect(HtmlSanitizer::sanitize($html))->toBe($html); +}); + +it('keeps http and https links unchanged', function (): void { + expect(HtmlSanitizer::sanitize('x')) + ->toBe('x') + ->and(HtmlSanitizer::sanitize('x')) + ->toBe('x'); +}); + +it('keeps relative links unchanged', function (): void { + expect(HtmlSanitizer::sanitize('x')) + ->toBe('x'); +}); + +it('keeps images unchanged', function (): void { + expect(HtmlSanitizer::sanitize('a')) + ->toBe('a'); +}); + +it('returns an empty string for empty or whitespace-only input', function (): void { + expect(HtmlSanitizer::sanitize(''))->toBe('') + ->and(HtmlSanitizer::sanitize(null))->toBe('') + ->and(HtmlSanitizer::sanitize(' '))->toBe(''); +}); From 8b490aef0955c743f9fd48b28ffbb4bcf141acac Mon Sep 17 00:00:00 2001 From: Luan Trindade Date: Sun, 20 Sep 2026 10:23:12 -0300 Subject: [PATCH 3/3] docs: record ADR-26 and close F-009 in the brain Mark F-009 as mitigated, add ADR-26 for the two-layer server-side HTML sanitization, update canonical state and backlog, document the discovery cache invalidation deploy step, and leave a handoff with the audit evidence. Co-Authored-By: Claude Opus 5 --- brain/audits/2026-06-13-system-audit.md | 2 ++ brain/canonico/CURRENT_STATE.md | 13 +++++++ brain/canonico/NEXT_ACTIONS.md | 3 +- ...026-09-20-server-side-html-sanitization.md | 34 +++++++++++++++++++ docs/DECISIONS.md | 16 +++++++++ docs/HUMAN-ACTIONS.md | 12 +++++++ 6 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 brain/handoffs/2026-09-20-server-side-html-sanitization.md diff --git a/brain/audits/2026-06-13-system-audit.md b/brain/audits/2026-06-13-system-audit.md index 76d85d1..a3b34cd 100644 --- a/brain/audits/2026-06-13-system-audit.md +++ b/brain/audits/2026-06-13-system-audit.md @@ -242,6 +242,8 @@ Fix before public deployment. Severity: Medium +Status: Mitigated in `fix/server-side-html-sanitization`. + Evidence: - Backend stores and returns `content_html` through `DocumentResource`. diff --git a/brain/canonico/CURRENT_STATE.md b/brain/canonico/CURRENT_STATE.md index 107cc5a..1f89115 100644 --- a/brain/canonico/CURRENT_STATE.md +++ b/brain/canonico/CURRENT_STATE.md @@ -24,6 +24,7 @@ The product is positioned as a documentation discovery platform for plugin ecosy - Angular SSR shell with lazy standalone routes, Signals-based UI state, Transloco i18n, command palette, protected routes, and reusable UI components. - Browser/SSR runtime configuration separates internal SSR API calls from public browser API, Reverb, allowed hosts, and canonical public origin. - Markdown viewer sanitization and heading/code enhancement. +- Server-side HTML sanitization of `content_html` in two layers: allowlist sanitizer applied at ingestion and again through a `Document` accessor on every read, so stored documents are served sanitized without rewriting the column. - SEO metadata, canonical URLs, Open Graph, JSON-LD, robots, sitemap, and portfolio screenshots, with SSR post-processing normalizing public origin metadata. - Development and production-like Docker Compose stacks. @@ -58,6 +59,7 @@ Important implementation anchors: - `App\Services\Ingestion\IngestionService` - `App\Services\Markdown\MarkdownParser` +- `App\Services\Markdown\HtmlSanitizer` - `App\Services\Discovery\DiscoveryAccess` - `App\Services\Discovery\DiscoveryCache` - `App\Contracts\GitHubClient` @@ -161,3 +163,14 @@ Latest analytics hardening phase: - `/api/v1/analytics/most-viewed` now validates `days` as integer `1..COMMANDSPHERE_ANALYTICS_MAX_DAYS`. - Analytics response metadata now includes `days` and `max_days`. - Local tests isolate command-view dedupe from Meilisearch when search indexing is not the behavior under test. + +Active sanitization remediation branch: `fix/server-side-html-sanitization`. + +Latest sanitization hardening phase: + +- F-009 server-side sanitization boundary (see ADR-26). +- Markdown conversion now runs with `html_input=escape` and `allow_unsafe_links=false`. +- `HtmlSanitizer` applies a tag/attribute allowlist over `DOMDocument`; `href`/`src` accept only `http`, `https`, `mailto`, and relative paths, with control bytes stripped before the scheme check. +- A `Document` accessor sanitizes `content_html` on every read, so rows stored before the fix are served sanitized without any data migration. +- Angular `MarkdownRendererService` is untouched and remains defense in depth. +- Gates: Pest `VALIDATED` (55 passed), Pint `VALIDATED` (118 files). `composer audit` reports 22 pre-existing advisories in 4 packages, and Jest has 2 pre-existing failures in `runtime-config.spec.ts`; both sets are identical to the `a3a8299` baseline and unrelated to this change. diff --git a/brain/canonico/NEXT_ACTIONS.md b/brain/canonico/NEXT_ACTIONS.md index 0b873a4..19a046f 100644 --- a/brain/canonico/NEXT_ACTIONS.md +++ b/brain/canonico/NEXT_ACTIONS.md @@ -249,7 +249,8 @@ Risks: ## Backlog - Add Content Security Policy on Laravel and SSR Node responses. -- Sanitize Markdown HTML server-side before storage or response. +- Update `league/commonmark`, `guzzlehttp/guzzle`, `guzzlehttp/psr7`, and `phpseclib/phpseclib` to clear the 22 advisories reported by `composer audit`, including CVE-2026-71478 in commonmark 2.8.2. ADR-26 already neutralizes that link-filter bypass class independently, so this is dependency hygiene, not an open XSS hole. +- Fix the two `runtime-config.spec.ts` Jest failures caused by Docker Compose environment variables leaking into the test `process.env`. - Harden GitHub client HTTP error taxonomy beyond 403/404. - Extract optional bearer-token user resolution shared by public discovery controllers. - Track Angular toolchain advisories from `npm audit`. diff --git a/brain/handoffs/2026-09-20-server-side-html-sanitization.md b/brain/handoffs/2026-09-20-server-side-html-sanitization.md new file mode 100644 index 0000000..6c72f25 --- /dev/null +++ b/brain/handoffs/2026-09-20-server-side-html-sanitization.md @@ -0,0 +1,34 @@ +# Handoff - Server-Side HTML Sanitization + +Date: 2026-09-20 +Branch: `fix/server-side-html-sanitization` + +## Scope + +Audit finding F-009: `content_html` derived from third-party Markdown was stored and returned without server-side sanitization, with the Angular renderer as the only barrier. + +## Implemented + +- `App\Services\Markdown\HtmlSanitizer`: tag/attribute allowlist over `DOMDocument`, no new dependency. `href`/`src` accept only `http`, `https`, `mailto`, and relative paths; control bytes are stripped before the scheme check; entity decoding and case folding are handled by the DOM layer. +- `MarkdownParser` converts with `html_input=escape` and `allow_unsafe_links=false`, then sanitizes before persistence. +- `Document` exposes a `contentHtml()` accessor that sanitizes on every read without rewriting the stored column, which is what covers rows ingested before the fix. +- Tests: `tests/Unit/Services/Markdown/HtmlSanitizerTest.php` (19 malicious cases plus 8 legitimate ones) and `tests/Feature/MarkdownSanitizationTest.php` (full parse pipeline, and a legacy row written straight to the database served sanitized by the API). +- See ADR-26 in `docs/DECISIONS.md`. + +## Validation + +Adversarial audit ran against an isolated `commandsphere_audit` database, which was dropped afterwards; the development database received no migrate or reset. + +- 19 payloads ingested through the real pipeline and read back through `GET /api/v1/documents/{id}`: 0 surviving executable tags, `on*` handlers, or `javascript:`/`data:` schemes. +- Second route `GET /api/v1/plugins/{slug}/versions/{version}/documents` checked against the "sanitized in one route only" failure mode: 38 documents, 0 failures. +- Legacy rows inserted through the query builder: stored column stays malicious, API output is sanitized. +- Legitimate rendering: 18/18 structure checks present, code blocks escaped rather than live tags. +- Pest 55 passed; Pint PASS on 118 files; no `skip`/`only`/`todo` in `backend/tests` or `frontend/src`. +- `composer audit` 22 advisories in 4 packages and 2 Jest failures in `runtime-config.spec.ts` are byte-identical to the `a3a8299` baseline; `composer.json`, `composer.lock`, and `frontend/` are unchanged by this branch. + +## Remaining Work + +- Deploy step, not a code defect: `DiscoveryCache` holds resolved payloads for 300 seconds, so entries cached before the deploy keep serving unsanitized HTML until they expire. Invalidate the cache on deploy, per `docs/HUMAN-ACTIONS.md`. +- Dependency advisories (`league/commonmark` 2.8.2 including CVE-2026-71478, `guzzlehttp/guzzle`, `guzzlehttp/psr7`, `phpseclib/phpseclib`) are tracked in `NEXT_ACTIONS.md`. The sanitizer neutralizes that bypass class on its own, so this is hygiene rather than an open hole. +- The `runtime-config.spec.ts` Jest failures come from Compose environment variables leaking into the test `process.env` and are unrelated to Markdown. +- Content Security Policy remains open under its own audit finding and would be the next layer above this one. diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 0ff2a4f..7e4e13b 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -290,3 +290,19 @@ Adicionar Dockerfiles multi-stage para backend PHP-FPM e frontend Node SSR, alé ### Consequências A stack pode ser validada localmente com `docker compose -f docker-compose.prod.yml up -d --build`, preservando separação entre runtime PHP-FPM, SSR Node e serviços de dados. O trade-off é manter dois caminhos Docker: dev com DX/testes e prod com empacotamento mais fiel ao deploy. + +## ADR-26 — Sanitização de HTML no servidor em duas camadas + +### Contexto +`content_html` é derivado de Markdown de terceiros vindo do GitHub. Até então a conversão usava `GithubFlavoredMarkdownConverter` sem configuração, o que mantém os padrões `html_input=allow` e links inseguros permitidos, e a sanitização existia apenas no Angular (`MarkdownRendererService`, ADR-18). Qualquer outro consumidor da API — export, integração, client alternativo, view administrativa — recebia HTML executável. Documentos ingeridos antes da correção já estavam gravados com esse HTML. + +### Decisão +Sanitizar no backend em duas camadas, com um allowlist próprio de tags e atributos sobre `DOMDocument` (`App\Services\Markdown\HtmlSanitizer`), sem dependência nova: + +1. Na ingestão, o conversor passa a usar `html_input=escape` e `allow_unsafe_links=false`, e a saída é sanitizada antes de persistir. +2. Na leitura, um accessor no model `Document` sanitiza `content_html` a cada acesso, sem reescrever a coluna. + +`href`/`src` aceitam apenas `http`, `https`, `mailto` e caminhos relativos, com remoção de bytes de controle antes da checagem de esquema. A sanitização do Angular permanece como defesa em profundidade. + +### Consequências +Documentos antigos ficam cobertos sem migração nem reprocessamento: a coluna armazenada permanece como está e a sanitização acontece na saída, o que satisfaz a lei DATA sem escrita em dado existente. A proteção não depende do filtro interno do `league/commonmark`, o que neutraliza a classe de bypass por bytes de controle do `CVE-2026-71478` (versão instalada 2.8.2) independentemente da atualização daquele pacote. O custo é sanitizar a cada leitura; a operação é idempotente e o resultado de discovery já é cacheado por `DiscoveryCache`. Entradas cacheadas antes do deploy continuam cruas até expirar (300s), então o deploy desta mudança exige invalidar o cache de discovery. diff --git a/docs/HUMAN-ACTIONS.md b/docs/HUMAN-ACTIONS.md index e642ee9..714a60f 100644 --- a/docs/HUMAN-ACTIONS.md +++ b/docs/HUMAN-ACTIONS.md @@ -11,3 +11,15 @@ DISCORD_REDIRECT_URI="${APP_URL}/api/v1/auth/discord/callback" ``` For local development and CI, `/api/v1/auth/dev-login` remains available only when `APP_ENV` is `local` or `testing`, so tests and backend work do not depend on external Discord credentials. + +## Invalidate the discovery cache when deploying ADR-26 + +`App\Services\Discovery\DiscoveryCache` stores already-resolved discovery payloads for 300 seconds, and `CatalogController` serves `document` and `versionDocuments` through it. Entries cached before the server-side sanitization deploy keep returning unsanitized `content_html` until they expire. + +After deploying the sanitization change, invalidate the discovery cache instead of waiting out the window: + +```bash +docker compose exec -T backend php artisan tinker --execute="app(App\Services\Discovery\DiscoveryCache::class)->invalidate();" +``` + +This is a one-time deploy step. New responses are sanitized on read, so the cache repopulates with sanitized payloads.