From be2f0505cc5cecfd1b9dc14c8c05c6094d74843b Mon Sep 17 00:00:00 2001 From: shrimbly Date: Tue, 10 Feb 2026 13:19:17 +1300 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20#59=20review=20=E2=80=94?= =?UTF-8?q?=2018=20bug,=20security,=20and=20robustness=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Providers: - fal: don't cache failed schema fetches, remove duplicate param spread, SSRF-validate mediaUrl - gemini: remove redundant header overrides, fix nano-banana model ID - kie: module-level MAX_MEDIA_SIZE, post-download size check, upload size guard - replicate: remove duplicate param spread, validate modelId format - wavespeed: post-download size check, NaN-safe parseInt, prevent prompt overwrite Executors: - generateVideo: validate stored fallback inputs same as regular path - nanoBanana: cap imageHistory at 50 entries - splitGrid: handle img.onerror with warning + null data - videoProcessing: add FileReader onerror/onabort handlers Utilities: - connectedInputs: use ?? instead of || for outputText/template - executionUtils: guard orphan children in topological sort, validate chunk() size Co-Authored-By: Claude Opus 4.6 --- src/app/api/generate/__tests__/route.test.ts | 2 +- src/app/api/generate/providers/fal.ts | 25 ++++++++++++------- src/app/api/generate/providers/gemini.ts | 9 ++----- src/app/api/generate/providers/kie.ts | 15 +++++++++-- src/app/api/generate/providers/replicate.ts | 15 ++++++++--- src/app/api/generate/providers/wavespeed.ts | 9 ++++--- src/store/execution/generateVideoExecutor.ts | 17 +++++++------ src/store/execution/nanoBananaExecutor.ts | 2 +- src/store/execution/splitGridExecutor.ts | 10 +++++++- .../execution/videoProcessingExecutors.ts | 8 ++++-- .../utils/__tests__/executionUtils.test.ts | 16 ++++++++++++ src/store/utils/connectedInputs.ts | 2 +- src/store/utils/executionUtils.ts | 6 ++++- 13 files changed, 96 insertions(+), 40 deletions(-) diff --git a/src/app/api/generate/__tests__/route.test.ts b/src/app/api/generate/__tests__/route.test.ts index 079f7f6e..974d2fd6 100644 --- a/src/app/api/generate/__tests__/route.test.ts +++ b/src/app/api/generate/__tests__/route.test.ts @@ -525,7 +525,7 @@ describe("/api/generate route", () => { expect(data.success).toBe(true); expect(mockGenerateContent).toHaveBeenCalledWith( expect.objectContaining({ - model: "gemini-2.5-flash-image", + model: "gemini-2.5-flash-preview-image-generation", }) ); }); diff --git a/src/app/api/generate/providers/fal.ts b/src/app/api/generate/providers/fal.ts index 99f3479b..cff1ff44 100644 --- a/src/app/api/generate/providers/fal.ts +++ b/src/app/api/generate/providers/fal.ts @@ -143,13 +143,14 @@ async function getFalInputMapping(modelId: string, apiKey: string | null): Promi } } } + + const result = { paramMap, arrayParams, schemaArrayParams, parameterTypes }; + falInputMappingCache.set(modelId, { result, timestamp: Date.now() }); + return result; } catch { - // Schema parsing failed - continue with empty mapping + // Schema parsing failed - return defaults without caching so next call retries + return { paramMap, arrayParams, schemaArrayParams, parameterTypes }; } - - const result = { paramMap, arrayParams, schemaArrayParams, parameterTypes }; - falInputMappingCache.set(modelId, { result, timestamp: Date.now() }); - return result; } export const MAX_UPLOAD_SIZE = 20 * 1024 * 1024; // 20 MB @@ -248,10 +249,8 @@ export async function generateWithFalQueue( // Fetch schema for type coercion and input mapping (cached) const { paramMap, arrayParams, schemaArrayParams, parameterTypes } = await getFalInputMapping(modelId, apiKey); - // Build request body, coercing parameter types from schema - const requestBody: Record = { - ...coerceParameterTypes(input.parameters, parameterTypes), - }; + // Build request body - parameters are applied per-path below to avoid double-spreading + const requestBody: Record = {}; // Upload base64 images to fal CDN to avoid sending large payloads inline const uploadImage = async (value: string | string[]): Promise => { @@ -265,6 +264,8 @@ export async function generateWithFalQueue( }; if (hasDynamicInputs) { + // Apply coerced parameters first, then dynamic inputs override + Object.assign(requestBody, coerceParameterTypes(input.parameters, parameterTypes)); const filteredInputs: Record = {}; for (const [key, value] of Object.entries(input.dynamicInputs!)) { if (value !== null && value !== undefined && value !== '') { @@ -476,6 +477,12 @@ export async function generateWithFalQueue( }; } + // Validate URL before fetching (SSRF protection) + const mediaUrlCheck = validateMediaUrl(mediaUrl); + if (!mediaUrlCheck.valid) { + return { success: false, error: `Invalid media URL: ${mediaUrlCheck.error}` }; + } + // Fetch the media and convert to base64 console.log(`[API:${requestId}] Fetching output from: ${mediaUrl.substring(0, 80)}...`); const mediaResponse = await fetch(mediaUrl); diff --git a/src/app/api/generate/providers/gemini.ts b/src/app/api/generate/providers/gemini.ts index aa2a3bf8..58bca493 100644 --- a/src/app/api/generate/providers/gemini.ts +++ b/src/app/api/generate/providers/gemini.ts @@ -12,7 +12,7 @@ import { GenerateResponse, ModelType } from "@/types"; * Map model types to Gemini model IDs */ export const MODEL_MAP: Record = { - "nano-banana": "gemini-2.5-flash-image", + "nano-banana": "gemini-2.5-flash-preview-image-generation", "nano-banana-pro": "gemini-3-pro-image-preview", }; @@ -154,12 +154,7 @@ export async function generateWithGemini( console.log(`[API:${requestId}] SUCCESS - Returning ${responseSizeMB}MB payload`); - // Create response with explicit headers to handle large payloads - const resp = NextResponse.json(responsePayload); - resp.headers.set('Content-Type', 'application/json'); - resp.headers.set('Content-Length', responseSize.toString()); - - return resp; + return NextResponse.json(responsePayload); } } diff --git a/src/app/api/generate/providers/kie.ts b/src/app/api/generate/providers/kie.ts index bdb82120..a00f61db 100644 --- a/src/app/api/generate/providers/kie.ts +++ b/src/app/api/generate/providers/kie.ts @@ -8,6 +8,9 @@ import { GenerationInput, GenerationOutput } from "@/lib/providers/types"; import { validateMediaUrl } from "@/utils/urlValidation"; +const MAX_MEDIA_SIZE = 500 * 1024 * 1024; // 500MB +const MAX_UPLOAD_SIZE = 20 * 1024 * 1024; // 20MB + /** * Get default required parameters for a Kie model * Many Kie models require specific parameters to be present even if not user-specified @@ -205,6 +208,10 @@ export async function uploadImageToKie( // Convert base64 to binary to detect actual type const binaryData = Buffer.from(imageData, "base64"); + if (binaryData.length > MAX_UPLOAD_SIZE) { + throw new Error(`[API:${requestId}] Image too large to upload (${(binaryData.length / (1024 * 1024)).toFixed(1)}MB, max ${MAX_UPLOAD_SIZE / (1024 * 1024)}MB)`); + } + // Detect actual image type from magic bytes (don't trust the declared MIME type) const detected = detectImageType(binaryData); const mimeType = detected.mimeType; @@ -557,7 +564,6 @@ export async function generateWithKie( return { success: false, error: `Failed to fetch output: ${mediaResponse.status}` }; } - const MAX_MEDIA_SIZE = 500 * 1024 * 1024; const mediaContentLength = parseInt(mediaResponse.headers.get("content-length") || "0", 10); if (mediaContentLength > MAX_MEDIA_SIZE) { return { success: false, error: `Media too large: ${(mediaContentLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; @@ -565,6 +571,9 @@ export async function generateWithKie( const contentType = mediaResponse.headers.get("content-type") || "video/mp4"; const mediaArrayBuffer = await mediaResponse.arrayBuffer(); + if (mediaArrayBuffer.byteLength > MAX_MEDIA_SIZE) { + return { success: false, error: `Media too large: ${(mediaArrayBuffer.byteLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; + } const mediaSizeMB = mediaArrayBuffer.byteLength / (1024 * 1024); console.log(`[API:${requestId}] Veo output: ${contentType}, ${mediaSizeMB.toFixed(2)}MB`); @@ -759,7 +768,6 @@ export async function generateWithKie( } // Check file size before downloading body - const MAX_MEDIA_SIZE = 500 * 1024 * 1024; // 500MB const mediaContentLength = parseInt(mediaResponse.headers.get("content-length") || "0", 10); if (mediaContentLength > MAX_MEDIA_SIZE) { return { success: false, error: `Media too large: ${(mediaContentLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; @@ -771,6 +779,9 @@ export async function generateWithKie( } const mediaArrayBuffer = await mediaResponse.arrayBuffer(); + if (mediaArrayBuffer.byteLength > MAX_MEDIA_SIZE) { + return { success: false, error: `Media too large: ${(mediaArrayBuffer.byteLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; + } const mediaSizeBytes = mediaArrayBuffer.byteLength; const mediaSizeMB = mediaSizeBytes / (1024 * 1024); diff --git a/src/app/api/generate/providers/replicate.ts b/src/app/api/generate/providers/replicate.ts index 5563953a..b82c30a7 100644 --- a/src/app/api/generate/providers/replicate.ts +++ b/src/app/api/generate/providers/replicate.ts @@ -27,6 +27,13 @@ export async function generateWithReplicate( const modelId = input.model.id; const [owner, name] = modelId.split("/"); + if (!owner || !name) { + return { + success: false, + error: `Invalid Replicate model ID "${modelId}": expected "owner/name" format`, + }; + } + // First, get the model to find the latest version const modelResponse = await fetch( `${REPLICATE_API_BASE}/models/${owner}/${name}`, @@ -61,13 +68,13 @@ export async function generateWithReplicate( const schema = modelData.latest_version?.openapi_schema as Record | undefined; const parameterTypes = getParameterTypesFromSchema(schema); - // Build input for the prediction, coercing parameter types from schema - const predictionInput: Record = { - ...coerceParameterTypes(input.parameters, parameterTypes), - }; + // Build input for the prediction - parameters are applied per-path below to avoid double-spreading + const predictionInput: Record = {}; // Add dynamic inputs if provided (these come from schema-mapped connections) if (hasDynamicInputs) { + // Apply coerced parameters first, then dynamic inputs override + Object.assign(predictionInput, coerceParameterTypes(input.parameters, parameterTypes)); const { schemaArrayParams } = getInputMappingFromSchema(schema); // Apply array wrapping based on schema type diff --git a/src/app/api/generate/providers/wavespeed.ts b/src/app/api/generate/providers/wavespeed.ts index 278675b2..d165ca72 100644 --- a/src/app/api/generate/providers/wavespeed.ts +++ b/src/app/api/generate/providers/wavespeed.ts @@ -91,10 +91,10 @@ export async function generateWithWaveSpeed( const isVideoModel = input.model.capabilities.includes("text-to-video") || input.model.capabilities.includes("image-to-video"); - // Build WaveSpeed payload + // Build WaveSpeed payload — spread parameters first so explicit prompt wins const payload: Record = { - prompt: input.prompt, ...input.parameters, + prompt: input.prompt, }; // Apply dynamic inputs (schema-mapped connections) @@ -334,11 +334,14 @@ export async function generateWithWaveSpeed( // Check file size before downloading body const MAX_MEDIA_SIZE_WS = 500 * 1024 * 1024; // 500MB const wsContentLength = parseInt(outputResponse.headers.get("content-length") || "0", 10); - if (wsContentLength > MAX_MEDIA_SIZE_WS) { + if (!isNaN(wsContentLength) && wsContentLength > MAX_MEDIA_SIZE_WS) { return { success: false, error: `Media too large: ${(wsContentLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; } const outputArrayBuffer = await outputResponse.arrayBuffer(); + if (outputArrayBuffer.byteLength > MAX_MEDIA_SIZE_WS) { + return { success: false, error: `Media too large: ${(outputArrayBuffer.byteLength / (1024 * 1024)).toFixed(0)}MB > 500MB limit` }; + } const outputSizeMB = outputArrayBuffer.byteLength / (1024 * 1024); const rawContentType = outputResponse.headers.get("content-type"); diff --git a/src/store/execution/generateVideoExecutor.ts b/src/store/execution/generateVideoExecutor.ts index c223db34..19efe872 100644 --- a/src/store/execution/generateVideoExecutor.ts +++ b/src/store/execution/generateVideoExecutor.ts @@ -45,6 +45,15 @@ export async function executeGenerateVideo( if (useStoredFallback) { images = connectedImages.length > 0 ? connectedImages : nodeData.inputImages; text = connectedText ?? nodeData.inputPrompt; + // Validate fallback inputs the same way as the regular path + const hasPrompt = text || dynamicInputs.prompt || dynamicInputs.negative_prompt; + if (!hasPrompt && images.length === 0) { + updateNodeData(node.id, { + status: "error", + error: "Missing required inputs", + }); + throw new Error("Missing required inputs"); + } } else { images = connectedImages; text = connectedText; @@ -59,14 +68,6 @@ export async function executeGenerateVideo( } } - if (useStoredFallback && !text) { - updateNodeData(node.id, { - status: "error", - error: "Missing text input", - }); - throw new Error("Missing text input"); - } - if (!nodeData.selectedModel?.modelId) { updateNodeData(node.id, { status: "error", diff --git a/src/store/execution/nanoBananaExecutor.ts b/src/store/execution/nanoBananaExecutor.ts index 7fc95dc4..3208fe0f 100644 --- a/src/store/execution/nanoBananaExecutor.ts +++ b/src/store/execution/nanoBananaExecutor.ts @@ -139,7 +139,7 @@ export async function executeNanoBanana( aspectRatio: nodeData.aspectRatio, model: nodeData.model, }; - const updatedHistory = [newHistoryItem, ...(nodeData.imageHistory || [])]; + const updatedHistory = [newHistoryItem, ...(nodeData.imageHistory || [])].slice(0, 50); updateNodeData(node.id, { outputImage: result.image, diff --git a/src/store/execution/splitGridExecutor.ts b/src/store/execution/splitGridExecutor.ts index f964b203..576827fa 100644 --- a/src/store/execution/splitGridExecutor.ts +++ b/src/store/execution/splitGridExecutor.ts @@ -60,7 +60,15 @@ export async function executeSplitGrid(ctx: NodeExecutionContext): Promise }); resolve(); }; - img.onerror = () => resolve(); + img.onerror = () => { + console.warn(`[splitGrid] Failed to load split image ${index} for node ${childSet.imageInput}`); + updateNodeData(childSet.imageInput, { + image: null, + filename: null, + dimensions: { width: 0, height: 0 }, + }); + resolve(); + }; img.src = splitImages[index]; }); } diff --git a/src/store/execution/videoProcessingExecutors.ts b/src/store/execution/videoProcessingExecutors.ts index 6ca3191e..ebe6471b 100644 --- a/src/store/execution/videoProcessingExecutors.ts +++ b/src/store/execution/videoProcessingExecutors.ts @@ -90,8 +90,10 @@ export async function executeVideoStitch(ctx: NodeExecutionContext): Promise((resolve) => { + outputVideo = await new Promise((resolve, reject) => { reader.onload = () => resolve(reader.result as string); + reader.onerror = () => reject(new Error("FileReader error while reading stitched video")); + reader.onabort = () => reject(new Error("FileReader aborted while reading stitched video")); reader.readAsDataURL(outputBlob); }); } @@ -218,8 +220,10 @@ export async function executeEaseCurve(ctx: NodeExecutionContext): Promise outputVideo = URL.createObjectURL(outputBlob); } else { const reader = new FileReader(); - outputVideo = await new Promise((resolve) => { + outputVideo = await new Promise((resolve, reject) => { reader.onload = () => resolve(reader.result as string); + reader.onerror = () => reject(new Error("FileReader error while reading ease curve video")); + reader.onabort = () => reject(new Error("FileReader aborted while reading ease curve video")); reader.readAsDataURL(outputBlob); }); } diff --git a/src/store/utils/__tests__/executionUtils.test.ts b/src/store/utils/__tests__/executionUtils.test.ts index b296aa13..d519cfa0 100644 --- a/src/store/utils/__tests__/executionUtils.test.ts +++ b/src/store/utils/__tests__/executionUtils.test.ts @@ -101,6 +101,22 @@ describe("chunk", () => { it("should handle exact multiples", () => { expect(chunk([1, 2, 3, 4], 2)).toEqual([[1, 2], [3, 4]]); }); + + it("should throw on size 0", () => { + expect(() => chunk([1, 2], 0)).toThrow("Invalid chunk size: must be a positive integer"); + }); + + it("should throw on negative size", () => { + expect(() => chunk([1, 2], -1)).toThrow("Invalid chunk size: must be a positive integer"); + }); + + it("should throw on NaN size", () => { + expect(() => chunk([1, 2], NaN)).toThrow("Invalid chunk size: must be a positive integer"); + }); + + it("should throw on Infinity size", () => { + expect(() => chunk([1, 2], Infinity)).toThrow("Invalid chunk size: must be a positive integer"); + }); }); describe("revokeBlobUrl", () => { diff --git a/src/store/utils/connectedInputs.ts b/src/store/utils/connectedInputs.ts index 2089cef0..5db53655 100644 --- a/src/store/utils/connectedInputs.ts +++ b/src/store/utils/connectedInputs.ts @@ -70,7 +70,7 @@ function getSourceOutput(sourceNode: WorkflowNode): { type: "image" | "text" | " return { type: "text", value: (sourceNode.data as PromptNodeData).prompt }; } else if (sourceNode.type === "promptConstructor") { const pcData = sourceNode.data as PromptConstructorNodeData; - return { type: "text", value: pcData.outputText || pcData.template || null }; + return { type: "text", value: pcData.outputText ?? pcData.template ?? null }; } else if (sourceNode.type === "llmGenerate") { return { type: "text", value: (sourceNode.data as LLMGenerateNodeData).outputText }; } diff --git a/src/store/utils/executionUtils.ts b/src/store/utils/executionUtils.ts index 5b66c2ca..a4b63f33 100644 --- a/src/store/utils/executionUtils.ts +++ b/src/store/utils/executionUtils.ts @@ -79,7 +79,8 @@ export function groupNodesByLevel( const nextLevel: string[] = []; for (const nodeId of currentLevel) { for (const child of adjList.get(nodeId) || []) { - const newDegree = (inDegree.get(child) || 1) - 1; + if (!inDegree.has(child)) continue; // skip orphan edge targets + const newDegree = inDegree.get(child)! - 1; inDegree.set(child, newDegree); if (newDegree === 0) { nextLevel.push(child); @@ -98,6 +99,9 @@ export function groupNodesByLevel( * Chunk an array into smaller arrays of specified size */ export function chunk(array: T[], size: number): T[][] { + if (!Number.isFinite(size) || size < 1) { + throw new Error("Invalid chunk size: must be a positive integer"); + } const chunks: T[][] = []; for (let i = 0; i < array.length; i += size) { chunks.push(array.slice(i, i + size));