fix(ffmpeg): preserve paths through filter and concat parsers

src/ffmpeg/escape.ts

@@ -1,28 +1,20 @@
 /**
- * Quote a value for safe use as a filter option value in an ffmpeg filter
- * graph. Wraps the value in single quotes and escapes internal backslashes
- * and single quotes per ffmpeg's filter escape rules.
+ * Escape a filter option value through FFmpeg's option and filtergraph parsers.
+ * The graph parser consumes the outer quotes first, so option-level escapes
+ * must survive it. In particular, a Windows drive colon still separates options
+ * without its own escape, even inside an outer quoted filtergraph value.
  *
- * This handles the FIRST level of escaping — filter option values — which
- * is enough for paths, color names, numeric expressions, and short text
- * snippets that go directly into a `key=value` pair.
- *
- * Some filter options (notably `drawtext` `text=`) interpret their content
- * as a SECOND-level expression where additional escaping rules apply.
- * For user-provided text, route the content through `textfile=<path>`
- * instead of inlining; this function quotes the path safely while keeping
- * the text content itself out of the filter graph entirely.
- *
- * See: https://ffmpeg.org/ffmpeg-filters.html#Filtergraph-syntax-1
- *
- * @example
- *   quoteFilterArg("don't")         // "'don\\'t'"
- *   quoteFilterArg('a:b,c[d]')      // "'a:b,c[d]'"
- *   quoteFilterArg('/tmp/x.txt')    // "'/tmp/x.txt'"
+ * These values are passed in an argument array, not through a shell. User text
+ * should still go through textfile rather than drawtext's text expansion.
+ * See https://ffmpeg.org/ffmpeg-filters.html#Notes-on-filtergraph-escaping
  */
 export function quoteFilterArg(value: string): string {
-  // Order matters: escape backslashes first, otherwise the backslash we add
-  // to escape a single quote would itself get re-doubled on a second pass.
-  const escaped = value.replace(/\\/g, '\\\\').replace(/'/g, "\\'");
-  return `'${escaped}'`;
+  const optionValue = value.replace(/[\\':]/g, (char) => `\\${char}`);
+  return `'${optionValue.replace(/'/g, "'\\''")}'`;
+}
+
+/** Quote a filename for the concat demuxer's single parsing layer. */
+export function quoteConcatPath(value: string): string {
+  if (/[\r\n\0]/.test(value)) throw new Error('Concat paths must not contain line breaks or NUL');
+  return `'${value.replace(/'/g, "'\\''")}'`;
 }

src/tools/concat.ts

@@ -3,7 +3,7 @@ import { unlink, writeFile } from 'node:fs/promises';
 import { resolve } from 'node:path';
 import { z } from 'zod';
 import { buildConcatArgs } from '../ffmpeg/args/concat.js';
-import { quoteFilterArg } from '../ffmpeg/escape.js';
+import { quoteConcatPath } from '../ffmpeg/escape.js';
 import { runFfmpeg } from '../ffmpeg/run.js';
 import { ensureWorkspace, getWorkspace, newOutputPath, resolveInput } from '../workspace.js';
 
@@ -28,7 +28,7 @@ export interface ConcatResult {
  * trailing newline) and small mistakes show up only at ffmpeg-runtime.
  */
 export function buildConcatListContent(absolutePaths: string[]): string {
-  return `${absolutePaths.map((path) => `file ${quoteFilterArg(path)}`).join('\n')}\n`;
+  return `${absolutePaths.map((path) => `file ${quoteConcatPath(path)}`).join('\n')}\n`;
 }
 
 export async function concat(input: ConcatInputType): Promise<ConcatResult> {

tests/add-text.test.ts

@@ -8,26 +8,24 @@ describe('quoteFilterArg', () => {
     expect(quoteFilterArg('hello')).toBe("'hello'");
   });
 
-  it('escapes internal single quotes', () => {
-    expect(quoteFilterArg("don't")).toBe("'don\\'t'");
+  it('preserves a quote through both parser layers', () => {
+    expect(quoteFilterArg("don't")).toBe(String.raw`'don\'\''t'`);
   });
 
-  it('escapes backslashes', () => {
-    expect(quoteFilterArg('a\\b')).toBe("'a\\\\b'");
+  it('preserves backslashes through both parser layers', () => {
+    expect(quoteFilterArg(String.raw`a\b`)).toBe(String.raw`'a\\b'`);
   });
 
-  it('escapes backslash before single quote (backslash escaped first)', () => {
-    // Input "\\'" (literal backslash then single quote) should become "\\\\\\'"
-    // — the backslash doubles to \\, and the quote becomes \'.
-    expect(quoteFilterArg("\\'")).toBe("'\\\\\\''");
+  it('preserves a backslash immediately before a quote', () => {
+    expect(quoteFilterArg(String.raw`\'`)).toBe(String.raw`'\\\'\'''`);
   });
 
   it('handles empty string', () => {
     expect(quoteFilterArg('')).toBe("''");
   });
 
-  it('leaves colons, commas, brackets alone (single-quoting handles them)', () => {
-    expect(quoteFilterArg('a:b,c[d];e')).toBe("'a:b,c[d];e'");
+  it('protects option colons as well as filtergraph delimiters', () => {
+    expect(quoteFilterArg('a:b,c[d];e')).toBe(String.raw`'a\:b,c[d];e'`);
   });
 
   it('handles unicode without modification', () => {
@@ -38,17 +36,14 @@ describe('quoteFilterArg', () => {
     expect(quoteFilterArg('/tmp/text-abc.txt')).toBe("'/tmp/text-abc.txt'");
   });
 
-  it('handles macOS /var/folders paths', () => {
-    const p = '/var/folders/dn/vk2rnbvx7k98/T/makemyclip-editor/add-text-123.txt';
-    expect(quoteFilterArg(p)).toBe(`'${p}'`);
+  it('keeps Windows drive colons and punctuation inside the filename', () => {
+    expect(quoteFilterArg(String.raw`C:\Users\Agent's work, [draft];\notes.txt`)).toBe(
+      String.raw`'C\:\\Users\\Agent\'\''s work, [draft];\\notes.txt'`,
+    );
   });
 
-  it('round-trips through a simulated parser', () => {
-    const original = "tricky: 'value' with\\backslash and: more, stuff";
-    const quoted = quoteFilterArg(original);
-    // Simulate ffmpeg: strip outer quotes, unescape \' then \\ in that order
-    const unquoted = quoted.slice(1, -1).replace(/\\'/g, "'").replace(/\\\\/g, '\\');
-    expect(unquoted).toBe(original);
+  it('preserves leading and trailing whitespace', () => {
+    expect(quoteFilterArg(' spaced name ')).toBe("' spaced name '");
   });
 });
 

tests/compile.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it } from 'vitest';
+import { quoteFilterArg } from '../src/ffmpeg/escape.js';
 import { type CompileContext, CompileError, compileTimeline } from '../src/timeline/compile.js';
 import { emptyComposition } from '../src/timeline/composition.js';
 import { applyOps, colorClip, mediaClip, textClip, videoTrack } from '../src/timeline/ops.js';
@@ -117,7 +118,7 @@ describe('compileTimeline — generated clips', () => {
     expect(step?.textFiles[0]?.content).toBe("It's a title: colons & commas");
     const fc = step?.args[step.args.indexOf('-filter_complex') + 1] ?? '';
     expect(fc).toContain('drawtext=');
-    expect(fc).toContain(`textfile='${step?.textFiles[0]?.path}'`);
+    expect(fc).toContain(`textfile=${quoteFilterArg(step?.textFiles[0]?.path ?? '')}`);
     // The raw text must NOT be inlined in the filter graph.
     expect(fc).not.toContain('a title: colons');
   });

tests/concat.test.ts

@@ -38,12 +38,20 @@ describe('buildConcatListContent', () => {
 
   it('escapes single quotes in paths', () => {
     const content = buildConcatListContent(["/tmp/it's a file.mp4"]);
-    expect(content).toBe("file '/tmp/it\\'s a file.mp4'\n");
+    expect(content).toBe("file '/tmp/it'\\''s a file.mp4'\n");
   });
 
-  it('escapes backslashes in paths', () => {
+  it('keeps backslashes literal inside a quoted concat path', () => {
     const content = buildConcatListContent(['C:\\videos\\clip.mp4']);
-    expect(content).toBe("file 'C:\\\\videos\\\\clip.mp4'\n");
+    expect(content).toBe("file 'C:\\videos\\clip.mp4'\n");
+  });
+
+  it.each([
+    'line\nbreak.mp4',
+    'carriage\rreturn.mp4',
+    'nul\0byte.mp4',
+  ])('rejects a path that cannot be represented as one list line: %s', (path) => {
+    expect(() => buildConcatListContent([path])).toThrow(/line breaks or NUL/);
   });
 
   it('handles unicode paths', () => {

tests/integration/filter-escape.test.ts

@@ -0,0 +1,83 @@
+import { mkdtemp, rmdir, unlink, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { describe, it } from 'vitest';
+import { buildAddTextArgs } from '../../src/ffmpeg/args/add-text.js';
+import { buildConcatArgs } from '../../src/ffmpeg/args/concat.js';
+import { runFfmpeg } from '../../src/ffmpeg/run.js';
+import { buildConcatListContent } from '../../src/tools/concat.js';
+
+// The real parser catches errors that a string-unescaping simulation misses.
+describe.skipIf(process.env.CI !== 'true')('filter option escaping with FFmpeg', () => {
+  it.each([
+    'plain note.txt',
+    "O'Brien, [notes]; v1.txt",
+    process.platform === 'win32' ? 'drive path.txt' : 'drive:path.txt',
+  ])('reads the exact textfile %s', async (name) => {
+    const dir = await mkdtemp(join(tmpdir(), 'makemyclip-escape-'));
+    const textfile = join(dir, name);
+    try {
+      await writeFile(textfile, 'Visible text', 'utf8');
+      const filter = buildAddTextArgs({
+        input: 'unused',
+        output: 'unused',
+        textfile,
+        position: 'center',
+        startSec: 0,
+        endSec: 1,
+        fontSize: 16,
+        box: false,
+      })[4];
+      if (!filter) throw new Error('Missing drawtext filter');
+      await runFfmpeg([
+        '-hide_banner',
+        '-loglevel',
+        'error',
+        '-f',
+        'lavfi',
+        '-i',
+        'color=c=black:s=96x64:r=10:d=0.2',
+        '-vf',
+        filter,
+        '-frames:v',
+        '1',
+        '-f',
+        'null',
+        '-',
+      ]);
+    } finally {
+      await unlink(textfile);
+      await rmdir(dir);
+    }
+  });
+
+  it('uses concat-list quoting without filter-option escapes', async () => {
+    const dir = await mkdtemp(join(tmpdir(), 'makemyclip-escape-'));
+    const input = join(dir, "O'Brien, [clip];.mp4");
+    const listFile = join(dir, 'list.txt');
+    const output = join(dir, 'joined.mp4');
+    try {
+      await runFfmpeg([
+        '-hide_banner',
+        '-loglevel',
+        'error',
+        '-f',
+        'lavfi',
+        '-i',
+        'color=c=black:s=96x64:r=10:d=0.2',
+        '-c:v',
+        'libx264',
+        '-pix_fmt',
+        'yuv420p',
+        input,
+      ]);
+      await writeFile(listFile, buildConcatListContent([input, input]), 'utf8');
+      await runFfmpeg(buildConcatArgs({ listFile, output }));
+    } finally {
+      for (const file of [input, listFile, output]) {
+        await unlink(file).catch(() => undefined);
+      }
+      await rmdir(dir);
+    }
+  });
+});