move away from express helmet (#927)

Co-authored-by: Kent C. Dodds <[email protected]>
Co-authored-by: Kent C. Dodds <[email protected]>

Author: nichtsam

app/entry.server.tsx

@@ -1,5 +1,6 @@
 import { PassThrough } from 'node:stream'
 import { styleText } from 'node:util'
+import { contentSecurity } from '@nichtsam/helmet/content'
 import { createReadableStreamFromReadable } from '@react-router/node'
 import * as Sentry from '@sentry/node'
 import { isbot } from 'isbot'
@@ -20,6 +21,8 @@ export const streamTimeout = 5000
 init()
 global.ENV = getEnv()
 
+const MODE = process.env.NODE_ENV ?? 'development'
+
 type DocRequestArgs = Parameters<HandleDocumentRequestFunction>
 
 export default async function handleRequest(...args: DocRequestArgs) {
@@ -64,6 +67,33 @@ export default async function handleRequest(...args: DocRequestArgs) {
 					const body = new PassThrough()
 					responseHeaders.set('Content-Type', 'text/html')
 					responseHeaders.append('Server-Timing', timings.toString())
+
+					contentSecurity(responseHeaders, {
+						crossOriginEmbedderPolicy: false,
+						contentSecurityPolicy: {
+							// NOTE: Remove reportOnly when you're ready to enforce this CSP
+							reportOnly: true,
+							directives: {
+								fetch: {
+									'connect-src': [
+										MODE === 'development' ? 'ws:' : undefined,
+										process.env.SENTRY_DSN ? '*.sentry.io' : undefined,
+										"'self'",
+									],
+									'font-src': ["'self'"],
+									'frame-src': ["'self'"],
+									'img-src': ["'self'", 'data:'],
+									'script-src': [
+										"'strict-dynamic'",
+										"'self'",
+										`'nonce-${nonce}'`,
+									],
+									'script-src-attr': [`'nonce-${nonce}'`],
+								},
+							},
+						},
+					})
+
 					resolve(
 						new Response(createReadableStreamFromReadable(body), {
 							headers: responseHeaders,

package-lock.json

@@ -53,6 +53,7 @@
     "@epic-web/totp": "^3.0.0",
     "@mjackson/form-data-parser": "^0.7.0",
     "@nasa-gcn/remix-seo": "^2.0.1",
+    "@nichtsam/helmet": "0.3.0",
     "@oslojs/crypto": "^1.0.1",
     "@oslojs/encoding": "^1.1.0",
     "@paralleldrive/cuid2": "^2.2.2",
@@ -89,7 +90,6 @@
     "express-rate-limit": "^7.5.0",
     "get-port": "^7.1.0",
     "glob": "^11.0.1",
-    "helmet": "^8.0.0",
     "input-otp": "^1.4.2",
     "intl-parse-accept-language": "^1.0.0",
     "isbot": "^5.1.22",

package.json

@@ -1,5 +1,6 @@
 import crypto from 'node:crypto'
 import { styleText } from 'node:util'
+import { helmet } from '@nichtsam/helmet/node-http'
 import { createRequestHandler } from '@react-router/express'
 import * as Sentry from '@sentry/node'
 import { ip as ipAddress } from 'address'
@@ -8,7 +9,6 @@ import compression from 'compression'
 import express from 'express'
 import rateLimit from 'express-rate-limit'
 import getPort, { portNumbers } from 'get-port'
-import helmet from 'helmet'
 import morgan from 'morgan'
 import { type ServerBuild } from 'react-router'
 
@@ -68,6 +68,12 @@ app.use(compression())
 // http://expressjs.com/en/advanced/best-practice-security.html#at-a-minimum-disable-x-powered-by-header
 app.disable('x-powered-by')
 
+app.use((_, res, next) => {
+	// The referrerPolicy breaks our redirectTo logic
+	helmet(res, { general: { referrerPolicy: false } })
+	next()
+})
+
 if (viteDevServer) {
 	app.use(viteDevServer.middlewares)
 } else {
@@ -110,39 +116,6 @@ app.use((_, res, next) => {
 	next()
 })
 
-app.use(
-	helmet({
-		xPoweredBy: false,
-		referrerPolicy: { policy: 'same-origin' },
-		crossOriginEmbedderPolicy: false,
-		contentSecurityPolicy: {
-			// NOTE: Remove reportOnly when you're ready to enforce this CSP
-			reportOnly: true,
-			directives: {
-				'connect-src': [
-					MODE === 'development' ? 'ws:' : null,
-					process.env.SENTRY_DSN ? '*.sentry.io' : null,
-					"'self'",
-				].filter(Boolean),
-				'font-src': ["'self'"],
-				'frame-src': ["'self'"],
-				'img-src': ["'self'", 'data:'],
-				'script-src': [
-					"'strict-dynamic'",
-					"'self'",
-					// @ts-expect-error
-					(_, res) => `'nonce-${res.locals.cspNonce}'`,
-				],
-				'script-src-attr': [
-					// @ts-expect-error
-					(_, res) => `'nonce-${res.locals.cspNonce}'`,
-				],
-				'upgrade-insecure-requests': null,
-			},
-		},
-	}),
-)
-
 // When running tests or running in development, we want to effectively disable
 // rate limiting because playwright tests are very fast and we don't want to
 // have to wait for the rate limit to reset between tests.