fix(media-use): kill shell command injection in probe/heygen-search

.github/workflows/ci.yml

@@ -33,6 +33,7 @@ jobs:
     outputs:
       code: ${{ steps.filter.outputs.code }}
       cli: ${{ steps.filter.outputs.cli }}
+      skills: ${{ steps.filter.outputs.skills }}
     steps:
       # Force git-based change detection instead of the pull_request REST API.
       # The API path can fail the whole workflow on transient listFiles
@@ -58,6 +59,10 @@ jobs:
               - "package.json"
               - "bun.lock"
               - ".github/workflows/ci.yml"
+            skills:
+              - "skills/**"
+              - "package.json"
+              - ".github/workflows/ci.yml"
 
   build:
     name: Build
@@ -224,6 +229,22 @@ jobs:
       - run: bun run --cwd packages/core build:hyperframes-runtime
       - run: bun run --filter '!@hyperframes/producer' test
 
+  # Runs the skills' own node:test suites (e.g. media-use), which live outside
+  # the workspace packages and are not covered by the `test` job above. Gated on
+  # the `skills` filter so a skills-only PR actually exercises them in CI.
+  test-skills:
+    name: Test (skills)
+    needs: changes
+    if: needs.changes.outputs.skills == 'true'
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
+        with:
+          node-version: 22
+      - run: node --test 'skills/**/*.test.mjs'
+
   cli-npx-shim:
     name: "CLI: npx shim (${{ matrix.os }})"
     needs: changes

package.json

@@ -33,6 +33,7 @@
     "format:check": "oxfmt --check .",
     "knip": "knip",
     "test:scripts": "node --import tsx --test scripts/validate-release-channel.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts",
+    "test:skills": "node --test 'skills/**/*.test.mjs'",
     "generate:previews": "tsx scripts/generate-template-previews.ts",
     "generate:catalog-previews": "tsx scripts/generate-catalog-previews.ts",
     "upload:docs-images": "bash scripts/upload-docs-images.sh",

skills/media-use/scripts/eval.mjs

@@ -15,7 +15,7 @@ import {
   writeFileSync,
 } from "node:fs";
 import { join, basename, resolve, dirname } from "node:path";
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 import { tmpdir } from "node:os";
 import { fileURLToPath } from "node:url";
 
@@ -33,11 +33,19 @@ const TEST_BLOCKS = [
   "registry/blocks/instagram-follow",
 ];
 
-function run(cmd, opts = {}) {
+// Run resolve.mjs with args as a literal argv array (no shell), so values
+// interpolated from manifest metadata (--intent prompt, --type) can't inject
+// shell. Mirrors the execFileSync fix in probe.mjs / heygen-search.mjs.
+function run(args, opts = {}) {
   try {
     return {
       ok: true,
-      output: execSync(cmd, { encoding: "utf8", timeout: 15000, stdio: "pipe", ...opts }).trim(),
+      output: execFileSync(process.execPath, [RESOLVE_SCRIPT, ...args], {
+        encoding: "utf8",
+        timeout: 15000,
+        stdio: "pipe",
+        ...opts,
+      }).trim(),
     };
   } catch (err) {
     return { ok: false, output: (err.stdout || "") + (err.stderr || ""), code: err.status };
@@ -92,7 +100,7 @@ function evalBlock(blockPath) {
     }
 
     // with media-use: run --adopt
-    const adoptResult = run(`node "${RESOLVE_SCRIPT}" --adopt --project "${tmp}" --json`);
+    const adoptResult = run(["--adopt", "--project", tmp, "--json"]);
     let adopted = { ok: false, adopted: 0, assets: [] };
     if (adoptResult.ok) {
       try {
@@ -129,9 +137,7 @@ function evalBlock(blockPath) {
     if (manifest.length > 0) {
       const first = manifest[0];
       const prompt = first.provenance?.prompt || first.description;
-      const r = run(
-        `node "${RESOLVE_SCRIPT}" --type ${first.type} --intent "${prompt}" --project "${tmp}" --json`,
-      );
+      const r = run(["--type", first.type, "--intent", prompt, "--project", tmp, "--json"]);
       if (r.ok) {
         try {
           resolveTest = JSON.parse(r.output);
@@ -142,9 +148,15 @@ function evalBlock(blockPath) {
     }
 
     // test resolve miss: try resolving something that doesn't exist
-    const missResult = run(
-      `node "${RESOLVE_SCRIPT}" --type bgm --intent "nonexistent query xyz" --project "${tmp}" --json`,
-    );
+    const missResult = run([
+      "--type",
+      "bgm",
+      "--intent",
+      "nonexistent query xyz",
+      "--project",
+      tmp,
+      "--json",
+    ]);
     let resolveMiss = null;
     if (!missResult.ok) {
       try {

skills/media-use/scripts/lib/heygen-search.mjs

@@ -1,20 +1,26 @@
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 
 export function heygenSearch(subcommand, query, { type, limit = 5, minScore } = {}) {
-  const q = query.replace(/'/g, "'\\''");
+  // execFileSync with an argv array (no shell), so query/type/etc. are passed as
+  // literal arguments — no quoting tricks, no command injection. subcommand is a
+  // hardcoded multi-word string (e.g. "audio sounds list"), split into tokens.
   // Tag the caller via the CLI's allowlisted attribution header (heygen >= v0.1.6).
-  const parts = [
-    `heygen --headers 'X-HeyGen-Client-Source: media-use' ${subcommand} --query '${q}'`,
+  const args = [
+    "--headers",
+    "X-HeyGen-Client-Source: media-use",
+    ...subcommand.split(" "),
+    "--query",
+    query,
   ];
-  if (type) parts.push(`--type ${type}`);
-  parts.push(`--limit ${limit}`);
+  if (type) args.push("--type", type);
+  args.push("--limit", String(limit));
   // Server-side score floor. Honored by `audio sounds list`; the `asset search`
   // backend rejects it, so only audio providers pass minScore (see image-provider).
-  if (minScore != null) parts.push(`--min-score ${minScore}`);
+  if (minScore != null) args.push("--min-score", String(minScore));
 
   let out;
   try {
-    out = execSync(parts.join(" "), {
+    out = execFileSync("heygen", args, {
       encoding: "utf8",
       timeout: 15000,
       stdio: ["pipe", "pipe", "pipe"],

skills/media-use/scripts/lib/probe.mjs

@@ -1,4 +1,4 @@
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 import { extname } from "node:path";
 
 const IMAGE_EXT = new Set([".jpg", ".jpeg", ".png", ".gif", ".webp", ".svg", ".ico"]);
@@ -8,8 +8,11 @@ export function probe(filePath) {
   if (ext === ".svg") return { width: null, height: null, duration: null, codec: "svg" };
 
   try {
-    const raw = execSync(
-      `ffprobe -v quiet -print_format json -show_format -show_streams "${filePath}"`,
+    // execFileSync (no shell) so a hostile filename like `"; rm -rf ~; ".png`
+    // can't break out of the quoting — filePath is passed as a literal argv entry.
+    const raw = execFileSync(
+      "ffprobe",
+      ["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", filePath],
       { encoding: "utf8", timeout: 5000 },
     );
     const info = JSON.parse(raw);

skills/media-use/scripts/lib/probe.test.mjs

@@ -0,0 +1,31 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync, existsSync, rmSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { probe } from "./probe.mjs";
+
+// Regression for the shell-injection fix: probe() must pass the path as a literal
+// argv entry, never through a shell. A filename containing shell metacharacters
+// must NOT execute. Under the old execSync(`ffprobe ... "${path}"`) the embedded
+// `touch` ran and created the marker; under execFileSync it cannot, regardless of
+// whether ffprobe is installed (the injected command never reaches a shell).
+test("probe does not execute shell metacharacters in a filename", () => {
+  const dir = mkdtempSync(join(tmpdir(), "probe-inject-"));
+  const marker = join(dir, "INJECTED");
+  // Slash-free basename (a real on-disk filename) that breaks out of the old
+  // double-quoted interpolation and would `touch INJECTED` in the cwd.
+  const evil = join(dir, `clip"; touch INJECTED; echo ".mp4`);
+  const prevCwd = process.cwd();
+  try {
+    writeFileSync(evil, "not real media");
+    process.chdir(dir); // so a leaked `touch INJECTED` would land next to `marker`
+    const meta = probe(evil);
+    assert.equal(existsSync(marker), false, "injected `touch` must not have run");
+    // Bogus/unreadable media still returns the null-shaped result, never throws.
+    assert.deepEqual(Object.keys(meta).sort(), ["codec", "duration", "height", "width"]);
+  } finally {
+    process.chdir(prevCwd);
+    rmSync(dir, { recursive: true, force: true });
+  }
+});