fix(skills): stage media downloads in private directories (#3787)

skills-manifest.json

@@ -50,7 +50,7 @@
       "files": 12
     },
     "media-use": {
-      "hash": "a397849a32047f7e",
+      "hash": "591daf2adb78e1cd",
       "files": 156
     },
     "motion-graphics": {

skills/media-use/scripts/lib/heygen-video-provider.mjs

@@ -1,4 +1,5 @@
 import { execFileSync } from "node:child_process";
+import { mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { freezeUrl } from "./freeze.mjs";
@@ -106,10 +107,20 @@ export async function heygenVideoGenerate(intent, ctx) {
     return null;
   }
 
-  const tmpPath = join(tmpdir(), `media-use-heygen-video-${process.pid}-${Date.now()}.mp4`);
+  let tmpDir;
+  let tmpPath;
   try {
+    tmpDir = mkdtempSync(join(tmpdir(), "media-use-heygen-video-"));
+    tmpPath = join(tmpDir, "video.mp4");
     await freezeUrl(videoUrl, tmpPath);
   } catch (err) {
+    if (tmpDir) {
+      try {
+        rmSync(tmpDir, { recursive: true, force: true });
+      } catch {
+        // Preserve the download failure if temporary-directory cleanup fails.
+      }
+    }
     console.error(`media-use: heygen video download failed: ${err.message}`);
     return null;
   }

skills/media-use/scripts/lib/heygen-video-provider.test.mjs

@@ -1,12 +1,32 @@
 import { strict as assert } from "node:assert";
-import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import {
+  chmodSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  statSync,
+  symlinkSync,
+  readdirSync,
+  writeFileSync,
+} from "node:fs";
 import http from "node:http";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { basename, dirname, join } from "node:path";
 import { test } from "node:test";
 import { HEYGEN_NOT_AUTHENTICATED_MESSAGE } from "./heygen-cli.mjs";
 import { AVATAR_VIDEO_SIGNIN_MESSAGE } from "./heygen-video-provider.mjs";
 
+function cleanupDownload(localPath) {
+  if (
+    basename(localPath) === "video.mp4" &&
+    basename(dirname(localPath)).startsWith("media-use-heygen-video-")
+  ) {
+    rmSync(dirname(localPath), { recursive: true, force: true });
+  } else {
+    rmSync(localPath, { force: true });
+  }
+}
+
 const VIDEO_FIXTURE = Buffer.from("tiny heygen video fixture");
 let importCount = 0;
 
@@ -148,7 +168,10 @@ test("downloads a generated avatar video and returns the generated MP4 result",
         });
         assert.ok(result);
         assert.equal(join(tmpdir(), result.localPath.slice(tmpdir().length + 1)), result.localPath);
-        assert.match(result.localPath, /media-use-heygen-video-\d+-\d+\.mp4$/);
+        assert.match(result.localPath, /media-use-heygen-video-[^/\\]+[/\\]video\.mp4$/);
+        if (process.platform !== "win32") {
+          assert.equal(statSync(dirname(result.localPath)).mode & 0o777, 0o700);
+        }
         assert.deepEqual(result, {
           localPath: result.localPath,
           ext: ".mp4",
@@ -163,7 +186,7 @@ test("downloads a generated avatar video and returns the generated MP4 result",
       },
     );
   } finally {
-    if (localPath) rmSync(localPath, { force: true });
+    if (localPath) cleanupDownload(localPath);
     await closeServer(server);
   }
 });
@@ -190,7 +213,7 @@ test("tags video creation but not avatar or voice discovery", async () => {
       },
     );
   } finally {
-    if (localPath) rmSync(localPath, { force: true });
+    if (localPath) cleanupDownload(localPath);
     await closeServer(server);
   }
 });
@@ -221,7 +244,7 @@ test("uses explicit avatar and voice overrides without discovery", async () => {
       },
     );
   } finally {
-    if (localPath) rmSync(localPath, { force: true });
+    if (localPath) cleanupDownload(localPath);
     await closeServer(server);
   }
 });
@@ -246,7 +269,7 @@ test("caches discovered avatar and voice IDs for the process", async () => {
       },
     );
   } finally {
-    for (const localPath of localPaths) rmSync(localPath, { force: true });
+    for (const localPath of localPaths) cleanupDownload(localPath);
     await closeServer(server);
   }
 });
@@ -342,3 +365,61 @@ test("download failure after a successful create returns null and logs a diagnos
     await closeServer(server);
   }
 });
+
+test("uses private unique downloads even when time is fixed and the old name is planted", async (t) => {
+  const root = mkdtempSync(join(tmpdir(), "hf-video-temp-test-"));
+  const previousTmpdir = process.env.TMPDIR;
+  process.env.TMPDIR = root;
+  t.mock.method(Date, "now", () => 123456);
+  t.mock.method(globalThis, "fetch", async () => ({
+    ok: true,
+    headers: { get: () => "4" },
+    body: [Buffer.from("new!")],
+  }));
+  try {
+    const victim = join(root, "victim");
+    writeFileSync(victim, "unchanged");
+    symlinkSync(victim, join(root, `media-use-heygen-video-${process.pid}-123456.mp4`));
+    await withFakeHeygen(
+      { response: JSON.stringify({ data: { video_url: "https://example.invalid/video.mp4" } }) },
+      async () => {
+        const generate = await freshGenerate();
+        const first = await generate("First", { avatarId: "a", voiceId: "v" });
+        const second = await generate("Second", { avatarId: "a", voiceId: "v" });
+        assert.ok(first && second);
+        assert.notEqual(first.localPath, second.localPath);
+        assert.equal(readFileSync(first.localPath, "utf8"), "new!");
+        assert.equal(readFileSync(second.localPath, "utf8"), "new!");
+        assert.equal(readFileSync(victim, "utf8"), "unchanged");
+      },
+    );
+  } finally {
+    if (previousTmpdir === undefined) delete process.env.TMPDIR;
+    else process.env.TMPDIR = previousTmpdir;
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
+test("removes private download staging on failure while returning null", async (t) => {
+  const root = mkdtempSync(join(tmpdir(), "hf-video-temp-fail-"));
+  const previousTmpdir = process.env.TMPDIR;
+  process.env.TMPDIR = root;
+  t.mock.method(globalThis, "fetch", async () => {
+    throw new Error("download failed");
+  });
+  t.mock.method(console, "error", () => {});
+  try {
+    await withFakeHeygen(
+      { response: JSON.stringify({ data: { video_url: "https://example.invalid/video.mp4" } }) },
+      async () => {
+        const generate = await freshGenerate();
+        assert.equal(await generate("Failure", { avatarId: "a", voiceId: "v" }), null);
+      },
+    );
+    assert.deepEqual(readdirSync(root), []);
+  } finally {
+    if (previousTmpdir === undefined) delete process.env.TMPDIR;
+    else process.env.TMPDIR = previousTmpdir;
+    rmSync(root, { recursive: true, force: true });
+  }
+});

skills/media-use/scripts/lib/lut-preset-provider.mjs

@@ -1,5 +1,5 @@
-import { existsSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
-import { join } from "node:path";
+import { existsSync, mkdtempSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
+import { dirname, join } from "node:path";
 import { withReservedFile, withReservedFileSync } from "./manifest.mjs";
 import { freezeUrl } from "./freeze.mjs";
 import { tokenOverlap } from "./match.mjs";
@@ -216,9 +216,10 @@ export async function freezeLibraryLut(match, { projectDir, type, localOnly = fa
         type,
         ".cube",
         async ({ id, localPath, fullPath }) => {
-          const tmpPath = `${fullPath}.tmp`;
+          const tmpDir = mkdtempSync(join(dirname(fullPath), ".lut-download-"));
+          const tmpPath = join(tmpDir, "download.cube");
           try {
-            // Download + validate at a .tmp path, then atomically rename. A crash
+            // Download + validate in a private directory, then atomically rename. A crash
             // (SIGKILL/OOM) between write and validate can't orphan an invalid .cube
             // at the final path — only a validated cube is ever renamed into place.
             await freezeUrl(match.url, tmpPath);
@@ -226,7 +227,7 @@ export async function freezeLibraryLut(match, { projectDir, type, localOnly = fa
             renameSync(tmpPath, fullPath);
             return libraryRecord(match, { id, localPath, fullPath, via: "url" });
           } finally {
-            rmSync(tmpPath, { force: true });
+            rmSync(tmpDir, { recursive: true, force: true });
           }
         },
       );

skills/media-use/scripts/lib/lut-preset-provider.test.mjs

@@ -1,5 +1,14 @@
 import { strict as assert } from "node:assert";
-import { mkdtempSync, rmSync, existsSync, readFileSync, readdirSync } from "node:fs";
+import {
+  mkdtempSync,
+  rmSync,
+  existsSync,
+  readFileSync,
+  readdirSync,
+  statSync,
+  symlinkSync,
+  writeFileSync,
+} from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { test } from "node:test";
@@ -127,6 +136,15 @@ test("url library entries respect localOnly and freeze through fetch", async ()
     globalThis.fetch = async (url) => {
       fetchCalls++;
       assert.equal(url, match.url);
+      const directory = join(projectDir, ".media/luts");
+      const staging = readdirSync(directory).filter((name) => name.startsWith(".lut-download-"));
+      assert.equal(staging.length, 1);
+      if (process.platform !== "win32") {
+        assert.equal(statSync(join(directory, staging[0])).mode & 0o777, 0o700);
+      }
+      const victim = join(projectDir, "victim.cube");
+      writeFileSync(victim, "unchanged");
+      symlinkSync(victim, join(directory, "lut_001.cube.tmp"));
       return {
         ok: true,
         headers: { get: () => String(body.length) },
@@ -139,6 +157,11 @@ test("url library entries respect localOnly and freeze through fetch", async ()
     assert.match(frozen.localPath, /^\.media\/luts\/lut_001\.cube$/);
     assert.equal(validateCubeFile(join(projectDir, frozen.localPath)).ok, true);
     assert.equal(frozen.metadata.provenance.via, "url");
+    assert.equal(readFileSync(join(projectDir, "victim.cube"), "utf8"), "unchanged");
+    assert.deepEqual(readdirSync(join(projectDir, ".media/luts")).sort(), [
+      "lut_001.cube",
+      "lut_001.cube.tmp",
+    ]);
   } finally {
     globalThis.fetch = originalFetch;
     rmSync(projectDir, { recursive: true, force: true });