fix(admin): detect redirect loops

.changeset/redirect-loop-detection.md

@@ -0,0 +1,6 @@
+---
+"@emdash-cms/admin": patch
+"emdash": patch
+---
+
+Fixes redirect loops causing the ERR_TOO_MANY_REDIRECTS error, by detecting circular chains when creating or editing redirects on the admin Redirects page.

e2e/tests/redirect-loop-detection.spec.ts

@@ -0,0 +1,300 @@
+/**
+ * Redirect Loop Detection E2E Tests
+ *
+ * Tests write-time loop prevention, pattern-aware detection,
+ * cache behavior, and admin UI warnings.
+ */
+
+import { test, expect } from "../fixtures";
+
+function apiHeaders(token: string, baseUrl: string) {
+	return {
+		"Content-Type": "application/json",
+		Authorization: `Bearer ${token}`,
+		"X-EmDash-Request": "1",
+		Origin: baseUrl,
+	};
+}
+
+/** Create a redirect via API, return the id */
+async function create(
+	page: import("@playwright/test").Page,
+	baseUrl: string,
+	token: string,
+	source: string,
+	destination: string,
+	options?: { enabled?: boolean },
+): Promise<string> {
+	const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, {
+		headers: apiHeaders(token, baseUrl),
+		data: { source, destination, ...options },
+	});
+	const body = await res.json();
+	if (!res.ok()) {
+		return body.error?.message ?? "unknown error";
+	}
+	return body.data.id;
+}
+
+/** Try to create a redirect, expect rejection. Return error message. */
+async function createExpectError(
+	page: import("@playwright/test").Page,
+	baseUrl: string,
+	token: string,
+	source: string,
+	destination: string,
+): Promise<string> {
+	const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, {
+		headers: apiHeaders(token, baseUrl),
+		data: { source, destination },
+	});
+	expect(res.ok(), `Expected rejection for ${source} → ${destination}`).toBe(false);
+	const body = await res.json();
+	return body.error?.message ?? "unknown error";
+}
+
+/** Try to create a redirect, expect success */
+async function createExpectSuccess(
+	page: import("@playwright/test").Page,
+	baseUrl: string,
+	token: string,
+	source: string,
+	destination: string,
+): Promise<void> {
+	const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, {
+		headers: apiHeaders(token, baseUrl),
+		data: { source, destination },
+	});
+	const body = await res.json();
+	expect(res.ok(), `Expected success for ${source} → ${destination}: ${JSON.stringify(body)}`).toBe(
+		true,
+	);
+}
+
+/** Delete all redirects */
+async function cleanup(
+	page: import("@playwright/test").Page,
+	baseUrl: string,
+	token: string,
+): Promise<void> {
+	const headers = apiHeaders(token, baseUrl);
+	const res = await page.request.get(`${baseUrl}/_emdash/api/redirects`, { headers });
+	if (!res.ok()) return;
+	const data = await res.json();
+	for (const item of data.data?.items ?? []) {
+		await page.request.delete(`${baseUrl}/_emdash/api/redirects/${item.id}`, { headers });
+	}
+}
+
+test.describe("redirect loop detection", () => {
+	test.beforeEach(async ({ admin, page, serverInfo }) => {
+		await admin.devBypassAuth();
+		await cleanup(page, serverInfo.baseUrl, serverInfo.token);
+	});
+
+	test.afterEach(async ({ page, serverInfo }) => {
+		await cleanup(page, serverInfo.baseUrl, serverInfo.token);
+	});
+
+	// -----------------------------------------------------------------------
+	// Pattern template loops
+	// -----------------------------------------------------------------------
+
+	test("rejects matching pattern template loop: [...path]", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]");
+		const msg = await createExpectError(page, baseUrl, token, "/new/[...path]", "/old/[...path]");
+		expect(msg).toContain("loop");
+	});
+
+	// -----------------------------------------------------------------------
+	// Admin UI warnings
+	// -----------------------------------------------------------------------
+
+	test("admin UI shows no loop banner when no loops exist", async ({ admin, page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/one", "/two");
+		await createExpectSuccess(page, baseUrl, token, "/two", "/three");
+
+		await admin.goto("/redirects");
+		await admin.waitForShell();
+		await admin.waitForLoading();
+
+		await expect(page.locator("text=Redirect loop detected")).toBeHidden();
+	});
+
+	// -----------------------------------------------------------------------
+	// Error message format
+	// -----------------------------------------------------------------------
+
+	test("error message shows template names, not __p__ dummy values", async ({
+		page,
+		serverInfo,
+	}) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]");
+		const msg = await createExpectError(page, baseUrl, token, "/articles/hello", "/blog/hello");
+		expect(msg).not.toContain("__p__");
+		expect(msg).toContain("/articles/hello");
+		expect(msg).toContain("/blog/hello");
+	});
+
+	// -----------------------------------------------------------------------
+	// Update-time loop detection
+	// -----------------------------------------------------------------------
+
+	test("rejects update that would create a loop", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		await createExpectSuccess(page, baseUrl, token, "/a", "/b");
+		await createExpectSuccess(page, baseUrl, token, "/b", "/c");
+		const id = await create(page, baseUrl, token, "/c", "/d");
+
+		const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { destination: "/a" },
+		});
+		expect(res.ok()).toBe(false);
+		const body = await res.json();
+		expect(body.error?.message).toContain("loop");
+	});
+
+	test("allows update that does not create a loop", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		await createExpectSuccess(page, baseUrl, token, "/a", "/b");
+		const id = await create(page, baseUrl, token, "/b", "/c");
+
+		const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { destination: "/d" },
+		});
+		expect(res.ok()).toBe(true);
+	});
+
+	test("rejects update changing both source and destination to create a loop", async ({
+		page,
+		serverInfo,
+	}) => {
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		await createExpectSuccess(page, baseUrl, token, "/a", "/b");
+		const id = await create(page, baseUrl, token, "/x", "/y");
+
+		const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { source: "/b", destination: "/a" },
+		});
+		expect(res.ok()).toBe(false);
+		const body = await res.json();
+		expect(body.error?.message).toContain("loop");
+	});
+
+	test("rejects update changing destination + enabling simultaneously", async ({
+		page,
+		serverInfo,
+	}) => {
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		await createExpectSuccess(page, baseUrl, token, "/a", "/b");
+		const id = await create(page, baseUrl, token, "/b", "/safe", { enabled: false });
+
+		const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { destination: "/a", enabled: true },
+		});
+		expect(res.ok()).toBe(false);
+		const body = await res.json();
+		expect(body.error?.message).toContain("loop");
+	});
+
+	// -----------------------------------------------------------------------
+	// Edge cases
+	// -----------------------------------------------------------------------
+
+	test("disabled redirect does not participate in loop detection", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		const id = await create(page, baseUrl, token, "/a", "/b");
+		await createExpectSuccess(page, baseUrl, token, "/b", "/c");
+
+		await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { enabled: false },
+		});
+
+		await createExpectSuccess(page, baseUrl, token, "/c", "/a");
+	});
+
+	test("re-enabling a disabled redirect that creates a loop is allowed", async ({
+		page,
+		serverInfo,
+	}) => {
+		// Users who had redirects before upgrade should be able to toggle
+		// them freely. The warning banner alerts them to the loop.
+		const { baseUrl, token } = serverInfo;
+		const headers = apiHeaders(token, baseUrl);
+		await createExpectSuccess(page, baseUrl, token, "/a", "/b");
+		await createExpectSuccess(page, baseUrl, token, "/b", "/c");
+		const id = await create(page, baseUrl, token, "/c", "/a", { enabled: false });
+
+		const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, {
+			headers,
+			data: { enabled: true },
+		});
+		expect(res.ok()).toBe(true);
+	});
+
+	// -----------------------------------------------------------------------
+	// Advanced pattern combinations
+	// -----------------------------------------------------------------------
+
+	test("rejects pattern with different param names that still loops", async ({
+		page,
+		serverInfo,
+	}) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]");
+		const msg = await createExpectError(page, baseUrl, token, "/articles/[id]", "/blog/[id]");
+		expect(msg).toContain("loop");
+	});
+
+	test("rejects catch-all loop even with deep nesting", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/v1/[...path]", "/v2/[...path]");
+		const msg = await createExpectError(
+			page,
+			baseUrl,
+			token,
+			"/v2/api/users/[slug]",
+			"/v1/api/users/[slug]",
+		);
+		expect(msg).toContain("loop");
+	});
+
+	test("multiple overlapping catch-alls: more specific loops back", async ({
+		page,
+		serverInfo,
+	}) => {
+		const { baseUrl, token } = serverInfo;
+		await createExpectSuccess(page, baseUrl, token, "/a/[...path]", "/b/[...path]");
+		await createExpectSuccess(page, baseUrl, token, "/a/sub/[...path]", "/c/[...path]");
+		const msg = await createExpectError(page, baseUrl, token, "/c/[...path]", "/a/sub/[...path]");
+		expect(msg).toContain("loop");
+	});
+
+	// -----------------------------------------------------------------------
+	// Long chains (20+)
+	// -----------------------------------------------------------------------
+
+	test("rejects loop at the end of a 25-redirect chain", async ({ page, serverInfo }) => {
+		const { baseUrl, token } = serverInfo;
+		for (let i = 1; i <= 24; i++) {
+			await createExpectSuccess(page, baseUrl, token, `/r${i}`, `/r${i + 1}`);
+		}
+		const msg = await createExpectError(page, baseUrl, token, "/r25", "/r1");
+		expect(msg).toContain("loop");
+		expect(msg).toContain("/r1");
+		expect(msg).toContain("/r25");
+	});
+});

packages/admin/src/components/DialogError.tsx

@@ -20,9 +20,15 @@ export function DialogError({
 	className?: string;
 }) {
 	if (!message) return null;
+	const lines = message.split("\n");
 	return (
-		<div className={cn("rounded-md bg-kumo-danger/10 p-3 text-sm text-kumo-danger", className)}>
-			{message}
+		<div
+			role="alert"
+			className={cn("rounded-md bg-kumo-danger/10 p-3 text-sm text-kumo-danger", className)}
+		>
+			{lines.map((line, i) => (
+				<div key={i}>{line}</div>
+			))}
 		</div>
 	);
 }

packages/admin/src/components/Redirects.tsx

@@ -6,6 +6,7 @@ import {
 	ArrowsLeftRight,
 	Trash,
 	PencilSimple,
+	WarningCircle,
 	X,
 } from "@phosphor-icons/react";
 import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
@@ -314,6 +315,7 @@ export function Redirects() {
 	}
 
 	const redirects = redirectsQuery.data?.items ?? [];
+	const loopRedirectIds = new Set(redirectsQuery.data?.loopRedirectIds ?? []);
 
 	return (
 		<div className="space-y-6">
@@ -397,6 +399,29 @@ export function Redirects() {
 						</select>
 					</div>
 
+					{/* Loop warning banner */}
+					{loopRedirectIds.size > 0 && (
+						<div
+							role="alert"
+							className="flex items-start gap-3 rounded-lg border border-kumo-warning/50 bg-kumo-warning-tint p-4"
+						>
+							<WarningCircle
+								size={20}
+								className="mt-0.5 shrink-0 text-kumo-warning"
+								weight="fill"
+								aria-hidden="true"
+							/>
+							<div>
+								<p className="text-sm font-medium text-kumo-warning">Redirect loop detected</p>
+								<p className="mt-1 text-sm text-kumo-subtle">
+									{loopRedirectIds.size}{" "}
+									{loopRedirectIds.size === 1 ? "redirect is" : "redirects are"} part of a loop.
+									Visitors hitting these paths will see an error.
+								</p>
+							</div>
+						</div>
+					)}
+
 					{/* Redirect list */}
 					{redirectsQuery.isLoading ? (
 						<div className="py-12 text-center text-kumo-subtle">Loading redirects...</div>
@@ -451,6 +476,17 @@ export function Redirects() {
 										/>
 									</div>
 									<div className="w-20 flex items-center justify-end gap-1">
+										{loopRedirectIds.has(r.id) && (
+											<span title="Part of a redirect loop" className="mr-1 inline-flex">
+												<WarningCircle
+													size={14}
+													weight="fill"
+													className="text-kumo-warning"
+													role="img"
+													aria-label="Part of a redirect loop"
+												/>
+											</span>
+										)}
 										{r.auto && (
 											<Badge variant="outline" className="mr-1 text-xs">
 												auto

packages/admin/src/lib/api/redirects.ts

@@ -54,6 +54,7 @@ export interface RedirectListOptions {
 export interface RedirectListResult {
 	items: Redirect[];
 	nextCursor?: string;
+	loopRedirectIds?: string[];
 }
 
 /**