Skip to content

Remove 'unsafe-inline' and 'unsafe-eval' from script-src - #773

Draft
Wavesonics wants to merge 3 commits into
developfrom
security/csp-hardening
Draft

Remove 'unsafe-inline' and 'unsafe-eval' from script-src#773
Wavesonics wants to merge 3 commits into
developfrom
security/csp-hardening

Conversation

@Wavesonics

Copy link
Copy Markdown
Collaborator

Follow-up to #772, which deliberately left CSP hardening out of a PageSpeed PR. Draft — stacked on perf/pagespeed-remediation; retarget to develop once that merges.

Lighthouse flagged two High-severity script-src findings. The policy allowed 'unsafe-inline' and 'unsafe-eval', which means an injected <script> from any XSS hole would have executed. Removing them requires every script to be an external file, so this PR converts all the page code that relied on them.

What had to change

44 inline event-handler attributes across 16 templates — pen-name editor, bio editor, community toggle, share/publish/review dialogs, whitelist rows, admin users. No nonce or hash can allow an on* attribute; only 'unsafe-inline' can. They become data-on-* attributes dispatched by a new actions.js:

<button data-on-click="bio-edit">Edit</button>
hammerActions({'bio-edit': enterBioEditMode});

Listeners live on document, so markup HTMX swaps in is wired up with no re-binding — which the old per-element listeners in admin-whitelist needed.

12 inline <script> blocks move to files under assets/js/. The values they interpolated (chart series, localized strings) now arrive as <script type="application/json"> islands — inert data, so CSP doesn't apply — built with a shared jsonIsland() / messagesIsland() in frontend/utils/JsonIsland.kt. ReviewFrontend had a private copy of that helper; it now uses the shared one.

Analytics no longer emits any inline script. AnalyticsProvider.eventBridge() (a JS string) is replaced by clientConfig(): Map<String, String>, rendered as data-* on the analytics.js tag; the vendor bootstrap, including Google's documented inline gtag snippet, lives in analytics.js.

Six htmx expressions that compile strings with new Function() — five hx-on::* and one bracketed hx-trigger filter (every 3s [autoRefresh.checked]) — become real event listeners and an htmx.trigger() poll.

A bug this surfaced

The pen-name and release forms gated themselves with onsubmit="return validateBeforeSubmit()" / return confirmRelease(). htmx issues its request from its own listener on the form, so cancelling the submit never stopped the request — declining the release confirmation still sent the DELETE. They now use htmx:confirm, which is the supported gate. Verified both branches in a browser: declining leaves the pen name claimed; accepting releases it.

Guard rails

Everything removed here fails silently in a browser — a dead button, not an exception — so CspComplianceTest fails the build on: a reintroduced inline handler, an executable inline script, an eval-requiring htmx attribute, or a data-on-* action whose handler is missing (and vice versa). DialogWiringTest renders the HTMX-swapped dialog partials and asserts their controls stay wired. AnalyticsCspTest asserts script-src contains neither unsafe directive.

docs/WEB-DESIGN-SYSTEM.md gains a "Client-Side Scripting" section documenting the pattern.

Scope

style-src keeps 'unsafe-inline' — 8 style attributes plus htmx's own injected styles, and Lighthouse's CSP audit only scores script-src. Nonces + strict-dynamic would additionally clear the "host allowlists can be bypassed" finding, but they collide with the ETag/304 path in PageETag.kt: a 304 would carry a fresh nonce while the cached body holds the old one, blocking every script on repeat visits. Trusted Types is untouched.

Verification

Full :server:test green. Driven in Chrome against a local --dev server with the new policy live, exercising every converted surface:

  • CSP is enforced — appending an inline <script> from the console does not execute.
  • Dashboard — claim pen name (debounced availability check, island-sourced strings), char counters, bio edit/save, community toggle, release-confirm gate (both branches).
  • Admin whitelist — expiry preset, add form reset after request (the hx-on replacement), edit dialogs opened from HTMX-swapped rows, close by button, dismiss by overlay click, and not closing on an inner click (the stopPropagation replacement).
  • Admin users — sort-controls toggle and its stored preference.
  • Admin settings — monitoring subfield enable/disable.
  • Monitoring — all four charts render from JSON islands with localized series names; endpoints table sort; copy-stack button; log auto-refresh polls only while checked (the bracketed-trigger replacement).

Not exercised live: the story-page share/publish dialogs, since the dev database has no story owned by an account I could log into. They are wired identically to the admin dialogs that were exercised, and DialogWiringTest covers their markup — but they are worth a manual click-through before merge.

Mobile scored 61 with an 11.1s LCP, driven by a 1.3 MiB hero JPEG and
~3.7s of render-blocking stylesheet chains.

- Hero: masthead.jpg (1.28 MiB) replaced with WebP variants (163 KiB
  desktop, 54 KiB mobile) plus a JPEG fallback via image-set, and
  preloaded with fetchpriority=high so it is discoverable from the HTML.
- error.css was linked on every page but only four templates use it; it
  now arrives via page_stylesheet.
- Lora moves from an @import inside base.css to a head link with
  preconnects, removing a three-deep request chain.
- Font Awesome loads non-blocking at media=print, promoted by
  async-css.js, with a noscript fallback.
- htmx gets defer.
- Kingthings gains font-display: swap and a WOFF2 (137 -> 50 KiB).
- Asset URLs carry ?v=<version>; versioned requests cache for a year as
  immutable, unversioned ones keep the existing windows.
- Accessibility: aria-labels on the download selects and links, heading
  hierarchy no longer skips levels, footer version text meets 4.5:1.
- Adds Cross-Origin-Opener-Policy: same-origin.
The CSP allowed inline scripts and eval, so an injected script tag would
have executed. Removing them means every script has to be an external
file, so this converts the page code that relied on them.

- 44 inline event-handler attributes across 16 templates become data-on-*
  attributes dispatched by a new actions.js. No nonce or hash can allow an
  on* attribute, so these had to go regardless of approach. Delegation from
  document also means HTMX-swapped markup needs no re-binding.
- 12 inline <script> blocks move to files under assets/js. Values they
  interpolated now arrive as <script type="application/json"> islands,
  built with the shared jsonIsland/messagesIsland helpers.
- The analytics provider no longer emits an inline bridge or Google's
  inline gtag bootstrap; analytics.js reads data-* off its own tag.
- Five hx-on::* attributes and one bracketed hx-trigger filter become real
  htmx event listeners, since htmx compiles those with new Function().

The pen-name forms gain a working gate: htmx issues its request from a
listener on the form, so the old onsubmit returning false never stopped it.
htmx:confirm does.

style-src keeps 'unsafe-inline' for the remaining style attributes.

CspComplianceTest fails the build on a reintroduced inline handler, inline
script, eval-requiring htmx attribute, or an action whose handler is
missing — all of which otherwise fail silently in the browser.
@codacy-production

codacy-production Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 141 complexity · 0 duplication

Metric Results
Complexity 141
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Base automatically changed from perf/pagespeed-remediation to develop July 25, 2026 16:52
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