fix(stream): make 1:1 recording possible, rate-limit Stream, lock down debug, give trials chat

__tests__/stream/recording-capability.test.ts

@@ -0,0 +1,112 @@
+/**
+ * @jest-environment node
+ */
+
+/**
+ * #1134 P1-6 — recording a 1:1 was not disabled, it was IMPOSSIBLE.
+ *
+ * `isAppointmentOwner` and `isRecordingEnabledForAppointment` each hand-rolled
+ * an if/else over `webinar` and `class` only. For a consultation or a
+ * subscription both fell through to `false`, so the consultant who owned the
+ * session failed the ownership check and `POST /api/stream/recordings/start`
+ * answered 403 — to the owner. `recording-info` reported `recordingEnabled:
+ * false` no matter what, above a comment noting the 1:1 plans had no such
+ * field.
+ *
+ * Both predicates now read one resolver, so they cannot disagree about which
+ * plan they are looking at — which is the shape of the original bug.
+ */
+
+import {
+  isAppointmentOwner,
+  isRecordingEnabledForAppointment,
+  resolveAppointmentPlan,
+  type AppointmentWithOwnership,
+  type OwnedPlan,
+} from "@/lib/stream/recording-utils";
+
+const OWNER = "consultant-profile-1";
+const OTHER = "consultant-profile-2";
+
+const withPlan = (kind: string, plan: OwnedPlan): AppointmentWithOwnership => {
+  switch (kind) {
+    case "webinar":
+      return { webinar: { webinarPlan: plan } };
+    case "class":
+      return { class: { classPlan: plan } };
+    case "consultation":
+      return { consultation: { consultationPlan: plan } };
+    default:
+      return { subscription: { subscriptionPlan: plan } };
+  }
+};
+
+const KINDS = ["webinar", "class", "consultation", "subscription"] as const;
+
+describe("recording capability covers every appointment kind", () => {
+  it.each(KINDS)("%s: the owning consultant is the owner", (kind) => {
+    const appointment = withPlan(kind, {
+      consultantProfileId: OWNER,
+      recordingEnabled: true,
+    });
+    expect(isAppointmentOwner(appointment, OWNER)).toBe(true);
+    expect(isAppointmentOwner(appointment, OTHER)).toBe(false);
+  });
+
+  it.each(KINDS)("%s: recordingEnabled is read from the plan", (kind) => {
+    expect(
+      isRecordingEnabledForAppointment(
+        withPlan(kind, { consultantProfileId: OWNER, recordingEnabled: true }),
+      ),
+    ).toBe(true);
+    expect(
+      isRecordingEnabledForAppointment(
+        withPlan(kind, { consultantProfileId: OWNER, recordingEnabled: false }),
+      ),
+    ).toBe(false);
+  });
+
+  it("defaults closed when the plan omits the flag", () => {
+    // A 1:1 is the most sensitive session type on the platform. An absent flag
+    // must never read as consent to record.
+    expect(
+      isRecordingEnabledForAppointment(
+        withPlan("consultation", { consultantProfileId: OWNER }),
+      ),
+    ).toBe(false);
+  });
+
+  it("never treats a missing consultantProfileId as ownership", () => {
+    // Guards the null-vs-null trap: an appointment whose plan has no consultant
+    // must not match a caller who also has none.
+    const appointment = withPlan("consultation", {
+      consultantProfileId: null,
+      recordingEnabled: true,
+    });
+    expect(isAppointmentOwner(appointment, null)).toBe(false);
+    expect(isAppointmentOwner(appointment, undefined)).toBe(false);
+  });
+
+  it("resolves nothing for an empty or absent appointment", () => {
+    expect(resolveAppointmentPlan(null)).toBeNull();
+    expect(resolveAppointmentPlan(undefined)).toBeNull();
+    expect(resolveAppointmentPlan({})).toBeNull();
+    expect(isAppointmentOwner(null, OWNER)).toBe(false);
+    expect(isRecordingEnabledForAppointment(null)).toBe(false);
+  });
+
+  it("the two predicates always read the same plan", () => {
+    // The original defect was divergence: ownership looked at one set of kinds
+    // and the recording flag at another. Assert they agree on every kind.
+    for (const kind of KINDS) {
+      const appointment = withPlan(kind, {
+        consultantProfileId: OWNER,
+        recordingEnabled: true,
+      });
+      const plan = resolveAppointmentPlan(appointment);
+      expect(plan?.consultantProfileId).toBe(OWNER);
+      expect(isAppointmentOwner(appointment, OWNER)).toBe(true);
+      expect(isRecordingEnabledForAppointment(appointment)).toBe(true);
+    }
+  });
+});

__tests__/stream/webhook-not-rate-limited.test.ts

@@ -0,0 +1,78 @@
+/**
+ * @jest-environment node
+ */
+
+/**
+ * #1134 P1-11 — the Stream webhook endpoint must not be edge rate-limited.
+ *
+ * The `stream: api` rule matched `/api/stream/` by prefix, which swept in
+ * `/api/stream/webhooks`. That is a worse failure than it sounds:
+ *
+ *   - Stream POSTs every delivery from its own infrastructure, so all of them
+ *     collapse onto a single rate-limit key rather than spreading across users.
+ *   - Bursts are the NORMAL shape. A 200-attendee webinar emits 200
+ *     `call.session_participant_joined` events at once.
+ *   - A 429 is not a deferral here. Stream retries inside a fifteen-second
+ *     total budget and then drops the event permanently.
+ *
+ * So throttling this path would have silently reintroduced the exact loss that
+ * #1137's persist-before-ack work exists to prevent — and done it in the
+ * middleware, before the route ever ran, where none of that machinery applies.
+ *
+ * Excluding it is safe because the endpoint is not open: it verifies an HMAC
+ * signature against the API secret and 401s anything unsigned before doing any
+ * work. The signature is the gate; the limiter never was.
+ *
+ * This asserts the ROUTE TABLE rather than booting the middleware, because the
+ * matcher predicates are the whole of the behaviour under test and the
+ * middleware itself pulls in Next's edge runtime, Redis and the maintenance
+ * store.
+ */
+
+import { readFileSync } from "fs";
+import { join } from "path";
+
+const middleware = readFileSync(
+  join(process.cwd(), "middleware.ts"),
+  "utf8",
+);
+
+/** The `stream: api` rule's match predicate, lifted from the source. */
+function streamApiMatches(pathname: string): boolean {
+  return (
+    pathname.startsWith("/api/stream/") &&
+    !pathname.startsWith("/api/stream/webhooks")
+  );
+}
+
+describe("the stream: api rate-limit rule", () => {
+  it("does NOT match the webhook endpoint", () => {
+    expect(streamApiMatches("/api/stream/webhooks")).toBe(false);
+  });
+
+  it("still matches the ordinary authenticated Stream routes", () => {
+    for (const p of [
+      "/api/stream/channels/search-appointments",
+      "/api/stream/recordings/start",
+      "/api/stream/search-consultees",
+      "/api/stream/debug",
+    ]) {
+      expect(streamApiMatches(p)).toBe(true);
+    }
+  });
+
+  it("is wired that way in middleware.ts, not just in this test", () => {
+    // The predicate above is a copy. This is the part that fails if someone
+    // simplifies the rule back to a bare prefix match.
+    expect(middleware).toContain('!p.startsWith("/api/stream/webhooks")');
+  });
+
+  it("does not claim the join rule is keyed per user", () => {
+    // `applyEdgeRateLimits` falls back to the client IP when a rule supplies no
+    // `key`, and the join rule supplies none. The comment used to assert
+    // per-user keying, which the code cannot do — this middleware is
+    // cookie-presence only, with no DB hit and no JWT parsing.
+    expect(middleware).not.toContain("keyed per user by the shared");
+    expect(middleware).toContain("Keyed by IP, NOT by user");
+  });
+});

app/api/stream/debug/route.ts

@@ -10,6 +10,8 @@
 import * as Sentry from "@sentry/nextjs";
 import { NextRequest, NextResponse } from "next/server";
 import { getStreamChatClient, isStreamConfigured } from "@/lib/stream-client";
+import { getSession } from "@/lib/auth-server";
+import { isPrivileged } from "@/lib/auth-helpers";
 import { streamLogger } from "@/lib/stream-logger";
 import prisma from "@/lib/prisma";
 
@@ -21,6 +23,23 @@ export async function GET(req: NextRequest) {
   // Security checks
   const isDev = process.env.NODE_ENV === "development";
 
+  // #1134 P1-12 — this route had NO session check at all. Its only production
+  // gate was a shared secret in the query string — which lands in access logs,
+  // browser history and any Referer header — and it dumps an arbitrary user's
+  // full Stream channel list. A session is now required everywhere, and staff
+  // or admin on top of that, so the secret is defence in depth rather than the
+  // whole defence.
+  const session = await getSession();
+  if (!session?.user?.id) {
+    return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
+  }
+  if (!isPrivileged(session.user.role)) {
+    streamLogger.warn("Non-privileged Stream debug attempt", {
+      userId: session.user.id,
+    });
+    return NextResponse.json({ error: "Forbidden" }, { status: 403 });
+  }
+
   if (!isDev && !ALLOW_IN_PRODUCTION) {
     return NextResponse.json(
       { error: "Debug endpoint not available in production" },
@@ -33,8 +52,12 @@ export async function GET(req: NextRequest) {
     const url = new URL(req.url);
     const secret = url.searchParams.get("secret");
 
-    if (!secret || secret !== DEBUG_SECRET) {
-      streamLogger.warn("Unauthorized debug attempt");
+    // A missing STREAM_DEBUG_SECRET must fail closed. `secret !== undefined`
+    // would have compared two undefineds and passed.
+    if (!DEBUG_SECRET || !secret || secret !== DEBUG_SECRET) {
+      streamLogger.warn("Unauthorized debug attempt", {
+        userId: session.user.id,
+      });
       return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
     }
   }

app/api/stream/meetings/[streamCallId]/recording-info/route.ts

@@ -8,6 +8,7 @@
 import { NextRequest, NextResponse } from "next/server";
 import prisma from "@/lib/prisma";
 import { isPaymentEntitled } from "@/lib/payments/utils/refund-balance";
+import { isRecordingEnabledForAppointment } from "@/lib/stream/recording-utils";
 import { isPrivileged } from "@/lib/auth-helpers";
 
 import { getSession } from "@/lib/auth-server";
@@ -60,7 +61,13 @@ export async function GET(req: NextRequest, { params }: RouteParams) {
                 consultation: {
                   include: {
                     consultationPlan: {
-                      select: { consultantProfileId: true },
+                      // #1134 P1-6 — recordingEnabled is new on the 1:1 plans;
+                      // without selecting it the resolver reports recording as
+                      // unavailable for every consultation.
+                      select: {
+                        consultantProfileId: true,
+                        recordingEnabled: true,
+                      },
                     },
                     requestedBy: {
                       select: { userId: true },
@@ -70,7 +77,10 @@ export async function GET(req: NextRequest, { params }: RouteParams) {
                 subscription: {
                   include: {
                     subscriptionPlan: {
-                      select: { consultantProfileId: true },
+                      select: {
+                        consultantProfileId: true,
+                        recordingEnabled: true,
+                      },
                     },
                     requestedBy: {
                       select: { userId: true },
@@ -173,15 +183,13 @@ export async function GET(req: NextRequest, { params }: RouteParams) {
       return NextResponse.json({ error: "Access denied" }, { status: 403 });
     }
 
-    // Determine if recording is enabled based on appointment type
-    let recordingEnabled = false;
-
-    if (appointment?.webinar?.webinarPlan) {
-      recordingEnabled = appointment.webinar.webinarPlan.recordingEnabled;
-    } else if (appointment?.class?.classPlan) {
-      recordingEnabled = appointment.class.classPlan.recordingEnabled;
-    }
-    // Consultations and subscriptions don't have recordingEnabled on their plans
+    // #1134 P1-6 — one resolver for all four plan kinds. This used to be a
+    // hand-rolled if/else over webinar and class with a comment explaining that
+    // consultations and subscriptions "don't have recordingEnabled on their
+    // plans" — true at the time, and the reason 1:1 recording was impossible
+    // rather than merely off. They have it now, and the shared helper means
+    // this can no longer disagree with the ownership check beside it.
+    const recordingEnabled = isRecordingEnabledForAppointment(appointment);
 
     return NextResponse.json({
       meetingSessionId: meetingSession.id,

app/api/stream/recordings/start/route.ts

@@ -67,6 +67,30 @@ export async function POST(req: NextRequest) {
                     },
                   },
                 },
+                // #1134 P1-6 — without these two the resolver sees no plan for a
+                // 1:1, so the actual owner fails isAppointmentOwner and start
+                // returns 403. Recording a consultation was not disabled, it was
+                // impossible.
+                consultation: {
+                  include: {
+                    consultationPlan: {
+                      select: {
+                        consultantProfileId: true,
+                        recordingEnabled: true,
+                      },
+                    },
+                  },
+                },
+                subscription: {
+                  include: {
+                    subscriptionPlan: {
+                      select: {
+                        consultantProfileId: true,
+                        recordingEnabled: true,
+                      },
+                    },
+                  },
+                },
               },
             },
           },

app/api/stream/recordings/stop/route.ts

@@ -67,6 +67,26 @@ export async function POST(req: NextRequest) {
                     },
                   },
                 },
+                // #1134 P1-6 — mirror the start route: without these the owner
+                // of a 1:1 cannot stop a recording they were able to start.
+                consultation: {
+                  include: {
+                    consultationPlan: {
+                      select: {
+                        consultantProfileId: true,
+                      },
+                    },
+                  },
+                },
+                subscription: {
+                  include: {
+                    subscriptionPlan: {
+                      select: {
+                        consultantProfileId: true,
+                      },
+                    },
+                  },
+                },
               },
             },
           },

lib/payments/webhooks/handlers.ts

@@ -943,7 +943,13 @@ ACTION REQUIRED: Customer was charged but appointment was NOT created!
         // cohered. One thread per relationship-context is the whole model now.
         if (
           (eventType === "CONSULTATION" && consultation) ||
-          (eventType === "SUBSCRIPTION" && subscription)
+          (eventType === "SUBSCRIPTION" && subscription) ||
+          // #1134 P1-16 — TRIAL had no branch here at all, so a trial buyer got
+          // video and no way to message the consultant before or after it. A
+          // trial is the platform's first impression; it is the LAST session
+          // type that should be mute. Same DM as any other 1:1, so it merges
+          // with their thread if they go on to book.
+          eventType === "TRIAL"
         ) {
           await createDirectMessageChannel(
             consultantUserId,

lib/rate-limit.ts

@@ -87,6 +87,23 @@ export const spamLimiter = makeLimiter(5, "1 h", "rl:spam");
  */
 export const cspReportLimiter = makeLimiter(120, "1 m", "rl:csp-report");
 
+/**
+ * #1134 P1-11 — Stream had NO rate limiting on any route or server action.
+ *
+ * Two shapes, two budgets:
+ *
+ * `streamJoinLimiter` guards the meeting join gate. It is the enumeration
+ * surface: call ids are deterministic (`slot-<anchorSlotId>`), so an attacker
+ * who has one slot id can walk neighbours. Generous enough that a flaky network
+ * retrying a join never trips it, tight enough that scanning is useless.
+ *
+ * `streamApiLimiter` guards the search / channel-create / block routes, which
+ * are ordinary authenticated reads and writes but were completely unbounded —
+ * every one of them costs a Stream API call we are billed for.
+ */
+export const streamJoinLimiter = makeLimiter(20, "1 m", "rl:stream-join");
+export const streamApiLimiter = makeLimiter(60, "1 m", "rl:stream-api");
+
 /** 3 per 24 hours — POST /api/trials (prevents flooding consultant inboxes) */
 export const trialRequestLimiter = makeLimiter(3, "24 h", "rl:trial-request");