Remove unsafe-inline from CSP script-src by externalising template JS #49
Loading…
Reference in a new issue
No description provided.
Delete branch "refs/pull/49/head"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #48
What changed
All UI JavaScript previously lived in inline
<script>blocks across 6 Jinja2 templates (dashboard.html,hashtag_streams.html,topical_tags.html,queue_preview.html,settings.html,stats.html). This forced'unsafe-inline'inContent-Security-Policy: script-src, which neutralises most XSS protection.Each inline block has been extracted to a dedicated file under
src/fenliu/static/assets/and the templates updated accordingly.'unsafe-inline'is now removed fromscript-src.How template variables were handled
{{ api_key }}(5 templates) — replaced with<meta name="fenliu-api-key" content="...">" in<head>; external JS reads it viadocument.querySelector`. Meta tags are not executable so CSP does not apply.stats.html) — replaced with a<script type="application/json" id="chart-data">JSON island.type=application/jsonmarks it as non-executable data; CSP ignores it.onclick/onchange/onsubmitattributes — converted todata-*attributes (for template-variable cases) oraddEventListenercalls in the external JS files.onclickattributes insettings.jsandqueue-preview.js— converted todata-*attributes with event delegation.CSP changes
script-src: removed'unsafe-inline'; addedhttps://cdn.jsdelivr.net(Chart.js CDN, already in use bystats.htmlbut missing from the old CSP)style-src: unchanged — PyView still injects per-view inline<style>blockspackages/fenliu/src/fenliu/static/assets/dashboard.jsline 1 — @marvin8🔴 Not addressing in this PR. The API key was previously embedded directly inside inline
<script>blocks (let apiKey = "{{ api_key }}"), so it was already equally readable from the DOM. A<meta>tag does not increase its exposure. Moving to HttpOnly cookies would require a significant auth-flow refactor and is out of scope here. The XSS mitigation delivered by this PR is the removal of'unsafe-inline'fromscript-src, which prevents injected scripts from running.packages/fenliu/src/fenliu/static/assets/queue-preview.jsline 1 — @marvin8🔴 Same finding as on
dashboard.js— see reply there. The meta tag does not increase the key's DOM visibility relative to its previous embedding inside<script>blocks. Not addressing in this PR.packages/fenliu/src/fenliu/static/assets/topical-tags.jsline 1 — @marvin8🔴 Same finding as on
dashboard.js— see reply there. Not addressing in this PR.packages/fenliu/src/fenliu/static/assets/stats.jsline 2 — @marvin8🔴 False positive.
<script type="application/json" id="chart-data">is rendered exclusively by the Jinja2 template via| tojsonon Python lists (aggregated post counts and hashtag labels from the database). It carries no user-supplied input and is not attacker-controllable.JSON.parseon server-rendered JSON data is standard practice and is not an injection vector.packages/fenliu/src/fenliu/static/assets/stats.jsline 2 — @marvin8🔴 Not addressing.
stats.jsis loaded exclusively fromstats.html, which renders<script id="chart-data">unconditionally. The element is structurally guaranteed by the template. A missing element would be immediately visible during development; defensive null-guarding for elements that are coupled to the loading template adds noise without real benefit.packages/fenliu/src/fenliu/static/assets/queue-preview.jsline 279 — @marvin8🔴 False positive. The
data-attachmentsattribute is rendered by Jinja2 with| tojson | replace("'", "'")from database-backedMediaAttachmentobjects fetched from the Fediverse API. It is entirely server-generated and not user-supplied in any direct sense. Parsing trusted server data withJSON.parseis standard practice.packages/fenliu/src/fenliu/middleware.pyline 111 — @marvin8🔴 Valid concern, not addressing in this PR. The CDN-without-SRI approach matches the pre-existing practice for Tailwind CSS (
cdn.tailwindcss.com) and Font Awesome (cdnjs.cloudflare.com), both already present before this PR. Addressing SRI consistently across all three CDN assets — pinning to specific versions and tracking hash updates — is the right scope for a dedicated follow-up issue.packages/fenliu/TODO.mdline 4 — @marvin8🔴 Pre-existing, not introduced by this PR. The only change made to that line was replacing
[ ]with[x]. The long description was written when the task was originally added to the TODO list.packages/fenliu/src/fenliu/static/assets/queue-preview.jsline 249 — @marvin8✅ Valid catch — the Escape-key path and the close-button path both called
modal.remove()without first removing thekeydownlistener; only the backdrop-click path cleaned it up correctly. Pre-existing bug in the original inline<script>that was faithfully extracted. Fixed in commit59565fb: all teardown paths now go through a singleteardown()function backed by anAbortController, socontroller.abort()removes every listener atomically regardless of how the modal is closed.packages/fenliu/src/fenliu/static/assets/queue-preview.jsline 60 — @marvin8🔴 Not addressing.
showFlashhastype = "success"as its default and is called only within this file with values"success","error","warning","info"— the complete set of defined keys. There is no call path that can supply an unknown type, so a defensive fallback would add dead code.packages/fenliu/src/fenliu/static/assets/queue-preview.jsline 283 — @marvin8🔴 Not addressing.
queue-preview.jsis loaded exclusively fromqueue_preview.htmlvia<script src="/static/assets/queue-preview.js" defer>. Every element referenced at the bottom of the file (statusFilter,sortBy,sortOrder,cleanupBtn,trimBtn) is always present in that template. The script cannot execute without the template that guarantees their presence.packages/fenliu/src/fenliu/static/assets/dashboard.jsline 20 — @marvin8🔴 Not addressing.
dashboard.jsis loaded exclusively fromdashboard.html, which unconditionally renders<div id="flash" ...>. The element is structurally guaranteed by the template that loads the script; null-guarding it adds noise.packages/fenliu/src/fenliu/static/assets/dashboard.jsline 69 — @marvin8🔴 Pre-existing, not introduced by this PR.
formatRelativeTimewas extracted verbatim from the original inline<script>block indashboard.html. The behaviour for future timestamps (clock skew, timezone drift) predates this change and is out of scope.packages/fenliu/src/fenliu/static/assets/dashboard.jsline 31 — @marvin8🔴 Out of scope and pre-existing. The
X-API-Keyheader mechanism is the existing API authentication approach, in place well before this PR. This PR only externalised the JavaScript; it did not alter any API authentication logic. Server-side rate limiting is a server-layer concern unrelated to script placement.