fix(security): limit SkillSpector to bundled skills (#2863)

fix(security): limit SkillSpector to bundled skills
2026-08-14 00:47:57 +00:00 · 2026-06-25 15:16:15 +08:00
parent f47f28e908
commit 3f66813e70
5 changed files with 268 additions and 42 deletions
@@ -9384,7 +9384,7 @@ export const updateReleaseScanResultsInternal = internalMutation({
 export const updateReleaseSkillSpectorAnalysisInternal = internalMutation({
  args: {
    releaseId: v.id("packageReleases"),
-    skillSpectorAnalysis: skillSpectorAnalysisValidator,
+    skillSpectorAnalysis: v.optional(skillSpectorAnalysisValidator),
  },
  handler: async (ctx, args) => {
    const release = await ctx.db.get(args.releaseId);
@@ -2997,6 +2997,39 @@ describe("securityScan", () => {
    expect(stored.issues[0]?.finding?.length).toBeLessThan(longSnippet.length);
  });

+  it("clears legacy plugin SkillSpector results when no new analysis is produced", async () => {
+    vi.stubEnv("SECURITY_SCAN_WORKER_TOKEN", "worker-secret");
+    const runQuery = vi.fn(async () => ({
+      job: {
+        _id: "securityScanJobs:plugin",
+        targetKind: "packageRelease",
+        leaseToken: "lease-token",
+      },
+      release: {
+        _id: "packageReleases:plugin",
+      },
+    }));
+    const runMutation = vi.fn(async (..._args: unknown[]) => ({ ok: true }));
+
+    await completeCodexScanJobHandler(
+      { runQuery, runMutation },
+      {
+        token: "worker-secret",
+        jobId: "securityScanJobs:plugin",
+        leaseToken: "lease-token",
+        llmAnalysis: { status: "clean", checkedAt: 123 },
+      },
+    );
+
+    expect(runMutation).toHaveBeenNthCalledWith(
+      1,
+      expect.anything(),
+      expect.objectContaining({ releaseId: "packageReleases:plugin" }),
+    );
+    const scanPatch = runMutation.mock.calls[0]?.[1] as Record<string, unknown>;
+    expect(scanPatch).not.toHaveProperty("skillSpectorAnalysis");
+  });
+
  it("persists an error ClawScan result when worker retries are exhausted", async () => {
    vi.stubEnv("SECURITY_SCAN_WORKER_TOKEN", "worker-secret");

@@ -2630,12 +2630,12 @@ export const completeCodexScanJob = action({
        llmAnalysis: args.llmAnalysis,
      });
    } else if (target.job.targetKind === "packageRelease" && target.release) {
-      if (args.skillSpectorAnalysis) {
-        await runMutationRef(ctx, internalRefs.packages.updateReleaseSkillSpectorAnalysisInternal, {
-          releaseId: target.release._id,
-          skillSpectorAnalysis: capSkillSpectorAnalysisForStorage(args.skillSpectorAnalysis),
-        });
-      }
+      await runMutationRef(ctx, internalRefs.packages.updateReleaseSkillSpectorAnalysisInternal, {
+        releaseId: target.release._id,
+        ...(args.skillSpectorAnalysis
+          ? { skillSpectorAnalysis: capSkillSpectorAnalysisForStorage(args.skillSpectorAnalysis) }
+          : {}),
+      });
      await runMutationRef(ctx, internalRefs.packages.updateReleaseLlmAnalysisInternal, {
        releaseId: target.release._id,
        llmAnalysis: args.llmAnalysis,
@@ -15,6 +15,7 @@ import {
  normalizeSkillSpectorAnalysis,
  processJob,
  resolveSkillSpectorScanInput,
+  resolveSkillSpectorScanInputs,
  writeArtifactWorkspace,
  writeJobDiagnostic,
 } from "./run-codex-scan-worker";
@@ -276,6 +277,37 @@ describe("run-codex-scan-worker diagnostics", () => {
    expect(prompt).not.toContain("OWASP");
  });

+  it("does not reuse plugin-level SkillSpector findings when no bundled skills are declared", () => {
+    const prompt = buildPrompt(
+      {
+        job: {
+          _id: "plugin-job",
+          hasMaliciousSignal: false,
+          leaseToken: "lease-secret",
+          source: "publish",
+          targetKind: "packageRelease",
+          waitForVtUntil: 0,
+        },
+        target: {
+          release: {
+            skillSpectorAnalysis: {
+              status: "suspicious",
+              issueCount: 1,
+              checkedAt: 123,
+              issues: [{ issueId: "SDI-1", severity: "HIGH", explanation: "plugin root" }],
+            },
+            pluginManifestSummary: {
+              bundledSkills: [],
+            },
+          },
+        },
+      },
+      [],
+    );
+
+    expect(prompt).not.toContain("plugin root");
+  });
+
  it("normalizes real SkillSpector JSON risk assessment fields", () => {
    const analysis = normalizeSkillSpectorAnalysis(
      JSON.stringify({
@@ -365,6 +397,63 @@ describe("run-codex-scan-worker diagnostics", () => {
    await expect(resolveSkillSpectorScanInput(workspace)).resolves.toBe("artifact");
  });

+  it("scans only bundled skill roots for plugin releases", async () => {
+    const workspace = await tempDir();
+    await mkdir(join(workspace, "artifact", "package"), { recursive: true });
+    await writeFile(join(workspace, "artifact.tgz"), "packed artifact");
+    await writeFile(join(workspace, "artifact", "package", "package.json"), "{}");
+
+    await expect(
+      resolveSkillSpectorScanInputs(workspace, {
+        job: {
+          _id: "package-job",
+          hasMaliciousSignal: false,
+          leaseToken: "lease-secret",
+          source: "publish",
+          targetKind: "packageRelease",
+          waitForVtUntil: 0,
+        },
+        target: {
+          release: {
+            pluginManifestSummary: {
+              bundledSkills: [
+                { rootPath: "skills/first" },
+                { rootPath: "./skills/second/" },
+                { rootPath: "../package-code" },
+              ],
+            },
+          },
+        },
+      }),
+    ).resolves.toEqual(["artifact/package/skills/first", "artifact/package/skills/second"]);
+  });
+
+  it("skips SkillSpector for plugin releases without bundled skills", async () => {
+    const workspace = await tempDir();
+    await mkdir(join(workspace, "artifact"), { recursive: true });
+    await writeFile(join(workspace, "artifact", "openclaw.plugin.json"), "{}");
+
+    await expect(
+      resolveSkillSpectorScanInputs(workspace, {
+        job: {
+          _id: "plugin-job",
+          hasMaliciousSignal: false,
+          leaseToken: "lease-secret",
+          source: "publish",
+          targetKind: "packageRelease",
+          waitForVtUntil: 0,
+        },
+        target: {
+          release: {
+            pluginManifestSummary: {
+              bundledSkills: [],
+            },
+          },
+        },
+      }),
+    ).resolves.toEqual([]);
+  });
+
  it("writes scanner metadata without lease tokens or signed file URLs", async () => {
    const workspace = await tempDir();

@@ -580,46 +580,144 @@ export async function resolveSkillSpectorScanInput(workspace: string) {
  return hasClawPackExtraction ? "artifact/package" : "artifact";
 }

+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 bundledSkills
+    .map((skill) => normalizedBundledSkillRoot(asRecord(skill)?.rootPath))
+    .filter((rootPath): rootPath is string => Boolean(rootPath));
+}
+
+export async function resolveSkillSpectorScanInputs(workspace: string, job: ClaimedJob) {
+  const bundledSkillRoots = bundledSkillRootsForJob(job);
+  if (job.job.targetKind !== "packageRelease") {
+    return [await resolveSkillSpectorScanInput(workspace)];
+  }
+  if (bundledSkillRoots.length === 0) return [];
+
+  const packageRoot = await resolveSkillSpectorScanInput(workspace);
+  const artifactRoot = resolve(workspace, packageRoot);
+  return bundledSkillRoots
+    .map((rootPath) => {
+      const skillRoot = resolve(artifactRoot, rootPath);
+      return skillRoot.startsWith(`${artifactRoot}/`) ? join(packageRoot, rootPath) : null;
+    })
+    .filter((path): path is string => Boolean(path));
+}
+
+function aggregateSkillSpectorAnalyses(analyses: SkillSpectorAnalysis[]) {
+  if (analyses.length === 1) return analyses[0];
+  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 = ["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))[0];
+  const recommendations = [
+    ...new Set(analyses.map((analysis) => analysis.recommendation).filter(Boolean)),
+  ];
+  const scannerVersions = [
+    ...new Set(analyses.map((analysis) => analysis.scannerVersion).filter(Boolean)),
+  ];
+  const summaries = analyses.map((analysis) => analysis.summary).filter(Boolean);
+  const errors = analyses.map((analysis) => analysis.error).filter(Boolean);
+  return {
+    status,
+    score: Math.max(...analyses.map((analysis) => analysis.score ?? 0)),
+    severity,
+    recommendation: recommendations.length > 0 ? recommendations.join("; ") : undefined,
+    issueCount: analyses.reduce((total, analysis) => total + analysis.issueCount, 0),
+    issues: analyses
+      .flatMap((analysis) => analysis.issues)
+      .slice(0, MAX_STORED_SKILLSPECTOR_ISSUES),
+    scannerVersion: scannerVersions.length > 0 ? 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,
+    checkedAt: Math.max(...analyses.map((analysis) => analysis.checkedAt)),
+  } satisfies SkillSpectorAnalysis;
+}
+
 async function runSkillSpector(
  workspace: string,
+  scanInputs: string[],
  onDiagnostic: (diagnostic: Partial<CodexCommandDiagnostic>) => void,
 ) {
-  const resultPath = join(workspace, "skillspector-report.json");
-  const scanInput = await resolveSkillSpectorScanInput(workspace);
-  const args = ["scan", scanInput, "--format", "json", "--output", resultPath];
-  onDiagnostic({ args });
-  try {
-    const output = await runCommand("skillspector", args, {
-      cwd: workspace,
-      timeoutMs: codexScanTimeoutMs(),
-    });
-    const raw = await readFile(resultPath, "utf8");
-    onDiagnostic({ exitCode: 0, rawResult: raw, stderr: output.stderr, stdout: output.stdout });
-    return normalizeSkillSpectorAnalysis(raw);
-  } catch (error) {
-    if (error instanceof CommandFailure) {
-      let rawResult: string | undefined;
-      try {
-        rawResult = await readFile(resultPath, "utf8");
-      } catch {
-        rawResult = undefined;
-      }
-      onDiagnostic({
-        exitCode: error.exitCode,
-        rawResult,
-        stderr: error.stderr,
-        stdout: error.stdout,
+  const analyses: SkillSpectorAnalysis[] = [];
+  for (const [index, scanInput] of scanInputs.entries()) {
+    const resultPath = join(workspace, `skillspector-report-${index}.json`);
+    const args = ["scan", scanInput, "--format", "json", "--output", resultPath];
+    onDiagnostic({ args });
+    try {
+      const output = await runCommand("skillspector", args, {
+        cwd: workspace,
+        timeoutMs: codexScanTimeoutMs(),
      });
-      if (rawResult) {
+      const raw = await readFile(resultPath, "utf8");
+      onDiagnostic({
+        exitCode: 0,
+        rawResult: raw,
+        stderr: output.stderr,
+        stdout: output.stdout,
+      });
+      analyses.push(normalizeSkillSpectorAnalysis(raw));
+    } catch (error) {
+      if (error instanceof CommandFailure) {
+        let rawResult: string | undefined;
        try {
-          return normalizeSkillSpectorAnalysis(rawResult);
+          rawResult = await readFile(resultPath, "utf8");
        } catch {
-          // Fall through to an error-shaped analysis; diagnostics keep the raw report.
+          rawResult = undefined;
+        }
+        onDiagnostic({
+          exitCode: error.exitCode,
+          rawResult,
+          stderr: error.stderr,
+          stdout: error.stdout,
+        });
+        if (rawResult) {
+          try {
+            analyses.push(normalizeSkillSpectorAnalysis(rawResult));
+            continue;
+          } catch {
+            // Fall through to an error-shaped analysis; diagnostics keep the raw report.
+          }
        }
      }
+      analyses.push(skillSpectorFailureAnalysis(error));
    }
-    return skillSpectorFailureAnalysis(error);
  }
+  return aggregateSkillSpectorAnalyses(analyses);
 }

 export function buildPrompt(
@@ -636,8 +734,11 @@ export function buildPrompt(
  );
  const skillSpector = JSON.stringify(
    skillSpectorAnalysis ??
-      (job.target.version as Record<string, unknown> | undefined)?.skillSpectorAnalysis ??
-      (job.target.release as Record<string, unknown> | undefined)?.skillSpectorAnalysis ??
+      (job.job.targetKind !== "packageRelease"
+        ? (job.target.version as Record<string, unknown> | undefined)?.skillSpectorAnalysis
+        : bundledSkillRootsForJob(job).length > 0
+          ? (job.target.release as Record<string, unknown> | undefined)?.skillSpectorAnalysis
+          : undefined) ??
      null,
    null,
    2,
@@ -1036,7 +1137,7 @@ function codexScanTimeoutMs() {
 async function runCodex(
  job: ClaimedJob,
  workspace: string,
-  skillSpectorAnalysis: SkillSpectorAnalysis,
+  skillSpectorAnalysis: SkillSpectorAnalysis | undefined,
  onDiagnostic: (diagnostic: Partial<CodexCommandDiagnostic>) => void,
 ) {
  const resultPath = join(workspace, "codex-result.json");
@@ -1115,9 +1216,12 @@ export async function processJob(
  let status: JobDiagnosticInput["status"] = "failed";
  try {
    await writeArtifactWorkspace(job, workspace);
-    skillSpectorAnalysis = await runSkillSpector(workspace, (next) => {
-      Object.assign(skillSpector, next);
-    });
+    const skillSpectorInputs = await resolveSkillSpectorScanInputs(workspace, job);
+    if (skillSpectorInputs.length > 0) {
+      skillSpectorAnalysis = await runSkillSpector(workspace, skillSpectorInputs, (next) => {
+        Object.assign(skillSpector, next);
+      });
+    }
    llmAnalysis = await runCodex(job, workspace, skillSpectorAnalysis, (next) => {
      Object.assign(codex, next);
    });