diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 639006d..73b2de2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,9 @@ jobs: - run: npm ci + - name: Unit tests + run: npm run test:unit + - name: Typecheck and build run: npm run build diff --git a/package.json b/package.json index 381a29a..7a734e6 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "build": "node api.js && tsc -b && vite build", "preview": "vite preview", "test:smoke": "node scripts/smoke-test.mjs", + "test:unit": "node --test --experimental-strip-types src/lib/*.test.ts", "visual:check": "node scripts/visual-check.mjs", "typecheck": "tsc -b --pretty false" }, diff --git a/src/lib/redirect.test.ts b/src/lib/redirect.test.ts new file mode 100644 index 0000000..573b76c --- /dev/null +++ b/src/lib/redirect.test.ts @@ -0,0 +1,41 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +import { safeRedirectTarget } from './redirect.ts'; + +test('safeRedirectTarget accepts path-only targets', () => { + assert.equal(safeRedirectTarget('/issue/1'), '/issue/1'); + assert.equal(safeRedirectTarget('/explore?focus=1'), '/explore?focus=1'); +}); + +test('safeRedirectTarget rejects protocol-relative URLs', () => { + assert.equal(safeRedirectTarget('//example.com'), '/explore'); + assert.equal(safeRedirectTarget('///example.com'), '/explore'); +}); + +test('safeRedirectTarget rejects backslash-prefixed hosts', () => { + assert.equal(safeRedirectTarget('/\\example.com'), '/explore'); + assert.equal(safeRedirectTarget('/\\\\example.com'), '/explore'); +}); + +test('safeRedirectTarget rejects absolute URLs and schemes', () => { + assert.equal(safeRedirectTarget('https://example.com'), '/explore'); + assert.equal(safeRedirectTarget('http://example.com/path'), '/explore'); + assert.equal(safeRedirectTarget('javascript:alert(1)'), '/explore'); +}); + +test('safeRedirectTarget rejects login targets to avoid redirect loops', () => { + assert.equal(safeRedirectTarget('/login'), '/explore'); + assert.equal(safeRedirectTarget('/login?next=/issue/1'), '/explore'); +}); + +test('safeRedirectTarget falls back for missing or empty values', () => { + assert.equal(safeRedirectTarget(null), '/explore'); + assert.equal(safeRedirectTarget(undefined), '/explore'); + assert.equal(safeRedirectTarget(''), '/explore'); +}); + +test('safeRedirectTarget honours a custom fallback', () => { + assert.equal(safeRedirectTarget('//example.com', '/'), '/'); + assert.equal(safeRedirectTarget(null, '/'), '/'); +}); diff --git a/src/lib/redirect.ts b/src/lib/redirect.ts new file mode 100644 index 0000000..7518fd7 --- /dev/null +++ b/src/lib/redirect.ts @@ -0,0 +1,25 @@ +/** + * safeRedirectTarget validates a `next` redirect target coming from a sign-in + * URL before the app navigates to it. + * + * Only same-origin, path-only values are accepted: a single leading `/` not + * followed by `/` or `\`, and no scheme. Anything else falls back to the + * fallback destination, so the sign-in page cannot be used as an open + * redirect (`/login?next=//example.com`). + */ +export function safeRedirectTarget(next: string | null | undefined, fallback = '/explore'): string { + if (!next) return fallback; + + // A path-only target must start with a single `/`. + if (next[0] !== '/') return fallback; + + // `//example.com` is a protocol-relative URL and `/\example.com` is treated + // the same way by browsers that normalise backslashes in paths — both are + // cross-origin. + if (next[1] === '/' || next[1] === '\\') return fallback; + + // Keep the login page from redirecting to itself. + if (next.startsWith('/login')) return fallback; + + return next; +} diff --git a/src/pages/auth.tsx b/src/pages/auth.tsx index b2ed2df..f38c85f 100644 --- a/src/pages/auth.tsx +++ b/src/pages/auth.tsx @@ -5,6 +5,7 @@ import { ArrowRight, ShieldCheck } from 'lucide-react'; import { useApp } from '../lib/store'; import { Brand, EASE, Mark } from '../components/ui'; import { PLATFORM } from '../lib/platform'; +import { safeRedirectTarget } from '../lib/redirect'; function AuthFrame({ kicker, title, sub, children, foot, @@ -45,7 +46,7 @@ export function ContributorLogin() { const [params] = useSearchParams(); const [name, setName] = useState(state.session.contributor ?? ''); const next = params.get('next'); - const dest = next && next.startsWith('/') && !next.startsWith('/login') ? next : '/explore'; + const dest = safeRedirectTarget(next); const submit = (e: FormEvent) => { e.preventDefault(); diff --git a/tsconfig.app.json b/tsconfig.app.json index a35d693..f1a99b1 100644 --- a/tsconfig.app.json +++ b/tsconfig.app.json @@ -11,6 +11,7 @@ "forceConsistentCasingInFileNames": true, "module": "ESNext", "moduleResolution": "Bundler", + "allowImportingTsExtensions": true, "resolveJsonModule": true, "isolatedModules": true, "noEmit": true, @@ -18,5 +19,6 @@ "noUnusedLocals": true, "noUnusedParameters": true }, - "include": ["src"] + "include": ["src"], + "exclude": ["src/**/*.test.ts"] }