fix(csp): make the policy correct enough to enforce, and make its violations visible (#10)

app/root.tsx

@@ -26,6 +26,7 @@ import { socialLinksData } from '#app/routes/_marketing+/__data'
 import tailwindStyleSheetUrl from '#app/styles/tailwind.css?url'
 import { usePosthogPageView } from '#app/utils/analytics'
 import { ClientHintCheck, getHints, useHints } from '#app/utils/client-hints'
+import { cspCaptureScript } from '#app/utils/csp'
 import { getEnv } from '#app/utils/env.server'
 import { honeypot } from '#app/utils/honeypot.server'
 import { getDomainUrl, getUrl } from '#app/utils/misc'
@@ -148,6 +149,17 @@ function Document({
 			className={`${theme || hintTheme || 'light'} h-full overflow-x-hidden`}
 		>
 			<head>
+				{/*
+				 * Must be the first thing in <head>. Parse-time CSP violations
+				 * (this document's own images, the Umami tracker below) fire
+				 * while the parser is still running, long before the
+				 * dynamically imported monitoring module could register a
+				 * listener. See app/utils/csp.ts.
+				 */}
+				<script
+					nonce={nonce}
+					dangerouslySetInnerHTML={{ __html: cspCaptureScript }}
+				/>
 				<ClientHintCheck nonce={nonce} />
 				<Meta />
 				<meta charSet="utf-8" />
@@ -172,6 +184,7 @@ function Document({
 				<ScrollRestoration nonce={nonce} />
 				<Scripts nonce={nonce} />
 				<script
+					nonce={nonce}
 					async
 					defer
 					src={`https://${env.UMAMI_DOMAIN}/${env.UMAMI_SCRIPT_NAME}`}

app/utils/csp.ts

@@ -0,0 +1,120 @@
+/**
+ * CSP violation capture.
+ *
+ * The policy in `server/index.ts` is `reportOnly`, so violations are never
+ * enforced — but the directives object also has no `report-uri` or `report-to`,
+ * which means the browser had nowhere to send them. They existed only as
+ * devtools console lines, which is how a policy that was wrong in two places
+ * stayed invisible.
+ *
+ * Registering the listener from `monitoring.client.tsx` is too late. That
+ * module is dynamically imported from `entry.client.tsx`, so it runs after the
+ * document has been parsed — and the violations most worth seeing (the Umami
+ * tracker, SSR images) are dispatched *during* parsing. Those events fire
+ * before any listener exists and are lost permanently.
+ *
+ * So the listener is installed by an inline nonced script at the top of
+ * `<head>` instead, which pushes into a bounded buffer. `drainCspViolations`
+ * is called once Sentry exists, forwarding both the buffered parse-time
+ * violations and anything that fires afterwards.
+ */
+
+/** Cap the buffer so a page in a violation loop cannot grow it without bound. */
+export const CSP_VIOLATION_LIMIT = 20
+
+export type CspViolation = {
+	blockedURI: string
+	violatedDirective: string
+	effectiveDirective: string
+	disposition: string
+	sourceFile: string
+	lineNumber: number
+	columnNumber: number
+	statusCode: number
+}
+
+/**
+ * Inline script source, rendered as the first child of `<head>`.
+ *
+ * Deliberately dependency-free and defensive: this runs before hydration, and
+ * a throw here would take down the whole document rather than just reporting.
+ */
+export const cspCaptureScript = `(function(){
+if (window.__cspViolations) return;
+var seen = [];
+window.__cspViolations = seen;
+window.addEventListener('securitypolicyviolation', function (event) {
+if (seen.length >= ${CSP_VIOLATION_LIMIT}) return;
+seen.push({
+blockedURI: String(event.blockedURI || ''),
+violatedDirective: String(event.violatedDirective || ''),
+effectiveDirective: String(event.effectiveDirective || ''),
+disposition: String(event.disposition || ''),
+sourceFile: String(event.sourceFile || ''),
+lineNumber: Number(event.lineNumber || 0),
+columnNumber: Number(event.columnNumber || 0),
+statusCode: Number(event.statusCode || 0)
+});
+});
+})();`
+
+/**
+ * Whether a violation was caused by Sentry's own transport being refused.
+ *
+ * Reporting a violation means sending an envelope to Sentry. If that envelope
+ * is itself blocked, forwarding the violation produces another blocked
+ * envelope, which produces another violation — an unbounded loop against the
+ * Sentry quota, triggered by nothing but a page load.
+ *
+ * `connect-src` is built from the DSN origin precisely so this cannot happen
+ * (`sentryOrigin` in `server/utils/csp.ts`). This is the second lock on the
+ * same door, for the case where the policy is tightened later and someone
+ * removes the Sentry host without realising what the violation handler does.
+ */
+export function isSentryViolation(
+	blockedURI: string,
+	sentryDsn: string | undefined,
+): boolean {
+	if (!sentryDsn || !blockedURI) return false
+	try {
+		return new URL(blockedURI).origin === new URL(sentryDsn).origin
+	} catch {
+		return false
+	}
+}
+
+declare global {
+	interface Window {
+		__cspViolations?: CspViolation[]
+	}
+}
+
+/**
+ * Hand the buffered violations to the caller and install a live listener for
+ * everything after this point, so violations are not double-reported.
+ */
+export function drainCspViolations(
+	onViolation: (violation: CspViolation) => void,
+): number {
+	const buffered = window.__cspViolations ?? []
+	window.__cspViolations = undefined
+
+	for (const violation of buffered.slice(0, CSP_VIOLATION_LIMIT)) {
+		onViolation(violation)
+	}
+
+	window.addEventListener('securitypolicyviolation', (event) => {
+		onViolation({
+			blockedURI: String(event.blockedURI || ''),
+			violatedDirective: String(event.violatedDirective || ''),
+			effectiveDirective: String(event.effectiveDirective || ''),
+			disposition: String(event.disposition || ''),
+			sourceFile: String(event.sourceFile || ''),
+			lineNumber: Number(event.lineNumber || 0),
+			columnNumber: Number(event.columnNumber || 0),
+			statusCode: Number(event.statusCode || 0),
+		})
+	})
+
+	return buffered.length
+}

app/utils/monitoring.client.tsx

@@ -6,6 +6,7 @@ import {
 	useLocation,
 	useNavigationType,
 } from 'react-router'
+import { drainCspViolations, isSentryViolation } from './csp'
 import { ensureCSSStyleDeclaration } from './cssom.client'
 
 export function init() {
@@ -47,6 +48,25 @@ export function init() {
 		replaysOnErrorSampleRate: 1.0,
 	})
 
+	// The policy is reportOnly with no report-uri, so violations used to exist
+	// only in the devtools console. The parse-time ones were captured by the
+	// inline script in root.tsx; this drains that buffer and covers the rest.
+	drainCspViolations((violation) => {
+		// Reporting this would mean sending an envelope to Sentry. If Sentry's
+		// own request is what got blocked, that envelope fails too and raises
+		// another violation, forever.
+		if (isSentryViolation(violation.blockedURI, ENV.SENTRY_DSN)) return
+
+		Sentry.captureMessage(`CSP: ${violation.violatedDirective}`, {
+			level: 'warning',
+			tags: {
+				csp_directive: violation.effectiveDirective,
+				csp_disposition: violation.disposition,
+			},
+			extra: violation,
+		})
+	})
+
 	startReplay()
 }
 

server/index.ts

@@ -9,6 +9,7 @@ import rateLimit, { ipKeyGenerator } from 'express-rate-limit'
 import getPort, { portNumbers } from 'get-port'
 import helmet from 'helmet'
 import morgan from 'morgan'
+import { cspConnectSrc, cspImgSrc } from './utils/csp.ts'
 import { decodeRequestUrl } from './utils/request-url.ts'
 
 const MODE = process.env.NODE_ENV ?? 'development'
@@ -131,17 +132,28 @@ app.use(
 		referrerPolicy: { policy: 'same-origin' },
 		crossOriginEmbedderPolicy: false,
 		contentSecurityPolicy: {
-			// NOTE: Remove reportOnly when you're ready to enforce this CSP
+			// NOTE: Remove reportOnly when you're ready to enforce this CSP.
+			//
+			// Read this before flipping it. `reportOnly: true` means the policy
+			// is *never applied* — the browser reports violations and loads
+			// everything anyway. So every gap below has been invisible and
+			// harmless so far, and nothing about the site working today is
+			// evidence the policy is correct. Flipping this turns all of it
+			// live at once, and any hole becomes a broken page.
+			//
+			// Violations are now captured in Sentry (see app/utils/csp.ts), so
+			// the safe order is: deploy with reportOnly, read the Sentry CSP
+			// messages until they are clean, then flip.
 			reportOnly: true,
 			directives: {
-				'connect-src': [
-					MODE === 'development' ? 'ws:' : null,
-					process.env.SENTRY_DSN ? '*.sentry.io' : null,
-					"'self'",
-				].filter(Boolean),
+				'connect-src': cspConnectSrc({
+					mode: MODE,
+					sentryDsn: process.env.SENTRY_DSN,
+					umamiDomain: process.env.UMAMI_DOMAIN,
+				}),
 				'font-src': ["'self'"],
 				'frame-src': ["'self'"],
-				'img-src': ["'self'", 'data:', '*.cloudinary.com'],
+				'img-src': cspImgSrc(),
 				'script-src': [
 					"'strict-dynamic'",
 					"'self'",

server/utils/csp.ts

@@ -0,0 +1,60 @@
+/**
+ * Build the CSP `connect-src` allowlist.
+ *
+ * Umami's tracker POSTs to `https://<UMAMI_DOMAIN>/api/send`, and Sentry ships
+ * every envelope to the host in its DSN. Both are cross-origin, so both have to
+ * be named here. Umami is a full `https://` origin rather than a wildcard so a
+ * compromised Umami host cannot widen the policy.
+ */
+
+/**
+ * The exact origin Sentry's envelopes go to, from `SENTRY_DSN`.
+ *
+ * `*.sentry.io` was the previous answer, and it is wrong for anyone pointing
+ * `SENTRY_DSN` at a self-hosted or custom Sentry domain: their envelopes get
+ * blocked by `connect-src`, and since `app/utils/monitoring.client.tsx` now
+ * forwards CSP violations *through Sentry*, each blocked envelope triggers
+ * another one. That is an unbounded loop against the Sentry quota.
+ *
+ * `new URL(dsn).origin` is exact — it allows the configured host and nothing
+ * else, rather than every subdomain of sentry.io.
+ */
+export function sentryOrigin(sentryDsn: string | undefined): string | null {
+	if (!sentryDsn) return null
+	try {
+		return new URL(sentryDsn).origin
+	} catch {
+		// A malformed DSN cannot be reached anyway; Sentry itself will refuse
+		// it. Omitting the host keeps the policy tight instead of guessing.
+		return null
+	}
+}
+
+export function cspConnectSrc({
+	mode,
+	sentryDsn,
+	umamiDomain,
+}: {
+	mode: string
+	sentryDsn: string | undefined
+	umamiDomain: string | undefined
+}): string[] {
+	return [
+		mode === 'development' ? 'ws:' : null,
+		sentryOrigin(sentryDsn),
+		umamiDomain ? `https://${umamiDomain}` : null,
+		"'self'",
+	].filter((value): value is string => value != null)
+}
+
+/**
+ * Build the CSP `img-src` allowlist.
+ *
+ * `*.cloudinary.com` backs the logo in `app/components/logo.tsx` and the
+ * project screenshots in `app/routes/_marketing+/__data.ts`.
+ * `*.hashnode.com` backs the blog/notes `coverImage.url` values served by the
+ * Hashnode GraphQL API (`app/graphql/gql.ts`) and rendered by `CardImage`.
+ */
+export function cspImgSrc(): string[] {
+	return ["'self'", 'data:', '*.cloudinary.com', '*.hashnode.com']
+}