Skip to content
Merged
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
19 changes: 19 additions & 0 deletions src/lib/video/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ export interface VideoProgress {

export type ProgressReporter = (update: VideoProgress) => Promise<void> | void;

const ALLOWED_IMAGE_TYPES = ["image/jpeg", "image/png", "image/gif", "image/webp", "image/bmp", "image/svg+xml", "image/tiff"];

async function validateImageUrl(url: string): Promise<void> {
try {
const response = await axios.head(url, { timeout: 5000 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The new URL validation performs a server-side request to a user-controlled URL without any private-network or host allowlist checks, which enables SSRF against internal services. Validate and reject localhost/private IP ranges (and optionally enforce an allowlist) before making the outbound request. [ssrf]

Severity Level: Critical 🚨
- ❌ Video pipeline can issue SSRF requests to internal hosts.
- ❌ POST /api/video/generate accepts attacker-controlled imageUrl targets.
- ⚠️ Internal metadata or admin APIs potentially reachable via HEAD.
- ⚠️ Background Inngest worker can be abused for network scanning.
Steps of Reproduction ✅
1. Issue an authenticated POST request to `/api/video/generate` (handler at
`src/app/api/video/generate/route.ts:22-105`) with a JSON body that passes
`generateVideoSchema` (defined in `src/lib/validations/video.ts:4-10`), setting `content`
to `null` and `imageUrl` to a URL pointing at an internal or otherwise sensitive endpoint,
e.g. `http://169.254.169.254/latest/meta-data/` or an RFC1918 IP address.

2. The route handler parses and validates the request via `parseAndValidateRequest(req,
generateVideoSchema)` at `route.ts:42`, then enqueues an Inngest event
`video/generate.requested` including the user-supplied `imageUrl` in the payload at
`route.ts:81-90`.

3. The Inngest function `generateVideo` at `src/inngest/functions.ts:72-80` receives the
event, extracts `imageUrl` from `event.data` (lines 79-80), and then calls
`runVideoPipeline({ content, imageUrl, baseUrl }, ...)` at `functions.ts:11-22`.

4. Inside `runVideoPipeline` in `src/lib/video/pipeline.ts:114-129`, when `content` is
undefined and `imageUrl` is present, the code enters the OCR branch at
`pipeline.ts:121-129`, calls `await validateImageUrl(imageUrl)` at `pipeline.ts:124`, and
`validateImageUrl` performs `axios.head(url, { timeout: 5000 })` at `pipeline.ts:43`,
causing the backend to issue a HEAD request directly to the attacker-controlled `imageUrl`
host, thereby enabling SSRF against internal services.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/video/pipeline.ts
**Line:** 43:43
**Comment:**
	*Ssrf: The new URL validation performs a server-side request to a user-controlled URL without any private-network or host allowlist checks, which enables SSRF against internal services. Validate and reject localhost/private IP ranges (and optionally enforce an allowlist) before making the outbound request.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target file and nearby symbols first.
ast-grep outline src/lib/video/pipeline.ts --view expanded || true

# Show the relevant section with line numbers.
sed -n '1,220p' src/lib/video/pipeline.ts | cat -n

# Search for the event payload source and any URL validation/allow-listing helpers.
rg -n "video/generate\.requested|imageUrl|axios\.head|Tesseract\.recognize|allow-list|private|link-local|https?://" src/lib/video -S || true

Repository: knoxiboy/DoubtDesk

Length of output: 10882


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the event handler and any request schema that populates `imageUrl`.
rg -n "video/generate\.requested|runVideoPipeline\(|imageUrl" . -S

# Inspect the likely event consumer / API entrypoints if present.
fd -H -t f ".*(api|inngest|event|video).*" . || true

Repository: knoxiboy/DoubtDesk

Length of output: 10917


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file that defines the event/request flow if search results are manageable.
rg -n "video/generate\.requested|runVideoPipeline\(" src app api lib -S || true

Repository: knoxiboy/DoubtDesk

Length of output: 744


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the video validation and the shared `safeUrl` definition.
sed -n '1,220p' src/lib/validations/video.ts | cat -n
printf '\n----\n'
rg -n "const safeUrl|safeUrl\s*=|safeUrl:" src/lib -S
printf '\n----\n'
# Read the file that defines safeUrl if found.
fd -H -t f ".*validations.*" src/lib | sort

Repository: knoxiboy/DoubtDesk

Length of output: 678


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the shared URL validator.
rg -n "safeUrl" src -S

Repository: knoxiboy/DoubtDesk

Length of output: 1004


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the request/event flow around the video generation endpoint and Inngest consumer.
sed -n '1,140p' src/app/api/video/generate/route.ts | cat -n
printf '\n----\n'
sed -n '330,380p' src/inngest/functions.ts | cat -n
printf '\n----\n'
sed -n '1,120p' src/lib/validations/common.ts | cat -n

Repository: knoxiboy/DoubtDesk

Length of output: 6716


Restrict server-side image fetches
imageUrl is only syntax-validated, so a caller can still point it at internal or link-local hosts. axios.head(url) and Tesseract.recognize(imageUrl, "eng") fetch that URL server-side without a host/scheme allow-list, private-IP check, or redirect restriction. Limit this to public https URLs and reject private/reserved destinations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/video/pipeline.ts` at line 42, Restrict the URL handling around the
axios.head call and Tesseract.recognize to validated public HTTPS URLs only:
enforce the https scheme, resolve and reject private, loopback, link-local, and
other reserved IP destinations, and prevent redirects from bypassing validation.
Apply the same validated URL to both server-side fetches and reject invalid or
unsafe imageUrl values before either request.

const contentType = String(response.headers["content-type"] || "").toLowerCase();

if (!ALLOWED_IMAGE_TYPES.some(type => contentType.includes(type))) {

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Substring match on content-type is weak.

contentType.includes(type) can match unintended values (e.g., a multipart/vendor content-type embedding one of the allowed strings as a substring). Prefer exact/prefix comparison after stripping charset parameters.

♻️ Suggested tightening
-    const contentType = String(response.headers["content-type"] || "").toLowerCase();
-    
-    if (!ALLOWED_IMAGE_TYPES.some(type => contentType.includes(type))) {
+    const contentType = String(response.headers["content-type"] || "")
+      .toLowerCase()
+      .split(";")[0]
+      .trim();
+
+    if (!ALLOWED_IMAGE_TYPES.includes(contentType)) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!ALLOWED_IMAGE_TYPES.some(type => contentType.includes(type))) {
const contentType = String(response.headers["content-type"] || "")
.toLowerCase()
.split(";")[0]
.trim();
if (!ALLOWED_IMAGE_TYPES.includes(contentType)) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/video/pipeline.ts` at line 46, The content-type validation in the
pipeline is too loose because `contentType.includes(type)` can accept unintended
matches; update the check in `pipeline` to use a stricter exact or prefix
comparison after normalizing the header by removing any charset or parameter
suffixes. Use the existing `ALLOWED_IMAGE_TYPES` logic in
`src/lib/video/pipeline.ts` and adjust the validation so only true image MIME
types are accepted, not substrings embedded in other content types.

throw new Error(`Invalid file type: ${contentType || "unknown"}. Only image files are allowed.`);
}
Comment on lines +45 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The MIME-type check uses substring matching, so crafted or malformed content-type values that merely contain an allowed token can bypass validation. Parse the MIME type up to ; and compare exact normalized values instead of using includes. [incorrect condition logic]

Severity Level: Major ⚠️
- ❌ Non-image payloads can bypass image-only validation logic.
- ⚠️ Tesseract OCR may process malformed or malicious files.
- ⚠️ Defensive filter weaker than intended due to substring matching.
- ⚠️ Potential crashes or undefined behavior in OCR stage.
Steps of Reproduction ✅
1. An attacker controls the HTTP server referenced by `imageUrl` and hosts a non-image
payload (for example, a text or binary file) while configuring the `Content-Type` header
to a non-standard value that embeds an allowed token, such as `application/x-image/jpeg`
or `foo/image/png`. This is fully under attacker control because `imageUrl` is only
constrained by `safeUrl` (standard URL format) in `src/lib/validations/common.ts:3-4` and
`generateVideoSchema` at `src/lib/validations/video.ts:4-7`.

2. The attacker sends a POST request to `/api/video/generate` (handler at
`src/app/api/video/generate/route.ts:22-105`), setting `content` to `null` and `imageUrl`
to the crafted URL. The request passes `generateVideoSchema` at `route.ts:42` and is
enqueued as `video/generate.requested` with the unmodified `imageUrl` at `route.ts:81-90`.

3. The Inngest `generateVideo` function in `src/inngest/functions.ts:72-80` receives the
event, extracts `imageUrl`, and calls `runVideoPipeline` at `functions.ts:11-22`. Inside
`runVideoPipeline` in `src/lib/video/pipeline.ts:121-129`, the OCR branch executes and
calls `await validateImageUrl(imageUrl)` at line 124.

4. `validateImageUrl` performs `axios.head(url, { timeout: 5000 })` at `pipeline.ts:43`,
reads the crafted `Content-Type` header into `contentType` at `pipeline.ts:44`, and
evaluates `ALLOWED_IMAGE_TYPES.some(type => contentType.includes(type))` at
`pipeline.ts:46`. Because the header string contains the substring `image/jpeg` or
`image/png`, the check passes even though the MIME type is non-standard for an image. No
error is thrown, and `runVideoPipeline` proceeds to call `Tesseract.recognize(imageUrl,
"eng")` at `pipeline.ts:125-127` on a non-image payload, defeating the intended MIME-type
validation.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/video/pipeline.ts
**Line:** 46:48
**Comment:**
	*Incorrect Condition Logic: The MIME-type check uses substring matching, so crafted or malformed `content-type` values that merely contain an allowed token can bypass validation. Parse the MIME type up to `;` and compare exact normalized values instead of using `includes`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

} catch (error) {
if (error instanceof Error && error.message.startsWith("Invalid file type")) {
throw error;
}
throw new Error("Failed to validate image file: unable to fetch file headers");
}
}

Comment on lines +38 to +55

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.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== file list ==\n'
git ls-files src/lib/video/pipeline.ts

printf '\n== outline ==\n'
ast-grep outline src/lib/video/pipeline.ts --view expanded || true

printf '\n== relevant slices ==\n'
sed -n '1,220p' src/lib/video/pipeline.ts | cat -n

Repository: knoxiboy/DoubtDesk

Length of output: 10370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== search for validateImageUrl and Tesseract.recognize usages ==\n'
rg -n "validateImageUrl|Tesseract\.recognize|content-type|ALLOWED_IMAGE_TYPES|axios\.head|axios\.get" src lib . -g '!node_modules' || true

Repository: knoxiboy/DoubtDesk

Length of output: 2631


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== surrounding lines near reported locations ==\n'
sed -n '30,150p' src/lib/video/pipeline.ts | cat -n
printf '\n== tail slice ==\n'
sed -n '150,320p' src/lib/video/pipeline.ts | cat -n

Repository: knoxiboy/DoubtDesk

Length of output: 11358


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== git diff stat for pipeline file ==\n'
git diff --stat -- src/lib/video/pipeline.ts || true

printf '\n== file size ==\n'
wc -l src/lib/video/pipeline.ts

Repository: knoxiboy/DoubtDesk

Length of output: 242


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== exact call flow from pipeline.ts ==\n'
python3 - <<'PY'
from pathlib import Path
p=Path('src/lib/video/pipeline.ts')
text=p.read_text()
for needle in ['validateImageUrl(', 'Tesseract.recognize(', 'axios.head(', 'axios.get(']:
    print(f"\n--- {needle} ---")
    for i,line in enumerate(text.splitlines(),1):
        if needle in line:
            start=max(1,i-8); end=min(len(text.splitlines()), i+12)
            for j in range(start,end+1):
                print(f"{j:4d}: {text.splitlines()[j-1]}")
            break
PY

Repository: knoxiboy/DoubtDesk

Length of output: 3133


Validate the OCR input bytes, not the HEAD header src/lib/video/pipeline.ts:41-56,124-127
validateImageUrl only checks a HEAD content-type, then Tesseract.recognize(imageUrl, "eng") fetches the URL again. A server can return image/* for HEAD and different bytes for the actual image, so this check can be bypassed. Read the file once, inspect its magic bytes, and pass that same buffer into OCR.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/video/pipeline.ts` around lines 39 - 56, The current validateImageUrl
flow in pipeline.ts only trusts the HEAD content-type, but OCR later re-fetches
the URL, so the same input must be validated from the actual bytes. Update
validateImageUrl and the Tesseract.recognize call site to fetch the image once,
verify its magic bytes from the downloaded buffer, and reuse that buffer for OCR
instead of passing the URL. Keep the image-type validation logic anchored around
validateImageUrl and the OCR invocation in pipeline.ts so the check cannot be
bypassed by mismatched HEAD vs body content.

export async function cleanupVideoArtifacts(tempDir: string, outputLocation: string): Promise<void> {
await Promise.all([
fs.promises.unlink(outputLocation).catch(() => {}),
Expand Down Expand Up @@ -109,6 +127,7 @@ export async function runVideoPipeline(
// 1. OCR if an image is provided and no text content was supplied.
if (imageUrl && !content) {
await onProgress({ progress: 10, step: "Reading image (OCR)…" });
await validateImageUrl(imageUrl);
const {
data: { text },
} = await Tesseract.recognize(imageUrl, "eng");
Expand Down
Loading