From 35810aefe20f245cbe4d291bbb3751d2ac62fb79 Mon Sep 17 00:00:00 2001
From: Oto Macenauer <oto.macenauer@absa.africa>
Date: Thu, 24 Sep 2026 16:21:29 +0200
Subject: [PATCH] fix: strip inline event handlers from sub-app HTML
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

script-src 'self' blocks onclick="…" exactly like an inline <script>, so a
handler that survives the build is dead in production. Where no CSP is
served (astro dev/preview) it runs, and the docs-example fixture's theme
toggle re-added `dark` there. transformSubAppHtml() now drops every on*
attribute; the element stays.

build-integrity asserts no on* attribute anywhere in dist/ (walked as a
parsed tree, so prose is not flagged) and that the fixture's toggle lost
its handler. HEADLESS_RULES.md gains a "scripts as files" section.

Closes #67

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 CLAUDE.md                     |  4 +---
 contract/HEADLESS_RULES.md    | 18 ++++++++++++++++++
 contract/RULES.md             |  4 +++-
 src/utils/transform.js        | 16 ++++++++++++++--
 tests/build-integrity.spec.js | 30 ++++++++++++++++++++++++++++++
 tests/transform.spec.js       | 25 +++++++++++++++++++++++++
 6 files changed, 91 insertions(+), 6 deletions(-)
diff --git a/CLAUDE.md b/CLAUDE.md
index d4e4761..1f094e2 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -120,9 +120,7 @@ Known limit: a `!important` declaration in a sub-app stylesheet outranks the fen
 
 ### Light Only
 
-The knowledge base has no dark mode: no theme toggle, no persisted theme, no `dark` class, no dark palette. A sub-app's own theme bootstrap is removed twice over — `hoist-inline-scripts.js` deletes it while it is still inline, and `transformSubAppHtml()` strips any that reaches Astro, along with a `dark` body class — so an embedding host's theme cannot bleed into the fragment.
-
-Known gap: inline `on*` handlers in sub-app HTML are not stripped (#67). They are inert under the CSP but not under `astro dev`.
+The knowledge base has no dark mode: no theme toggle, no persisted theme, no `dark` class, no dark palette. A sub-app's own theme bootstrap is removed twice over — `hoist-inline-scripts.js` deletes it while it is still inline, and `transformSubAppHtml()` strips any that reaches Astro, along with a `dark` body class — so an embedding host's theme cannot bleed into the fragment. `transformSubAppHtml()` also strips every inline `on*` handler: inert under the CSP anyway, and under `astro dev` a sub-app theme toggle's `onclick` would re-add `dark`.
 
 ### URL Rewriting
 
diff --git a/contract/HEADLESS_RULES.md b/contract/HEADLESS_RULES.md
index d46fe7f..e2279c1 100644
--- a/contract/HEADLESS_RULES.md
+++ b/contract/HEADLESS_RULES.md
@@ -131,6 +131,23 @@ The `<body>` of each doc page must follow this structure:
 
 ---
 
+## Required: scripts as files
+
+The knowledge base serves `script-src 'self'`. Anything a page runs must come from a
+`<script src>` in your artifact:
+
+- **No inline event handlers.** `onclick="…"`, `onload="…"` and every other `on*`
+  attribute is stripped when the knowledge base re-hosts the page — the element stays,
+  the handler is gone. Attach listeners from a script file instead
+  ([KB-HTML-005](./RULES.md#kb-html-005--no-inline-event-handler-attributes)).
+- **No inline `<script>` blocks.** They are moved into files at build time so already
+  published bundles keep working, but ship them as files yourself
+  ([KB-HTML-004](./RULES.md#kb-html-004--no-inline-script-blocks)).
+- **No `javascript:` URLs.** Blocked like an inline handler
+  ([KB-HTML-006](./RULES.md#kb-html-006--no-javascript-urls)).
+
+---
+
 ## Required: design tokens
 
 Your CSS must define (or import) the canonical design tokens.  
@@ -206,5 +223,6 @@ Before opening a PR to add your app to `apps.json`:
 - [ ] No `<header class="fixed top-0...">` present in any headless HTML page
 - [ ] `data-kb-headless="true"` is on the `<html>` element
 - [ ] All asset paths are relative (no leading `/`)
+- [ ] No `on*` attributes — listeners are attached from script files
 - [ ] Design tokens are defined in your CSS
 - [ ] A GitHub Release with `kb-docs.tar.gz` exists on your repo
diff --git a/contract/RULES.md b/contract/RULES.md
index aae0d93..c118095 100644
--- a/contract/RULES.md
+++ b/contract/RULES.md
@@ -149,7 +149,9 @@ not scripts and are not reported.
 **Severity:** warning
 
 `onclick="…"` and every other `on*` attribute is blocked by `script-src 'self'`: the
-handler never runs. Attach listeners from a script file.
+handler never runs. The knowledge base strips every one when it re-hosts the page, so
+the element stays and does nothing — in development too, where no policy is served.
+Attach listeners from a script file.
 
 ### KB-HTML-006 — No `javascript:` URLs
 
diff --git a/src/utils/transform.js b/src/utils/transform.js
index d630f98..924805a 100644
--- a/src/utils/transform.js
+++ b/src/utils/transform.js
@@ -139,6 +139,9 @@ function detach(node) {
 const childElement = (node, tagName) =>
   (node?.childNodes ?? []).find((c) => c.tagName === tagName);
 
+/** `onclick`, `onload`, … — every attribute a browser treats as an inline handler. */
+const isEventHandler = (name) => /^on./i.test(name);
+
 // ── Light-only enforcement ────────────────────────────────────────────────────
 
 // isThemeBootstrap() lives in theme.js, dependency-free, so the publishing
@@ -152,8 +155,8 @@ export { isThemeBootstrap };
  * needs to re-host it.
  *
  * Steps (in order):
- *  1. Rewrite every URL-bearing attribute to an absolute /{prefix}/{slug}/… path
- *     and drop any <base> tag
+ *  1. Rewrite every URL-bearing attribute to an absolute /{prefix}/{slug}/… path,
+ *     drop any <base> tag and strip every inline `on*` event handler
  *  2. Stamp data-astro-transition-persist on every stylesheet link, and wrap
  *     every inline <style> in the sub-app cascade layer (css-layers.js)
  *  3. Split off <head> contents, <body> attributes and <body> contents
@@ -185,6 +188,15 @@ export function transformSubAppHtml(html, slug, fileRelDir, prefix) {
     //    written out in a code sample stays the string the author typed.
     if (el.tagName === 'base') { doomed.push(el); continue; }
 
+    // 1a. Inline event handlers. `script-src 'self'` blocks them in production,
+    //     so the handler is dead code there — and where no CSP is served
+    //     (astro dev/preview) it runs, which is how a sub-app's theme toggle
+    //     re-added `dark`. Stripping is what production already does, made
+    //     true everywhere.
+    if (el.attrs?.some((a) => isEventHandler(a.name))) {
+      el.attrs = el.attrs.filter((a) => !isEventHandler(a.name));
+    }
+
     for (const attr of el.attrs ?? []) {
       if (URL_ATTRS.has(attr.name) || (attr.name === 'data' && el.tagName === 'object')) {
         const abs = resolveUrl(attr.value, base, prefix, slug);
diff --git a/tests/build-integrity.spec.js b/tests/build-integrity.spec.js
index aa7de95..b226078 100644
--- a/tests/build-integrity.spec.js
+++ b/tests/build-integrity.spec.js
@@ -14,6 +14,7 @@ import { test, expect } from '@playwright/test';
 import { readFileSync, existsSync, readdirSync } from 'node:fs';
 import { fileURLToPath } from 'node:url';
 import { dirname, join, relative } from 'node:path';
+import { parse } from 'parse5';
 import { isThemeBootstrap } from '../src/utils/transform.js';
 import { LAYER_ORDER, SUB_APP_LAYER } from '../src/utils/css-layers.js';
 
@@ -414,6 +415,35 @@ test.describe('no inline scripts in the build output', () => {
     }
   });
 
+  test('no element in dist/ carries an inline event handler', () => {
+    // script-src 'self' blocks onclick="…" exactly like an inline <script>, so
+    // one that survives is dead in production and live under astro dev. Walked
+    // as a parsed tree: `onclick="…"` quoted in a code sample is prose, not an
+    // attribute.
+    const offenders = [];
+    const walk = (node, file) => {
+      for (const child of node.childNodes ?? []) {
+        for (const attr of child.attrs ?? []) {
+          if (/^on./i.test(attr.name)) offenders.push(`${relative(DIST, file)}: <${child.tagName} ${attr.name}>`);
+        }
+        if (child.content) walk(child.content, file);
+        walk(child, file);
+      }
+    };
+    for (const file of htmlFiles(DIST)) walk(parse(readFileSync(file, 'utf8')), file);
+    expect(offenders, "inline on* handler in the output — script-src 'self' blocks it; transform.js should strip it").toEqual([]);
+  });
+
+  test('the fixture theme toggle survives without its handler', () => {
+    // The docs-example fixture ships `<button id="theme-toggle" onclick="…">`,
+    // so this asserts the strip ran on a real handler rather than that there
+    // was nothing to strip.
+    const html = read('user-guide/docs/index.html');
+    const button = html.match(/<button\b[^>]*\bid="theme-toggle"[^>]*>/)?.[0];
+    expect(button, 'fixture no longer ships the theme toggle — pick another handler-bearing element').toBeTruthy();
+    expect(button).not.toMatch(/\bon[a-z]+=/i);
+  });
+
   test('the sub-app theme bootstrap is deleted, not hoisted into a file', () => {
     // The docs-example fixture ships a `localStorage`-driven dark-mode bootstrap.
     // Hoisting it would turn it into an external script the light-only strip can
diff --git a/tests/transform.spec.js b/tests/transform.spec.js
index eb266ba..8ad064f 100644
--- a/tests/transform.spec.js
+++ b/tests/transform.spec.js
@@ -82,6 +82,31 @@ test.describe('light-only enforcement', () => {
   });
 });
 
+test.describe('inline event handlers', () => {
+  test('every on* attribute is stripped, the element and its other attributes kept', () => {
+    const { bodyHtml, headHtml } = run(doc(
+      `<button id="theme-toggle" aria-label="Toggle" onclick="document.body.classList.toggle('dark')">t</button>` +
+      '<img src="a.png" ONERROR="x()" onload="y()">' +
+      '<template><a href="#" onmouseover="z()">in template</a></template>',
+      '<link rel="stylesheet" href="s.css" onload="this.media=\'all\'">',
+    ));
+    expect(bodyHtml + headHtml).not.toMatch(/\son[a-z]+=/i);
+    expect(bodyHtml).toContain('<button id="theme-toggle" aria-label="Toggle">t</button>');
+    expect(bodyHtml).toContain('src="/knowledge-base/demo/a.png"');
+    expect(bodyHtml).toContain('in template');
+  });
+
+  test('a bare `on` attribute is not a handler', () => {
+    const { bodyHtml } = run(doc('<div on="x" onfoo="y">d</div>'));
+    expect(bodyHtml).toContain('<div on="x">d</div>');
+  });
+
+  test('onclick quoted in prose is text, not an attribute', () => {
+    const { bodyHtml } = run(doc('<pre><code>&lt;button onclick="go()"&gt;</code></pre>'));
+    expect(bodyHtml).toContain('onclick="go()"');
+  });
+});
+
 test.describe('URL rewriting', () => {
   test('rewrites the single-URL attributes', () => {
     const { bodyHtml } = run(doc(
