AbsaOSS / AbsaOSS/knowledge-base

Inline event handlers in sub-app HTML survive the build: dead under CSP, and a dark-mode leak without it

Offen
#67 0 Kommentare 0 Reaktionen 1 zugewiesene Person Beansprucht von @oto-macenauer-absa Auf GitHub ansehen
Vorherrschende Sprache
JavaScript
Sterne
0
Forks
0
Ø Merge
7 Std. 35 Min.
Gemergte PRs (30 T.)
22

Beschreibung

## Summary

The "nothing inline in the output" invariant is enforced for `` elements only. Inline **event-handler attributes** are neither hoisted nor stripped, and `script-src 'self'` blocks them exactly like an inline script.

The vendored docs-example fixture ships one, and it reaches `dist/`:

```html
<button id="theme-toggle" aria-label="Toggle theme" onclick="
const r=document.getElementById('docs-root');
r.classList.toggle('dark');
localStorage.setItem('theme',r.classList.contains('dark')?'dark':'light');
">
```

## Two problems

1. **It is dead code in production.** `script-src 'self'` with no `'unsafe-inline'` blocks inline handlers, so the button is inert and logs a CSP violation when clicked. `scripts/hoist-inline-scripts.js` exists precisely so that already-published bundles keep working under the policy; it does not cover this case, so any doc app whose interactivity is written as `onclick=` is quietly broken instead.
2. **It is a light-only leak wherever the CSP is absent.** `astro dev` and `astro preview` serve no CSP, so there the toggle works and adds `dark` to the sub-app root — the exact thing `transform.js` strips the theme bootstrap to prevent (#48).

`tests/build-integrity.spec.js` asserts every `<script>` has a `src`, which is why this got through. Nothing asserts on `on*` attributes.

## Suggested fix

Decide between the two available policies and apply it in `src/utils/transform.js`, where the document is already parsed:

- **Strip `on*` attributes.** Honest about what production does — the handler cannot run anyway — and closes the dev-mode dark leak. Cost: a doc app's inline interactivity disappears rather than failing loudly.
- **Hoist them**, the way `<script>` bodies are hoisted: turn `onclick="…"` into a listener registered from a generated file. Preserves behaviour, but needs a stable element handle and is a good deal more machinery.

Either way, add the assertion to `tests/build-integrity.spec.js` so the invariant covers attributes and not just elements, and say so in `contract/HEADLESS_RULES.md` — this is a rule onboarding repositories need to know about.

Separately worth deciding whether the docs-example fixture should keep shipping a dark-mode toggle at all, given the marketplace is light-only.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.