Harden Cutline imports and test portability

plugins/klarkxy/cutline/PATCHES.md

@@ -371,9 +371,14 @@ session or the runtime restarts. Existing sessions and active jobs are not timed
 out or evicted. Provenance lists `mp4-demux.js` only in the client payload.
 
 Test fixtures live under repository `.fablecut-acceptance/` (or the explicit
-`FABLECUT_TEST_ROOT` override), outside the installable package. Verbatim font
-license whitespace and the upstream `analyze.js` final blank line remain intact;
-these produce existing `git diff --cached --check` diagnostics.
+`FABLECUT_TEST_ROOT` override), outside the installable package. Media-dependent
+tests skip explicitly when FFmpeg is not on PATH, so a Node-and-Git-only CI
+runner still exercises every non-media test. The private-redirect counter
+listens on `127.0.0.1`, while the URL still names `127.0.0.2`: a correct
+implementation never connects to that target, so macOS does not have to
+configure the second loopback address. Verbatim font license whitespace and the
+upstream `analyze.js` final blank line remain intact; these produce existing
+`git diff --cached --check` diagnostics.
 
 ## Export recovery messages
 
@@ -387,7 +392,7 @@ mode.
 
 `fablecut_analyze_reference` accepts only video and audio extensions before it copies an explicit local path into `media/`. This prevents arbitrary files from being exposed through the editor's media route. Supported extensions are `.mp4`, `.webm`, `.mov`, `.mkv`, `.m4v`, `.avi`, `.mp3`, `.wav`, `.ogg`, `.m4a`, `.aac` and `.flac`. The regression test verifies that a `.pem` file is rejected and never copied.
 
-Managed `/media/` and `/library/` URLs are resolved with `realpath` containment in both the HTTP server and MCP media reader; a symlink inside either managed root that targets outside it is refused. Local media copies use an exclusive-create loop so concurrent imports cannot overwrite a same-named file. The upstream `paths.js` now requires `FABLECUT_DATA_DIR` to be set before loading, preventing its standalone fallbacks from writing into the package or a home-directory folder if the entry initialization order changes.
+Managed `/media/` and `/library/` URLs are resolved with `realpath` containment in both the HTTP server and MCP media reader; a symlink inside either managed root that targets outside it is refused. Local media copies use an exclusive-create loop so concurrent imports cannot overwrite a same-named file. Single-file imports and reference analysis also resolve symbolic links before checking the extension, so a media-looking alias cannot copy a non-media target into `media/`. The upstream `paths.js` requires `FABLECUT_DATA_DIR` to be set before loading, preventing its standalone fallbacks from writing into the package or a home-directory folder if the entry initialization order changes.
 
 ## Branding and identity (Cutline)
 

plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/mcp-server.js

@@ -1744,7 +1744,8 @@ async function callTool(name, args, attempt = 0) {
       let file = /^\/media\//i.test(src)
         ? folderFile(MEDIA_DIR, path.join(MEDIA_DIR, entryRel(src, "/media/")), "managed media reference")
         : src;
-      if (!file || !fs.existsSync(file) || !fs.statSync(file).isFile())
+      file = realPath(file);
+      if (!file || !fs.statSync(file).isFile())
         throw new Error("File not found: " + src);
       const ext = path.extname(file).toLowerCase();
       const referenceKinds = new Set([".mp4", ".webm", ".mov", ".mkv", ".m4v", ".avi", ".mp3", ".wav", ".ogg", ".m4a", ".aac", ".flac"]);
@@ -1796,16 +1797,17 @@ async function callTool(name, args, attempt = 0) {
         kind = kindFromName(got.name);
         await maybeFaststart(target);
       } else {
-        if (!fs.existsSync(src) || !fs.statSync(src).isFile())
+        const file = realPath(src);
+        if (!file || !fs.statSync(file).isFile())
           throw new Error("File not found: " + src);
-        const ext = path.extname(src).toLowerCase();
-        /* The kind is detected BEFORE anything is copied: a font, a script or
-           any other non-media file is refused here, so it never reaches
-           media/. */
-        kind = kindFromName(src);
+        const ext = path.extname(file).toLowerCase();
+        /* The kind is detected from the real path BEFORE anything is copied:
+           a media-looking symlink cannot smuggle a non-media target into
+           media/. Fonts, scripts and other non-media files are refused too. */
+        kind = kindFromName(file);
         if (!kind) throw new Error(`Unsupported extension ${ext} — only media files ` +
           `(video, audio, image, SVG) can be imported`);
-        target = copyToMedia(src, path.basename(src));
+        target = copyToMedia(file, path.basename(src));
       }
       if (!kind) throw new Error("Unsupported media type");
       const entry = {

plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/paths.js

@@ -10,10 +10,10 @@
      DATA_DIR  the user's work — project.json, imported media, exports, cached
                analysis. Must outlive an update.
 
-   Standalone (`node server.js`) they are the same directory, which is exactly
-   how FableCut has always behaved. Set FABLECUT_DATA_DIR to split them; the
-   plugin sets it to ${CLAUDE_PLUGIN_DATA}. Installed from npm (the code sits
-   under node_modules, e.g. the npx cache) the default is ~/FableCut.
+   The Mini App entry must set FABLECUT_DATA_DIR before loading this module.
+   There are deliberately no standalone, package-directory or home-directory
+   fallbacks: an unset value stops startup instead of writing to an unexpected
+   location.
    ═══════════════════════════════════════════════════════════════════════════ */
 "use strict";
 const crypto = require("crypto");

plugins/klarkxy/cutline/tests/editing.test.mjs

@@ -8,7 +8,7 @@ import assert from 'node:assert/strict';
 import { existsSync, readFileSync, statSync } from 'node:fs';
 import path from 'node:path';
 
-import { fixtureDir, getJson, makeClip, mcpSession, mcpTool, startApp } from './helpers.mjs';
+import { FFMPEG_SKIP, fixtureDir, getJson, makeClip, mcpSession, mcpTool, startApp } from './helpers.mjs';
 
 const app = await startApp();
 const { sessionId } = await mcpSession(app.base);
@@ -26,7 +26,7 @@ function firstJson(text) {
   return JSON.parse(text.slice(text.indexOf('{'), text.lastIndexOf('}') + 1));
 }
 
-test('a file is imported into the library and registered in the project', async () => {
+test('a file is imported into the library and registered in the project', { skip: FFMPEG_SKIP }, async () => {
   longClip = path.join(app.dataDir, 'source', 'long.mp4');
   shortClip = path.join(app.dataDir, 'source', 'short.mp4');
   await makeClip(longClip, 4);
@@ -45,7 +45,7 @@ test('a file is imported into the library and registered in the project', async
   assert.ok(statSync(copied).size > 0);
 });
 
-test('a frame is taken from a non-zero moment of the clip', async () => {
+test('a frame is taken from a non-zero moment of the clip', { skip: FFMPEG_SKIP }, async () => {
   const project = JSON.parse((await mcpTool(app.base, sessionId, 'fablecut_get_project')).text);
   const src = project.media[0].src;
 
@@ -57,7 +57,7 @@ test('a frame is taken from a non-zero moment of the clip', async () => {
   assert.ok(image.data.length > 100, 'the image has pixels');
 });
 
-test('a contact sheet surveys a range that starts at a non-zero moment', async () => {
+test('a contact sheet surveys a range that starts at a non-zero moment', { skip: FFMPEG_SKIP }, async () => {
   const project = JSON.parse((await mcpTool(app.base, sessionId, 'fablecut_get_project')).text);
   const src = project.media[0].src;
 
@@ -73,7 +73,7 @@ test('a contact sheet surveys a range that starts at a non-zero moment', async (
   assert.ok(image.data.length > 100, 'the image has pixels');
 });
 
-test('a clip is placed on the timeline', async () => {
+test('a clip is placed on the timeline', { skip: FFMPEG_SKIP }, async () => {
   const project = JSON.parse((await mcpTool(app.base, sessionId, 'fablecut_get_project')).text);
   const media = project.media[0];
 
@@ -93,7 +93,7 @@ test('a clip is placed on the timeline', async () => {
   assert.equal(after.clips[0].mediaId, media.id);
 });
 
-test('replacing the file behind a media entry trims the clips to fit', async () => {
+test('replacing the file behind a media entry trims the clips to fit', { skip: FFMPEG_SKIP }, async () => {
   const project = JSON.parse((await mcpTool(app.base, sessionId, 'fablecut_get_project')).text);
   const media = project.media[0];
 
@@ -116,7 +116,7 @@ test('replacing the file behind a media entry trims the clips to fit', async ()
   assert.ok(clip.in <= replaced.duration, 'the in point fits the shorter file');
 });
 
-test('the project is written under the data directory', async () => {
+test('the project is written under the data directory', { skip: FFMPEG_SKIP }, async () => {
   const file = path.join(app.dataDir, 'project.json');
   assert.ok(existsSync(file), 'project.json lives in the data directory');
   const saved = JSON.parse(readFileSync(file, 'utf8'));
@@ -267,7 +267,7 @@ test('a forced save never rewinds the revision', async () => {
   assert.equal(final.json.revision, onDisk + 1, 'the revision did not move again');
 });
 
-test('the MCP client may look at a file outside the library', async () => {
+test('the MCP client may look at a file outside the library', { skip: FFMPEG_SKIP }, async () => {
   // The user asked for it: the local MCP client can read any file the user
   // can, exactly as upstream. The page cannot — that is the next test.
   const outsideDir = fixtureDir('external-frame');
@@ -279,7 +279,7 @@ test('the MCP client may look at a file outside the library', async () => {
   assert.ok(reply.json.result.content.some((/** @type {any} */ block) => block.type === 'image'));
 });
 
-test('the page may not read a file outside the library', async () => {
+test('the page may not read a file outside the library', { skip: FFMPEG_SKIP }, async () => {
   const outsideDir = fixtureDir('external-frame');
   const outside = path.join(outsideDir, 'elsewhere.mp4');
 
@@ -298,7 +298,7 @@ test('the page may not read a file outside the library', async () => {
   assert.equal(plain.status, 403);
 });
 
-test('a path inside the data directory is readable', async () => {
+test('a path inside the data directory is readable', { skip: FFMPEG_SKIP }, async () => {
   const { reply, text } = await mcpTool(app.base, sessionId, 'fablecut_frame', { path: longClip, t: 1 });
   assert.equal(reply.json.result.isError, undefined, text);
   assert.ok(reply.json.result.content.some((/** @type {any} */ block) => block.type === 'image'));

plugins/klarkxy/cutline/tests/helpers.mjs

@@ -6,7 +6,7 @@
 // test runner gives every test file its own process, which is why one file
 // here starts one Mini App and the others share it.
 
-import { spawn } from 'node:child_process';
+import { spawn, spawnSync } from 'node:child_process';
 import { createServer } from 'node:http';
 import { mkdtempSync, mkdirSync, existsSync } from 'node:fs';
 
@@ -19,6 +19,11 @@ export const PACKAGE_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.
 /** Keep test artifacts outside the installable package and the OS temp. */
 const FIXTURE_ROOT = process.env.FABLECUT_TEST_ROOT || path.resolve(PACKAGE_ROOT, '../../../.fablecut-acceptance/test-runs');
 
+/** Media fixtures need the real executable; CI runners may have Node and Git only. */
+const FFMPEG_AVAILABLE = spawnSync('ffmpeg', ['-version'], { stdio: 'ignore' }).status === 0;
+/** Add to a media-dependent test's options: { skip: FFMPEG_SKIP }. */
+export const FFMPEG_SKIP = FFMPEG_AVAILABLE ? false : 'FFmpeg is required for this media test';
+
 /**
  * A directory for a test fixture. The fixtures are files the tests need to
  * exist (a clip outside the library, a Cutline data folder to import from);

plugins/klarkxy/cutline/tests/import-safety.test.mjs

@@ -9,7 +9,7 @@ import assert from 'node:assert/strict';
 import { existsSync, mkdirSync, readdirSync, readFileSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs';
 import path from 'node:path';
 
-import { fixtureDir, getJson, makeClip, mcpSession, mcpTool, sleep, startApp } from './helpers.mjs';
+import { FFMPEG_SKIP, fixtureDir, getJson, makeClip, mcpSession, mcpTool, sleep, startApp } from './helpers.mjs';
 
 const app = await startApp();
 const { sessionId } = await mcpSession(app.base);
@@ -109,7 +109,7 @@ test('a project id that is a path is refused', async () => {
   assert.notEqual(after.json.name, 'Sneaky', 'the project outside the folder was not imported');
 });
 
-test('a media path that climbs out of the source folder is refused', async () => {
+test('a media path that climbs out of the source folder is refused', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-traversal');
   // a file next to the media folder, not inside it
   await makeClip(path.join(root, 'secret.mp4'), 1);
@@ -130,7 +130,7 @@ test('a media path that climbs out of the source folder is refused', async () =>
   assert.ok(!existsSync(path.join(app.dataDir, 'media', 'secret.mp4')), 'the file outside media/ was not copied');
 });
 
-test('an encoded media path that climbs out of the source folder is refused', async () => {
+test('an encoded media path that climbs out of the source folder is refused', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-encoded');
   await makeClip(path.join(root, 'secret.mp4'), 1);
   await makeSource(root, {
@@ -149,7 +149,7 @@ test('an encoded media path that climbs out of the source folder is refused', as
   assert.deepEqual(mediaFiles(), mediaBefore, 'no file was copied');
 });
 
-test('a link out of the source media folder is refused', async () => {
+test('a link out of the source media folder is refused', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-junction');
   const outside = fixtureDir('safety-junction-outside');
   await makeClip(path.join(outside, 'linked.mp4'), 1);
@@ -182,7 +182,7 @@ test('a link out of the source media folder is refused', async () => {
   }
 });
 
-test('a media file in a subfolder of the source media folder is imported', async () => {
+test('a media file in a subfolder of the source media folder is imported', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-nested');
   const clip = await makeSourceMedia(root, 'sub/shot.mp4', 2);
   await makeSource(root, {
@@ -205,7 +205,7 @@ test('a media file in a subfolder of the source media folder is imported', async
   assert.ok(existsSync(clip), 'the original file is still there');
 });
 
-test('a media file that is not in the source folder stops the import', async () => {
+test('a media file that is not in the source folder stops the import', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-missing');
   await makeSourceMedia(root, 'here.mp4', 1);
   await makeSource(root, {
@@ -228,7 +228,7 @@ test('a media file that is not in the source folder stops the import', async ()
   assert.deepEqual(mediaFiles(), mediaBefore, 'not even the file that was there was copied');
 });
 
-test('the project that was open is kept as a version before the import', async () => {
+test('the project that was open is kept as a version before the import', { skip: FFMPEG_SKIP }, async () => {
   const patch = await mcpTool(app.base, sessionId, 'fablecut_patch_project', {
     ops: [{ op: 'setProject', set: { name: 'Work In Progress' } }],
   });
@@ -259,7 +259,7 @@ test('the project that was open is kept as a version before the import', async (
   assert.match(versions.text, /before agent edits/, versions.text);
 });
 
-test('an import refuses to overwrite a project that changed while it ran', async () => {
+test('an import refuses to overwrite a project that changed while it ran', { skip: FFMPEG_SKIP }, async () => {
   const root = fixtureDir('safety-race');
   await makeSourceMedia(root, 'race-a.mp4', 1);
   await makeSourceMedia(root, 'race-b.mp4', 1);
@@ -296,7 +296,7 @@ test('an import refuses to overwrite a project that changed while it ran', async
   assert.ok(!existsSync(path.join(app.dataDir, 'media', 'race-a.mp4')), 'the copies of the refused import were taken away');
 });
 
-test('a file that is not media stops the import', async () => {
+test('a file that is not media stops the import', { skip: FFMPEG_SKIP }, async () => {
   // The media folder is served from the editor's own origin: a file a browser
   // would run there would run there.
   const root = fixtureDir('safety-active');
@@ -322,6 +322,32 @@ test('a file that is not media stops the import', async () => {
   assert.ok(!existsSync(path.join(app.dataDir, 'media', 'evil.html')), 'the file was not copied');
 });
 
+test('a media-looking symlink cannot import a non-media target', async (t) => {
+  const root = fixtureDir('safety-media-link-target');
+  const target = path.join(root, 'not-media.txt');
+  const link = path.join(root, 'disguised.mp4');
+  writeFileSync(target, '<script>alert(1)</script>');
+  try {
+    symlinkSync(target, link, 'file');
+  } catch (error) {
+    if (['EPERM', 'EACCES', 'ENOTSUP'].includes(error.code)) return t.skip(`symlinks unavailable: ${error.code}`);
+    throw error;
+  }
+  const mediaBefore = mediaFiles();
+
+  const imported = await mcpTool(app.base, sessionId, 'fablecut_import_media', { path: link });
+  assert.equal(imported.reply.json.result.isError, true, 'single-file import is refused');
+  assert.match(imported.text, /Unsupported extension \.txt/, imported.text);
+
+  const analyzed = await mcpTool(app.base, sessionId, 'fablecut_analyze_reference', {
+    path: link,
+    registerMusic: false,
+  });
+  assert.equal(analyzed.reply.json.result.isError, true, 'reference import is refused');
+  assert.match(analyzed.text, /Unsupported reference media extension \.txt/, analyzed.text);
+  assert.deepEqual(mediaFiles(), mediaBefore, 'neither entry point copied the non-media target');
+});
+
 test('an upload of a file that is not media is refused', async () => {
   const before = mediaFiles();
 

plugins/klarkxy/cutline/tests/import-url-security.test.mjs

@@ -247,7 +247,10 @@ test('the name stays in the Host header and in the certificate check', async ()
 });
 
 test('a redirect to a private address is refused before anything is sent', async () => {
-  const privateTarget = await startCounter('127.0.0.2');
+  // The counter listens on an address every platform has. It need not bind the
+  // advertised target: a correct downloader never connects there, and the
+  // request stub independently proves that no request was attempted.
+  const privateTarget = await startCounter('127.0.0.1');
   const redirector = await startRedirector(`https://127.0.0.2:${privateTarget.port}/secret.mp4`);
   const requests = stubHttpsRequest(() => fakeResponse(200));