diff --git a/app/routes.ts b/app/routes.ts index 6a2d3786..d549ea8e 100644 --- a/app/routes.ts +++ b/app/routes.ts @@ -176,4 +176,5 @@ export default [ route('users', 'features/platform-admin/routes/users.tsx'), ]), ]), + route('*', 'shell/not-found.tsx'), ] satisfies RouteConfig diff --git a/app/shell/not-found.test.ts b/app/shell/not-found.test.ts new file mode 100644 index 00000000..a74393e0 --- /dev/null +++ b/app/shell/not-found.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' + +import { action, loader } from './not-found' + +describe('not-found route', () => { + it.each([ + ['loader', loader], + ['action', action], + ])('answers 404 from its %s', (_name, handler) => { + let thrown: unknown + try { + handler() + } catch (error) { + thrown = error + } + + expect(thrown).toBeInstanceOf(Response) + expect((thrown as Response).status).toBe(404) + }) +}) diff --git a/app/shell/not-found.tsx b/app/shell/not-found.tsx new file mode 100644 index 00000000..432b5b20 --- /dev/null +++ b/app/shell/not-found.tsx @@ -0,0 +1,22 @@ +// Catch-all, registered last in app/routes.ts. When no route matches, React Router answers the 404 +// before any middleware runs, so the page left without the security headers (and its scripts +// without the CSP nonce) that the root route gives every other response. Matching here runs the +// root middleware and loader; the root ErrorBoundary still renders the page. +function notFound(): never { + throw new Response(null, { status: 404, statusText: 'Not Found' }) +} + +export function loader() { + notFound() +} + +// Without it, a form posted to an unknown path would answer 405 instead of 404. +export function action() { + notFound() +} + +// Never rendered. Without a component this would be a resource route, and the 404 would leave as +// an empty response instead of the root ErrorBoundary's page. +export default function NotFound() { + return null +} diff --git a/app/tests/e2e/not-found.spec.ts b/app/tests/e2e/not-found.spec.ts new file mode 100644 index 00000000..aa7629eb --- /dev/null +++ b/app/tests/e2e/not-found.spec.ts @@ -0,0 +1,21 @@ +import { expect, test } from '@playwright/test' + +test.describe('Unknown path', () => { + // Regression: when no route matched, no middleware ran, so the 404 page left without the + // security headers every other response carries. + test('answers 404 with the security headers', async ({ request }) => { + const response = await request.get('/this-path-does-not-exist') + + expect(response.status()).toBe(404) + expect(response.headers()['x-frame-options']).toBe('DENY') + expect(response.headers()['x-content-type-options']).toBe('nosniff') + expect(response.headers()['content-security-policy-report-only']).toContain("default-src 'self'") + }) + + test('still renders the not-found page', async ({ page }) => { + const response = await page.goto('/this-path-does-not-exist') + + expect(response?.status()).toBe(404) + await expect(page.getByText('404')).toBeVisible() + }) +})