Merge branch 'worktree-agent-a70be2e5'

src/__tests__/hooks/use-all-user-movies.test.ts

@@ -0,0 +1,122 @@
+/**
+ * @vitest-environment jsdom
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { renderHook, waitFor } from "@testing-library/react";
+import React from "react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+
+// Track every method call against the supabase client so we can assert on
+// the exact filter chain the hook constructs.
+type Call = { method: string; args: unknown[] };
+const calls: Call[] = [];
+
+let mockRows: Array<Record<string, unknown>> = [];
+let mockUserId: string | null = "user-1";
+
+function makeQueryBuilder() {
+  const builder: Record<string, unknown> = {};
+  const record = (method: string) =>
+    vi.fn((...args: unknown[]) => {
+      calls.push({ method, args });
+      return builder;
+    });
+
+  builder.select = record("select");
+  builder.eq = record("eq");
+  builder.in = record("in");
+  builder.order = record("order");
+  // The query is awaited; PostgREST builders are thenable.
+  builder.then = (resolve: (v: { data: unknown; error: null }) => unknown) =>
+    resolve({ data: mockRows, error: null });
+  return builder;
+}
+
+const fromMock = vi.fn(() => makeQueryBuilder());
+
+vi.mock("@/lib/supabase/client", () => ({
+  getSupabaseBrowserClient: () => ({
+    from: fromMock,
+    auth: {
+      getUser: async () => ({ data: { user: mockUserId ? { id: mockUserId } : null } }),
+      onAuthStateChange: () => ({
+        data: { subscription: { unsubscribe: () => {} } },
+      }),
+    },
+  }),
+}));
+
+import { useAllUserMovies } from "@/hooks/use-all-user-movies";
+
+function wrapper({ children }: { children: React.ReactNode }) {
+  const client = new QueryClient({
+    defaultOptions: { queries: { retry: false, gcTime: 0 } },
+  });
+  return React.createElement(QueryClientProvider, { client }, children);
+}
+
+beforeEach(() => {
+  calls.length = 0;
+  fromMock.mockClear();
+  mockRows = [];
+  mockUserId = "user-1";
+});
+
+describe("useAllUserMovies", () => {
+  it("queries movies via the browser client and never filters by a client-supplied group list", async () => {
+    mockRows = [
+      { id: "m1", tmdb_id: 100, group_id: "g1", title: "A", watched: false },
+      { id: "m2", tmdb_id: 200, group_id: "g2", title: "B", watched: false },
+    ];
+
+    const { result } = renderHook(() => useAllUserMovies(), { wrapper });
+
+    await waitFor(() => expect(result.current.isSuccess).toBe(true));
+
+    // Hits the movies table directly.
+    expect(fromMock).toHaveBeenCalledWith("movies");
+
+    // Filters on watched=false, but NEVER on group_id with a client list.
+    const eqCalls = calls.filter((c) => c.method === "eq");
+    expect(eqCalls).toContainEqual({ method: "eq", args: ["watched", false] });
+    for (const c of eqCalls) {
+      expect(c.args[0]).not.toBe("group_id");
+    }
+
+    // Critically: no .in("group_id", [...]) filter (would mean the client
+    // is supplying the group list instead of trusting RLS).
+    const inCalls = calls.filter((c) => c.method === "in");
+    for (const c of inCalls) {
+      expect(c.args[0]).not.toBe("group_id");
+    }
+  });
+
+  it("returns an empty array when the supabase client returns no rows (simulating RLS rejecting a forged group id)", async () => {
+    // Simulate RLS filtering: the user's auth.uid() doesn't match any
+    // group_members row for the requested data, so the server returns [].
+    mockRows = [];
+
+    const { result } = renderHook(() => useAllUserMovies(), { wrapper });
+
+    await waitFor(() => expect(result.current.isSuccess).toBe(true));
+    expect(result.current.data).toEqual([]);
+  });
+
+  it("dedupes results by tmdb_id, keeping the first occurrence", async () => {
+    mockRows = [
+      { id: "m1", tmdb_id: 100, group_id: "g1", title: "First", watched: false },
+      { id: "m2", tmdb_id: 100, group_id: "g2", title: "Duplicate", watched: false },
+      { id: "m3", tmdb_id: 200, group_id: "g1", title: "Other", watched: false },
+      { id: "m4", tmdb_id: 200, group_id: "g3", title: "Another dup", watched: false },
+    ];
+
+    const { result } = renderHook(() => useAllUserMovies(), { wrapper });
+
+    await waitFor(() => expect(result.current.isSuccess).toBe(true));
+
+    expect(result.current.data).toHaveLength(2);
+    expect(result.current.data?.[0]?.id).toBe("m1");
+    expect(result.current.data?.[0]?.title).toBe("First");
+    expect(result.current.data?.[1]?.id).toBe("m3");
+  });
+});

src/hooks/use-all-user-movies.ts

@@ -0,0 +1,85 @@
+"use client";
+
+import { useEffect, useState } from "react";
+import { useQuery } from "@tanstack/react-query";
+import { getSupabaseBrowserClient } from "@/lib/supabase/client";
+import type { Database } from "@/types/database";
+
+type MovieRow = Database["public"]["Tables"]["movies"]["Row"];
+
+/**
+ * Fetches the flattened unwatched movie pool across every group the current
+ * user is a member of.
+ *
+ * SECURITY: this hook intentionally does NOT accept any caller-supplied
+ * group ID list. It uses the anon-key browser Supabase client and relies
+ * entirely on RLS policies on `movies` (which check membership in
+ * `group_members` via `auth.uid()`) to scope the rows the user is allowed
+ * to see. Routing this through the service-role key or filtering by a
+ * client-derived group list would defeat that trust boundary.
+ */
+async function fetchAllUserMovies(): Promise<MovieRow[]> {
+  const supabase = getSupabaseBrowserClient();
+
+  // RLS on `movies` filters by group membership using auth.uid().
+  // We deliberately do NOT add a `.in("group_id", ...)` filter — the server
+  // is the sole authority on which rows the user may see.
+  const { data, error } = await supabase
+    .from("movies")
+    .select("*")
+    .eq("watched", false)
+    .order("added_at", { ascending: false });
+
+  if (error) throw error;
+  if (!data) return [];
+
+  const seen = new Set<number>();
+  const deduped: MovieRow[] = [];
+  for (const movie of data) {
+    if (seen.has(movie.tmdb_id)) continue;
+    seen.add(movie.tmdb_id);
+    deduped.push(movie);
+  }
+  return deduped;
+}
+
+/**
+ * Tracks the current Supabase auth user id so the query key changes when
+ * the session changes (sign-in, sign-out, recovery claim, etc.). The id is
+ * NOT used as a filter — RLS still does that — it only keys the cache.
+ */
+function useCurrentUserId(): string | null | undefined {
+  const [userId, setUserId] = useState<string | null | undefined>(undefined);
+
+  useEffect(() => {
+    const supabase = getSupabaseBrowserClient();
+    let cancelled = false;
+
+    supabase.auth.getUser().then(({ data }) => {
+      if (!cancelled) setUserId(data.user?.id ?? null);
+    });
+
+    const { data: sub } = supabase.auth.onAuthStateChange((_event, session) => {
+      setUserId(session?.user?.id ?? null);
+    });
+
+    return () => {
+      cancelled = true;
+      sub.subscription.unsubscribe();
+    };
+  }, []);
+
+  return userId;
+}
+
+export function useAllUserMovies() {
+  const userId = useCurrentUserId();
+
+  return useQuery({
+    queryKey: ["all-user-movies", userId ?? null],
+    queryFn: fetchAllUserMovies,
+    enabled: !!userId,
+    staleTime: 2 * 60 * 1000,
+    gcTime: 2 * 60 * 1000,
+  });
+}