From 46f2274d315f8b23583c72bdfd7e408b6dfc0b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 15 Jul 2026 14:44:57 +0000 Subject: [PATCH] fix(skills): preserve nested staged asset paths --- skills-manifest.json | 8 ++-- .../faceless-explainer/scripts/lib/assets.mjs | 33 +++++++++++---- skills/pr-to-video/scripts/lib/assets.mjs | 33 +++++++++++---- .../scripts/lib/assets.mjs | 33 +++++++++++---- .../scripts/lib/assets.test.mjs | 42 +++++++++++++++++++ 5 files changed, 118 insertions(+), 31 deletions(-) create mode 100644 skills/product-launch-video/scripts/lib/assets.test.mjs diff --git a/skills-manifest.json b/skills-manifest.json index 4868c30dd2..97ce1399c0 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -6,7 +6,7 @@ "files": 144 }, "faceless-explainer": { - "hash": "e486bec3499bd84d", + "hash": "3873fd4a76a59907", "files": 19 }, "figma": { @@ -58,12 +58,12 @@ "files": 132 }, "pr-to-video": { - "hash": "739774abd773a3d3", + "hash": "706dc7fbf2362ab9", "files": 28 }, "product-launch-video": { - "hash": "86fcf6421ab9aa8f", - "files": 23 + "hash": "dae461ff0dc67f35", + "files": 24 }, "remotion-to-hyperframes": { "hash": "c96bb2f0af9e1143", diff --git a/skills/faceless-explainer/scripts/lib/assets.mjs b/skills/faceless-explainer/scripts/lib/assets.mjs index 0f06ac5da1..c3466bfbc5 100644 --- a/skills/faceless-explainer/scripts/lib/assets.mjs +++ b/skills/faceless-explainer/scripts/lib/assets.mjs @@ -2,18 +2,30 @@ // Shared by stage-assets.mjs (Step 4 close, BEFORE the frame workers run) and // assemble-index.mjs (Step 5, idempotent backstop). Only assets a frame names // in `asset_candidates` are staged; unnamed assets never reach the project. -// asset_candidates value form: "assets/ — desc; assets/… — …". +// asset_candidates value form: "assets/ — desc; assets/… — …". import { copyFileSync, existsSync, mkdirSync } from "node:fs"; -import { basename, join } from "node:path"; +import { basename, dirname, join, posix } from "node:path"; -export function basenamesFromCandidates(value) { +export function assetPathsFromCandidates(value) { if (typeof value !== "string") return []; return value .split(";") .map((seg) => seg.split(/\s+[—–-]\s+/)[0].trim()) // strip the " — description" .filter(Boolean) - .map((p) => basename(p.replace(/^assets\//, ""))); + .map((path) => path.replaceAll("\\", "/").replace(/^assets\//, "")) + .map((path) => posix.normalize(path)) + .filter( + (path) => + path !== "." && + !path.startsWith("../") && + !posix.isAbsolute(path) && + !/^[A-Za-z]:\//.test(path), + ); +} + +export function basenamesFromCandidates(value) { + return assetPathsFromCandidates(value).map((path) => basename(path)); } // Copy each frame's asset_candidates from capture/{assets,assets/videos, @@ -22,7 +34,7 @@ export function basenamesFromCandidates(value) { export function stageAssets({ hyperframesDir, frames }) { const wanted = new Set(); for (const f of frames) { - for (const b of basenamesFromCandidates(f.extra?.asset_candidates)) wanted.add(b); + for (const path of assetPathsFromCandidates(f.extra?.asset_candidates)) wanted.add(path); } const captureDirs = [ join(hyperframesDir, "capture/assets"), @@ -34,19 +46,22 @@ export function stageAssets({ hyperframesDir, frames }) { let staged = 0; if (wanted.size > 0) { mkdirSync(assetsDir, { recursive: true }); - for (const b of wanted) { - const dest = join(assetsDir, b); + for (const assetPath of wanted) { + const dest = join(assetsDir, assetPath); if (existsSync(dest)) { staged++; continue; } // first-wins / already staged - const src = captureDirs.map((d) => join(d, b)).find((p) => existsSync(p)); + const src = captureDirs + .flatMap((dir) => [join(dir, assetPath), join(dir, basename(assetPath))]) + .find((path) => existsSync(path)); if (src) { + mkdirSync(dirname(dest), { recursive: true }); copyFileSync(src, dest); staged++; } else { anomalies.push( - `asset "${b}" named by a frame but not found under capture/ — frame will 404 it`, + `asset "${assetPath}" named by a frame but not found under capture/ — frame will 404 it`, ); } } diff --git a/skills/pr-to-video/scripts/lib/assets.mjs b/skills/pr-to-video/scripts/lib/assets.mjs index 0f06ac5da1..c3466bfbc5 100644 --- a/skills/pr-to-video/scripts/lib/assets.mjs +++ b/skills/pr-to-video/scripts/lib/assets.mjs @@ -2,18 +2,30 @@ // Shared by stage-assets.mjs (Step 4 close, BEFORE the frame workers run) and // assemble-index.mjs (Step 5, idempotent backstop). Only assets a frame names // in `asset_candidates` are staged; unnamed assets never reach the project. -// asset_candidates value form: "assets/ — desc; assets/… — …". +// asset_candidates value form: "assets/ — desc; assets/… — …". import { copyFileSync, existsSync, mkdirSync } from "node:fs"; -import { basename, join } from "node:path"; +import { basename, dirname, join, posix } from "node:path"; -export function basenamesFromCandidates(value) { +export function assetPathsFromCandidates(value) { if (typeof value !== "string") return []; return value .split(";") .map((seg) => seg.split(/\s+[—–-]\s+/)[0].trim()) // strip the " — description" .filter(Boolean) - .map((p) => basename(p.replace(/^assets\//, ""))); + .map((path) => path.replaceAll("\\", "/").replace(/^assets\//, "")) + .map((path) => posix.normalize(path)) + .filter( + (path) => + path !== "." && + !path.startsWith("../") && + !posix.isAbsolute(path) && + !/^[A-Za-z]:\//.test(path), + ); +} + +export function basenamesFromCandidates(value) { + return assetPathsFromCandidates(value).map((path) => basename(path)); } // Copy each frame's asset_candidates from capture/{assets,assets/videos, @@ -22,7 +34,7 @@ export function basenamesFromCandidates(value) { export function stageAssets({ hyperframesDir, frames }) { const wanted = new Set(); for (const f of frames) { - for (const b of basenamesFromCandidates(f.extra?.asset_candidates)) wanted.add(b); + for (const path of assetPathsFromCandidates(f.extra?.asset_candidates)) wanted.add(path); } const captureDirs = [ join(hyperframesDir, "capture/assets"), @@ -34,19 +46,22 @@ export function stageAssets({ hyperframesDir, frames }) { let staged = 0; if (wanted.size > 0) { mkdirSync(assetsDir, { recursive: true }); - for (const b of wanted) { - const dest = join(assetsDir, b); + for (const assetPath of wanted) { + const dest = join(assetsDir, assetPath); if (existsSync(dest)) { staged++; continue; } // first-wins / already staged - const src = captureDirs.map((d) => join(d, b)).find((p) => existsSync(p)); + const src = captureDirs + .flatMap((dir) => [join(dir, assetPath), join(dir, basename(assetPath))]) + .find((path) => existsSync(path)); if (src) { + mkdirSync(dirname(dest), { recursive: true }); copyFileSync(src, dest); staged++; } else { anomalies.push( - `asset "${b}" named by a frame but not found under capture/ — frame will 404 it`, + `asset "${assetPath}" named by a frame but not found under capture/ — frame will 404 it`, ); } } diff --git a/skills/product-launch-video/scripts/lib/assets.mjs b/skills/product-launch-video/scripts/lib/assets.mjs index 0f06ac5da1..c3466bfbc5 100644 --- a/skills/product-launch-video/scripts/lib/assets.mjs +++ b/skills/product-launch-video/scripts/lib/assets.mjs @@ -2,18 +2,30 @@ // Shared by stage-assets.mjs (Step 4 close, BEFORE the frame workers run) and // assemble-index.mjs (Step 5, idempotent backstop). Only assets a frame names // in `asset_candidates` are staged; unnamed assets never reach the project. -// asset_candidates value form: "assets/ — desc; assets/… — …". +// asset_candidates value form: "assets/ — desc; assets/… — …". import { copyFileSync, existsSync, mkdirSync } from "node:fs"; -import { basename, join } from "node:path"; +import { basename, dirname, join, posix } from "node:path"; -export function basenamesFromCandidates(value) { +export function assetPathsFromCandidates(value) { if (typeof value !== "string") return []; return value .split(";") .map((seg) => seg.split(/\s+[—–-]\s+/)[0].trim()) // strip the " — description" .filter(Boolean) - .map((p) => basename(p.replace(/^assets\//, ""))); + .map((path) => path.replaceAll("\\", "/").replace(/^assets\//, "")) + .map((path) => posix.normalize(path)) + .filter( + (path) => + path !== "." && + !path.startsWith("../") && + !posix.isAbsolute(path) && + !/^[A-Za-z]:\//.test(path), + ); +} + +export function basenamesFromCandidates(value) { + return assetPathsFromCandidates(value).map((path) => basename(path)); } // Copy each frame's asset_candidates from capture/{assets,assets/videos, @@ -22,7 +34,7 @@ export function basenamesFromCandidates(value) { export function stageAssets({ hyperframesDir, frames }) { const wanted = new Set(); for (const f of frames) { - for (const b of basenamesFromCandidates(f.extra?.asset_candidates)) wanted.add(b); + for (const path of assetPathsFromCandidates(f.extra?.asset_candidates)) wanted.add(path); } const captureDirs = [ join(hyperframesDir, "capture/assets"), @@ -34,19 +46,22 @@ export function stageAssets({ hyperframesDir, frames }) { let staged = 0; if (wanted.size > 0) { mkdirSync(assetsDir, { recursive: true }); - for (const b of wanted) { - const dest = join(assetsDir, b); + for (const assetPath of wanted) { + const dest = join(assetsDir, assetPath); if (existsSync(dest)) { staged++; continue; } // first-wins / already staged - const src = captureDirs.map((d) => join(d, b)).find((p) => existsSync(p)); + const src = captureDirs + .flatMap((dir) => [join(dir, assetPath), join(dir, basename(assetPath))]) + .find((path) => existsSync(path)); if (src) { + mkdirSync(dirname(dest), { recursive: true }); copyFileSync(src, dest); staged++; } else { anomalies.push( - `asset "${b}" named by a frame but not found under capture/ — frame will 404 it`, + `asset "${assetPath}" named by a frame but not found under capture/ — frame will 404 it`, ); } } diff --git a/skills/product-launch-video/scripts/lib/assets.test.mjs b/skills/product-launch-video/scripts/lib/assets.test.mjs new file mode 100644 index 0000000000..748b06fb40 --- /dev/null +++ b/skills/product-launch-video/scripts/lib/assets.test.mjs @@ -0,0 +1,42 @@ +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; + +import { assetPathsFromCandidates, stageAssets } from "./assets.mjs"; + +test("rejects asset candidates that escape the project assets directory", () => { + assert.deepEqual( + assetPathsFromCandidates( + "assets/../secret.png — traversal; /tmp/secret.png — absolute; C:/secret.png — drive", + ), + [], + ); +}); + +test("recognizes an asset candidate already present in a nested assets directory", () => { + const hyperframesDir = mkdtempSync(join(tmpdir(), "hyperframes-stage-assets-")); + try { + const keyframesDir = join(hyperframesDir, "assets/keyframes"); + mkdirSync(keyframesDir, { recursive: true }); + writeFileSync(join(keyframesDir, "scene-01-meeting.png"), "fixture"); + + const result = stageAssets({ + hyperframesDir, + frames: [ + { + extra: { + asset_candidates: "assets/keyframes/scene-01-meeting.png — approved meeting keyframe", + }, + }, + ], + }); + + assert.equal(result.staged, 1); + assert.deepEqual([...result.wanted], ["keyframes/scene-01-meeting.png"]); + assert.deepEqual(result.anomalies, []); + } finally { + rmSync(hyperframesDir, { recursive: true, force: true }); + } +});