From 4dea4620bdcc9c1800db90d11f12796d6d87a03b Mon Sep 17 00:00:00 2001 From: thomas Date: Sun, 16 Aug 2026 16:59:00 +0200 Subject: [PATCH] fix(website): harden security following audit - Fix open redirect in safePath (backslash-normalization bypass) - Add security headers + CSP (server middleware and vercel.json for CDN-served pages) - Sanitize generated markdown HTML with DOMPurify before it reaches bypassSecurityTrustHtml - Enable TypeScript strict mode and strictTemplates - Warn when SITE_ORIGIN is unset in production (OAuth redirect_uri falls back to client headers) Co-Authored-By: Claude Fable 5 --- website/package.json | 3 +- website/pnpm-lock.yaml | 16 +++++++++ website/src/server.ts | 22 ++++++------ website/src/server/auth.ts | 14 +++++++- website/src/server/security-headers.ts | 37 +++++++++++++++++++ website/tools/generate-content.mjs | 49 +++++++++++++++++++++----- website/tsconfig.json | 4 ++- website/vercel.json | 18 +++++++++- 8 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 website/src/server/security-headers.ts diff --git a/website/package.json b/website/package.json index 117d0fe62..794a30256 100644 --- a/website/package.json +++ b/website/package.json @@ -38,6 +38,7 @@ "@tailwindcss/typography": "^0.5.20", "@types/express": "^5.0.1", "@types/node": "^20.17.19", + "dompurify": "^3.4.13", "github-slugger": "^2.0.0", "gray-matter": "^4.0.3", "jsdom": "^28.0.0", @@ -48,4 +49,4 @@ "typescript": "~6.0.2", "vitest": "^4.0.8" } -} \ No newline at end of file +} diff --git a/website/pnpm-lock.yaml b/website/pnpm-lock.yaml index 589754422..cc674a9d8 100644 --- a/website/pnpm-lock.yaml +++ b/website/pnpm-lock.yaml @@ -75,6 +75,9 @@ importers: '@types/node': specifier: ^20.17.19 version: 20.19.43 + dompurify: + specifier: ^3.4.13 + version: 3.4.13 github-slugger: specifier: ^2.0.0 version: 2.0.0 @@ -1511,6 +1514,9 @@ packages: '@types/serve-static@2.2.0': resolution: {integrity: sha512-8mam4H1NHLtu7nmtalF7eyBH14QyOASmcxHhSfEoRyr0nP/YdoesEtU+uSRvMe96TW/HPTtkoKqQLl53N7UXMQ==} + '@types/trusted-types@2.0.7': + resolution: {integrity: sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==} + '@types/unist@3.0.3': resolution: {integrity: sha512-ko/gIFJRv177XgZsZcBwnqJN5x/Gien8qNOn0D5bQU/zAzVf9Zt3BlcUiLqhV9y4ARk0GbT3tnUiPNgnTXzc/Q==} @@ -1778,6 +1784,9 @@ packages: resolution: {integrity: sha512-cgwlv/1iFQiFnU96XXgROh8xTeetsnJiDsTc7TYCLFd9+/WNkIqPTxiM/8pSd8VIrhXGTf1Ny1q1hquVqDJB5w==} engines: {node: '>= 4'} + dompurify@3.4.13: + resolution: {integrity: sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==} + domutils@3.2.2: resolution: {integrity: sha512-6kZKyUajlDuqlHKVX1w7gyslj9MPIXzIFiz/rGu35uC1wMi+kMhQwGhl4lt9unC9Vb9INnY9Z3/ZA3+FhASLaw==} @@ -4140,6 +4149,9 @@ snapshots: '@types/http-errors': 2.0.5 '@types/node': 20.19.43 + '@types/trusted-types@2.0.7': + optional: true + '@types/unist@3.0.3': {} '@ungap/structured-clone@1.3.3': {} @@ -4402,6 +4414,10 @@ snapshots: dependencies: domelementtype: 2.3.0 + dompurify@3.4.13: + optionalDependencies: + '@types/trusted-types': 2.0.7 + domutils@3.2.2: dependencies: dom-serializer: 2.0.0 diff --git a/website/src/server.ts b/website/src/server.ts index f1cf30179..f628102d7 100644 --- a/website/src/server.ts +++ b/website/src/server.ts @@ -8,6 +8,7 @@ import express from 'express'; import { join } from 'node:path'; import { githubApi } from './server/github-api'; import { authRoutes } from './server/auth'; +import { SECURITY_HEADERS } from './server/security-headers'; const browserDistFolder = join(import.meta.dirname, '../browser'); @@ -18,16 +19,15 @@ const angularApp = new AngularNodeAppEngine({ }); /** - * Example Express Rest API endpoints can be defined here. - * Uncomment and define endpoints as necessary. - * - * Example: - * ```ts - * app.get('/api/{*splat}', (req, res) => { - * // Handle API request - * }); - * ``` + * Security headers on every SSR response. Static/prerendered files served by + * the Vercel CDN get the same set from the `headers` block in vercel.json. */ +app.use((req, res, next) => { + for (const [name, value] of Object.entries(SECURITY_HEADERS)) { + res.setHeader(name, value); + } + next(); +}); /** * JSON API backed by the GitHub REST API (cached server-side). @@ -56,9 +56,7 @@ app.use( app.use((req, res, next) => { angularApp .handle(req) - .then((response) => - response ? writeResponseToNodeResponse(response, res) : next(), - ) + .then((response) => (response ? writeResponseToNodeResponse(response, res) : next())) .catch(next); }); diff --git a/website/src/server/auth.ts b/website/src/server/auth.ts index ec1c50aa2..e75506e6b 100644 --- a/website/src/server/auth.ts +++ b/website/src/server/auth.ts @@ -28,11 +28,21 @@ export function readAuthCookie(req: Request): string | null { * forwarded headers are client-supplied and must not decide the OAuth `redirect_uri` * nor whether cookies get the `Secure` attribute. */ +let warnedMissingOrigin = false; + function siteOrigin(req: Request): string { const configured = process.env['SITE_ORIGIN']; if (configured) { return configured.replace(/\/+$/, ''); } + if (!warnedMissingOrigin && process.env['NODE_ENV'] === 'production') { + warnedMissingOrigin = true; + console.warn( + 'SITE_ORIGIN is not set: falling back to client-supplied x-forwarded-* headers ' + + 'to build the OAuth redirect_uri and decide the Secure cookie flag. ' + + 'Set SITE_ORIGIN in production.', + ); + } const forwardedHost = String(req.headers['x-forwarded-host'] ?? '') .split(',')[0] .trim(); @@ -57,7 +67,9 @@ function safeEqual(a: string, b: string): boolean { /** Only same-site paths are allowed as post-login redirect targets. */ function safePath(value: unknown): string { - const path = String(value ?? '/'); + // Browsers treat `\` as `/` in the Location header, so `/\evil.com` would + // become a protocol-relative redirect to evil.com. Normalize before checking. + const path = String(value ?? '/').replace(/\\/g, '/'); return path.startsWith('/') && !path.startsWith('//') ? path : '/'; } diff --git a/website/src/server/security-headers.ts b/website/src/server/security-headers.ts new file mode 100644 index 000000000..53280b09d --- /dev/null +++ b/website/src/server/security-headers.ts @@ -0,0 +1,37 @@ +/** + * Security headers applied to every SSR response. Keep in sync with the + * `headers` block in vercel.json, which applies the same set to the static + * and prerendered pages served straight from the Vercel CDN. + * + * The CSP allow-list covers the third parties the site actually loads: + * Google Tag Manager / Analytics and AdSense (injected after consent, see + * src/app/consent.ts), the giscus comments iframe, GitHub avatars and the + * GitHub-hosted demo videos embedded in challenge docs. `unsafe-inline` for + * scripts is required by the two bootstrap scripts in index.html and + * Angular's hydration event-replay script; for styles by Angular's inlined + * component styles and shiki's inline color attributes. + */ +export const CONTENT_SECURITY_POLICY = [ + "default-src 'self'", + "script-src 'self' 'unsafe-inline' https://www.googletagmanager.com https://pagead2.googlesyndication.com https://googleads.g.doubleclick.net https://tpc.googlesyndication.com https://ep2.adtrafficquality.google https://giscus.app", + "style-src 'self' 'unsafe-inline'", + // https: because AdSense creatives load images from arbitrary Google CDNs. + "img-src 'self' data: https:", + "font-src 'self' data:", + "connect-src 'self' https://*.google-analytics.com https://*.analytics.google.com https://*.googletagmanager.com https://pagead2.googlesyndication.com https://ep1.adtrafficquality.google", + 'frame-src https://giscus.app https://googleads.g.doubleclick.net https://tpc.googlesyndication.com https://www.google.com https://ep2.adtrafficquality.google', + "media-src 'self' https://github.com https://user-images.githubusercontent.com https://private-user-images.githubusercontent.com", + "object-src 'none'", + "base-uri 'self'", + "form-action 'self' https://github.com", + "frame-ancestors 'self'", +].join('; '); + +export const SECURITY_HEADERS: Record = { + 'Content-Security-Policy': CONTENT_SECURITY_POLICY, + 'X-Content-Type-Options': 'nosniff', + 'X-Frame-Options': 'SAMEORIGIN', + 'Referrer-Policy': 'strict-origin-when-cross-origin', + 'Strict-Transport-Security': 'max-age=31536000', + 'Permissions-Policy': 'camera=(), microphone=(), geolocation=()', +}; diff --git a/website/tools/generate-content.mjs b/website/tools/generate-content.mjs index f8ef8077d..e0c907e0c 100644 --- a/website/tools/generate-content.mjs +++ b/website/tools/generate-content.mjs @@ -9,6 +9,8 @@ import matter from 'gray-matter'; import { Marked } from 'marked'; import { createHighlighter } from 'shiki'; import GithubSlugger from 'github-slugger'; +import createDOMPurify from 'dompurify'; +import { JSDOM } from 'jsdom'; const CONTENT_DIR = new URL('../src/content', import.meta.url).pathname; const OUT_DIR = new URL('../src/app/generated', import.meta.url).pathname; @@ -74,8 +76,19 @@ function resolveAuthor(slug) { } const SHIKI_LANGS = [ - 'typescript', 'javascript', 'html', 'css', 'json', 'bash', 'shell', - 'yaml', 'diff', 'angular-html', 'angular-ts', 'jsx', 'tsx', + 'typescript', + 'javascript', + 'html', + 'css', + 'json', + 'bash', + 'shell', + 'yaml', + 'diff', + 'angular-html', + 'angular-ts', + 'jsx', + 'tsx', ]; /** Light colors inline, dark colors in `--shiki-dark*` vars (see styles.css). */ @@ -89,6 +102,20 @@ const highlighter = await createHighlighter({ langs: SHIKI_LANGS, }); +/** + * The rendered HTML is injected with `bypassSecurityTrustHtml` (doc-page.ts), so + * Angular's sanitizer never sees it. marked passes raw HTML through untouched, + * which would let a `