diff --git a/apps/sim/lib/copilot/tools/server/base-tool.ts b/apps/sim/lib/copilot/tools/server/base-tool.ts index 2af97ba7490..d2365d0b301 100644 --- a/apps/sim/lib/copilot/tools/server/base-tool.ts +++ b/apps/sim/lib/copilot/tools/server/base-tool.ts @@ -1,5 +1,6 @@ import type { z } from 'zod' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' +import { combineExecutionAbortSignals } from '@/lib/core/execution-limits' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' export interface ServerToolContext { @@ -28,6 +29,18 @@ export interface ServerToolContext { resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry } +/** + * One signal covering every way a tool call can be cancelled, for tools that + * hold a killable resource (a child process, a long stream) rather than merely + * checking between steps as {@link assertServerToolNotAborted} does. + */ +export function resolveServerToolAbortSignal(context?: ServerToolContext): AbortSignal | undefined { + const signals = [context?.abortSignal, context?.userStopSignal].filter( + (signal): signal is AbortSignal => Boolean(signal) + ) + return signals.length > 0 ? combineExecutionAbortSignals(signals) : undefined +} + export function assertServerToolNotAborted( context?: ServerToolContext, message = 'Request aborted before tool mutation could be applied.' diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts index 26912076ead..2868184a357 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts @@ -42,6 +42,7 @@ vi.mock('@/lib/copilot/application/execute-file-use-case', () => ({ })) vi.mock('@/lib/media/ffmpeg', () => ({ + MAX_FFMPEG_INPUTS: 10, runFfmpegOperation: runFfmpegOperationMock, })) @@ -58,6 +59,7 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () })) import { ffmpegServerTool } from '@/lib/copilot/tools/server/media/ffmpeg' +import { MAX_FFMPEG_INPUTS } from '@/lib/media/ffmpeg' const EXACT_EMPTY = { status: 'exact' as const, entries: [] } const TRACKED = { @@ -308,4 +310,37 @@ describe('ffmpeg server tool secret provenance', () => { message: 'ffmpeg convert failed: The media operation failed safely', }) }) + + it('rejects more inputs than the cap before downloading any of them', async () => { + const result = await ffmpegServerTool.execute( + { + operation: 'concat', + inputs: { + files: Array.from({ length: MAX_FFMPEG_INPUTS + 1 }, () => ({ path: 'files/input.mp4' })), + }, + }, + context + ) + + expect(result.success).toBe(false) + expect(result.message).toContain(`At most ${MAX_FFMPEG_INPUTS} input files`) + expect(resolveWorkspaceFileReferenceMock).not.toHaveBeenCalled() + expect(runFfmpegOperationMock).not.toHaveBeenCalled() + }) + + it('forwards the abort signal so a cancelled turn can kill the transcode', async () => { + const abortSignal = new AbortController().signal + + await ffmpegServerTool.execute( + { operation: 'convert', inputs: { files: [{ path: 'files/input.mp4' }] } }, + { ...context, abortSignal } + ) + + expect(runFfmpegOperationMock).toHaveBeenCalledWith( + 'convert', + expect.anything(), + expect.anything(), + { signal: abortSignal } + ) + }) }) diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts index a4673a1db54..82745f01a47 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts @@ -8,11 +8,17 @@ import { Ffmpeg } from '@/lib/copilot/generated/tool-catalog-v1' import { assertServerToolNotAborted, type BaseServerTool, + resolveServerToolAbortSignal, type ServerToolContext, } from '@/lib/copilot/tools/server/base-tool' import { writeCopilotWorkspaceFileByPath } from '@/lib/copilot/vfs/resource-writer' import { MAX_MEDIA_BYTES } from '@/lib/media/falai' -import { type FfmpegOperation, type MediaFile, runFfmpegOperation } from '@/lib/media/ffmpeg' +import { + type FfmpegOperation, + MAX_FFMPEG_INPUTS, + type MediaFile, + runFfmpegOperation, +} from '@/lib/media/ffmpeg' import { createWorkspaceFileSecretProvenanceFromRegistry, getBoundWorkspaceFileSecretProvenance, @@ -90,6 +96,14 @@ export const ffmpegServerTool: BaseServerTool = { if (inputPaths.length === 0) { return { success: false, message: 'At least one input file is required in inputs.files' } } + // Bounded before any download: the byte budget alone still permits hundreds + // of small clips, and concat re-encodes every one of them serially. + if (inputPaths.length > MAX_FFMPEG_INPUTS) { + return { + success: false, + message: `At most ${MAX_FFMPEG_INPUTS} input files are allowed per ffmpeg operation (got ${inputPaths.length}).`, + } + } let inputRequiresOpaqueError = false try { @@ -138,19 +152,26 @@ export const ffmpegServerTool: BaseServerTool = { inputRequiresOpaqueError ||= inputProvenance.status === 'unknown' || inputProvenance.entries.length > 0 assertServerToolNotAborted(context) - const result = await runFfmpegOperation(params.operation, mediaFiles, { - text: params.text, - position: params.position, - start: params.start, - end: params.end, - width: params.width, - height: params.height, - aspectRatio: params.aspectRatio, - volume: params.volume, - musicVolume: params.musicVolume, - loopToVideo: params.loopToVideo, - format: params.format, - }) + const result = await runFfmpegOperation( + params.operation, + mediaFiles, + { + text: params.text, + position: params.position, + start: params.start, + end: params.end, + width: params.width, + height: params.height, + aspectRatio: params.aspectRatio, + volume: params.volume, + musicVolume: params.musicVolume, + loopToVideo: params.loopToVideo, + format: params.format, + }, + // A transcode outlives its request unless the child is killed, so the + // cancellation signal must reach FFmpeg itself, not just the steps around it. + { signal: resolveServerToolAbortSignal(context) } + ) // probe reports metadata only — no file written. if (params.operation === 'probe') { diff --git a/apps/sim/lib/media/ffmpeg-probe-precedence.test.ts b/apps/sim/lib/media/ffmpeg-probe-precedence.test.ts new file mode 100644 index 00000000000..673e21f53b3 --- /dev/null +++ b/apps/sim/lib/media/ffmpeg-probe-precedence.test.ts @@ -0,0 +1,50 @@ +/** + * Standalone file: the resolved ffprobe path is memoized at module scope, so + * the first resolution in a process wins. Testing precedence therefore needs a + * module whose memo no other test has populated. + * + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' + +const { execSyncMock, execFileMock, existsSyncMock } = vi.hoisted(() => ({ + execSyncMock: vi.fn(), + execFileMock: vi.fn(), + existsSyncMock: vi.fn(), +})) + +vi.mock('node:child_process', () => ({ + execSync: execSyncMock, + execFile: execFileMock, +})) + +vi.mock('node:fs', () => ({ + existsSync: existsSyncMock, +})) + +import { runFfmpegOperation } from '@/lib/media/ffmpeg' + +describe('ffprobe lookup precedence', () => { + it('prefers ffprobe on PATH over a sibling of the ffmpeg binary', async () => { + // ffmpeg resolves into a directory whose ffprobe sibling may be stray or + // unusable; a real PATH entry must win. Both lookups go through execSync, + // so they are distinguished by the command. + execSyncMock.mockImplementation((cmd: string) => { + if (cmd.includes('ffprobe')) return '/usr/bin/ffprobe\n' + if (cmd.includes('ffmpeg')) return '/opt/broken/ffmpeg\n' + throw new Error(`unexpected command: ${cmd}`) + }) + // The sibling exists on disk — without this the test cannot tell the two + // orderings apart, because a non-existent sibling is skipped either way. + existsSyncMock.mockImplementation((p: string) => p === '/opt/broken/ffprobe') + execFileMock.mockImplementation((_bin, _args, _opts, cb) => { + cb(null, JSON.stringify({ format: {}, streams: [] }), '') + return {} + }) + + await runFfmpegOperation('probe', [{ buffer: Buffer.from('media'), mimeType: 'video/mp4' }]) + + expect(execFileMock.mock.calls[0][0]).toBe('/usr/bin/ffprobe') + expect(execFileMock.mock.calls[0][0]).not.toBe('/opt/broken/ffprobe') + }) +}) diff --git a/apps/sim/lib/media/ffmpeg-probe-resolution.test.ts b/apps/sim/lib/media/ffmpeg-probe-resolution.test.ts new file mode 100644 index 00000000000..05d2afbae58 --- /dev/null +++ b/apps/sim/lib/media/ffmpeg-probe-resolution.test.ts @@ -0,0 +1,73 @@ +/** + * Standalone file: the ffmpeg module memoizes its binary lookup at module + * scope, so exercising the "no ffmpeg installed" branch needs a fresh module + * state that a shared file would already have consumed. + * + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { execSyncMock, execFileMock } = vi.hoisted(() => ({ + execSyncMock: vi.fn(), + execFileMock: vi.fn(), +})) + +vi.mock('node:child_process', () => ({ + execSync: execSyncMock, + execFile: execFileMock, +})) + +import { runFfmpegOperation } from '@/lib/media/ffmpeg' + +const PROBE_JSON = JSON.stringify({ + format: { duration: '3', format_name: 'mov,mp4' }, + streams: [{ codec_type: 'video', codec_name: 'h264', width: 640, height: 480 }], +}) + +describe('probing without a discoverable ffmpeg binary', () => { + beforeEach(() => { + vi.clearAllMocks() + // No ffmpeg on this host. + execSyncMock.mockImplementation(() => { + throw new Error('which: no ffmpeg in PATH') + }) + execFileMock.mockImplementation((_bin, _args, _opts, cb) => { + cb(null, PROBE_JSON, '') + return {} + }) + }) + + it('keeps probing across repeated calls', async () => { + const file = { buffer: Buffer.from('media'), mimeType: 'video/mp4' } + + // The second call is the regression: the binary lookup is memoized after + // the first, and an ffmpeg-required check here would throw from then on + // even though ffprobe is perfectly usable. + for (const _ of [1, 2, 3]) { + const result = await runFfmpegOperation('probe', [file]) + expect(result.probe).toMatchObject({ hasVideo: true, width: 640, height: 480 }) + } + + expect(execFileMock).toHaveBeenCalledTimes(3) + expect(execFileMock.mock.calls[0][0]).toContain('ffprobe') + }) + + it('always hands ffprobe a positive timeout', async () => { + // Node reads `timeout: 0` as "no timeout", so the computed cap is floored. + // Asserted on a healthy budget rather than an expired one: forcing the + // expired window means racing the abort timer, which makes the test flaky. + await runFfmpegOperation('probe', [{ buffer: Buffer.from('media'), mimeType: 'video/mp4' }]) + + const opts = execFileMock.mock.calls[0][2] as { timeout: number } + expect(opts.timeout).toBeGreaterThan(0) + expect(opts.timeout).toBeLessThanOrEqual(15_000) + }) + + it('still refuses to transcode, which genuinely needs ffmpeg', async () => { + await expect( + runFfmpegOperation('convert', [{ buffer: Buffer.from('m'), mimeType: 'video/mp4' }], { + format: 'mp3', + }) + ).rejects.toThrow('FFmpeg not found') + }) +}) diff --git a/apps/sim/lib/media/ffmpeg.test.ts b/apps/sim/lib/media/ffmpeg.test.ts new file mode 100644 index 00000000000..99ce87b8b3d --- /dev/null +++ b/apps/sim/lib/media/ffmpeg.test.ts @@ -0,0 +1,138 @@ +/** + * @vitest-environment node + */ +import fs from 'node:fs/promises' +import { describe, expect, it, vi } from 'vitest' +import { MAX_FFMPEG_INPUTS, type MediaFile, runFfmpegOperation } from '@/lib/media/ffmpeg' + +function mediaFile(mimeType = 'video/mp4'): MediaFile { + return { buffer: Buffer.from('media'), mimeType, name: 'clip.mp4' } +} + +describe('runFfmpegOperation input bounds', () => { + it('rejects more inputs than the cap before touching the filesystem', async () => { + const inputs = Array.from({ length: MAX_FFMPEG_INPUTS + 1 }, () => mediaFile()) + + await expect(runFfmpegOperation('concat', inputs)).rejects.toThrow( + `At most ${MAX_FFMPEG_INPUTS} input files` + ) + }) + + it('still requires at least one input', async () => { + await expect(runFfmpegOperation('convert', [], { format: 'mp3' })).rejects.toThrow( + 'At least one input file is required' + ) + }) +}) + +describe('runFfmpegOperation output format validation', () => { + it.each([ + ['../../escape.mp4', 'traversal'], + ['../pwned.mp3', 'parent segment'], + ['/etc/cron.d/x.mp4', 'absolute path'], + ['mp4/../../x', 'embedded separator'], + ])('rejects %s as an output format (%s)', async (format) => { + await expect(runFfmpegOperation('convert', [mediaFile()], { format })).rejects.toThrow( + 'Unsupported output format' + ) + }) + + it('rejects a format with no known muxer', async () => { + await expect(runFfmpegOperation('convert', [mediaFile()], { format: 'exe' })).rejects.toThrow( + 'Unsupported output format' + ) + }) + + it('rejects a traversal format on extract_audio too', async () => { + await expect( + runFfmpegOperation('extract_audio', [mediaFile()], { format: '../../escape.mp3' }) + ).rejects.toThrow('Unsupported output format') + }) + + it('keeps the formats the input MIME map already supported', async () => { + // Asserted through the rejection's own "Supported:" list rather than by + // converting for real: a `not.toThrow` on a live transcode passes on any + // rejection, including "FFmpeg not found". + const error = await runFfmpegOperation('convert', [mediaFile()], { format: 'exe' }).catch( + (e: Error) => e + ) + + for (const format of ['mp4', 'mov', 'webm', 'mp3', 'wav', 'gif', 'webp']) { + expect(error.message).toContain(format) + } + }) + + it('rejects weba, which FFmpeg has no muxer for', async () => { + // The extension appears in the input MIME map, but `ffmpeg out.weba` fails + // with "Error initializing the muxer" — webm is the muxer's real name. + await expect(runFfmpegOperation('convert', [mediaFile()], { format: 'weba' })).rejects.toThrow( + 'Unsupported output format' + ) + }) +}) + +describe('runFfmpegOperation scale bounds', () => { + it.each([ + [30000, 30000], + [1, 1], + [4097, 1080], + [1920, 0], + [0, 1080], + [1920.5, 1080], + ])('rejects scale_pad at %sx%s', async (width, height) => { + await expect(runFfmpegOperation('scale_pad', [mediaFile()], { width, height })).rejects.toThrow( + 'must be an integer between 16 and 4096' + ) + }) +}) + +describe('runFfmpegOperation per-operation validation', () => { + // Each rule is asserted at the operation that owns it. The complementary + // property — that an operation ignores options it never reads — cannot be + // asserted without running a real transcode, so it is left to review. + it('rejects an out-of-range volume on mix_audio, which consumes it', async () => { + await expect( + runFfmpegOperation('mix_audio', [mediaFile('audio/mpeg'), mediaFile('audio/mpeg')], { + volume: 15, + }) + ).rejects.toThrow('volume must be a number between 0 and 10') + }) + + it('rejects a trim whose end precedes its start', async () => { + await expect(runFfmpegOperation('trim', [mediaFile()], { start: 10, end: 5 })).rejects.toThrow( + 'end (5s) must be greater than or equal to start (10s)' + ) + }) + + it('allows webm for extract_audio but not weba', async () => { + const error = await runFfmpegOperation('extract_audio', [mediaFile()], { + format: 'weba', + }).catch((e: Error) => e) + + expect(error.message).toContain('Unsupported output format') + expect(error.message).toContain('webm') + }) + + it('restricts extract_audio to audio containers', async () => { + await expect( + runFfmpegOperation('extract_audio', [mediaFile()], { format: 'png' }) + ).rejects.toThrow('Unsupported output format') + }) +}) + +describe('runFfmpegOperation abort handling', () => { + it('refuses to start once the signal is already aborted', async () => { + const controller = new AbortController() + controller.abort() + const mkdtemp = vi.spyOn(fs, 'mkdtemp') + + await expect( + runFfmpegOperation('convert', [mediaFile()], { format: 'mp3' }, { signal: controller.signal }) + ).rejects.toThrow(/aborted/i) + + // "Refuses to start" means exactly this: no temp dir, so no input was ever + // written and no process was ever spawned. + expect(mkdtemp).not.toHaveBeenCalled() + mkdtemp.mockRestore() + }) +}) diff --git a/apps/sim/lib/media/ffmpeg.ts b/apps/sim/lib/media/ffmpeg.ts index bcfa0b6adf0..2f850106c39 100644 --- a/apps/sim/lib/media/ffmpeg.ts +++ b/apps/sim/lib/media/ffmpeg.ts @@ -1,25 +1,26 @@ -import { execSync } from 'node:child_process' +import { execFile, execSync } from 'node:child_process' +import { existsSync } from 'node:fs' import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' import ffmpeg from 'fluent-ffmpeg' +import { + createTimeoutAbortController, + getRemainingExecutionMs, + type TimeoutAbortController, +} from '@/lib/core/execution-limits' const logger = createLogger('MediaFfmpeg') let ffmpegInitialized = false let ffmpegPath: string | null = null +let ffprobePath: string | null = null -/** Lazy system FFmpeg binary resolution, mirroring lib/audio/extractor.ts. */ -function ensureFfmpeg(): void { - if (ffmpegInitialized) { - if (!ffmpegPath) { - throw new Error( - 'FFmpeg not found. Install: brew install ffmpeg (macOS) / apk add ffmpeg (Alpine) / apt-get install ffmpeg (Ubuntu)' - ) - } - return - } +/** Lazy system FFmpeg binary resolution, mirroring lib/audio/extractor.ts. Never throws. */ +function initFfmpegPath(): void { + if (ffmpegInitialized) return ffmpegInitialized = true try { @@ -31,6 +32,74 @@ function ensureFfmpeg(): void { } } +/** + * Transcoding requires ffmpeg itself. Probing does not — kept separate from + * {@link initFfmpegPath} so a host with only ffprobe can still probe. + */ +function ensureFfmpeg(): void { + initFfmpegPath() + if (!ffmpegPath) { + throw new Error( + 'FFmpeg not found. Install: brew install ffmpeg (macOS) / apk add ffmpeg (Alpine) / apt-get install ffmpeg (Ubuntu)' + ) + } +} + +function lookupOnPath(binary: string): string | null { + try { + const cmd = process.platform === 'win32' ? `where ${binary}` : `which ${binary}` + return execSync(cmd, { encoding: 'utf-8' }).trim().split('\n')[0] || null + } catch { + return null + } +} + +/** + * Mirrors fluent-ffmpeg's resolution order — FFPROBE_PATH, then PATH, then + * ffmpeg's own directory — so replacing its ffprobe call does not narrow where + * the binary may live for self-hosters. PATH outranks the sibling deliberately: + * a stray or unusable file next to ffmpeg must not mask a working install. + */ +function resolveFfprobePath(): string { + if (ffprobePath) return ffprobePath + + const binary = process.platform === 'win32' ? 'ffprobe.exe' : 'ffprobe' + const configured = process.env.FFPROBE_PATH?.trim() + if (configured && existsSync(configured)) { + ffprobePath = configured + return ffprobePath + } + + const onPath = lookupOnPath(binary) + if (onPath) { + ffprobePath = onPath + return ffprobePath + } + + // Deliberately initFfmpegPath, not ensureFfmpeg: a missing ffmpeg must not + // stop a probe when ffprobe is installed on its own. + initFfmpegPath() + const sibling = ffmpegPath ? path.join(path.dirname(ffmpegPath), binary) : undefined + ffprobePath = sibling && existsSync(sibling) ? sibling : binary + return ffprobePath +} + +/** + * Hard bounds for a single operation. FFmpeg runs in the request-serving + * process, so every attacker-influenced dimension needs a ceiling: an + * unbounded input count, filter dimension, or runtime is a whole-instance + * CPU/RAM denial of service, not a single failed request. + */ +export const MAX_FFMPEG_INPUTS = 10 +export const MIN_SCALE_DIMENSION = 16 +export const MAX_SCALE_DIMENSION = 4096 +export const DEFAULT_FFMPEG_TIMEOUT_MS = 5 * 60 * 1000 +const PROBE_TIMEOUT_MS = 15 * 1000 +const PROBE_MAX_OUTPUT_BYTES = 8 * 1024 * 1024 + +const TIME_BUDGET_EXCEEDED = + 'FFmpeg operation exceeded its time budget — try fewer, shorter, or lower-resolution inputs' + export type FfmpegOperation = | 'overlay_audio' | 'mux' @@ -84,6 +153,17 @@ export interface FfmpegResult { probe?: MediaProbe } +/** Execution bounds for one operation, separate from its media parameters. */ +export interface FfmpegRunOptions { + /** Aborts and SIGKILLs every process spawned for the operation. */ + signal?: AbortSignal + /** + * Wall-clock budget for the whole operation. Defaults to, and is capped at, + * DEFAULT_FFMPEG_TIMEOUT_MS — a caller may shorten the ceiling, never raise it. + */ + timeoutMs?: number +} + const MIME_TO_EXT: Record = { 'video/mp4': 'mp4', 'video/mpeg': 'mp4', @@ -129,16 +209,110 @@ const EXT_TO_MIME: Record = { jpg: 'image/jpeg', jpeg: 'image/jpeg', gif: 'image/gif', + webp: 'image/webp', +} + +/** + * The formats a caller may name as an output, declared explicitly rather than + * derived from EXT_TO_MIME: that map answers "what content type is this?", and + * letting an addition there silently widen what may be written couples a + * security allowlist to an unrelated lookup table. + */ +const OUTPUT_EXTS = new Set([ + 'mp4', + 'mov', + 'webm', + 'mkv', + 'avi', + 'mp3', + 'm4a', + 'wav', + 'ogg', + 'flac', + 'aac', + 'opus', + 'png', + 'jpg', + 'jpeg', + 'gif', + 'webp', +]) + +/** + * extract_audio can only name an audio container; the rest would silently + * produce nothing useful. `webm`, not `weba` — FFmpeg's muxer is named webm and + * it refuses a .weba output, so allowlisting that extension would only produce + * a muxer error at encode time. + */ +const AUDIO_EXTS = new Set(['mp3', 'm4a', 'wav', 'ogg', 'flac', 'aac', 'opus', 'webm']) + +/** Containers shared with video, whose content type differs for an audio-only output. */ +const AUDIO_ONLY_MIME: Record = { + webm: 'audio/webm', +} + +/** + * Temp-file names are built as `${prefix}.${ext}` and joined against the temp + * dir, so an extension carrying `/` or `..` escapes that dir once `path.join` + * normalizes it. Both extension sources are attacker-influenced (a stored file's + * MIME type, and the caller-supplied `format`), so neither reaches a path + * unsanitized. + */ +const SAFE_EXT_PATTERN = /^[a-z0-9]{1,8}$/ + +function isSafeExt(ext: string): boolean { + return SAFE_EXT_PATTERN.test(ext) } function extFromMime(mime: string): string { - return MIME_TO_EXT[mime] || mime.split('/')[1] || 'bin' + const known = MIME_TO_EXT[mime] + if (known) return known + const derived = (mime.split('/')[1] || '').toLowerCase() + return isSafeExt(derived) ? derived : 'bin' } function mimeFromExt(ext: string): string { return EXT_TO_MIME[ext] || 'application/octet-stream' } +/** Only formats with a known muxer and a safe file name may name an output. */ +function resolveOutputExt(format: string, allowed: Set = OUTPUT_EXTS): string { + const ext = format.trim().toLowerCase() + if (!isSafeExt(ext) || !allowed.has(ext)) { + throw new Error(`Unsupported output format "${format}". Supported: ${[...allowed].join(', ')}`) + } + return ext +} + +/** Scale targets land in a filter graph, where an oversized value allocates per-frame buffers. */ +function resolveScaleDimension(value: number, label: 'width' | 'height'): number { + if (!Number.isInteger(value) || value < MIN_SCALE_DIMENSION || value > MAX_SCALE_DIMENSION) { + throw new Error( + `${label} must be an integer between ${MIN_SCALE_DIMENSION} and ${MAX_SCALE_DIMENSION} (got ${value})` + ) + } + return value +} + +function clampProbedDimension(value: number | undefined, fallback: number): number { + if (!Number.isInteger(value) || (value as number) < MIN_SCALE_DIMENSION) return fallback + return Math.min(value as number, MAX_SCALE_DIMENSION) +} + +function resolveNonNegativeSeconds(value: number, label: string): number { + if (!Number.isFinite(value) || value < 0) { + throw new Error(`${label} must be a non-negative number of seconds (got ${value})`) + } + return value +} + +function resolveVolume(value: number, label: string): number { + if (!Number.isFinite(value) || value < 0 || value > 10) { + throw new Error(`${label} must be a number between 0 and 10 (got ${value})`) + } + return value +} + const ASPECT_TARGETS: Record = { '16:9': { w: 1920, h: 1080 }, '9:16': { w: 1080, h: 1920 }, @@ -173,8 +347,35 @@ function escapeDrawtext(text: string): string { return text.replace(/\\/g, '\\\\').replace(/:/g, '\\:').replace(/'/g, "\\'").replace(/%/g, '\\%') } +/** + * One deadline for the whole operation, not per spawned process: `concat` runs + * an encode per clip, so a per-command timeout would still multiply out to an + * unbounded total. Every spawn shares this signal and is SIGKILLed when it + * fires — whether from the deadline or the caller's own cancellation. + */ +interface RunContext { + dir: string + limit: TimeoutAbortController +} + +function createOperationLimit(runOptions: FfmpegRunOptions): TimeoutAbortController { + const requested = runOptions.timeoutMs + const timeoutMs = + typeof requested === 'number' && requested > 0 + ? Math.min(requested, DEFAULT_FFMPEG_TIMEOUT_MS) + : DEFAULT_FFMPEG_TIMEOUT_MS + return createTimeoutAbortController(timeoutMs, runOptions.signal) +} + +function abortError(limit: TimeoutAbortController): Error { + return new Error(limit.isTimedOut() ? TIME_BUDGET_EXCEEDED : 'FFmpeg operation aborted') +} + +function assertOperationLive(limit: TimeoutAbortController): void { + if (limit.signal.aborted) throw abortError(limit) +} + async function withTempDir(fn: (dir: string) => Promise): Promise { - ensureFfmpeg() const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'media-ffmpeg-')) try { return await fn(dir) @@ -183,50 +384,162 @@ async function withTempDir(fn: (dir: string) => Promise): Promise { } } +/** + * Resolves a name inside the operation's temp dir and refuses anything that + * escapes it. The invariant this module needs is "nothing is read or written + * outside the temp dir" — asserting it here makes that structural, rather than + * depending on every present and future filename source remembering to + * sanitize itself. + */ +function tempPath(dir: string, name: string): string { + const resolved = path.resolve(dir, name) + if (resolved !== dir && !resolved.startsWith(dir + path.sep)) { + throw new Error(`Refusing to use a path outside the working directory: ${name}`) + } + return resolved +} + async function writeInput(dir: string, file: MediaFile, index: number): Promise { const ext = extFromMime(file.mimeType) - const filePath = path.join(dir, `in-${index}.${ext}`) + const filePath = tempPath(dir, `in-${index}.${ext}`) await fs.writeFile(filePath, file.buffer) return filePath } -function runCommand(command: ffmpeg.FfmpegCommand, outputPath: string): Promise { +function runCommand( + command: ffmpeg.FfmpegCommand, + outputPath: string, + limit: TimeoutAbortController +): Promise { + ensureFfmpeg() + assertOperationLive(limit) return new Promise((resolve, reject) => { - command - .on('end', () => resolve()) - .on('error', (err) => reject(new Error(`FFmpeg error: ${err.message}`))) - .save(outputPath) + let settled = false + + /** SIGKILL, not SIGTERM: a wedged encoder must not get to ignore the signal. */ + function hardKill(): void { + try { + command.kill('SIGKILL') + } catch { + // Already gone, or not yet spawned; onStart covers the latter. + } + } + function settle(err?: Error): void { + if (settled) return + settled = true + limit.signal.removeEventListener('abort', onAbort) + if (err) reject(err) + else resolve() + } + /** + * fluent-ffmpeg's kill() is a silent no-op until the child exists, and + * `.save()` spawns asynchronously (it may shell out for capability checks + * first). A kill landing in that window would otherwise reject the promise + * while the encode goes on to spawn orphaned and unkillable — so re-issue + * it once the process is up. 'start' fires immediately after the spawn. + */ + function onStart(): void { + if (settled) hardKill() + } + function onAbort(): void { + hardKill() + settle(abortError(limit)) + } + + limit.signal.addEventListener('abort', onAbort, { once: true }) + + try { + command + .on('start', onStart) + .on('end', () => settle()) + .on('error', (err) => settle(new Error(`FFmpeg error: ${err.message}`))) + .save(outputPath) + } catch (err) { + // Keeps the abort listener from outliving a command that never started. + settle(toError(err)) + } }) } -export async function probeMedia(file: MediaFile): Promise { - return withTempDir(async (dir) => { - const inputPath = await writeInput(dir, file, 0) - return probeFile(inputPath) - }) +interface FfprobeOutput { + format?: { duration?: string | number; format_name?: string } + streams?: Array<{ + codec_type?: string + codec_name?: string + width?: number + height?: number + }> } -function probeFile(filePath: string): Promise { - ensureFfmpeg() +/** + * Distinguishes the three ways a probe fails. Node's `execFile` error message + * is `Command failed: ` plus stderr, which both leaks the server's + * binary and temp paths to the caller and — once stderr is quiet — renders a + * timeout, a corrupt file, and a missing file byte-identical. + */ +function describeProbeFailure(err: Error & { killed?: boolean; code?: unknown }, stderr: string) { + if (err.code === 'ABORT_ERR') return 'aborted' + if (err.killed) return 'timed out' + const detail = stderr.trim().split('\n').pop() + if (!detail) return 'unreadable media' + // ffprobe prefixes its diagnostic with the input path; keep the diagnostic, + // drop the server's directory layout. + return detail.replace( + /(^|\s)(\/\S+)/g, + (_match, lead: string, abs: string) => `${lead}${path.basename(abs)}` + ) +} + +/** + * Spawned directly rather than through `fluent-ffmpeg.ffprobe`, which gives no + * handle on the child and so cannot be killed: a crafted input that wedges + * ffprobe would otherwise hang forever holding a request. + */ +function probeFile(filePath: string, limit: TimeoutAbortController): Promise { + assertOperationLive(limit) + const remaining = getRemainingExecutionMs(limit.signal) ?? PROBE_TIMEOUT_MS + // Floored at 1ms: Node reads `timeout: 0` as "no timeout", so an expired + // budget would otherwise remove the probe cap entirely — the opposite of what + // an exhausted budget should do. Reachable between the deadline passing and + // the abort timer firing, where assertOperationLive still sees a live signal. + const timeout = Math.max(1, Math.min(PROBE_TIMEOUT_MS, remaining)) return new Promise((resolve, reject) => { - ffmpeg.ffprobe(filePath, (err, metadata) => { - if (err) { - reject(new Error(`FFprobe error: ${err.message}`)) - return + execFile( + resolveFfprobePath(), + ['-v', 'error', '-print_format', 'json', '-show_format', '-show_streams', '-i', filePath], + { + timeout, + killSignal: 'SIGKILL', + maxBuffer: PROBE_MAX_OUTPUT_BYTES, + signal: limit.signal, + }, + (err, stdout, stderr) => { + if (err) { + reject(new Error(`FFprobe error: ${describeProbeFailure(err, stderr)}`)) + return + } + let metadata: FfprobeOutput + try { + metadata = JSON.parse(stdout) as FfprobeOutput + } catch { + reject(new Error('FFprobe error: unreadable metadata')) + return + } + const streams = metadata.streams ?? [] + const video = streams.find((s) => s.codec_type === 'video') + const audio = streams.find((s) => s.codec_type === 'audio') + resolve({ + durationSeconds: Number(metadata.format?.duration) || 0, + format: metadata.format?.format_name || 'unknown', + width: video?.width, + height: video?.height, + videoCodec: video?.codec_name, + audioCodec: audio?.codec_name, + hasAudio: Boolean(audio), + hasVideo: Boolean(video), + }) } - const video = metadata.streams.find((s) => s.codec_type === 'video') - const audio = metadata.streams.find((s) => s.codec_type === 'audio') - resolve({ - durationSeconds: Number(metadata.format?.duration) || 0, - format: metadata.format?.format_name || 'unknown', - width: video?.width, - height: video?.height, - videoCodec: video?.codec_name, - audioCodec: audio?.codec_name, - hasAudio: Boolean(audio), - hasVideo: Boolean(video), - }) - }) + ) }) } @@ -237,61 +550,79 @@ function probeFile(filePath: string): Promise { export async function runFfmpegOperation( operation: FfmpegOperation, inputs: MediaFile[], - options: FfmpegOptions = {} + options: FfmpegOptions = {}, + runOptions: FfmpegRunOptions = {} ): Promise { if (inputs.length === 0) { throw new Error('At least one input file is required') } - - if (operation === 'probe') { - return { probe: await probeMedia(inputs[0]) } + if (inputs.length > MAX_FFMPEG_INPUTS) { + throw new Error( + `At most ${MAX_FFMPEG_INPUTS} input files are allowed per operation (got ${inputs.length})` + ) } - return withTempDir(async (dir) => { - const inputPaths = await Promise.all(inputs.map((f, i) => writeInput(dir, f, i))) - - switch (operation) { - case 'overlay_audio': - case 'mux': - return overlayAudio(dir, inputPaths, options) - case 'mix_audio': - return mixAudio(dir, inputPaths, options) - case 'concat': - return concat(dir, inputPaths) - case 'trim': - return trim(dir, inputPaths[0], inputs[0], options) - case 'scale_pad': - return scalePad(dir, inputPaths[0], options) - case 'overlay_image': - return overlayImage(dir, inputPaths, options) - case 'add_text': - return addText(dir, inputPaths[0], options) - case 'fade': - return fade(dir, inputPaths[0], inputs[0], options) - case 'extract_audio': - return extractAudio(dir, inputPaths[0], options) - case 'convert': - return convert(dir, inputPaths[0], options) - case 'thumbnail': - return thumbnail(dir, inputPaths[0], options) - default: - throw new Error(`Unsupported ffmpeg operation: ${operation}`) - } - }) + const limit = createOperationLimit(runOptions) + assertOperationLive(limit) + + try { + return await withTempDir(async (dir) => { + const ctx: RunContext = { dir, limit } + if (operation === 'probe') { + return { probe: await probeFile(await writeInput(dir, inputs[0], 0), limit) } + } + + const inputPaths = await Promise.all(inputs.map((f, i) => writeInput(dir, f, i))) + + switch (operation) { + case 'overlay_audio': + case 'mux': + return overlayAudio(ctx, inputPaths, options) + case 'mix_audio': + return mixAudio(ctx, inputPaths, options) + case 'concat': + return concat(ctx, inputPaths) + case 'trim': + return trim(ctx, inputPaths[0], inputs[0], options) + case 'scale_pad': + return scalePad(ctx, inputPaths[0], options) + case 'overlay_image': + return overlayImage(ctx, inputPaths, options) + case 'add_text': + return addText(ctx, inputPaths[0], options) + case 'fade': + return fade(ctx, inputPaths[0], inputs[0], options) + case 'extract_audio': + return extractAudio(ctx, inputPaths[0], options) + case 'convert': + return convert(ctx, inputPaths[0], options) + case 'thumbnail': + return thumbnail(ctx, inputPaths[0], options) + default: + throw new Error(`Unsupported ffmpeg operation: ${operation}`) + } + }) + } finally { + limit.cleanup() + } } -async function readOut(outputPath: string, ext: string): Promise { +async function readOut( + outputPath: string, + ext: string, + contentType = mimeFromExt(ext) +): Promise { const buffer = await fs.readFile(outputPath) - return { buffer, ext, contentType: mimeFromExt(ext) } + return { buffer, ext, contentType } } async function overlayAudio( - dir: string, + { dir, limit }: RunContext, inputPaths: string[], options: FfmpegOptions ): Promise { if (inputPaths.length < 2) throw new Error('overlay_audio requires [video, audio]') - const outputPath = path.join(dir, 'out.mp4') + const outputPath = tempPath(dir, 'out.mp4') const command = ffmpeg().input(inputPaths[0]) if (options.loopToVideo) { command.input(inputPaths[1]).inputOptions(['-stream_loop', '-1']) @@ -309,19 +640,19 @@ async function overlayAudio( 'aac', '-shortest', ]) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'mp4') } async function mixAudio( - dir: string, + { dir, limit }: RunContext, inputPaths: string[], options: FfmpegOptions ): Promise { if (inputPaths.length < 2) throw new Error('mix_audio requires [voice, music]') - const outputPath = path.join(dir, 'out.mp3') - const voiceVol = options.volume ?? 1 - const musicVol = options.musicVolume ?? 0.3 + const outputPath = tempPath(dir, 'out.mp3') + const voiceVol = resolveVolume(options.volume ?? 1, 'volume') + const musicVol = resolveVolume(options.musicVolume ?? 0.3, 'musicVolume') const command = ffmpeg() .input(inputPaths[0]) .input(inputPaths[1]) @@ -331,13 +662,13 @@ async function mixAudio( `[v][m]amix=inputs=2:duration=longest:dropout_transition=0[a]`, ]) .outputOptions(['-map', '[a]']) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'mp3') } -async function concat(dir: string, inputPaths: string[]): Promise { +async function concat({ dir, limit }: RunContext, inputPaths: string[]): Promise { if (inputPaths.length < 2) throw new Error('concat requires at least 2 clips') - const probes = await Promise.all(inputPaths.map(probeFile)) + const probes = await Promise.all(inputPaths.map((p) => probeFile(p, limit))) probes.forEach((p, i) => { if (!p.hasVideo) { throw new Error( @@ -345,8 +676,11 @@ async function concat(dir: string, inputPaths: string[]): Promise ) } }) - const width = probes[0].width || 1280 - const height = probes[0].height || 720 + // Clamped, not trusted: these come from the first clip's container metadata, + // and a crafted file can declare dimensions that make the normalize pass + // allocate gigabytes per frame. + const width = clampProbedDimension(probes[0].width, 1280) + const height = clampProbedDimension(probes[0].height, 720) const fps = 30 // Normalize every clip to identical codec/size/fps/pixfmt, and SYNTHESIZE silent @@ -355,7 +689,7 @@ async function concat(dir: string, inputPaths: string[]): Promise // non-existent [i:a]), which is the "Error binding filtergraph inputs/outputs" failure. const normalized: string[] = [] for (let i = 0; i < inputPaths.length; i++) { - const out = path.join(dir, `norm-${i}.mp4`) + const out = tempPath(dir, `norm-${i}.mp4`) const cmd = ffmpeg().input(inputPaths[i]) const maps: string[] = ['-map', '0:v:0'] const extra: string[] = [] @@ -396,85 +730,99 @@ async function concat(dir: string, inputPaths: string[]): Promise '2', ...extra, ]) - await runCommand(cmd, out) + await runCommand(cmd, out, limit) normalized.push(out) } // Concatenate the now-uniform clips with the concat demuxer (stream copy: fast + reliable). - const listPath = path.join(dir, 'concat-list.txt') + const listPath = tempPath(dir, 'concat-list.txt') await fs.writeFile( listPath, normalized.map((p) => `file '${p.replace(/'/g, "'\\''")}'`).join('\n') ) - const outputPath = path.join(dir, 'out.mp4') + const outputPath = tempPath(dir, 'out.mp4') const concatCmd = ffmpeg() .input(listPath) .inputOptions(['-f', 'concat', '-safe', '0']) .outputOptions(['-c', 'copy', '-movflags', '+faststart']) - await runCommand(concatCmd, outputPath) + await runCommand(concatCmd, outputPath, limit) return readOut(outputPath, 'mp4') } async function trim( - dir: string, + { dir, limit }: RunContext, inputPath: string, input: MediaFile, options: FfmpegOptions ): Promise { const ext = extFromMime(input.mimeType) - const outputPath = path.join(dir, `out.${ext}`) - const start = options.start ?? 0 + const outputPath = tempPath(dir, `out.${ext}`) + const start = resolveNonNegativeSeconds(options.start ?? 0, 'start') const command = ffmpeg(inputPath).setStartTime(start) if (options.end !== undefined) { - command.setDuration(Math.max(0, options.end - start)) + const end = resolveNonNegativeSeconds(options.end, 'end') + // Without this, `end < start` clamps to a zero-length output that is written + // to the workspace and reported as a success. + if (end < start) { + throw new Error(`end (${end}s) must be greater than or equal to start (${start}s)`) + } + command.setDuration(end - start) } - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, ext) } async function scalePad( - dir: string, + { dir, limit }: RunContext, inputPath: string, options: FfmpegOptions ): Promise { let width = options.width let height = options.height - if ((!width || !height) && options.aspectRatio && ASPECT_TARGETS[options.aspectRatio]) { + // Only an omitted dimension falls back to the aspect ratio. A supplied 0 is a + // bad value, not an absent one, and must reach the bounds check to say so. + if ( + (width === undefined || height === undefined) && + options.aspectRatio && + ASPECT_TARGETS[options.aspectRatio] + ) { width = ASPECT_TARGETS[options.aspectRatio].w height = ASPECT_TARGETS[options.aspectRatio].h } - if (!width || !height) { + if (width === undefined || height === undefined) { throw new Error('scale_pad requires width+height or a known aspectRatio (e.g. 9:16)') } - const outputPath = path.join(dir, 'out.mp4') + const scaleWidth = resolveScaleDimension(width, 'width') + const scaleHeight = resolveScaleDimension(height, 'height') + const outputPath = tempPath(dir, 'out.mp4') const command = ffmpeg(inputPath) .videoFilters( - `scale=${width}:${height}:force_original_aspect_ratio=decrease,pad=${width}:${height}:(ow-iw)/2:(oh-ih)/2,setsar=1` + `scale=${scaleWidth}:${scaleHeight}:force_original_aspect_ratio=decrease,pad=${scaleWidth}:${scaleHeight}:(ow-iw)/2:(oh-ih)/2,setsar=1` ) .outputOptions(['-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'mp4') } async function overlayImage( - dir: string, + { dir, limit }: RunContext, inputPaths: string[], options: FfmpegOptions ): Promise { if (inputPaths.length < 2) throw new Error('overlay_image requires [video, image]') const xy = OVERLAY_POSITION[options.position || 'top-right'] || OVERLAY_POSITION['top-right'] - const outputPath = path.join(dir, 'out.mp4') + const outputPath = tempPath(dir, 'out.mp4') const command = ffmpeg() .input(inputPaths[0]) .input(inputPaths[1]) .complexFilter([`[0:v][1:v]overlay=${xy}[v]`]) .outputOptions(['-map', '[v]', '-map', '0:a?', '-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'mp4') } async function addText( - dir: string, + { dir, limit }: RunContext, inputPath: string, options: FfmpegOptions ): Promise { @@ -490,70 +838,72 @@ async function addText( `x=${pos.x}`, `y=${pos.y}`, ].join(':') - const outputPath = path.join(dir, 'out.mp4') + const outputPath = tempPath(dir, 'out.mp4') const command = ffmpeg(inputPath) .videoFilters(`drawtext=${drawtext}`) .outputOptions(['-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'mp4') } async function fade( - dir: string, + { dir, limit }: RunContext, inputPath: string, input: MediaFile, _options: FfmpegOptions ): Promise { - const probe = await probeFile(inputPath) + const probe = await probeFile(inputPath, limit) const duration = probe.durationSeconds || 0 const fadeDur = Math.min(0.5, duration / 4 || 0.5) const outStart = Math.max(0, duration - fadeDur) const isVideo = input.mimeType.startsWith('video/') || probe.hasVideo const ext = isVideo ? 'mp4' : extFromMime(input.mimeType) - const outputPath = path.join(dir, `out.${ext}`) + const outputPath = tempPath(dir, `out.${ext}`) const command = ffmpeg(inputPath) if (isVideo) { command.videoFilters([`fade=t=in:st=0:d=${fadeDur}`, `fade=t=out:st=${outStart}:d=${fadeDur}`]) } command.audioFilters([`afade=t=in:st=0:d=${fadeDur}`, `afade=t=out:st=${outStart}:d=${fadeDur}`]) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, ext) } async function extractAudio( - dir: string, + { dir, limit }: RunContext, inputPath: string, options: FfmpegOptions ): Promise { - const ext = (options.format || 'mp3').toLowerCase() - const outputPath = path.join(dir, `out.${ext}`) + const ext = resolveOutputExt(options.format || 'mp3', AUDIO_EXTS) + const outputPath = tempPath(dir, `out.${ext}`) const command = ffmpeg(inputPath).noVideo() - await runCommand(command, outputPath) - return readOut(outputPath, ext) + await runCommand(command, outputPath, limit) + // A container shared with video (webm) resolves to a video content type by + // default, but this output has had its video stream dropped. + return readOut(outputPath, ext, AUDIO_ONLY_MIME[ext] ?? mimeFromExt(ext)) } async function convert( - dir: string, + { dir, limit }: RunContext, inputPath: string, options: FfmpegOptions ): Promise { if (!options.format) throw new Error('convert requires a target format') - const ext = options.format.toLowerCase() - const outputPath = path.join(dir, `out.${ext}`) - await runCommand(ffmpeg(inputPath), outputPath) + const ext = resolveOutputExt(options.format) + const outputPath = tempPath(dir, `out.${ext}`) + await runCommand(ffmpeg(inputPath), outputPath, limit) return readOut(outputPath, ext) } async function thumbnail( - dir: string, + { dir, limit }: RunContext, inputPath: string, options: FfmpegOptions ): Promise { - const outputPath = path.join(dir, 'out.jpg') + const outputPath = tempPath(dir, 'out.jpg') const command = ffmpeg(inputPath) - .seekInput(options.start ?? 0) + .seekInput(resolveNonNegativeSeconds(options.start ?? 0, 'start')) .frames(1) - await runCommand(command, outputPath) + await runCommand(command, outputPath, limit) return readOut(outputPath, 'jpg') }