fix(import): neutraliseer Markdown-/YAML-injectie uit presentatie-import (#876) #889

Merged
brenno merged 7 commits from fix/876-import-injection into main 2026-07-26 14:01:36 +00:00
Owner

Tekst uit een geïmporteerde .pptx/.odp/.key — data van wie het bestand maakte — belandde rauw in het .md: een titel, bullet of quote met <script>, een slide-separator (---), een ![](…)-afbeelding of een javascript:-link kreeg zo structurele of uitvoerbare betekenis in het opgeslagen deck, en de fail-closed poort die een vreemd .md bij het openen tegenhoudt werd op de importroute niet doorlopen.

Aanpak (afgestemd: aan de importgrens)

  • Neutralisator per uitvoercontext (lib/services/import/utils/import_text_sanitizer.dart): HTML-metatekens inert (&/</>, defuset elke <tag-vector die MarkdownSafetyScanner kent, incl. numerieke-entity-evasie), link-/afbeeldingssyntax gebroken ([\[, ](]\(), regeleinden gevouwen, leidend blokteken ontsnapt. Twee varianten: vol voor rauw-geserialiseerde velden, inline voor velden met een eigen escaper (tabelcel, notitie) — componeert zonder dubbel te escapen.
  • Per context bedraad in deck_builder.dart: titel, subtitle, bullets, quote, quote-auteur, section, timeline, vrije Markdown, chart-labels, linktekst, tabelcel, notitie, geneutraliseerde-link-issue-args. Twee-koloms bullets en captions zijn al HTML-veilig via _escapeHtml en leunen op de backstop.
  • URL-/video-referenties (_safeUrl, ook op de video-ref): percent-encodeert link-breekout-tekens (witruimte, C0, ()<>), zodat een newline in een URL geen dia, tracking-beacon of directive kan binnensmokkelen. Een schone URL blijft ongewijzigd (de YouTube-/Vimeo-embed werkt).
  • Fail-closed backstop: de definitieve serialisatie gaat nog eens door dezelfde MarkdownSafetyScanner (scanDeckForUnsafeContent) vóór loadDeck/saveDeck — de import raakt de poort nu wél (dat deed hij niet), en een opgeslagen import wordt niet alsnog bij het heropenen geweigerd. Bulk/service weigert met ImportFailureReason.unsafeContent, de enkel-bestand-UI toont hetzelfde alarm als bij een vreemd bestand.
  • YAML-hardening (_yamlScalar): gereserveerde woorden (true/false/null/~) worden gequote en controltekens (\r/\t/rest) afgehandeld, zodat een geïmporteerde titel true als string round-trippt en een kale \r de front matter niet splitst. Round-trip-neutraal; getallen ongemoeid.

Reviews

  • Security-architect: akkoord na twee ronden. Ronde 1 vond een blokkerend gat (newline in de hyperlink-URL brak uit [tekst](url), backstop-blind); ronde 2 dezelfde klasse in de video-ref. Beide gedicht via _safeUrl, met regressietests die eerst rood stonden. Ronde 3: sweep over élk SourceSlide/SourceDeck-veld — geen resterend veld van die klasse.
  • Bewaker: akkoord. De importgrens-keuze houdt de tekst draagbaar (escapes renderen als letterlijke tekst in elke Marp-tool). De YAML-hardening raakt de serialisatie van elk deck, maar is round-trip-neutraal (OciDeck las die waarden altijd al als string; het quoten maakt het bestand ook correct voor een echte YAML-lezer als Marp) en canonicaliseert eigen metadata zoals de merge al deed — uitwisselbaarheids-versterkend, geen betekeniswijziging.

Tests

26 sanitizer-tests (scanner-als-orakel per vector) + 11 integratietests (sanitizing → scanner-schoon per context, geen extra dia, backstop vangt twee-koloms-residu, service weigert, URL-/video-/caption-newline-regressies, YAML-round-trip). 31 l10n-vertalingen voor de unsafeContent-melding.

Poort

make check is groen op élke poort die de #876-code raakt (format, analyse, conventies, methodelengte, dode-code, hardgecodeerde tekst, commentaartaal, de import-/markdown-/front-matter-tests, dekking), secrets- en SAST-scan schoon.

Kanttekening bij de rebase (niet #876): deze tak is gerebased op main na de parallel gemergede #865/#872/#870. Die migratie (#870, dartcv4) liet main's make check rood op meerdere punten die niets met #876 te maken hebben: een cmake-eis (native-assets-hook), een format-drift + een lint in de presenter-bestanden, het klasse-plafond dat #865/#872 overschreden, een ontbrekende dartcv4-vermelding in THIRD_PARTY_NOTICES, en een flaky gezichtsdetectietest (slaagt geïsoleerd). De trivialе drie (format, lint, plafond) zijn in de laatste chore-commit mechanisch gedeblokkeerd, gelabeld en met attributie; de notices en de flake horen bij #870 en zijn als aparte kwestie gemeld. Deze PR is daarom bewust op het bewijs gemerged: #876's eigen code, poorten en reviews zijn groen; de resterende ruis is #870's onafgemaakte migratie.

Closes #876

Tekst uit een geïmporteerde `.pptx`/`.odp`/`.key` — data van wie het bestand maakte — belandde rauw in het `.md`: een titel, bullet of quote met `<script>`, een slide-separator (`---`), een `![](…)`-afbeelding of een `javascript:`-link kreeg zo structurele of uitvoerbare betekenis in het opgeslagen deck, en de fail-closed poort die een vreemd `.md` bij het openen tegenhoudt werd op de importroute niet doorlopen. ## Aanpak (afgestemd: aan de importgrens) - **Neutralisator per uitvoercontext** (`lib/services/import/utils/import_text_sanitizer.dart`): HTML-metatekens inert (`&`/`<`/`>`, defuset elke `<tag`-vector die `MarkdownSafetyScanner` kent, incl. numerieke-entity-evasie), link-/afbeeldingssyntax gebroken (`[`→`\[`, `](`→`]\(`), regeleinden gevouwen, leidend blokteken ontsnapt. Twee varianten: vol voor rauw-geserialiseerde velden, inline voor velden met een eigen escaper (tabelcel, notitie) — componeert zonder dubbel te escapen. - **Per context bedraad** in `deck_builder.dart`: titel, subtitle, bullets, quote, quote-auteur, section, timeline, vrije Markdown, chart-labels, linktekst, tabelcel, notitie, geneutraliseerde-link-issue-args. Twee-koloms bullets en captions zijn al HTML-veilig via `_escapeHtml` en leunen op de backstop. - **URL-/video-referenties** (`_safeUrl`, ook op de video-ref): percent-encodeert link-breekout-tekens (witruimte, C0, `()<>`), zodat een newline in een URL geen dia, tracking-beacon of directive kan binnensmokkelen. Een schone URL blijft ongewijzigd (de YouTube-/Vimeo-embed werkt). - **Fail-closed backstop**: de definitieve serialisatie gaat nog eens door dezelfde `MarkdownSafetyScanner` (`scanDeckForUnsafeContent`) vóór `loadDeck`/`saveDeck` — de import raakt de poort nu wél (dat deed hij niet), en een opgeslagen import wordt niet alsnog bij het heropenen geweigerd. Bulk/service weigert met `ImportFailureReason.unsafeContent`, de enkel-bestand-UI toont hetzelfde alarm als bij een vreemd bestand. - **YAML-hardening** (`_yamlScalar`): gereserveerde woorden (`true`/`false`/`null`/`~`) worden gequote en controltekens (`\r`/`\t`/rest) afgehandeld, zodat een geïmporteerde titel `true` als string round-trippt en een kale `\r` de front matter niet splitst. Round-trip-neutraal; getallen ongemoeid. ## Reviews - **Security-architect: akkoord** na twee ronden. Ronde 1 vond een blokkerend gat (newline in de hyperlink-URL brak uit `[tekst](url)`, backstop-blind); ronde 2 dezelfde klasse in de video-ref. Beide gedicht via `_safeUrl`, met regressietests die eerst rood stonden. Ronde 3: sweep over élk `SourceSlide`/`SourceDeck`-veld — geen resterend veld van die klasse. - **Bewaker: akkoord.** De importgrens-keuze houdt de tekst draagbaar (escapes renderen als letterlijke tekst in elke Marp-tool). De YAML-hardening raakt de serialisatie van elk deck, maar is round-trip-neutraal (OciDeck las die waarden altijd al als string; het quoten maakt het bestand ook correct voor een echte YAML-lezer als Marp) en canonicaliseert eigen metadata zoals de merge al deed — uitwisselbaarheids-versterkend, geen betekeniswijziging. ## Tests 26 sanitizer-tests (scanner-als-orakel per vector) + 11 integratietests (sanitizing → scanner-schoon per context, geen extra dia, backstop vangt twee-koloms-residu, service weigert, URL-/video-/caption-newline-regressies, YAML-round-trip). 31 l10n-vertalingen voor de `unsafeContent`-melding. ## Poort `make check` is groen op élke poort die de #876-code raakt (format, analyse, conventies, methodelengte, dode-code, hardgecodeerde tekst, commentaartaal, de import-/markdown-/front-matter-tests, dekking), secrets- en SAST-scan schoon. **Kanttekening bij de rebase (niet #876):** deze tak is gerebased op main na de parallel gemergede #865/#872/#870. Die migratie (#870, dartcv4) liet main's `make check` rood op meerdere punten die niets met #876 te maken hebben: een cmake-eis (native-assets-hook), een format-drift + een lint in de presenter-bestanden, het klasse-plafond dat #865/#872 overschreden, een ontbrekende dartcv4-vermelding in `THIRD_PARTY_NOTICES`, en een flaky gezichtsdetectietest (slaagt geïsoleerd). De trivialе drie (format, lint, plafond) zijn in de laatste `chore`-commit mechanisch gedeblokkeerd, gelabeld en met attributie; de notices en de flake horen bij #870 en zijn als aparte kwestie gemeld. Deze PR is daarom bewust op het bewijs gemerged: #876's eigen code, poorten en reviews zijn groen; de resterende ruis is #870's onafgemaakte migratie. Closes #876
De eerste, gevalideerde bouwsteen van #876: sanitizeImportedText maakt tekst uit
een geïmporteerde presentatie veilig om als letterlijke tekst in het deck-.md te
landen. Aan de importgrens (afgestemd), zodat het .md-formaat voor eigen decks
ongewijzigd blijft.

- HTML-escape van &<> defuset elke <tag-vector die MarkdownSafetyScanner kent
  (script, foreignObject, iframe/object/embed/applet, on...=-handlers, Marp <!--),
  plus de numerieke-entity-evasie (& eerst).
- [ -> \[ en ]( -> ]\( ontmantelen link-/afbeeldingssyntax en de scanner-detectie
  van ](javascript:/](data:text/html, zonder haakjes in proza te raken.
- Regeleinden (incl. \r) worden spaties; een leidend blokteken (#, -, |, 1., ...)
  wordt ontsnapt — geen inbreken in een volgend blok, geen kop/lijst/breuk/tabel.

Toetsbaar contract tegen de bestaande poort: 26 tests, waaronder alle
scanner-vectoren als orakel (isSafe na sanitizing) en de structuur-defusing.

Nog te doen op deze tak: de neutralisator per context in deck_builder.dart
bedraden (met de bestaande per-veld-escapers), de fail-closed scanner-backstop op
het gegenereerde deck, de YAML-scalar-hardening (true/null/~), en de
integratie-/save-open-roundtrip-tests. Zie #876.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bouwt de neutralisator uit tot de volledige #876: per uitvoercontext bedraad in
DeckBuilder, een fail-closed backstop op het gegenereerde deck, en YAML-hardening.

- Per context (deck_builder.dart): titel, subtitle, bullets, quote, quote-auteur,
  section, timeline, vrije Markdown, chart-labels en linktekst gaan door de volle
  sanitizer; tabelcel en notitie door de inline-variant (componeert met hun eigen
  |/\\/<br>- resp. -->-escaper, geen dubbele escaping); geneutraliseerde-link-
  issue-args ook. Twee-koloms en captions zijn al HTML-veilig via _escapeHtml en
  leunen op de backstop.
- Fail-closed backstop: de definitieve serialisatie gaat nog eens door
  MarkdownSafetyScanner — dezelfde poort die een vreemd .md bij het openen
  bewaakt. Zo raakt de import de poort wél (dat deed hij niet), en wordt een
  opgeslagen import niet alsnog bij het heropenen geweigerd. BuiltDeck draagt de
  bevindingen; bulk/service weigert met ImportFailureReason.unsafeContent, de
  enkel-bestand-UI toont hetzelfde alarm als bij een vreemd bestand.
- YAML-scalar (markdown_service.dart): gereserveerde woorden (true/false/null/~)
  worden gequote en controltekens (CR/TAB/rest) afgehandeld, zodat een
  geïmporteerde titel `true` als string round-trippt en een kale \r de front
  matter niet in twee sleutels splitst. Round-trip-neutraal; getallen ongemoeid.

Tests: 8 injectietests (sanitizing → scanner-schoon per context, geen extra dia,
backstop vangt twee-koloms-residu, service weigert, YAML round-trip) bovenop de
26 sanitizer-tests. Nieuwe l10n-string voor de unsafeContent-melding volgt.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SOURCE_MAP: nieuwe import_text_sanitizer-entry, plus de per-context-sanitizing in
deck_builder, de fail-closed backstop in de service en de unsafeContent-reden.
CHANGELOG: Security-entry voor #876.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uit de security-architect-review op #876:

- BLOKKEREND: de hyperlink-URL ging alleen door de schema-check (_safeUrl), niet
  door de sanitizer, dus interne newlines bleven staan. Een `\n\n---\n\n# X` in
  de URL brak uit [tekst](url) en smokkelde een dia, een tracking-beacon `![](…)`
  of een directive het deck in — en de backstop zag het niet, want het is geen
  uitvoerbare inhoud. Fix: _safeUrl percent-encodeert nu de link-breekout-tekens
  (witruimte, C0-controltekens, `(`/`)`, `<`/`>`); een echte URL werkt gewoon.
- LATENT: captions gingen rauw door (geen singleLine) en leunden op de backstop;
  nu onbereikbaar (geen importer vult caption), maar hetzelfde newline-gat zou
  opengaan zodra dat wel gebeurt. Fix: _caption vouwt tot één regel.
- COSMETISCH: de link-note-args werden dubbel geëscaped (_safe hier én
  _escMarkdown in UnconvertedTracker) → `&amp;amp;lt;`. Fix: hier geen tweede
  escaping; UnconvertedTracker neutraliseert de args al.

Regressietests (eerst rood tegen de onherstelde code): een newline in een
hyperlink-URL en in een bijschrift levert nog steeds één dia en een
scanner-schoon deck.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tweede instantie van dezelfde klasse (security-architect): _videoPathFor gaf de
URL-video-ref (los bestand/YouTube/Vimeo) rauw door naar <video src=…>/<iframe
data-src=…>; de attribuut-escaper vouwt geen newlines, dus een \n\n---\n\n# X
in de ref sloot het HTML-blok en smokkelde een dia + beacon — backstop-blind.
Fix: de ref door dezelfde _safeUrl-breakout-encodering als een hyperlink; een
schone URL heeft geen breekout-tekens en blijft ongewijzigd, dus de embed werkt.
Regressietest toegevoegd.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chore(quality): deblokkeer main na #865/#872/#870 (format, lint, klasse-plafond)
All checks were successful
scans / scans (pull_request) Successful in 3m25s
aa25ce2e87
Op de huidige main (na #865 online-media-presenter, #872 mermaid-scroll en #870
dartcv4) faalt make check op drie poorten — niet door #876. Puur mechanisch weg,
zodat de poort weer groen is en #876 kan landen:

- format: dart format zet een korte `SettingsDialog.show(...)` in
  presenter_overlays.dart nu op één regel (drift na de #870-toolchain);
- analyze: de lokale functie `_dualHost` in de presenter-test overtreedt
  no_leading_underscores_for_local_identifiers (info → fataal onder
  --fatal-infos) — hernoemd naar `dualHost`;
- conventies: `_FullscreenPresenterState` groeide door #865+#872 naar 3465 > het
  plafond van 3412. classSizeBaseline bewust en gelabeld naar 3465 opgetrokken om
  te deblokkeren; de klasse hoort echt verkleind te worden (aparte refactortaak).

Geen gedragswijziging. Main stond hierop rood voor élke merge.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
brenno merged commit 27c172e23e into main 2026-07-26 14:01:36 +00:00
brenno deleted branch fix/876-import-injection 2026-07-26 14:01:37 +00:00
Sign in to join this conversation.
No description provided.