move away from express helmet

[nichtsam]

previously: #924
resolves: #920

Issue

The current implementation of Express Helmet applies security headers globally, resulting in HTML-specific headers, such as Content-Security-Policy and X-Download-Options, being applied to non-HTML routes.

Solution

This PR replaces Express Helmet with a custom package I developed, @nichtsam/helmet, which more effectively distinguishes between different layers of security headers.

Test Plan

Checklist

• Tests updated
• Docs updated

Screenshots

[kentcdodds]

I'm very excited about this. Thank you so much for taking on the task of maintaining this package. I'm looking forward to merging this. I just have a couple questions.

[kentcdodds]

This is a nitpick, but is there any chance you could add exports config in your package.json so that we don't have to dive deep into your package? Instead, this could be more like this for example:

Suggested change

	import { contentSecurity } from '@nichtsam/helmet/rules/content/index'
	import { contentSecurity } from '@nichtsam/helmet/content'

[nichtsam]

I also didn't like that, but didn't know how to path them better, thanks for the idea!
I will update the package to simplify the import path.

[kentcdodds]

Can you help me understand what this does, why we need this, and what's in the entry.server.tsx?

[nichtsam]

These lines ensure that all routes are protected, not just Remix routes.

When working on CSP and other security headers, I realized that some security headers are specific to the response type. Some apply to all types of resources, such as X-Content-Type-Options, Strict-Transport-Security, and Cross-Origin-Resource-Policy. Others are content-specific, typically for text/html, some (like CSP) can also apply to application/xml, application/pdf,image/svg+xml and more (see this).

To improve clarity and control, I categorized the headers into three groups:
1. General – Applied to all resources.
2. Content – Applied to content types.
3. Resource Sharing – Related to cross-origin policies.

Here, we apply the general security headers, ensuring that assets, JS files, and other resources are covered. In entry.server.tsx, we handle content-specific headers for text/html, such as CSP.

The main helmet works on Web Fetch API's Headers, this import is essentially a wrapper on it to work with http.ServerResponse.

I hope this is clear, feel free to ask for more details!

[kentcdodds]

Also, could you take a look at the Playwright test and make sure that those are passing?

[kentcdodds]

That's clear to me. Thanks so much!

[kentcdodds]

Sorry about the merge, commit. I'll wait for you to update the paths and things, and then we'll get this merged. Thanks a ton!

[kentcdodds]

Awesome! Thank you for this. Very very cool.

[kentcdodds]

Looks like we still need to have the playwright tests passing. I saw your issue and I’ve noticed some flakiness as well, though the CI normally passes. I would prefer to wait to merge this until we have a green CI on this PR.

[nichtsam]

Looks like we still need to have the playwright tests passing. I saw your issue and I’ve noticed some flakiness as well, though the CI normally passes. I would prefer to wait to merge this until we have a green CI on this PR.

Same here. I don’t think this should cause test failures, but I’m not too familiar with Playwright. I’ll need to find some time to look into it.

[kentcdodds]

Hmmm... I tried re-running the playwright test for this PR and it failed again. All the playwright tests on the main branch have passed without issue. I'm worried this change may be affecting the test outcome in some way so I'm not going to merge this until we resolve the test failures (which maybe involves finally solving #931).

[nichtsam]

Hmmm... I tried re-running the playwright test for this PR and it failed again. All the playwright tests on the main branch have passed without issue. I'm worried this change may be affecting the test outcome in some way so I'm not going to merge this until we resolve the test failures (which maybe involves finally solving #931).

No worries! The tests on the main branch are failing on my device as well. I’m not sure why they pass consistently on your side but not here—maybe I didn’t set up the environment correctly.

[nichtsam]

I realized the Referrer-Policy security header will cause redirectTo to behave differently, turned it off now.

epic-stack/app/routes/_auth+/auth_.$provider.ts

Lines 13 to 34 in fdeda42

	export async function action({ request, params }: Route.ActionArgs) {
	const providerName = ProviderNameSchema.parse(params.provider)
	try {
	await handleMockAction(providerName, request)
	return await authenticator.authenticate(providerName, request)
	} catch (error: unknown) {
	if (error instanceof Response) {
	const formData = await request.formData()
	const rawRedirectTo = formData.get('redirectTo')
	const redirectTo =
	typeof rawRedirectTo === 'string'
	? rawRedirectTo
	: getReferrerRoute(request)
	const redirectToCookie = getRedirectCookieHeader(redirectTo)
	if (redirectToCookie) {
	error.headers.append('set-cookie', redirectToCookie)
	}
	}
	throw error
	}
	}

[kentcdodds]

The tests all pass locally for me, so I'm going to go ahead and merge it. Thanks a lot!

[kentcdodds]

I absolutely love this, thank you so much!