diff --git a/plugins/klarkxy/cutline/PATCHES.md b/plugins/klarkxy/cutline/PATCHES.md index 5809206..5513dfc 100644 --- a/plugins/klarkxy/cutline/PATCHES.md +++ b/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) diff --git a/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/mcp-server.js b/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/mcp-server.js index db8d7db..0d7ec0c 100644 --- a/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/mcp-server.js +++ b/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 = { diff --git a/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/paths.js b/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/paths.js index b683900..3911366 100644 --- a/plugins/klarkxy/cutline/miniapp/node/vendor/fablecut/paths.js +++ b/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"); diff --git a/plugins/klarkxy/cutline/tests/editing.test.mjs b/plugins/klarkxy/cutline/tests/editing.test.mjs index 7a2f3de..fe79c35 100644 --- a/plugins/klarkxy/cutline/tests/editing.test.mjs +++ b/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')); diff --git a/plugins/klarkxy/cutline/tests/helpers.mjs b/plugins/klarkxy/cutline/tests/helpers.mjs index 67cab35..22b979d 100644 --- a/plugins/klarkxy/cutline/tests/helpers.mjs +++ b/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); diff --git a/plugins/klarkxy/cutline/tests/import-safety.test.mjs b/plugins/klarkxy/cutline/tests/import-safety.test.mjs index 052d02e..5204981 100644 --- a/plugins/klarkxy/cutline/tests/import-safety.test.mjs +++ b/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, ''); + 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(); diff --git a/plugins/klarkxy/cutline/tests/import-url-security.test.mjs b/plugins/klarkxy/cutline/tests/import-url-security.test.mjs index a8e28d4..11d1f7d 100644 --- a/plugins/klarkxy/cutline/tests/import-url-security.test.mjs +++ b/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)); diff --git a/plugins/klarkxy/cutline/tests/import.test.mjs b/plugins/klarkxy/cutline/tests/import.test.mjs index 5f6dcae..1dbe404 100644 --- a/plugins/klarkxy/cutline/tests/import.test.mjs +++ b/plugins/klarkxy/cutline/tests/import.test.mjs @@ -8,7 +8,7 @@ import assert from 'node:assert/strict'; import { existsSync, mkdirSync, readFileSync, writeFileSync } 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); @@ -40,7 +40,7 @@ async function makeSourceFolder(root, name, seconds) { return { root, clip, doc }; } -test('a project folder is imported with its media, and the originals stay', async () => { +test('a project folder is imported with its media, and the originals stay', { skip: FFMPEG_SKIP }, async () => { const source = await makeSourceFolder(fixtureDir('import-source'), 'Older Project', 3); // A file with the same name already sits in this library: the import must @@ -76,7 +76,7 @@ test('a project folder is imported with its media, and the originals stay', asyn assert.ok(existsSync(path.join(app.dataDir, 'media', 'shot.mp4')), 'the earlier file was not overwritten'); }); -test('the imported project is saved at the actual current revision', async () => { +test('the imported project is saved at the actual current revision', { skip: FFMPEG_SKIP }, async () => { const before = await getJson(app.base, '/api/project'); const source = await makeSourceFolder(fixtureDir('import-source-2'), 'Another One', 2); @@ -124,7 +124,7 @@ test('a folder without a project is refused', async () => { assert.match(text, /no project to import/); }); -test('the same import can be done with the existing tools alone', async () => { +test('the same import can be done with the existing tools alone', { skip: FFMPEG_SKIP }, async () => { // The recipe the skill documents: import every file, then write the // document with the new ids and the actual current revision. const source = await makeSourceFolder(fixtureDir('import-source-4'), 'By Hand', 2); diff --git a/plugins/klarkxy/cutline/tests/paths.test.mjs b/plugins/klarkxy/cutline/tests/paths.test.mjs new file mode 100644 index 0000000..d8b66c8 --- /dev/null +++ b/plugins/klarkxy/cutline/tests/paths.test.mjs @@ -0,0 +1,21 @@ +// @ts-check +// The vendored runtime must never guess where durable state belongs. + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + +const pathsModule = fileURLToPath(new URL('../miniapp/node/vendor/fablecut/paths.js', import.meta.url)); + +test('paths.js refuses to load without an explicit data directory', () => { + const env = { ...process.env }; + delete env.FABLECUT_DATA_DIR; + const result = spawnSync(process.execPath, ['-e', 'require(process.argv[1])', pathsModule], { + env, + encoding: 'utf8', + }); + + assert.notEqual(result.status, 0, 'loading without FABLECUT_DATA_DIR fails'); + assert.match(result.stderr, /FABLECUT_DATA_DIR must be set by the Mini App entry/); +}); diff --git a/plugins/klarkxy/cutline/tests/reference-path.test.mjs b/plugins/klarkxy/cutline/tests/reference-path.test.mjs index 53d1b6c..158389c 100644 --- a/plugins/klarkxy/cutline/tests/reference-path.test.mjs +++ b/plugins/klarkxy/cutline/tests/reference-path.test.mjs @@ -2,7 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { existsSync, mkdirSync, symlinkSync, writeFileSync } from 'node:fs'; import path from 'node:path'; -import { startApp, fixtureDir, makeClip, mcpSession, mcpTool } from './helpers.mjs'; +import { FFMPEG_SKIP, startApp, fixtureDir, makeClip, mcpSession, mcpTool } from './helpers.mjs'; const app = await startApp(); const { sessionId } = await mcpSession(app.base); test.after(() => app.dispose()); @@ -48,7 +48,7 @@ test('managed media symlinks cannot expose files outside media', async (t) => { assert.notEqual(await served.text(), 'private'); }); -test('reference analysis accepts nested media and explicit source paths', async () => { +test('reference analysis accepts nested media and explicit source paths', { skip: FFMPEG_SKIP }, async () => { const external = path.join(fixtureDir('reference-input'), 'explicit.mp4'); await makeClip(external, 1); mkdirSync(path.join(app.dataDir, 'media', 'nested'), { recursive: true });