fix(media): bound the copilot ffmpeg tool's inputs, runtime, and output paths

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.'

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 }
+    )
+  })
 })

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<FfmpegArgs, FfmpegResult> = {
     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<FfmpegArgs, FfmpegResult> = {
       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') {

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')
+  })
+})

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')
+  })
+})