Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions apps/web/actions/videos/get-available-translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import { db } from "@cap/database";
import { videos } from "@cap/database/schema";
import { Storage } from "@cap/web-backend";
import type { Video } from "@cap/web-domain";
import { provideOptionalAuth, Storage, VideosPolicy } from "@cap/web-backend";
import { Policy, type Video } from "@cap/web-domain";
import { eq } from "drizzle-orm";
import { Effect } from "effect";
import { Effect, Exit } from "effect";
import * as EffectRuntime from "@/lib/server";
import { runPromise } from "@/lib/server";
import { decodeStorageVideo } from "@/lib/video-storage";
import {
Expand Down Expand Up @@ -37,10 +38,24 @@ export async function getAvailableTranslations(
};
}

const query = await db()
.select({ video: videos })
.from(videos)
.where(eq(videos.id, videoId));
const exit = await Effect.gen(function* () {
const videosPolicy = yield* VideosPolicy;

return yield* Effect.promise(() =>
db().select({ video: videos }).from(videos).where(eq(videos.id, videoId)),
).pipe(Policy.withPublicPolicy(videosPolicy.canView(videoId)));
}).pipe(provideOptionalAuth, EffectRuntime.runPromiseExit);

if (Exit.isFailure(exit)) {
return {
success: false,
hasOriginal: false,
translations: [],
message: "Video not found",
};
}

const query = exit.value;

if (query.length === 0 || !query[0]?.video) {
return {
Expand Down
24 changes: 17 additions & 7 deletions apps/web/actions/videos/get-transcript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import { db } from "@cap/database";
import { getCurrentUser } from "@cap/database/auth/session";
import { videos } from "@cap/database/schema";
import { Storage } from "@cap/web-backend";
import type { Video } from "@cap/web-domain";
import { provideOptionalAuth, Storage, VideosPolicy } from "@cap/web-backend";
import { Policy, type Video } from "@cap/web-domain";
import { eq } from "drizzle-orm";
import { Effect, Option } from "effect";
import { Effect, Exit, Option } from "effect";
import * as EffectRuntime from "@/lib/server";
import { runPromise } from "@/lib/server";
import { decodeStorageVideo } from "@/lib/video-storage";

Expand All @@ -22,10 +23,19 @@ export async function getTranscript(
};
}

const query = await db()
.select({ video: videos })
.from(videos)
.where(eq(videos.id, videoId));
const exit = await Effect.gen(function* () {
const videosPolicy = yield* VideosPolicy;

return yield* Effect.promise(() =>
db().select({ video: videos }).from(videos).where(eq(videos.id, videoId)),
).pipe(Policy.withPublicPolicy(videosPolicy.canView(videoId)));
}).pipe(provideOptionalAuth, EffectRuntime.runPromiseExit);
Comment on lines +26 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Double DB round-trip for the same video row

VideosPolicy.canView calls repo.getById(videoId) internally (see buildCanView in packages/web-backend/src/Videos/VideosPolicy.ts), then the outer Effect.promise(() => db().select({ video: videos })…) fetches the exact same row a second time. The same pattern is present in get-available-translations.ts, translate-transcript.ts, and api/video/ai/route.ts. Since withPublicPolicy runs the policy before the wrapped effect, every request now incurs two sequential reads for the same video. If this becomes a measurable latency concern, the results from canView's internal fetch could be threaded out and reused instead of re-querying.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/videos/get-transcript.ts
Line: 26-32

Comment:
**Double DB round-trip for the same video row**

`VideosPolicy.canView` calls `repo.getById(videoId)` internally (see `buildCanView` in `packages/web-backend/src/Videos/VideosPolicy.ts`), then the outer `Effect.promise(() => db().select({ video: videos })…)` fetches the exact same row a second time. The same pattern is present in `get-available-translations.ts`, `translate-transcript.ts`, and `api/video/ai/route.ts`. Since `withPublicPolicy` runs the policy *before* the wrapped effect, every request now incurs two sequential reads for the same video. If this becomes a measurable latency concern, the results from `canView`'s internal fetch could be threaded out and reused instead of re-querying.

How can I resolve this? If you propose a fix, please make it concise.


if (Exit.isFailure(exit)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this treats any failure (including unexpected DB/runtime errors) as "Video not found". If that’s intentional for the client response, it might still be worth logging the failure so real issues don’t get silently masked in production.

return { success: false, message: "Video not found" };
}

const query = exit.value;

if (query.length === 0) {
return { success: false, message: "Video not found" };
Expand Down
29 changes: 22 additions & 7 deletions apps/web/actions/videos/translate-transcript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import { db } from "@cap/database";
import { videos } from "@cap/database/schema";
import { Storage } from "@cap/web-backend";
import type { Video } from "@cap/web-domain";
import { provideOptionalAuth, Storage, VideosPolicy } from "@cap/web-backend";
import { Policy, type Video } from "@cap/web-domain";
import { eq } from "drizzle-orm";
import { Effect, Option } from "effect";
import { Effect, Exit, Option } from "effect";
import { GROQ_MODEL, getGroqClient } from "@/lib/groq-client";
import { isRateLimited, RATE_LIMIT_IDS } from "@/lib/rate-limit";
import * as EffectRuntime from "@/lib/server";
import { runPromise } from "@/lib/server";
import { decodeStorageVideo } from "@/lib/video-storage";
import {
Expand Down Expand Up @@ -46,10 +48,23 @@ export async function translateTranscript(
};
}

const query = await db()
.select({ video: videos })
.from(videos)
.where(eq(videos.id, videoId));
if (await isRateLimited(RATE_LIMIT_IDS.TRANSLATE_TRANSCRIPT)) {
return { success: false, message: "Too many requests" };
}

const exit = await Effect.gen(function* () {
const videosPolicy = yield* VideosPolicy;

return yield* Effect.promise(() =>
db().select({ video: videos }).from(videos).where(eq(videos.id, videoId)),
).pipe(Policy.withPublicPolicy(videosPolicy.canView(videoId)));
}).pipe(provideOptionalAuth, EffectRuntime.runPromiseExit);

if (Exit.isFailure(exit)) {
return { success: false, message: "Video not found" };
}

const query = exit.value;

if (query.length === 0 || !query[0]?.video) {
return { success: false, message: "Video not found" };
Expand Down
25 changes: 20 additions & 5 deletions apps/web/app/api/video/ai/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ import { db } from "@cap/database";
import { getCurrentUser } from "@cap/database/auth/session";
import { users, videos } from "@cap/database/schema";
import type { VideoMetadata } from "@cap/database/types";
import type { Video } from "@cap/web-domain";
import { provideOptionalAuth, VideosPolicy } from "@cap/web-backend";
import { Policy, type Video } from "@cap/web-domain";
import { eq } from "drizzle-orm";
import { Effect, Exit } from "effect";
import type { NextRequest } from "next/server";
import { startAiGeneration } from "@/lib/generate-ai";
import * as EffectRuntime from "@/lib/server";
import { isAiGenerationEnabled } from "@/utils/flags";

export const dynamic = "force-dynamic";
Expand All @@ -27,10 +30,22 @@ export async function GET(request: NextRequest) {
);
}

const result = await db()
.select()
.from(videos)
.where(eq(videos.id, videoId));
const exit = await Effect.gen(function* () {
const videosPolicy = yield* VideosPolicy;

return yield* Effect.promise(() =>
db().select().from(videos).where(eq(videos.id, videoId)),
).pipe(Policy.withPublicPolicy(videosPolicy.canView(videoId)));
}).pipe(provideOptionalAuth, EffectRuntime.runPromiseExit);

if (Exit.isFailure(exit)) {
return Response.json(
{ error: true, message: "Video not found" },
{ status: 404 },
);
}

const result = exit.value;
if (result.length === 0 || !result[0]) {
return Response.json(
{ error: true, message: "Video not found" },
Expand Down
76 changes: 76 additions & 0 deletions apps/web/lib/rate-limit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { checkRateLimit } from "@vercel/firewall";
import { headers as nextHeaders } from "next/headers";

/**
* Best-effort per-key rate limiting backed by the Vercel Firewall.
*
* IMPORTANT: each `ruleId` passed here must also be configured as a Rate Limit
* rule in the Vercel Firewall dashboard (Firewall → Rate Limiting) with a
* `@vercel/firewall` rule condition whose ID matches `ruleId`, plus the desired
* window / limit / action. An ID that has no matching dashboard rule fails
* OPEN (`checkRateLimit` returns `{ rateLimited: false, error: "not-found" }`),
* so this helper never breaks self-hosted deploys that lack the firewall — but
* it also provides no protection until the rule exists.
*
* Mirrors the existing pattern in `actions/collections/password.ts` and
* `actions/send-download-link.ts`:
* - only enforced in production,
* - best-effort (any error → not limited) so a firewall/IP-header outage can
* never take down the underlying feature.
*
* @param ruleId Stable rule id, also configured in the Vercel Firewall.
* @param opts.key Optional bucket key (e.g. per-email / per-user). Defaults to
* the caller IP (the firewall's default behaviour).
* @param opts.headers Optional request headers (required inside Hono handlers
* where `next/headers` is unavailable; defaults to the App
* Router request headers).
* @returns `true` when the request should be rejected.
*/
export async function isRateLimited(
ruleId: string,
opts?: { key?: string; headers?: Headers },
): Promise<boolean> {
if (process.env.NODE_ENV !== "production") return false;

try {
const headersList = opts?.headers ?? (await nextHeaders());
const request = new Request("https://cap.so/api/rate-limit", {
method: "POST",
headers: headersList,
});

const { rateLimited } = await checkRateLimit(ruleId, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since checkRateLimit can fail open with error: "not-found", it feels easy to think you’re protected when the dashboard rule is missing. Consider warning when that happens (ideally once per ruleId) so misconfig doesn’t silently disable the limit.

Suggested change
const { rateLimited } = await checkRateLimit(ruleId, {
const { rateLimited, error } = await checkRateLimit(ruleId, {
request,
...(opts?.key ? { rateLimitKey: opts.key } : {}),
});
if (error === "not-found") {
console.warn(`Rate limit rule not found for "${ruleId}" (Vercel Firewall).`);
}
return rateLimited;

request,
...(opts?.key ? { rateLimitKey: opts.key } : {}),
});

return rateLimited;
} catch (error) {
console.warn(`Rate limit check failed for "${ruleId}":`, error);
return false;
}
}

/**
* Canonical Vercel Firewall rate-limit rule ids introduced by the security
* hardening pass. Each MUST be created in the Vercel Firewall dashboard for the
* corresponding protection to take effect (see `isRateLimited`).
*/
export const RATE_LIMIT_IDS = {
/** Email OTP verification attempts (brute-force guard). Suggested: 10 / 10m per key (email). */
AUTH_OTP_VERIFY: "rl_auth_otp_verify",
/** Email OTP / magic-link send (mailbomb + token-reseed guard). Suggested: 5 / 10m per key (email). */
AUTH_OTP_SEND: "rl_auth_otp_send",
/** Unauthed Loom download/convert (ffmpeg + memory DoS). Suggested: 10 / 1m per IP. */
LOOM_DOWNLOAD: "rl_loom_download",
/** Unauthed transcript translation (Groq cost). Suggested: 10 / 1m per IP. */
TRANSLATE_TRANSCRIPT: "rl_translate_transcript",
/** Anonymous support-chat messages (Groq + Supermemory cost). Suggested: 20 / 1m per IP. */
MESSENGER_MESSAGE: "rl_messenger_message",
/** Unauthed analytics view tracking (Tinybird ingest + notifications). Suggested: 60 / 1m per IP. */
ANALYTICS_TRACK: "rl_analytics_track",
/** Unauthed guest checkout (Stripe object/cost abuse). Suggested: 10 / 10m per IP. */
GUEST_CHECKOUT: "rl_guest_checkout",
/** Unauthed desktop log → Discord forwarding (spam). Suggested: 10 / 1m per IP. */
DESKTOP_LOGS: "rl_desktop_logs",
} as const;
Comment on lines +59 to +76

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 RATE_LIMIT_IDS entries without corresponding call-sites in this PR

Several IDs — AUTH_OTP_VERIFY, AUTH_OTP_SEND, LOOM_DOWNLOAD, MESSENGER_MESSAGE, ANALYTICS_TRACK, GUEST_CHECKOUT, DESKTOP_LOGS — are defined here but isRateLimited is only wired up for TRANSLATE_TRANSCRIPT in this PR. The doc-comment already explains that an ID with no matching Vercel Firewall dashboard rule fails open, so the constants themselves are harmless. That said, it is worth verifying the IDs are either already used elsewhere or are tracked for follow-up — a constant that silently provides no protection could be misleading.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/lib/rate-limit.ts
Line: 59-76

Comment:
**`RATE_LIMIT_IDS` entries without corresponding call-sites in this PR**

Several IDs — `AUTH_OTP_VERIFY`, `AUTH_OTP_SEND`, `LOOM_DOWNLOAD`, `MESSENGER_MESSAGE`, `ANALYTICS_TRACK`, `GUEST_CHECKOUT`, `DESKTOP_LOGS` — are defined here but `isRateLimited` is only wired up for `TRANSLATE_TRANSCRIPT` in this PR. The doc-comment already explains that an ID with no matching Vercel Firewall dashboard rule fails open, so the constants themselves are harmless. That said, it is worth verifying the IDs are either already used elsewhere or are tracked for follow-up — a constant that silently provides no protection could be misleading.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Loading