fix(url): harden redirect path validation

Add safe redirect path helpers and reject protocol-relative or backslash-based redirect targets before appending redirect parameters.

Tested: pnpm run lint && pnpm run typecheck && pnpm test tests/url.test.ts

Author: kiki-kanri

src/url.ts

@@ -5,18 +5,15 @@
  *
  * @param {string} url - The target URL to modify
  * @param {string} redirectPath - The path to use as the redirect destination
- * (must be an absolute path starting with '/')
+ * (must be a safe same-origin application path starting with `/`)
  *
  * @returns {string} A new URL string with the `redirect` query parameter
  *
- * @throws {Error} If redirectPath is not a valid relative path (must start with '/')
+ * @throws {Error} If redirectPath is not a safe same-origin application path
  */
 export function appendRedirectParamToUrl(url: string, redirectPath: string) {
-    if (!redirectPath.startsWith('/')) {
-        throw new Error(
-            `Invalid redirect path: "${redirectPath}". Redirect paths must be absolute paths starting with '/'.`,
-        );
-    }
+    // eslint-disable-next-line style/max-len
+    if (!isSafeRedirectPath(redirectPath)) throw new Error(`Invalid redirect path: "${redirectPath}". Redirect paths must be safe same-origin paths starting with '/'.`);
 
     const [baseAndHash, rawQuery = ''] = url.split('?');
     const [base, hash] = (baseAndHash || '').split('#');
@@ -25,3 +22,38 @@ export function appendRedirectParamToUrl(url: string, redirectPath: string) {
     const queryString = searchParams.toString();
     return hash ? `${base}?${queryString}#${hash}` : `${base}?${queryString}`;
 }
+
+/**
+ * Returns whether a value is a safe same-origin redirect path.
+ *
+ * Safe redirect paths are absolute application paths such as `/dashboard`.
+ * Protocol-relative URLs (`//example.com`), absolute URLs, backslash paths,
+ * and non-string values are rejected.
+ *
+ * @param {unknown} value - The value to check
+ *
+ * @returns {boolean} Whether the value is safe to use as an application redirect path
+ */
+export function isSafeRedirectPath(value: unknown): value is string {
+    if (typeof value !== 'string') return false;
+    if (!value.startsWith('/')) return false;
+    if (value.startsWith('//')) return false;
+    if (value.includes('\\')) return false;
+    return true;
+}
+
+/**
+ * Normalizes a redirect value into a safe same-origin application path.
+ *
+ * If an array is provided, the first value is checked. Unsafe values fall back
+ * to the provided fallback path.
+ *
+ * @param {unknown} value - The redirect value to normalize
+ * @param {string} fallback - The safe fallback path
+ *
+ * @returns {string} A safe redirect path
+ */
+export function normalizeRedirectPath(value: unknown, fallback = '/') {
+    const redirectPath = Array.isArray(value) ? value[0] : value;
+    return isSafeRedirectPath(redirectPath) ? redirectPath : fallback;
+}

tests/url.test.ts

@@ -3,7 +3,47 @@ import {
     it,
 } from 'vitest';
 
-import { appendRedirectParamToUrl } from '../src/url';
+import {
+    appendRedirectParamToUrl,
+    isSafeRedirectPath,
+    normalizeRedirectPath,
+} from '../src/url';
+
+describe.concurrent('isSafeRedirectPath', () => {
+    it('should accept same-origin application paths', ({ expect }) => {
+        expect(isSafeRedirectPath('/')).toBe(true);
+        expect(isSafeRedirectPath('/dashboard')).toBe(true);
+        expect(isSafeRedirectPath('/dashboard?tab=home#section')).toBe(true);
+    });
+
+    it('should reject unsafe redirect paths', ({ expect }) => {
+        expect(isSafeRedirectPath('')).toBe(false);
+        expect(isSafeRedirectPath('dashboard')).toBe(false);
+        expect(isSafeRedirectPath('https://evil.com')).toBe(false);
+        expect(isSafeRedirectPath('http://evil.com')).toBe(false);
+        expect(isSafeRedirectPath('//evil.com')).toBe(false);
+        expect(isSafeRedirectPath('/\\evil.com')).toBe(false);
+        expect(isSafeRedirectPath(123)).toBe(false);
+        expect(isSafeRedirectPath(undefined)).toBe(false);
+    });
+});
+
+describe.concurrent('normalizeRedirectPath', () => {
+    it('should return safe redirect paths', ({ expect }) => {
+        expect(normalizeRedirectPath('/dashboard')).toBe('/dashboard');
+        expect(normalizeRedirectPath([
+            '/dashboard',
+            '//evil.com',
+        ])).toBe('/dashboard');
+    });
+
+    it('should return fallback for unsafe redirect paths', ({ expect }) => {
+        expect(normalizeRedirectPath('//evil.com')).toBe('/');
+        expect(normalizeRedirectPath('https://evil.com')).toBe('/');
+        expect(normalizeRedirectPath('/\\evil.com')).toBe('/');
+        expect(normalizeRedirectPath(undefined, '/fallback')).toBe('/fallback');
+    });
+});
 
 describe.concurrent('appendRedirectParamToUrl', () => {
     it('should append redirect param to URL with no existing query', ({ expect }) => {
@@ -35,6 +75,14 @@ describe.concurrent('appendRedirectParamToUrl', () => {
         expect(() => appendRedirectParamToUrl('/login', 'http://evil.com')).toThrow();
     });
 
+    it('should throw error for protocol-relative URL as redirectPath', ({ expect }) => {
+        expect(() => appendRedirectParamToUrl('/login', '//evil.com')).toThrow();
+    });
+
+    it('should throw error for backslash redirectPath', ({ expect }) => {
+        expect(() => appendRedirectParamToUrl('/login', '/\\evil.com')).toThrow();
+    });
+
     it('should handle URL starting with question mark', ({ expect }) => {
         const result = appendRedirectParamToUrl('?foo=bar', '/dashboard');
         expect(result).toBe('?foo=bar&redirect=%2Fdashboard');