graham/aprs.me
Watch
1
0
Fork
You've already forked aprs.me
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: remove permissive CDN content security sources #27

Merged
graham merged 2 commits from fix/w10-csp-origins into main 2026-09-15 08:54:39 -05:00
Conversation 2 Commits 2 Files changed 2 +29 -30
graham commented 2026-09-14 23:33:04 -05:00
Owner
Copy link

Fixes W10 from bugs.md.

Drops unpkg, jsDelivr and cdnjs from script-src and unpkg from style-src. Nothing loads scripts from those CDNs at runtime — Leaflet, Chart.js and plugins ship from the local vendor bundle (scripts/build-vendor-bundle.js fetches them at build time), so the allow-list only offered arbitrary-npm-package script execution to any future HTML-injection bug in packet-rendering templates. The moduledoc now states why the CDNs are absent and why style-src 'unsafe-inline' remains (runtime-computed inline style attributes for symbol sprites, chart sizing and marker positioning cannot be hashed; nonce-splitting needs conn access in LiveView components and is left as a documented follow-up).

Verification: focused CSP tests passed (12) with a new negative assertion for all three CDNs, full suite passed (2236), mix credo --strict, commit hooks including Dialyzer, and a live dev-server smoke confirmed the served header contains no CDN origins.

Fixes W10 from bugs.md. Drops unpkg, jsDelivr and cdnjs from `script-src` and unpkg from `style-src`. Nothing loads scripts from those CDNs at runtime — Leaflet, Chart.js and plugins ship from the local vendor bundle (`scripts/build-vendor-bundle.js` fetches them at build time), so the allow-list only offered arbitrary-npm-package script execution to any future HTML-injection bug in packet-rendering templates. The moduledoc now states why the CDNs are absent and why `style-src 'unsafe-inline'` remains (runtime-computed inline style attributes for symbol sprites, chart sizing and marker positioning cannot be hashed; nonce-splitting needs conn access in LiveView components and is left as a documented follow-up). Verification: focused CSP tests passed (12) with a new negative assertion for all three CDNs, full suite passed (2236), `mix credo --strict`, commit hooks including Dialyzer, and a live dev-server smoke confirmed the served header contains no CDN origins.
fix: remove permissive CDN content security sources
All checks were successful
Elixir CI / Dialyzer (pull_request) Successful in 39s
Details
Elixir CI / Build and test (pull_request) Successful in 1m39s
Details
Elixir CI / Build and Push Docker Image (pull_request) Has been skipped
Details
skippy-bot/review Skippy review: clean — 1 non-blocking note open
Details
8debe9eda2
skippy-bot reviewed 2026-09-14 23:34:57 -05:00
skippy-bot left a comment
Copy link

🤖 Skippy PR review

1 finding — none blocking.

Severity Location Issue
🔵 Suggestion lib/aprsme_web/plugs/content_security_policy.ex:33 js.sentry-cdn.com is dead weight too — nothing in the repo uses Sentry

Reviewed 8debe9eda2f3. Comment skippy review to re-run.

### 🤖 Skippy PR review **1 finding** — none blocking. | Severity | Location | Issue | | --- | --- | --- | | 🔵 Suggestion | `lib/aprsme_web/plugs/content_security_policy.ex:33` | js.sentry-cdn.com is dead weight too — nothing in the repo uses Sentry | <sub>Reviewed `8debe9eda2f3`. Comment `skippy review` to re-run.</sub> <!-- skippy-pr-review -->
lib/aprsme_web/plugs/content_security_policy.ex Outdated
@ -25,3 +33,1 @@
@external_style_sources [
"https://unpkg.com"
"https://js.sentry-cdn.com"
skippy-bot commented 2026-09-14 23:34:57 -05:00
First-time contributor
Copy link

🔵 Suggestion — js.sentry-cdn.com is dead weight too — nothing in the repo uses Sentry

The argument that retired unpkg/jsDelivr/cdnjs applies to this entry: a case-insensitive grep -r sentry over the tree at 8debe9e returns no Sentry at all — no sentry dep in mix.exs/mix.lock, no Sentry.init in assets/js, no script tag in layouts/root.html.heex, nothing in config/, rel/ or k8s/. The only hits are this plug and its test, so script-src still allow-lists a third-party script host for an integration that does not exist, and the new moduledoc bullet list still advertises "Sentry error tracking". If the browser SDK is coming back, one line here saying so is enough; if not, drop this entry together with the two connect-src Sentry origins (https://*.ingest.sentry.io, https://*.sentry.io), which are equally unexercised. Not blocking — nothing fails today, it is just residual surface on a PR whose whole point is removing residual surface.

**🔵 Suggestion — js.sentry-cdn.com is dead weight too — nothing in the repo uses Sentry** The argument that retired unpkg/jsDelivr/cdnjs applies to this entry: a case-insensitive `grep -r sentry` over the tree at 8debe9e returns no Sentry at all — no `sentry` dep in `mix.exs`/`mix.lock`, no `Sentry.init` in `assets/js`, no script tag in `layouts/root.html.heex`, nothing in `config/`, `rel/` or `k8s/`. The only hits are this plug and its test, so `script-src` still allow-lists a third-party script host for an integration that does not exist, and the new moduledoc bullet list still advertises "Sentry error tracking". If the browser SDK is coming back, one line here saying so is enough; if not, drop this entry together with the two `connect-src` Sentry origins (`https://*.ingest.sentry.io`, `https://*.sentry.io`), which are equally unexercised. Not blocking — nothing fails today, it is just residual surface on a PR whose whole point is removing residual surface.
skippy-bot marked this conversation as resolved
fix: remove unused Sentry CSP sources
All checks were successful
skippy-bot/review Skippy review: clean — no open findings
Details
Elixir CI / Dialyzer (pull_request) Successful in 40s
Details
Elixir CI / Build and test (pull_request) Successful in 1m38s
Details
Elixir CI / Build and Push Docker Image (pull_request) Has been skipped
Details
6104ae7aed
skippy-bot commented 2026-09-15 08:53:33 -05:00
First-time contributor
Copy link

Resolved 1 of 1 earlier findings — the Sentry script-src host and both *.sentry.io connect-src origins are gone from content_security_policy.ex, the moduledoc bullet was dropped, and the test now asserts all three absent.

Nothing new in 8debe9e..6104ae7 — the follow-on commit only removes those origins and reworks the tests accordingly. 0 still open.

Resolved 1 of 1 earlier findings — the Sentry `script-src` host and both `*.sentry.io` `connect-src` origins are gone from `content_security_policy.ex`, the moduledoc bullet was dropped, and the test now asserts all three absent. Nothing new in `8debe9e..6104ae7` — the follow-on commit only removes those origins and reworks the tests accordingly. 0 still open. <!-- skippy-pr-review -->
graham merged commit 2240011141 into main 2026-09-15 08:54:39 -05:00
graham deleted branch fix/w10-csp-origins 2026-09-15 08:54:39 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
graham/aprs.me!27
No description provided.