Stop sending X-Frame-Options with the embed frame-ancestors carve-out

next.config.js

@@ -7,7 +7,11 @@ const nextConfig = {
   async headers() {
     return [
       {
-        source: "/:path*",
+        // Every route except /embed. The blanket rule has to leave /embed out: a later, more
+        // specific rule cannot remove a header the earlier one set, and /embed was being
+        // served with both `X-Frame-Options: DENY` and `frame-ancestors *`. Browsers that
+        // let frame-ancestors win framed it; any that honoured DENY would not.
+        source: "/((?!embed(?:/|$)).*)",
         headers: [
           { key: "X-Content-Type-Options", value: "nosniff" },
           { key: "Referrer-Policy", value: "strict-origin-when-cross-origin" },
@@ -28,8 +32,10 @@ const nextConfig = {
         // page meant to be framed by other sites. It renders no account
         // state - public map data only - so allowing any origin to frame it
         // isn't a privacy or clickjacking risk the way framing an
-        // authenticated page would be. A more specific `source` here takes
-        // precedence over the blanket DENY above for the same header.
+        // authenticated page would be. The rule above skips this path, so no
+        // X-Frame-Options header is sent and `frame-ancestors` is the only
+        // framing policy. Anything added to /embed that reads account state
+        // needs a review of this carve-out first (see #311).
         source: "/embed/:path*",
         headers: [
           { key: "X-Content-Type-Options", value: "nosniff" },

src/components/layout/navbar.test.tsx

@@ -0,0 +1,54 @@
+import { cleanup, render } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { Navbar } from "@/components/layout/navbar";
+
+const { pathname, createClient, getUser } = vi.hoisted(() => ({
+  pathname: { current: "/map" },
+  createClient: vi.fn(),
+  getUser: vi.fn(),
+}));
+
+vi.mock("next/navigation", () => ({
+  usePathname: () => pathname.current,
+  useRouter: () => ({ refresh: vi.fn(), push: vi.fn() }),
+}));
+vi.mock("next/dynamic", () => ({ default: () => () => null }));
+vi.mock("@/lib/supabase/client", () => ({ createClient }));
+vi.mock("@/lib/supabase/config", () => ({ isSupabaseConfigured: () => true }));
+vi.mock("@/components/layout/theme-toggle", () => ({ ThemeToggle: () => null }));
+
+beforeEach(() => {
+  getUser.mockReset().mockResolvedValue({ data: { user: null } });
+  createClient.mockReset().mockReturnValue({
+    auth: {
+      getUser,
+      onAuthStateChange: () => ({ data: { subscription: { unsubscribe: vi.fn() } } }),
+    },
+  });
+});
+
+afterEach(cleanup);
+
+describe("Navbar auth lookup", () => {
+  it("asks Supabase who the visitor is on a normal page", () => {
+    pathname.current = "/map";
+    const { container } = render(<Navbar />);
+
+    expect(createClient).toHaveBeenCalled();
+    expect(getUser).toHaveBeenCalled();
+    expect(container.querySelector("header")).not.toBeNull();
+  });
+
+  it.each(["/embed", "/embed/anything"])(
+    "renders nothing and never touches Supabase on %s (the third-party iframe)",
+    (path) => {
+      pathname.current = path;
+      const { container } = render(<Navbar />);
+
+      expect(container.innerHTML).toBe("");
+      expect(createClient).not.toHaveBeenCalled();
+      expect(getUser).not.toHaveBeenCalled();
+    },
+  );
+});

src/components/layout/navbar.tsx

@@ -41,6 +41,9 @@ export function Navbar() {
   const [searchLoaded, setSearchLoaded] = React.useState(false);
 
   const authEnabled = isSupabaseConfigured();
+  // The embeddable map widget renders chrome-free in a third-party iframe - no nav, no
+  // sign-in, nothing account-aware. See /docs/embedding-widget.
+  const isEmbed = pathname.startsWith("/embed");
 
   function openSearch() {
     setSearchLoaded(true);
@@ -69,6 +72,8 @@ export function Navbar() {
   }, []);
 
   React.useEffect(() => {
+    // Nothing on /embed shows login state, so do not ask Supabase about the visitor at all.
+    if (isEmbed) return;
     const supabase = createClient();
     if (!supabase) return;
     supabase.auth
@@ -79,7 +84,7 @@ export function Navbar() {
       data: { subscription },
     } = supabase.auth.onAuthStateChange((_, session) => setLoggedIn(!!session));
     return () => subscription.unsubscribe();
-  }, []);
+  }, [isEmbed]);
 
   async function handleSignOut() {
     const supabase = createClient();
@@ -91,9 +96,7 @@ export function Navbar() {
   const isActive = (href: string) =>
     href === "/" ? pathname === "/" : pathname.startsWith(href);
 
-  // The embeddable map widget renders chrome-free in a third-party iframe -
-  // no nav, no sign-in, nothing account-aware. See /docs/embedding-widget.
-  if (pathname.startsWith("/embed")) return null;
+  if (isEmbed) return null;
 
   return (
     <header className="fixed top-0 inset-x-0 z-[1100] w-full border-b bg-background/95 backdrop-blur-sm supports-[backdrop-filter]:bg-background/80">

src/lib/embed-frame-policy.test.ts

@@ -0,0 +1,80 @@
+import { readFileSync } from "node:fs";
+import path from "node:path";
+import { createRequire } from "node:module";
+import { describe, expect, it } from "vitest";
+
+// Guards the /embed carve-out reviewed in #311: it is the one route other sites may frame, so
+// it must not grow account state, and its framing headers must not contradict each other.
+
+const require = createRequire(import.meta.url);
+const root = path.resolve(__dirname, "../..");
+const nextConfig = require("../../next.config.js") as {
+  headers: () => Promise<{ source: string; headers: { key: string; value: string }[] }[]>;
+};
+
+const headerNames = (rule: { headers: { key: string }[] }) =>
+  rule.headers.map((h) => h.key.toLowerCase());
+
+/** Does a Next.js `source` pattern like `/((?!embed(?:/|$)).*)` match `pathname`? */
+function matchesBlanket(source: string, pathname: string) {
+  const inner = source.replace(/^\//, "");
+  return new RegExp(`^${inner}$`).test(pathname.replace(/^\//, ""));
+}
+
+describe("next.config.js framing headers", () => {
+  it("denies framing everywhere except /embed", async () => {
+    const rules = await nextConfig.headers();
+    const blanket = rules.find((r) => headerNames(r).includes("x-frame-options"));
+
+    expect(blanket).toBeDefined();
+    expect(blanket!.headers.find((h) => h.key === "X-Frame-Options")?.value).toBe("DENY");
+
+    for (const pathname of [
+      "/",
+      "/map",
+      "/login",
+      "/docs/embedding-widget",
+      "/embedded-things",
+    ]) {
+      expect(matchesBlanket(blanket!.source, pathname), pathname).toBe(true);
+    }
+    for (const pathname of ["/embed", "/embed/anything"]) {
+      expect(matchesBlanket(blanket!.source, pathname), pathname).toBe(false);
+    }
+  });
+
+  it("lets /embed be framed with frame-ancestors only, never together with X-Frame-Options", async () => {
+    const rules = await nextConfig.headers();
+    const embed = rules.find((r) => r.source === "/embed/:path*");
+
+    expect(embed).toBeDefined();
+    expect(embed!.headers.find((h) => h.key === "Content-Security-Policy")?.value).toBe(
+      "frame-ancestors *",
+    );
+    expect(headerNames(embed!)).not.toContain("x-frame-options");
+  });
+});
+
+describe("/embed renders no account state", () => {
+  const files = [
+    "src/app/embed/page.tsx",
+    "src/components/map/embed-map.tsx",
+    "src/components/map/embed-widget.tsx",
+  ];
+
+  it.each(files)("%s does not touch Supabase, auth or user data", (file) => {
+    const source = readFileSync(path.join(root, file), "utf8");
+    const imports = source
+      .split("\n")
+      .filter(
+        (line) =>
+          /^\s*(import|export)\b.*\bfrom\b/.test(line) || /^\s*import\s+["']/.test(line),
+      )
+      .join("\n");
+
+    expect(imports).not.toMatch(/supabase/i);
+    expect(imports).not.toMatch(
+      /\bauth\b|AuthProvider|use-user|user-places|user-profile/i,
+    );
+  });
+});