Skip to content

fix(api): enforce ownership check on AI generation endpoint - #1749

Closed
MinitJain wants to merge 5 commits into
CapSoftware:mainfrom
MinitJain:fix/video-ai-idor-ownership-check
Closed

fix(api): enforce ownership check on AI generation endpoint#1749
MinitJain wants to merge 5 commits into
CapSoftware:mainfrom
MinitJain:fix/video-ai-idor-ownership-check

Conversation

@MinitJain

@MinitJain MinitJain commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • GET /api/video/ai authenticated the caller but never verified they own the target video
  • Any authenticated user could pass ?videoId=<any_video_id> and trigger AI generation billed to the video owner's account
  • Added eq(videos.ownerId, user.id) to the DB query — non-owners get a 404 (same as "not found"), so the existence of other users' videos is not leaked

Security Impact

This is an IDOR (Insecure Direct Object Reference). An attacker with a free account could exhaust another user's paid AI generation quota by repeatedly triggering generation on their videos.

Test plan

  • Owner can trigger AI generation on their own video (existing behavior unchanged)
  • Authenticated non-owner gets 404 when passing another user's video ID
  • Unauthenticated request still gets 401

@superagent-security superagent-security Bot added contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis. labels May 12, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@MinitJain
MinitJain force-pushed the fix/video-ai-idor-ownership-check branch from 1b13b9a to 601731b Compare June 19, 2026 16:18
@superagent-security superagent-security Bot removed the pr:verified PR passed security analysis. label Jun 19, 2026
@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@superagent-security superagent-security Bot removed the contributor:verified Contributor passed trust analysis. label Jun 19, 2026
Comment thread apps/web/app/api/video/ai/route.ts Outdated
import { and, eq } from "drizzle-orm";
import type { NextRequest } from "next/server";
import { startAiGeneration } from "@/lib/generate-ai";
import * as EffectRuntime from "@/lib/server";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

EffectRuntime looks unused now that the Effect/Policy code path is gone — worth dropping to avoid unused-import lint failures.

Comment thread apps/web/app/api/video/ai/route.ts Outdated
}

const result = exit.value;
const result = await db()

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: since videos.id is unique, you can limit(1) and avoid the result[0] dance.

Suggested change
const result = await db()
const [video] = await db()
.select()
.from(videos)
.where(and(eq(videos.id, videoId), eq(videos.ownerId, user.id)))
.limit(1);
if (!video) {
return Response.json(
{ error: true, message: "Video not found" },
{ status: 404 },
);
}

… type

- Add @fortawesome/fontawesome-svg-core to package.json (was missing,
  causing "Cannot find module" typecheck error in Footer.tsx)
- Fix createVideo return type in caption-tracks.test.ts to include
  FakeVideo intersection so video.dispatch() typechecks correctly
Comment thread apps/web/package.json Outdated
"@effect/rpc": "^0.71.0",
"@effect/sql-mysql2": "^0.47.0",
"@effect/workflow": "^0.11.3",
"@fortawesome/fontawesome-svg-core": "^6.7.2",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Did you mean to include the workspace lockfile update (e.g. pnpm-lock.yaml) with this dependency bump? If CI runs with a frozen lockfile, it’ll fail if the lock isn’t updated.

Replace `import type { IconDefinition } from "@fortawesome/fontawesome-svg-core"`
with a local type alias `type IconDefinition = typeof faDiscord`.
fontawesome-svg-core is a transitive dep not listed in package.json;
importing it directly breaks CI's frozen-lockfile install.
Comment thread apps/web/app/(site)/Footer.tsx Outdated
faXTwitter,
} from "@fortawesome/free-brands-svg-icons";

type IconDefinition = typeof faDiscord;

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: having type IconDefinition = ... between imports can trip import/first / import-order rules. If you hit lint noise, consider moving this alias below the full import block.

Comment on lines 28 to +32
}

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]) {
const [video] = await db()
.select()
.from(videos)

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 perf: this handler only needs metadata, transcriptionStatus, and ownerId — selecting the whole videos row may pull large columns unnecessarily.

Suggested change
}
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]) {
const [video] = await db()
.select()
.from(videos)
const [video] = await db()
.select({
ownerId: videos.ownerId,
metadata: videos.metadata,
transcriptionStatus: videos.transcriptionStatus,
})
.from(videos)
.where(and(eq(videos.id, videoId), eq(videos.ownerId, user.id)))
.limit(1);

…y needed video columns

- Move `type IconDefinition` below all import statements to satisfy
  Biome organizeImports ordering rule
- Select only ownerId/metadata/transcriptionStatus from videos table
  instead of SELECT * (tembo perf suggestion)
@richiemcilroy

Copy link
Copy Markdown
Member

Closing: already covered on main. The /api/video/ai route now gates reads through Policy.withPublicPolicy(videosPolicy.canView(videoId)) (from #1926) and scopes AI generation to the video owner. The ownership gap this PR targeted is no longer present. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants