fix(security): prevent package scans from exhausting worker memory (#3393)

* fix(security): scope SkillSpector to bundled roots

* fix(security): bound bundled SkillSpector scans

* chore(security): format SkillSpector worker changes

* fix(security): import path separator for scan roots

scripts/security/run-codex-scan-worker-clawscan.test.ts

@@ -7,7 +7,12 @@ import { join } from "node:path";
 import { promisify } from "node:util";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import type { ClaimedJob } from "./run-codex-scan-worker";
-import { processJob, runClawScan } from "./run-codex-scan-worker";
+import {
+  aggregateSkillSpectorAnalyses,
+  processJob,
+  resolveBundledSkillSpectorScanInputs,
+  runClawScan,
+} from "./run-codex-scan-worker";
 
 const tempDirs: string[] = [];
 const execFileAsync = promisify(execFile);
@@ -88,7 +93,12 @@ function claimedJob(input: {
     target: {
       ...input.target,
       ...(input.targetKind === "packageRelease"
-        ? { release: { vtAnalysis: input.vtAnalysis ?? null } }
+        ? {
+            release: {
+              ...(input.target.release ?? {}),
+              vtAnalysis: input.vtAnalysis ?? null,
+            },
+          }
         : { version: { vtAnalysis: input.vtAnalysis ?? null } }),
     },
   };
@@ -220,6 +230,216 @@ function clawScanArtifactJson(options?: {
 }
 
 describe("run-codex-scan-worker clawscan authority", () => {
+  it.each([
+    {
+      name: "zero roots",
+      roots: [],
+      expected: [],
+    },
+    {
+      name: "one root",
+      roots: ["skills/alpha"],
+      expected: [{ rootPath: "skills/alpha", scanPath: "artifact/package/skills/alpha" }],
+    },
+    {
+      name: "multiple roots in deterministic order",
+      roots: ["skills/zeta/", "./skills/alpha", "skills/zeta"],
+      expected: [
+        { rootPath: "skills/alpha", scanPath: "artifact/package/skills/alpha" },
+        { rootPath: "skills/zeta", scanPath: "artifact/package/skills/zeta" },
+      ],
+    },
+    {
+      name: "traversal roots rejected",
+      roots: ["../outside", "skills/../../outside", "/absolute", ".", "skills/safe"],
+      expected: [{ rootPath: "skills/safe", scanPath: "artifact/package/skills/safe" }],
+    },
+  ])("selects $name only from the package manifest", async ({ name, roots, expected }) => {
+    const workspace = await tempDir();
+    await mkdir(join(workspace, "artifact", "package"), { recursive: true });
+    await writeFile(join(workspace, "artifact", "package", "package.json"), "{}\n");
+    const job = claimedJob({
+      jobId: `securityScanJobs:${name.replaceAll(" ", "-")}`,
+      source: "publish",
+      targetKind: "packageRelease",
+      target: {
+        release: {
+          pluginManifestSummary: {
+            bundledSkills: roots.map((rootPath) => ({ rootPath })),
+          },
+        },
+      },
+    });
+
+    await expect(resolveBundledSkillSpectorScanInputs(workspace, job)).resolves.toEqual(expected);
+  });
+
+  it("produces an explicit result when a package has no bundled skills", () => {
+    expect(aggregateSkillSpectorAnalyses([])).toEqual({
+      applicable: false,
+      status: "clean",
+      risk_assessment: {
+        score: 0,
+        severity: "NONE",
+        recommendation: "NOT_APPLICABLE",
+      },
+      issue_count: 0,
+      filtered_findings: [],
+      metadata: { skillspector_version: "skillspector" },
+      summary: "Package declares no bundled skills; SkillSpector was not applicable.",
+    });
+  });
+
+  it("aggregates bundled SkillSpector reports deterministically", () => {
+    const analyses = [
+      {
+        status: "clean",
+        score: 5,
+        severity: "LOW",
+        recommendation: "ALLOW",
+        issueCount: 0,
+        issues: [],
+        scannerVersion: "2.0.0",
+        summary: "alpha clean",
+        checkedAt: 20,
+      },
+      {
+        status: "suspicious",
+        score: 80,
+        severity: "HIGH",
+        recommendation: "REVIEW",
+        issueCount: 1,
+        issues: [
+          {
+            issueId: "SDI-2",
+            severity: "HIGH",
+            explanation: "review beta",
+          },
+        ],
+        scannerVersion: "1.0.0",
+        summary: "beta suspicious",
+        checkedAt: 10,
+      },
+    ];
+
+    expect(aggregateSkillSpectorAnalyses(analyses)).toEqual(
+      aggregateSkillSpectorAnalyses([...analyses].reverse()),
+    );
+    expect(aggregateSkillSpectorAnalyses(analyses)).toMatchObject({
+      applicable: true,
+      status: "suspicious",
+      risk_assessment: {
+        score: 80,
+        severity: "HIGH",
+        recommendation: "ALLOW; REVIEW",
+      },
+      issue_count: 1,
+      metadata: { skillspector_version: "1.0.0, 2.0.0" },
+      summary: "Scanned 2 bundled skills. alpha clean beta suspicious",
+    });
+  });
+
+  it("never passes a package root to SkillSpector", async () => {
+    const workspace = await tempDir();
+    const packageRoot = join(workspace, "artifact", "package");
+    await mkdir(join(packageRoot, "skills", "alpha"), { recursive: true });
+    await mkdir(join(packageRoot, "skills", "beta"), { recursive: true });
+    await writeFile(join(packageRoot, "package.json"), "{}\n");
+    await writeFile(join(packageRoot, "skills", "alpha", "SKILL.md"), "# alpha\n");
+    await writeFile(join(packageRoot, "skills", "beta", "SKILL.md"), "# beta\n");
+
+    const fakeSkillSpector = join(workspace, "skillspector");
+    const skillSpectorTargets = join(workspace, "skillspector-targets.log");
+    await writeFakeClawScanCommand(
+      fakeSkillSpector,
+      `target="$2"
+printf '%s\\n' "$target" >> ${JSON.stringify(skillSpectorTargets)}
+out=""
+while [[ $# -gt 0 ]]; do
+  case "$1" in
+    --output)
+      out="$2"
+      shift 2
+      ;;
+    *)
+      shift
+      ;;
+  esac
+done
+cat > "$out" <<JSON
+{"status":"suspicious","risk_score":25,"risk_severity":"MEDIUM","risk_recommendation":"REVIEW","issue_count":1,"issues":[{"id":"same-name","severity":"MEDIUM","file":"SKILL.md","explanation":"review"}],"scanner_version":"test","summary":"$(basename "$target") suspicious"}
+JSON`,
+    );
+
+    const fakeClawScan = join(workspace, "fake-clawscan");
+    const copiedFixture = join(workspace, "skillspector-fixture.json");
+    await writeFakeClawScanCommand(
+      fakeClawScan,
+      `out=""
+fixture=""
+while [[ $# -gt 0 ]]; do
+  case "$1" in
+    --output)
+      out="$2"
+      shift 2
+      ;;
+    --scanner-result)
+      fixture="\${2#skillspector=}"
+      shift 2
+      ;;
+    *)
+      shift
+      ;;
+  esac
+done
+cp "$fixture" ${JSON.stringify(copiedFixture)}
+cat > "$out" <<'JSON'
+${clawScanArtifactJson()}
+JSON`,
+    );
+
+    const previousClawScan = process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND;
+    const previousPath = process.env.PATH;
+    process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND = fakeClawScan;
+    process.env.PATH = `${workspace}:${previousPath ?? ""}`;
+    try {
+      const job = claimedJob({
+        jobId: "securityScanJobs:bundled-roots-only",
+        source: "publish",
+        targetKind: "packageRelease",
+        target: {
+          release: {
+            pluginManifestSummary: {
+              bundledSkills: [{ rootPath: "skills/beta" }, { rootPath: "skills/alpha" }],
+            },
+          },
+        },
+      });
+
+      await runClawScan(job, workspace, () => {});
+
+      expect((await readFile(skillSpectorTargets, "utf8")).trim().split("\n")).toEqual([
+        "artifact/package/skills/alpha",
+        "artifact/package/skills/beta",
+      ]);
+      expect(JSON.parse(await readFile(copiedFixture, "utf8"))).toMatchObject({
+        applicable: true,
+        status: "suspicious",
+        issue_count: 2,
+        filtered_findings: [{ file: "skills/alpha/SKILL.md" }, { file: "skills/beta/SKILL.md" }],
+        summary: "Scanned 2 bundled skills. alpha suspicious beta suspicious",
+      });
+    } finally {
+      if (previousClawScan === undefined) delete process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND;
+      else process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND = previousClawScan;
+      if (previousPath === undefined) {
+        delete process.env.PATH;
+      } else {
+        process.env.PATH = previousPath;
+      }
+    }
+  });
+
   it("uses ClawScan as the only skillVersion scan implementation", async () => {
     const workspace = await tempDir();
     const fakeClawScan = join(workspace, "fake-clawscan");
@@ -483,7 +703,12 @@ JSON`,
           expect.arrayContaining(["--profile", "clawhub", "--output"]),
         );
         expect(invocationArgs).not.toContain("--context");
-        expect(invocationArgs).not.toContain("--scanner-result");
+        if (targetKind === "packageRelease") {
+          expect(invocationArgs).toContain("--scanner-result");
+          expect(invocationArgs.some((arg) => arg.startsWith("skillspector="))).toBe(true);
+        } else {
+          expect(invocationArgs).not.toContain("--scanner-result");
+        }
         expect((await readFile(filesLog, "utf8")).trim().split("\n")).toContain(expectedFile);
       } finally {
         if (previousCommand === undefined) delete process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND;

scripts/security/run-codex-scan-worker.source-test.mjs

@@ -0,0 +1,27 @@
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import test from "node:test";
+
+const source = readFileSync(new URL("./run-codex-scan-worker.ts", import.meta.url), "utf8");
+const functionStart = source.indexOf("async function runBundledSkillSpector(");
+const functionEnd = source.indexOf("\nconst REQUIRED_CLAWHUB_RESULT_KEYS", functionStart);
+const functionSource = source.slice(functionStart, functionEnd);
+
+test("bundled SkillSpector roots share one deadline", () => {
+  const deadlineIndex = functionSource.indexOf(
+    "const deadlineMs = Date.now() + clawScanTimeoutMs();",
+  );
+  const loopIndex = functionSource.indexOf("for (const [index, scanInput]");
+  assert.ok(deadlineIndex >= 0, "expected a shared deadline");
+  assert.ok(loopIndex > deadlineIndex, "deadline must be captured before the per-root loop");
+  assert.match(functionSource, /const remainingMs = deadlineMs - Date\.now\(\);/);
+});
+
+test("expired budgets fail before starting another root", () => {
+  const expiryIndex = functionSource.indexOf("if (remainingMs <= 0)");
+  const commandIndex = functionSource.indexOf('await runCommand("skillspector"');
+  assert.ok(expiryIndex >= 0, "expected an exhausted-budget guard");
+  assert.ok(commandIndex > expiryIndex, "budget guard must run before SkillSpector");
+  assert.match(functionSource, /timeoutMs: remainingMs/);
+  assert.doesNotMatch(functionSource, /timeoutMs: clawScanTimeoutMs\(\)/);
+});

scripts/security/run-codex-scan-worker.ts

@@ -2,7 +2,7 @@ import { spawn } from "node:child_process";
 import { mkdirSync, readFileSync } from "node:fs";
 import { appendFile, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { basename, dirname, join, resolve } from "node:path";
+import { basename, dirname, join, resolve, sep } from "node:path";
 import { pathToFileURL } from "node:url";
 import { ConvexHttpClient } from "convex/browser";
 import { api } from "../../convex/_generated/api";
@@ -92,6 +92,28 @@ export type SkillSpectorAnalysis = {
   checkedAt: number;
 };
 
+type SkillSpectorScannerResult = {
+  applicable: boolean;
+  error?: string;
+  filtered_findings: SkillSpectorIssue[];
+  issue_count: number;
+  metadata?: {
+    skillspector_version: string;
+  };
+  risk_assessment: {
+    recommendation?: string;
+    score?: number;
+    severity?: string;
+  };
+  status: string;
+  summary?: string;
+};
+
+type BundledSkillSpectorScanInput = {
+  rootPath: string;
+  scanPath: string;
+};
+
 type ClawScanCommandDiagnostic = {
   args?: string[];
   artifactPath?: string;
@@ -1156,6 +1178,230 @@ function clawScanTimeoutMs() {
   return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_CLAWSCAN_TIMEOUT_MS;
 }
 
+function normalizedBundledSkillRoot(value: unknown) {
+  if (typeof value !== "string") return null;
+  const normalized = value
+    .trim()
+    .replaceAll("\\", "/")
+    .replace(/^\.\/+/, "")
+    .replace(/\/+$/, "");
+  if (
+    !normalized ||
+    normalized === "." ||
+    normalized.startsWith("/") ||
+    normalized.split("/").some((segment) => segment === "..")
+  ) {
+    return null;
+  }
+  return normalized;
+}
+
+function bundledSkillRootsForJob(job: ClaimedJob) {
+  if (job.job.targetKind !== "packageRelease") return [];
+  const release = asRecord(job.target.release);
+  const pluginManifestSummary = asRecord(release?.pluginManifestSummary);
+  const bundledSkills = pluginManifestSummary?.bundledSkills;
+  if (!Array.isArray(bundledSkills)) return [];
+  return [
+    ...new Set(
+      bundledSkills
+        .map((skill) => normalizedBundledSkillRoot(asRecord(skill)?.rootPath))
+        .filter((rootPath): rootPath is string => Boolean(rootPath)),
+    ),
+  ].sort();
+}
+
+export async function resolveBundledSkillSpectorScanInputs(workspace: string, job: ClaimedJob) {
+  if (job.job.targetKind !== "packageRelease") return [];
+  const packageRoot = await resolveClawScanTarget(workspace, job);
+  const artifactRoot = resolve(workspace, packageRoot);
+  return bundledSkillRootsForJob(job)
+    .map((rootPath) => {
+      const skillRoot = resolve(artifactRoot, rootPath);
+      return skillRoot.startsWith(`${artifactRoot}${sep}`)
+        ? { rootPath, scanPath: join(packageRoot, rootPath) }
+        : null;
+    })
+    .filter((input): input is BundledSkillSpectorScanInput => Boolean(input));
+}
+
+function packageRelativeSkillSpectorIssueFile(rootPath: string, file: string) {
+  const normalized = file
+    .trim()
+    .replaceAll("\\", "/")
+    .replace(/^\.\/+/, "");
+  if (normalized === rootPath || normalized.startsWith(`${rootPath}/`)) return normalized;
+  const rootedSuffix = `/${rootPath}/`;
+  const rootedIndex = normalized.lastIndexOf(rootedSuffix);
+  if (rootedIndex >= 0) return normalized.slice(rootedIndex + 1);
+  const relative = normalized
+    .split("/")
+    .filter((segment) => segment && segment !== "." && segment !== "..")
+    .join("/");
+  return relative ? `${rootPath}/${relative}` : rootPath;
+}
+
+function prefixSkillSpectorFindingPaths(
+  analysis: SkillSpectorAnalysis,
+  rootPath: string,
+): SkillSpectorAnalysis {
+  return {
+    ...analysis,
+    issues: analysis.issues.map((issue) =>
+      issue.file
+        ? {
+            ...issue,
+            file: packageRelativeSkillSpectorIssueFile(rootPath, issue.file),
+          }
+        : issue,
+    ),
+  };
+}
+
+function compareSkillSpectorIssues(left: SkillSpectorIssue, right: SkillSpectorIssue) {
+  return JSON.stringify(left).localeCompare(JSON.stringify(right));
+}
+
+export function aggregateSkillSpectorAnalyses(
+  analyses: SkillSpectorAnalysis[],
+): SkillSpectorScannerResult {
+  if (analyses.length === 0) {
+    return {
+      applicable: false,
+      status: "clean",
+      risk_assessment: {
+        score: 0,
+        severity: "NONE",
+        recommendation: "NOT_APPLICABLE",
+      },
+      issue_count: 0,
+      filtered_findings: [],
+      metadata: { skillspector_version: "skillspector" },
+      summary: "Package declares no bundled skills; SkillSpector was not applicable.",
+    };
+  }
+  if (analyses.length === 1) {
+    const [analysis] = analyses;
+    return {
+      applicable: true,
+      status: analysis.status,
+      risk_assessment: {
+        score: analysis.score,
+        severity: analysis.severity,
+        recommendation: analysis.recommendation,
+      },
+      issue_count: analysis.issueCount,
+      filtered_findings: analysis.issues,
+      metadata: analysis.scannerVersion
+        ? { skillspector_version: analysis.scannerVersion }
+        : undefined,
+      summary: analysis.summary,
+      error: analysis.error,
+    };
+  }
+
+  const statuses = analyses.map((analysis) => analysis.status);
+  const status = statuses.some((value) => value === "error" || value === "failed")
+    ? "error"
+    : statuses.includes("malicious")
+      ? "malicious"
+      : statuses.includes("suspicious")
+        ? "suspicious"
+        : "clean";
+  const severityRank = ["NONE", "UNKNOWN", "LOW", "MEDIUM", "HIGH", "CRITICAL"];
+  const severity = analyses
+    .map((analysis) => analysis.severity?.toUpperCase())
+    .filter((value): value is string => Boolean(value))
+    .sort(
+      (left, right) =>
+        severityRank.indexOf(right) - severityRank.indexOf(left) || left.localeCompare(right),
+    )[0];
+  const recommendations = [
+    ...new Set(analyses.map((analysis) => analysis.recommendation).filter(Boolean)),
+  ].sort();
+  const scannerVersions = [
+    ...new Set(analyses.map((analysis) => analysis.scannerVersion).filter(Boolean)),
+  ].sort();
+  const summaries = analyses
+    .map((analysis) => analysis.summary)
+    .filter((summary): summary is string => Boolean(summary))
+    .sort();
+  const errors = analyses
+    .map((analysis) => analysis.error)
+    .filter((error): error is string => Boolean(error))
+    .sort();
+  return {
+    applicable: true,
+    status,
+    risk_assessment: {
+      score: Math.max(...analyses.map((analysis) => analysis.score ?? 0)),
+      severity,
+      recommendation: recommendations.length > 0 ? recommendations.join("; ") : undefined,
+    },
+    issue_count: analyses.reduce((total, analysis) => total + analysis.issueCount, 0),
+    filtered_findings: analyses
+      .flatMap((analysis) => analysis.issues)
+      .sort(compareSkillSpectorIssues)
+      .slice(0, MAX_STORED_SKILLSPECTOR_ISSUES),
+    metadata:
+      scannerVersions.length > 0 ? { skillspector_version: scannerVersions.join(", ") } : undefined,
+    summary:
+      summaries.length > 0
+        ? `Scanned ${analyses.length} bundled skills. ${summaries.join(" ")}`
+        : `Scanned ${analyses.length} bundled skills.`,
+    error: errors.length > 0 ? errors.join("; ") : undefined,
+  };
+}
+
+async function runBundledSkillSpector(
+  workspace: string,
+  scanInputs: BundledSkillSpectorScanInput[],
+) {
+  const analyses: SkillSpectorAnalysis[] = [];
+  const deadlineMs = Date.now() + clawScanTimeoutMs();
+  for (const [index, scanInput] of scanInputs.entries()) {
+    const remainingMs = deadlineMs - Date.now();
+    if (remainingMs <= 0) {
+      throw new Error("SkillSpector bundled-skill scan deadline exceeded");
+    }
+    const resultPath = join(workspace, `skillspector-report-${index}.json`);
+    const args = ["scan", scanInput.scanPath, "--format", "json", "--output", resultPath];
+    try {
+      await runCommand("skillspector", args, {
+        cwd: workspace,
+        timeoutMs: remainingMs,
+      });
+      analyses.push(
+        prefixSkillSpectorFindingPaths(
+          normalizeSkillSpectorAnalysis(await readFile(resultPath, "utf8"), 0),
+          scanInput.rootPath,
+        ),
+      );
+    } catch (error) {
+      if (error instanceof CommandFailure) {
+        const rawResult = await readFile(resultPath, "utf8").catch(() => undefined);
+        if (rawResult) {
+          const analysis = prefixSkillSpectorFindingPaths(
+            normalizeSkillSpectorAnalysis(rawResult, 0),
+            scanInput.rootPath,
+          );
+          const validFindingsExit =
+            error.exitCode === 1 &&
+            (analysis.status === "suspicious" || analysis.status === "malicious") &&
+            analysis.issueCount > 0 &&
+            analysis.issues.length > 0;
+          if (validFindingsExit) {
+            analyses.push(analysis);
+            continue;
+          }
+        }
+      }
+      throw error;
+    }
+  }
+  return aggregateSkillSpectorAnalyses(analyses);
+}
+
 const REQUIRED_CLAWHUB_RESULT_KEYS = [
   ...CLAWHUB_OUTPUT_SCHEMA_CONTRACT.requiredResultKeys,
   "artifact_inspection",
@@ -1383,7 +1629,21 @@ export async function runClawScan(
   const command = process.env.CODEX_SECURITY_SCAN_CLAWSCAN_COMMAND ?? "clawscan";
   const artifactPath = join(workspace, "clawscan-artifact.json");
   const target = await resolveClawScanTarget(workspace, job);
-  const args = [target, "--profile", "clawhub", "--output", artifactPath];
+  const args = [target, "--profile", "clawhub"];
+  if (job.job.targetKind === "packageRelease") {
+    // SkillSpector only understands skills. ClawHub owns the plugin manifest
+    // boundary, so never let ClawScan fall back to scanning the whole package.
+    const scanInputs = await resolveBundledSkillSpectorScanInputs(workspace, job);
+    const skillSpectorResultPath = join(workspace, "skillspector-aggregate.json");
+    const skillSpectorResult = await runBundledSkillSpector(workspace, scanInputs);
+    await writeFile(
+      skillSpectorResultPath,
+      `${JSON.stringify(skillSpectorResult, null, 2)}\n`,
+      "utf8",
+    );
+    args.push("--scanner-result", `skillspector=${skillSpectorResultPath}`);
+  }
+  args.push("--output", artifactPath);
   onDiagnostic({ args: [command, ...args], artifactPath });
 
   const captureArtifact = async () => {

specs/security-moderation.md

@@ -326,6 +326,11 @@ See also: [acceptable-usage.md](./acceptable-usage.md) for the marketplace polic
 - ClawScan verdicts come from a GitHub Actions Codex worker, not a single
   hosted LLM call. Codex reviews the materialized artifact workspace with
   SkillSpector and static scan evidence as context.
+- In the external Codex security worker, package-release SkillSpector runs scan
+  only normalized bundled-skill roots declared by the stored plugin manifest
+  summary. The plugin package root is never a fallback SkillSpector target;
+  packages with no bundled roots provide ClawScan an explicit not-applicable
+  result. This is not a prepublication-worker contract.
 - Current skill and plugin scans are queued through `securityScanJobs` and
   completed by the external Codex worker.
 - VirusTotal telemetry remains a separate Security audit signal and is not an