feat(security): fail closed on dangerous skill installs

2026-04-09 16:21:15 +00:00 · 2026-03-31 23:27:10 +09:00
parent 98c0c38186
commit 0d7f1e2c84
21 changed files with 362 additions and 129 deletions
--- a/src/plugins/clawhub.ts
+++ b/src/plugins/clawhub.ts
@@ -13,6 +13,7 @@ import {
  type ClawHubPackageFamily,
 } from "../infra/clawhub.js";
 import { resolveCompatibilityHostVersion } from "../version.js";
+import type { InstallSafetyOverrides } from "./install-security-scan.js";
 import { installPluginFromArchive, type InstallPluginResult } from "./install.js";

 export const CLAWHUB_INSTALL_ERROR_CODE = {
@@ -223,16 +224,17 @@ function logClawHubPackageSummary(params: {
  }
 }

-export async function installPluginFromClawHub(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  spec: string;
-  baseUrl?: string;
-  token?: string;
-  logger?: PluginInstallLogger;
-  mode?: "install" | "update";
-  dryRun?: boolean;
-  expectedPluginId?: string;
-}): Promise<
+export async function installPluginFromClawHub(
+  params: InstallSafetyOverrides & {
+    spec: string;
+    baseUrl?: string;
+    token?: string;
+    logger?: PluginInstallLogger;
+    mode?: "install" | "update";
+    dryRun?: boolean;
+    expectedPluginId?: string;
+  },
+): Promise<
  | ({
      ok: true;
    } & Extract<InstallPluginResult, { ok: true }> & {
--- a/src/plugins/install-security-scan.runtime.ts
+++ b/src/plugins/install-security-scan.runtime.ts
@@ -3,6 +3,7 @@ import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan-
 import { scanDirectoryWithSummary } from "../security/skill-scanner.js";
 import { getGlobalHookRunner } from "./hook-runner-global.js";
 import { createBeforeInstallHookPayload } from "./install-policy-context.js";
+import type { InstallSafetyOverrides } from "./install-security-scan.js";

 type InstallScanLogger = {
  warn?: (message: string) => void;
@@ -162,6 +163,28 @@ function logDangerousForceUnsafeInstall(params: {
  );
 }

+function resolveBuiltinScanDecision(
+  params: InstallSafetyOverrides & {
+    builtinScan: BuiltinInstallScan;
+    logger: InstallScanLogger;
+    targetLabel: string;
+  },
+): InstallSecurityScanResult | undefined {
+  const builtinBlocked = buildBlockedScanResult({
+    builtinScan: params.builtinScan,
+    dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
+    targetLabel: params.targetLabel,
+  });
+  if (params.dangerouslyForceUnsafeInstall && params.builtinScan.critical > 0) {
+    logDangerousForceUnsafeInstall({
+      findings: params.builtinScan.findings,
+      logger: params.logger,
+      targetLabel: params.targetLabel,
+    });
+  }
+  return builtinBlocked;
+}
+
 async function scanFileTarget(params: {
  logger: InstallScanLogger;
  path: string;
@@ -262,16 +285,17 @@ async function runBeforeInstallHook(params: {
  return undefined;
 }

-export async function scanBundleInstallSourceRuntime(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  logger: InstallScanLogger;
-  pluginId: string;
-  sourceDir: string;
-  requestKind?: PluginInstallRequestKind;
-  requestedSpecifier?: string;
-  mode?: "install" | "update";
-  version?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanBundleInstallSourceRuntime(
+  params: InstallSafetyOverrides & {
+    logger: InstallScanLogger;
+    pluginId: string;
+    sourceDir: string;
+    requestKind?: PluginInstallRequestKind;
+    requestedSpecifier?: string;
+    mode?: "install" | "update";
+    version?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const builtinScan = await scanDirectoryTarget({
    logger: params.logger,
    path: params.sourceDir,
@@ -279,18 +303,12 @@ export async function scanBundleInstallSourceRuntime(params: {
    targetName: params.pluginId,
    warningMessage: `WARNING: Bundle "${params.pluginId}" contains dangerous code patterns`,
  });
-  const builtinBlocked = buildBlockedScanResult({
+  const builtinBlocked = resolveBuiltinScanDecision({
    builtinScan,
+    logger: params.logger,
    dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
    targetLabel: `Bundle "${params.pluginId}" installation`,
  });
-  if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) {
-    logDangerousForceUnsafeInstall({
-      findings: builtinScan.findings,
-      logger: params.logger,
-      targetLabel: `Bundle "${params.pluginId}" installation`,
-    });
-  }

  const hookResult = await runBeforeInstallHook({
    logger: params.logger,
@@ -314,19 +332,20 @@ export async function scanBundleInstallSourceRuntime(params: {
  return hookResult?.blocked ? hookResult : builtinBlocked;
 }

-export async function scanPackageInstallSourceRuntime(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  extensions: string[];
-  logger: InstallScanLogger;
-  packageDir: string;
-  pluginId: string;
-  requestKind?: PluginInstallRequestKind;
-  requestedSpecifier?: string;
-  mode?: "install" | "update";
-  packageName?: string;
-  manifestId?: string;
-  version?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanPackageInstallSourceRuntime(
+  params: InstallSafetyOverrides & {
+    extensions: string[];
+    logger: InstallScanLogger;
+    packageDir: string;
+    pluginId: string;
+    requestKind?: PluginInstallRequestKind;
+    requestedSpecifier?: string;
+    mode?: "install" | "update";
+    packageName?: string;
+    manifestId?: string;
+    version?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const forcedScanEntries: string[] = [];
  for (const entry of params.extensions) {
    const resolvedEntry = path.resolve(params.packageDir, entry);
@@ -352,18 +371,12 @@ export async function scanPackageInstallSourceRuntime(params: {
    targetName: params.pluginId,
    warningMessage: `WARNING: Plugin "${params.pluginId}" contains dangerous code patterns`,
  });
-  const builtinBlocked = buildBlockedScanResult({
+  const builtinBlocked = resolveBuiltinScanDecision({
    builtinScan,
+    logger: params.logger,
    dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
    targetLabel: `Plugin "${params.pluginId}" installation`,
  });
-  if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) {
-    logDangerousForceUnsafeInstall({
-      findings: builtinScan.findings,
-      logger: params.logger,
-      targetLabel: `Plugin "${params.pluginId}" installation`,
-    });
-  }

  const hookResult = await runBeforeInstallHook({
    logger: params.logger,
@@ -389,14 +402,15 @@ export async function scanPackageInstallSourceRuntime(params: {
  return hookResult?.blocked ? hookResult : builtinBlocked;
 }

-export async function scanFileInstallSourceRuntime(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  filePath: string;
-  logger: InstallScanLogger;
-  mode?: "install" | "update";
-  pluginId: string;
-  requestedSpecifier?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanFileInstallSourceRuntime(
+  params: InstallSafetyOverrides & {
+    filePath: string;
+    logger: InstallScanLogger;
+    mode?: "install" | "update";
+    pluginId: string;
+    requestedSpecifier?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const builtinScan = await scanFileTarget({
    logger: params.logger,
    path: params.filePath,
@@ -404,18 +418,12 @@ export async function scanFileInstallSourceRuntime(params: {
    targetName: params.pluginId,
    warningMessage: `WARNING: Plugin file "${params.pluginId}" contains dangerous code patterns`,
  });
-  const builtinBlocked = buildBlockedScanResult({
+  const builtinBlocked = resolveBuiltinScanDecision({
    builtinScan,
+    logger: params.logger,
    dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
    targetLabel: `Plugin file "${params.pluginId}" installation`,
  });
-  if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) {
-    logDangerousForceUnsafeInstall({
-      findings: builtinScan.findings,
-      logger: params.logger,
-      targetLabel: `Plugin file "${params.pluginId}" installation`,
-    });
-  }

  const hookResult = await runBeforeInstallHook({
    logger: params.logger,
--- a/src/plugins/install-security-scan.ts
+++ b/src/plugins/install-security-scan.ts
@@ -2,6 +2,10 @@ type InstallScanLogger = {
  warn?: (message: string) => void;
 };

+export type InstallSafetyOverrides = {
+  dangerouslyForceUnsafeInstall?: boolean;
+};
+
 export type InstallSecurityScanResult = {
  blocked?: {
    code?: "security_scan_blocked" | "security_scan_failed";
@@ -19,45 +23,48 @@ async function loadInstallSecurityScanRuntime() {
  return await import("./install-security-scan.runtime.js");
 }

-export async function scanBundleInstallSource(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  logger: InstallScanLogger;
-  pluginId: string;
-  sourceDir: string;
-  requestKind?: PluginInstallRequestKind;
-  requestedSpecifier?: string;
-  mode?: "install" | "update";
-  version?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanBundleInstallSource(
+  params: InstallSafetyOverrides & {
+    logger: InstallScanLogger;
+    pluginId: string;
+    sourceDir: string;
+    requestKind?: PluginInstallRequestKind;
+    requestedSpecifier?: string;
+    mode?: "install" | "update";
+    version?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const { scanBundleInstallSourceRuntime } = await loadInstallSecurityScanRuntime();
  return await scanBundleInstallSourceRuntime(params);
 }

-export async function scanPackageInstallSource(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  extensions: string[];
-  logger: InstallScanLogger;
-  packageDir: string;
-  pluginId: string;
-  requestKind?: PluginInstallRequestKind;
-  requestedSpecifier?: string;
-  mode?: "install" | "update";
-  packageName?: string;
-  manifestId?: string;
-  version?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanPackageInstallSource(
+  params: InstallSafetyOverrides & {
+    extensions: string[];
+    logger: InstallScanLogger;
+    packageDir: string;
+    pluginId: string;
+    requestKind?: PluginInstallRequestKind;
+    requestedSpecifier?: string;
+    mode?: "install" | "update";
+    packageName?: string;
+    manifestId?: string;
+    version?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const { scanPackageInstallSourceRuntime } = await loadInstallSecurityScanRuntime();
  return await scanPackageInstallSourceRuntime(params);
 }

-export async function scanFileInstallSource(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  filePath: string;
-  logger: InstallScanLogger;
-  mode?: "install" | "update";
-  pluginId: string;
-  requestedSpecifier?: string;
-}): Promise<InstallSecurityScanResult | undefined> {
+export async function scanFileInstallSource(
+  params: InstallSafetyOverrides & {
+    filePath: string;
+    logger: InstallScanLogger;
+    mode?: "install" | "update";
+    pluginId: string;
+    requestedSpecifier?: string;
+  },
+): Promise<InstallSecurityScanResult | undefined> {
  const { scanFileInstallSourceRuntime } = await loadInstallSecurityScanRuntime();
  return await scanFileInstallSourceRuntime(params);
 }
--- a/src/plugins/install.ts
+++ b/src/plugins/install.ts
@@ -10,6 +10,7 @@ import {
 import { type NpmIntegrityDrift, type NpmSpecResolution } from "../infra/install-source-utils.js";
 import { CONFIG_DIR, resolveUserPath } from "../utils.js";
 import type { InstallSecurityScanResult } from "./install-security-scan.js";
+import type { InstallSafetyOverrides } from "./install-security-scan.js";
 import {
  resolvePackageExtensionEntries,
  type PackageManifest as PluginPackageManifest,
@@ -230,8 +231,7 @@ function buildBlockedInstallResult(params: {
  };
 }

-type PackageInstallCommonParams = {
-  dangerouslyForceUnsafeInstall?: boolean;
+type PackageInstallCommonParams = InstallSafetyOverrides & {
  extensionsDir?: string;
  timeoutMs?: number;
  logger?: PluginInstallLogger;
@@ -794,18 +794,19 @@ export async function installPluginFromFile(params: {
  return buildFileInstallResult(pluginId, targetFile);
 }

-export async function installPluginFromNpmSpec(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  spec: string;
-  extensionsDir?: string;
-  timeoutMs?: number;
-  logger?: PluginInstallLogger;
-  mode?: "install" | "update";
-  dryRun?: boolean;
-  expectedPluginId?: string;
-  expectedIntegrity?: string;
-  onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise<boolean>;
-}): Promise<InstallPluginResult> {
+export async function installPluginFromNpmSpec(
+  params: InstallSafetyOverrides & {
+    spec: string;
+    extensionsDir?: string;
+    timeoutMs?: number;
+    logger?: PluginInstallLogger;
+    mode?: "install" | "update";
+    dryRun?: boolean;
+    expectedPluginId?: string;
+    expectedIntegrity?: string;
+    onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise<boolean>;
+  },
+): Promise<InstallPluginResult> {
  const runtime = await loadPluginInstallRuntime();
  const { logger, timeoutMs, mode, dryRun } = runtime.resolveTimedInstallModeOptions(
    params,
--- a/src/plugins/marketplace.ts
+++ b/src/plugins/marketplace.ts
@@ -9,6 +9,7 @@ import { runCommandWithTimeout } from "../process/exec.js";
 import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js";
 import { sanitizeForLog } from "../terminal/ansi.js";
 import { resolveUserPath } from "../utils.js";
+import type { InstallSafetyOverrides } from "./install-security-scan.js";
 import { installPluginFromPath, type InstallPluginResult } from "./install.js";

 const DEFAULT_GIT_TIMEOUT_MS = 120_000;
@@ -1030,16 +1031,17 @@ export async function resolveMarketplaceInstallShortcut(
  };
 }

-export async function installPluginFromMarketplace(params: {
-  dangerouslyForceUnsafeInstall?: boolean;
-  marketplace: string;
-  plugin: string;
-  logger?: MarketplaceLogger;
-  timeoutMs?: number;
-  mode?: "install" | "update";
-  dryRun?: boolean;
-  expectedPluginId?: string;
-}): Promise<MarketplaceInstallResult> {
+export async function installPluginFromMarketplace(
+  params: InstallSafetyOverrides & {
+    marketplace: string;
+    plugin: string;
+    logger?: MarketplaceLogger;
+    timeoutMs?: number;
+    mode?: "install" | "update";
+    dryRun?: boolean;
+    expectedPluginId?: string;
+  },
+): Promise<MarketplaceInstallResult> {
  const loaded = await loadMarketplace({
    source: params.marketplace,
    logger: params.logger,