Skip to content

fix(web): F14 — close 5-expert audit findings (7 HIGH + 7 MEDIUM/LOW)#41

Merged
explosivebit merged 1 commit into
developfrom
fix/post-audit-f14
May 6, 2026
Merged

fix(web): F14 — close 5-expert audit findings (7 HIGH + 7 MEDIUM/LOW)#41
explosivebit merged 1 commit into
developfrom
fix/post-audit-f14

Conversation

@explosivebit
Copy link
Copy Markdown
Contributor

Summary

5-expert audit (TS / Frontend / Security / Performance / UX) on F11+F12+F13 returned 0 CRITICAL, 7 HIGH, ~12 MEDIUM, ~10 LOW. This PR closes all 7 HIGH + 7 MEDIUM/LOW findings.

HIGH

ID Finding Fix
TS-H1 renderBody return type was plain string — caller could feed any string to {@html} Branded SafeHtml = string & { readonly __safeHtml: unique symbol }
TS-H2 marked.parse(...) as string cast valid only with async: false — silent breakage if option removed // TODO(marked-types) comment + revisit on marked v19
FE-H1 Matrix impact-mode never faded — CSS selector targeted .node only; Matrix has .row-header / .col-header Selector extended via :is(.node, .row-header, .col-header)
FE-H2 Notification.permission access throws in sandboxed iframes / restrictive Safari → toggle effect crashed try/catch in notificationPermission, requestPermission, fire
UX-H1 Long code blocks pushed horizontal scrollbar to entire panel .artifact-body pre / pre code get max-width: 100%; white-space: pre-wrap; word-break: break-word
PERF-H1 marked + DOMPurify (~21 KB gzip) shipped on first paint despite body collapsed by default Dynamic import inside bodyExpanded branch — first-paint bundle drops by 21 KB gzip

MEDIUM / LOW

  • FE-M2 — clipboard fallback (textarea + execCommand('copy')) for http:// non-secure contexts.
  • FE-M3liveText prefixed with zero-width-space + liveSeq counter so screen readers re-announce identical breach text.
  • FE-M4 — Notify-on first-fire-storm: prime prevHealthSnapshot when user flips notify on mid-session.
  • FE-L1prefers-reduced-motion query extended from svg * to *, *::before, *::after.
  • UX-M1 — tooltips on Show downstream / upstream buttons.
  • UX-M3.links ul { max-height: 30vh; overflow-y: auto }.
  • SEC-L1 — DOMPurify afterSanitizeAttributes hook forces rel="noopener noreferrer" on target="_blank" anchors (CWE-1022 guard).

Backlog (deferred)

  • UX-H2 mermaid latent (no current PRD/RFC uses mermaid; ADR for diagram renderer when needed).
  • TS-M3..M8, L9..L12 (cosmetic).
  • SEC-L2 redaction of secret patterns in body excerpt (needs thoughtful regex set — separate artifact).
  • SEC-L3 kindFilter validation (low impact).
  • FE-L2..L4 (rare edge cases).

Verify

  • npx svelte-check — 0/0/436.
  • npm test — 70/70 (68 baseline + 2 new tests for noopener hook + iframe-throw).
  • node scripts/smoke.mjs — PASS.
  • Playwright DOM check — 0 console errors.

Test plan

  • CI smoke matrix (3-OS × Node 22) green.
  • svelte-check clean on PR.
  • vitest 70/70.
  • Manual: open ArtifactPanel for an Epic with many children — link list scrolls within max-height: 30vh.
  • Manual: clear all filters in Matrix view with impact-mode active — non-impacted headers visibly fade.
  • Manual: open in iframe — Notify toggle does not crash the page.

Refs: PRD-006 RFC-005 PRD-007 RFC-006

🤖 Generated with Claude Code

5-expert audit (TS / Frontend / Security / Performance / UX) on
F11+F12+F13 returned 0 CRITICAL, 7 HIGH, ~12 MEDIUM, ~10 LOW. This
PR closes all 7 HIGH + 7 MEDIUM/LOW.

HIGH:
- TS-H1: renderBody returns branded SafeHtml type (string &
  { readonly __safeHtml: unique symbol }). Untrusted strings cannot
  reach {@html} via this signature.
- TS-H2: TODO(marked-types) comment near `as string` cast.
- FE-H1: Matrix impact-mode selector extended to include row-header
  and col-header (was only .node — Matrix never faded).
- FE-H2: try/catch around Notification.permission and
  Notification.requestPermission and new Notification ctor.
  Sandboxed iframes no longer throw on access.
- UX-H1: artifact-body pre / pre code get max-width: 100%,
  white-space: pre-wrap, word-break: break-word. Long shell lines
  wrap instead of cascading horizontal scroll to panel.
- PERF-H1: marked + DOMPurify dynamic-imported only when user clicks
  "+ Show body". First-paint −21 KB gzip.

MEDIUM/LOW:
- FE-M2: clipboard fallback for non-secure contexts (textarea +
  document.execCommand('copy')).
- FE-M3: liveText prefixed with zero-width-space + liveSeq counter
  so identical breach text re-announces in screen readers.
- FE-M4: notify-on first-fire-storm prevention — prime
  prevHealthSnapshot when user flips notify on mid-session.
- FE-L1: prefers-reduced-motion media query covers all *, not just
  svg *.
- UX-M1: tooltips on Show downstream / upstream buttons.
- UX-M3: .links ul capped at max-height: 30vh + overflow-y: auto.
- SEC-L1: DOMPurify afterSanitizeAttributes hook adds
  rel="noopener noreferrer" to target="_blank" anchors (CWE-1022).

Tests: 68 → 70.
- markdown-renderer.test.ts: target=_blank gets rel=noopener.
- notify.test.ts: iframe-throw fallback returns 'unsupported'.

Verify:
- svelte-check 0/0/436.
- npm test 70/70.
- npm run smoke PASS.
- Playwright DOM check: 0 console errors.

Bundle: 64 KB raw / 21 KB gzip of marked + DOMPurify moved from
first-paint chunk to lazy chunk loaded on body-toggle click.

Refs: PRD-006 RFC-005 PRD-007 RFC-006
@explosivebit explosivebit merged commit 3347af6 into develop May 6, 2026
3 checks passed
@explosivebit explosivebit deleted the fix/post-audit-f14 branch May 6, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant