Venice Background Input Guard #1611
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded JSON input validation, error handling, and truncation to the Venice background API route; adjusted BackgroundGenerator fetch error handling to extract and surface server error messages to the user. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds input validation and sanitization to the Venice background API route to prevent malformed requests and control API costs. The implementation uses zod schemas to validate incoming requests, reject invalid payloads early with clear 400 errors, and enforce a 2,000-character limit on user input before it reaches the LLM.
- Introduced schema-based validation with zod to enforce input constraints (presence, size limits)
- Centralized JSON parsing and error handling in a dedicated
parseUserInputfunction - Added custom
BadRequestErrorclass for consistent HTTP 400 responses
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.error("Unexpected error parsing request body:", error); | ||
| return new Response("Invalid request body", { status: 400 }); | ||
| } | ||
| const truncatedInput = userInput.slice(0, MAX_INPUT_CHARS); // never log or send more than the limit |
There was a problem hiding this comment.
The truncation on line 36 is redundant. The requestSchema validation on line 10 already enforces a maximum length of 2000 characters with .max(MAX_INPUT_CHARS, ...), which means any input exceeding this limit will be rejected before reaching this point. The parseUserInput function will throw a BadRequestError if the input is too long, so userInput is guaranteed to be within the limit. This line can be removed, and truncatedInput can be replaced with userInput throughout.
| async function parseUserInput(request: Request) { | ||
| let payload: unknown; | ||
| try { | ||
| payload = await request.json(); | ||
| } catch { | ||
| throw new BadRequestError("Invalid JSON payload"); | ||
| } | ||
| const result = requestSchema.safeParse(payload); | ||
| if (!result.success) { | ||
| const issue = result.error.issues.at(0); | ||
| throw new BadRequestError(issue?.message ?? "Invalid request body"); | ||
| } | ||
| return result.data.text; | ||
| } |
There was a problem hiding this comment.
The new parseUserInput function and validation logic should have test coverage. Consider adding tests to verify:
- Invalid JSON payloads are rejected with "Invalid JSON payload"
- Missing
textfield is rejected with "User input is missing" - Empty or whitespace-only
textis rejected (after trim) - Input exceeding 2000 characters is rejected with the appropriate error message
- Valid input is correctly parsed and returned
The project uses vitest (see package.json), and similar validation patterns in other API routes (e.g., src/pages/api/notifications/index.ts) could serve as examples for structuring these tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/api/venice/background/route.ts (2)
26-36: Centralized parsing + truncation behavior look correct (note minor redundancy)The
parseUserInputtry/catch andBadRequestErrorhandling give you a clean separation between user mistakes and unexpected failures, and returning a 400 with a specific message matches the PR goals.
truncatedInput = userInput.slice(0, MAX_INPUT_CHARS)is technically redundant becauserequestSchema.max(MAX_INPUT_CHARS)already guaranteesuserInput.length <= MAX_INPUT_CHARS, but it’s harmless and slightly future‑proof if the schema changes later. No changes strictly required here.
1-17: Validation + error handling are solid; optional refinements suggestedThe overall flow—schema-based validation, central
parseUserInput, andBadRequestErrorfor 400s—is clean and will reliably reject empty/oversized/malformed payloads before hitting Venice.
zod(v3.22.4) is already in your dependencies, so no concerns there.One optional polish suggestion:
Optional: Normalize validation error surface
You're currently surfacing the first zod issue's raw.message. That's fine, but if you ever want a more stable API surface, you might instead normalize all validation errors to a generic"Invalid request body"and logresult.errorserver-side for debugging.parseUserInputalready infersPromise<string>, but you could make this explicit for stronger type contracts if preferred.This is purely optional; the current implementation is correct and readable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/api/venice/background/route.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/api/venice/background/route.ts (1)
src/app/api/venice/config.ts (1)
VENICE_API_KEY(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
🔇 Additional comments (1)
src/app/api/venice/background/route.ts (1)
58-58: UsingtruncatedInputin the prompt achieves the intended ceilingSwapping in
truncatedInputfor the placeholder keeps the user context within the configured character limit before it ever reaches Venice, aligning with the “end‑to‑end 2,000‑character ceiling” requirement without altering the rest of the prompt logic.
|
good thinking to guard the inputs here and show useful error messages to the user, but after quick testing it's clear that this is not ready for prod. i wasn't able to notice any improvements from the tests i ran, but I did notice things that work currently in prod but do not work in this branch, plus fail silently with no error message in the UI (example below). the error from the logs is that the input exceeds 2,000 characters, but using the same input works in prod. where did the character limit come from? why is there no error message displayed in the UI? please figure out a reasonable character limit (2k is way too low) and ensure that an error message is displayed to the user if they click 'generate' and have exceeded it.
ie. generating with the following context works in prod but not this branch: please remove the moving dots, and bring the background to life <style> body, html { margin: 0; padding: 0; height: 100%; overflow: hidden; } |
…messages - Increase character limit from 2000 to 10000 to support complex HTML/CSS inputs - Add proper error handling in frontend to display error messages via toast - Update API to return JSON error responses for better error handling - Fix silent failures - users now see clear error messages when validation fails
- Upgrade Next.js from 15.2.2 to 15.5.9 - Fixes critical security vulnerability CVE-2025-66478 - Prevents remote code execution in React Server Components
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/api/venice/background/route.ts (1)
42-42: Redundant truncation after schema validation.The
requestSchemaalready enforcesmax(MAX_INPUT_CHARS), so any input reaching this point is guaranteed to be within the limit. The.slice()is defensive but unnecessary given the current flow.If you want to keep it as a defense-in-depth measure (guarding against future refactors that might bypass validation), that's acceptable. Otherwise, you can simplify by using
userInputdirectly.
🧹 Nitpick comments (3)
src/app/api/venice/background/route.ts (3)
12-17: Consider setting thenameproperty for better error identification.Custom error classes should set
this.nameto improve stack traces and error identification in logs.🔎 Suggested improvement
class BadRequestError extends Error { status = 400; constructor(message: string) { super(message); + this.name = "BadRequestError"; } }
22-24: Consider returning 500 for missing API key.A missing
VENICE_API_KEYis a server configuration error, not a client error. Status 500 (Internal Server Error) or 503 (Service Unavailable) would be more semantically accurate than 400.🔎 Suggested change
if (!VENICE_API_KEY) { - return new Response("API key is missing", { status: 400 }); + console.error("VENICE_API_KEY environment variable is not configured"); + return new Response("Service temporarily unavailable", { status: 503 }); }
72-75: Consider adding a timeout to the Venice API fetch.The external fetch has no explicit timeout. While
maxDuration = 300limits the overall request, a hung connection to Venice could consume the entire budget on the first model, preventing fallback attempts.🔎 Add AbortController timeout
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 60000); // 60s per model const fetchResponse = await fetch( "https://api.venice.ai/api/v1/chat/completions", - options, + { ...options, signal: controller.signal }, ); + clearTimeout(timeoutId);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/api/venice/background/route.ts(4 hunks)src/common/lib/theme/BackgroundGenerator.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/api/venice/background/route.ts (1)
src/app/api/venice/config.ts (1)
VENICE_API_KEY(7-7)
🔇 Additional comments (5)
src/common/lib/theme/BackgroundGenerator.tsx (2)
100-116: LGTM! Robust error extraction with appropriate fallbacks.The error handling correctly prioritizes JSON extraction, falls back to plain text, and uses
statusTextas a last resort. This addresses the PR reviewer's concern about missing UI feedback when requests fail.
127-136: Good addition of user-facing error feedback.The toast now displays the extracted error message, providing clear feedback to users when background generation fails. This directly addresses the reviewer's request for visible UI errors.
One minor note: the 8-second duration is reasonable, but consider whether this is consistent with other error toasts in the application.
src/app/api/venice/background/route.ts (3)
1-11: Good schema-driven validation setup.The zod schema correctly trims input, enforces presence (min 1), and caps at the limit. The increased 10,000 character limit addresses the reviewer's concern about complex HTML/CSS inputs being rejected.
26-41: Solid error handling with structured JSON responses.The try-catch correctly distinguishes
BadRequestErrorfrom unexpected errors, returning structured JSON that the frontend can parse and display. This aligns well with theBackgroundGenerator.tsxchanges that extract error messages from JSON responses.
98-111: Clean input parsing with proper validation.The
parseUserInputfunction correctly:
- Catches JSON parsing errors
- Uses
safeParseto avoid exceptions from zod- Extracts the first issue message for user-friendly errors
|
looks great now from testing perspective, ty @Jhonattan2121 |
Summary
parseUserInput, ensuring we never log or forward malformed content.Detail
Strict request contract
Introduced
zodinsrc/app/api/venice/background/route.tsand definedrequestSchemathat trims, validates presence, and capstextat 2 000 chars. CustomBadRequestErrorensures consistent HTTP 400 responses.Hardened parsing path
parseUserInputnow owns JSON decoding, schema validation, and error translation. All parsing failures (invalid JSON, missing text, size overrun) are surfaced early without touching Venice.Bounded context before execution
Even valid inputs are sliced to
MAX_INPUT_CHARSbefore being interpolated intoPROMPT, guaranteeing logs and Venice calls never exceed the agreed ceiling.Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.