Skip to content

fix(security): scheme-check custom htmlAttributes + deny dangerous custom tags#155

Merged
DavidBabinec merged 1 commit into
mainfrom
fix/htmlattributes-xss
Jul 3, 2026
Merged

fix(security): scheme-check custom htmlAttributes + deny dangerous custom tags#155
DavidBabinec merged 1 commit into
mainfrom
fix/htmlattributes-xss

Conversation

@DavidBabinec

@DavidBabinec DavidBabinec commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

What & why

Custom htmlAttributes on base.link/button/image/container/text nodes were name-filtered only (event handlers, class, style, reserved data-*). href, src, srcdoc, formaction, xlink:href, … were permitted and their values never scheme-checked; the custom-tag escape hatch also accepted iframe, base, object, script, …

A low-privilege editor holding site.content.edit (including the Client role) — or a merely-imported HTML file — could store:

  • href="javascript:…" (shadowing a module's own checked href — first duplicate href wins), or
  • an <iframe srcdoc="<script>…"> (zero-click).

Because htmlAttributes is an object it bypassed escapeProps/sanitizeNodeProps, so the payload survived persistence and publish. It executed for published-page visitors and — critically — inside the admin editor canvas, which renders these trusted modules same-origin as /admin with no script-src CSP. When a higher-privileged Owner/Admin opened a page authored by a low-trust Client, the script ran in the admin origin → session/credential theft and privilege escalation.

Fix (single chokepoint + tag denylist)

All four emit/collect paths already funnel through one name gate; I extended that into a value-aware gate and applied it everywhere:

  • sanitizeRenderableHtmlAttribute(name, value) in @core/htmlAttributes — rejects non-renderable names, the raw-HTML sink srcdoc, and any value carrying a dangerous URL scheme (javascript:/vbscript:/data:) via the existing isSafeUrl. Value-level scheme checking covers every URL attribute without enumerating them; ordinary text (titles, ARIA labels, viewBox) passes unchanged.
  • Routed the shared publisher/canvas normaliser (normalizeHtmlAttributes), the <body> emit (bodyHtmlAttributes), and the HTML-import harvest (collectHtmlAttributes) through it.
  • resolveHtmlTag custom-tag escape hatch now denies dangerous elements (script, iframe, frame, frameset, object, embed, applet, base, link, meta, style) → coerced to div.

Not affected: base.video emits its own trusted <iframe> via its module htmlTag (returns 'iframe' directly), not resolveHtmlTag, so video/YouTube embeds keep working.

Impact

  • Developer: one shared, value-aware attribute gate instead of a name-only check duplicated across four sites.
  • User: closes a low-privilege → admin-canvas stored-XSS / account-takeover path. Legitimate attributes, safe URLs, and benign custom tags are unchanged.

Complementary follow-up (not in this PR): ship the admin script-src CSP that securityHeaders.ts currently defers — defense-in-depth behind this fix.

Verification

  • bun test src/__tests__/security/htmlAttributesXss.test.ts — 17 pass (srcdoc, javascript:/data:/vbscript: values, and dangerous custom tags dropped; safe attrs/URLs/tags preserved; publisher emit verified end-to-end).
  • bun test across publisher, htmlImport, base-modules, siteImport, htmlAttributes panels — 916 pass, 0 fail.
  • bunx tsc -b clean; eslint on touched files clean; no-core-barrel-deep-imports gate passes (new @core/htmlAttributes → @core/html-sanitize leaf import is allowed).
  • Pre-existing unrelated failures (icon-catalog-integrity) confirmed failing on the untouched base; not part of this change.

🤖 Generated with Claude Code

@DavidBabinec @claude
fix(security): scheme-check custom htmlAttributes + deny dangerous cu…
4cf03d6 
…stom tags

Custom `htmlAttributes` on base.link/button/image/container/text nodes were
name-filtered only (on*/class/style/reserved). `href`, `src`, `srcdoc`,
`formaction`, `xlink:href`, … were permitted and their VALUES never
scheme-checked, and the custom-tag escape hatch accepted `iframe`, `base`,
`object`, `script`, … A low-privilege editor (`site.content.edit`, incl. the
Client role) — or a merely-imported HTML file — could store:

  - `href="javascript:…"` shadowing a module's own checked href, or
  - an `<iframe srcdoc="<script>…">`,

which executed for published-page visitors AND, critically, inside the admin
editor canvas (these trusted modules render same-origin as /admin with no
script-src CSP) when another admin opened the page → admin session/credential
theft and privilege escalation.

Fix at the single chokepoint every emit path already funnels through:

- Add `sanitizeRenderableHtmlAttribute(name, value)` in `@core/htmlAttributes`:
  rejects non-renderable names, the raw-HTML sink `srcdoc`, and any value with
  a dangerous URL scheme (javascript:/vbscript:/data:, via the existing
  `isSafeUrl`). Value-level scheme checking covers every URL attribute without
  enumerating them; ordinary text values pass unchanged.
- Route all four emit/collect paths through it: the shared publisher/canvas
  normaliser (`normalizeHtmlAttributes`), the `<body>` emit
  (`bodyHtmlAttributes`), and the HTML-import harvest (`collectHtmlAttributes`).
- Deny dangerous elements in the custom-tag escape hatch (`resolveHtmlTag`):
  script/iframe/frame/frameset/object/embed/applet/base/link/meta/style →
  coerced to `div`. `base.video` emits its own trusted <iframe> via its module
  `htmlTag`, not this resolver, so video embeds are unaffected.

Tests: new `htmlAttributesXss.test.ts` proves srcdoc/javascript:/data:/vbscript:
values and dangerous custom tags are dropped while safe attributes/URLs/tags
pass; the publisher emit is verified end-to-end.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DavidBabinec
DavidBabinec marked this pull request as ready for review July 3, 2026 13:49
@DavidBabinec
DavidBabinec merged commit c0087ec into main Jul 3, 2026
6 checks passed
@DavidBabinec
DavidBabinec deleted the fix/htmlattributes-xss branch July 3, 2026 13:50
@DavidBabinec DavidBabinec mentioned this pull request Jul 3, 2026
Merged
DavidBabinec added a commit that referenced this pull request Jul 3, 2026
@DavidBabinec @claude
test(agent): carry the base64 fixture through text content, not a dat…
348ea1a 
…a: attribute (#178)

The read-surface cleaning test planted its base64 payload in a custom
htmlAttributes value (`data-preview="data:image/png;base64,…"`). #155's
stored-XSS hardening now rejects data:-scheme values in custom attributes
(sanitizeRenderableHtmlAttribute), so the attribute never renders and the
cleaner has nothing to clean — the test has been red on main since (CI only
runs CodeQL, not bun test).

The production behavior is correct; the fixture was stale. Base64 blobs can
now only reach the agent read surface through text content (URL attributes
are scheme-checked by isSafeUrl, which also blocks data:), so the fixture
pastes the blob into a paragraph — which is also the realistic shape the
cleaner exists for.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lauer3912 pushed a commit to ClawCopilot/Instatic that referenced this pull request Jul 20, 2026
A5050
cherry-pick: fix(security): scheme-check custom htmlAttributes + deny…
81d2483 
… dangerous custom tags (CoreBunch#155) [upstream c0087ec]
lauer3912 pushed a commit to ClawCopilot/Instatic that referenced this pull request Jul 20, 2026
A5050
merge: integrate upstream v0.0.8-v0.0.11 fixes and features
7cc1b6c 
Cherry-picked 15 upstream commits across 5 phases:

Phase 1 — Security:
  - ae7ed0d: CSP base-uri + object-src (CoreBunch#158)
  - c0087ec: XSS scheme-check htmlAttributes (CoreBunch#155)

Phase 2 — Windows compatibility:
  - 75086cf: Windows port probing (CoreBunch#170)
  - fdfcb15: vite through entrypoint (CoreBunch#202)
  - 8c957fe: Windows-safe slot symlink swap (CoreBunch#187)

Phase 3 — Editor fixes (7 commits):
  - Font weights, malformed fonts tolerance, empty container,
    canvas pan, layer icons, explorer tabs, responsive nav

Phase 4 — CMS core:
  - 43abba0: Transactional site-document save (43 files)
  - 61fcebc: Stop forcing table kind 'data' (CoreBunch#216)
  - 3a4a6fc: Custom data-table fields editing (CoreBunch#182)

Phase 5 — Other:
  - f139d3c: YouTube iframe routing (CoreBunch#151)
  - 6447a65: JSON-import media validation (CoreBunch#154)
  - 9a3af2d: Publisher media background optimization
  - 081e0d3: Collapse data.rows SQL query (CoreBunch#159)

Additional workspace fixes:
  - Migration 020 conflict resolved (renumbered to 021)
  - staticArtefact.ts: Windows plain-file slot swap
  - 15 test files: EBUSY-safe cleanup on Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@DavidBabinec