From 95109066695572ff784e3f3e273244d2b23d583f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 08:55:31 +0100 Subject: [PATCH] fix: stop hook fallback after security blocks --- CHANGELOG.md | 1 + src/cli/plugins-cli.install.test.ts | 77 +++++++++++++++++++++++++++++ src/cli/plugins-install-command.ts | 18 +++---- 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a17323ecb58..71dcbbb7b06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- CLI/plugins: stop security-blocked plugin installs from retrying as hook packs, so normal plugin packages report the scanner failure without a misleading "not a valid hook pack" follow-up. Fixes #61175; supersedes #64102. Thanks @KonsultDigital and @ziyincody. - CLI/update: keep the automatic post-update completion refresh on the core-command tree so it no longer stages bundled plugin runtime deps before the Gateway restart path, avoiding `.24` update hangs and 1006 disconnect cascades. Fixes #72665. Thanks @sakalaboator and @He-Pin. - Agents/Bedrock: stop heartbeat runs from persisting blank user transcript turns and repair existing blank user text messages before replay, preventing AWS Bedrock `ContentBlock` blank-text validation failures. Fixes #72640 and #72622. Thanks @goldzulu. - Agents/LM Studio: promote standalone bracketed local-model tool requests into registered tool calls and hide unsupported bracket blocks from visible replies, so MemPalace MCP lookups do not print raw `[tool]` JSON scaffolding in chat. Fixes #66178. Thanks @detroit357. diff --git a/src/cli/plugins-cli.install.test.ts b/src/cli/plugins-cli.install.test.ts index 19c1f149249..afee5e71a06 100644 --- a/src/cli/plugins-cli.install.test.ts +++ b/src/cli/plugins-cli.install.test.ts @@ -191,6 +191,20 @@ function primeHookPackNpmFallback() { return { cfg, installedCfg }; } +function primeBlockedNpmPluginInstall(params: { + spec: string; + pluginId: string; + code?: "security_scan_blocked" | "security_scan_failed"; +}) { + loadConfig.mockReturnValue({} as OpenClawConfig); + mockClawHubPackageNotFound(params.spec); + installPluginFromNpmSpec.mockResolvedValue({ + ok: false, + error: `Plugin "${params.pluginId}" installation blocked: dangerous code patterns detected: finding details`, + code: params.code ?? "security_scan_blocked", + }); +} + function primeHookPackPathFallback(params: { tmpRoot: string; pluginInstallError: string; @@ -606,6 +620,30 @@ describe("plugins cli install", () => { ); }); + it("does not fall back to hook pack for linked path when a no-flag security scan blocks", async () => { + const localPluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-link-plugin-")); + const pluginInstallError = "plugin blocked by security scan"; + + loadConfig.mockReturnValue({} as OpenClawConfig); + installPluginFromPath.mockResolvedValue({ + ok: false, + error: pluginInstallError, + code: "security_scan_blocked", + }); + + try { + await expect( + runPluginsCommand(["plugins", "install", localPluginDir, "--link"]), + ).rejects.toThrow("__exit__:1"); + } finally { + fs.rmSync(localPluginDir, { recursive: true, force: true }); + } + + expect(installHooksFromPath).not.toHaveBeenCalled(); + expect(runtimeErrors.at(-1)).toContain(pluginInstallError); + expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack"); + }); + it("passes dangerous force unsafe install to local hook-pack fallback installs", async () => { const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hook-install-")); primeHookPackPathFallback({ @@ -740,6 +778,30 @@ describe("plugins cli install", () => { ).toBe(true); }); + it("does not fall back to hook pack for local path when a no-flag security scan fails", async () => { + const localPluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-local-plugin-")); + const pluginInstallError = "plugin security scan failed"; + + loadConfig.mockReturnValue({} as OpenClawConfig); + installPluginFromPath.mockResolvedValue({ + ok: false, + error: pluginInstallError, + code: "security_scan_failed", + }); + + try { + await expect(runPluginsCommand(["plugins", "install", localPluginDir])).rejects.toThrow( + "__exit__:1", + ); + } finally { + fs.rmSync(localPluginDir, { recursive: true, force: true }); + } + + expect(installHooksFromPath).not.toHaveBeenCalled(); + expect(runtimeErrors.at(-1)).toContain(pluginInstallError); + expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack"); + }); + it("does not fall back to hook pack for local path when dangerous force unsafe install is set", async () => { const localPluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-local-plugin-")); const cfg = {} as OpenClawConfig; @@ -822,6 +884,21 @@ describe("plugins cli install", () => { expect(runtimeErrors.at(-1)).toContain(pluginInstallError); }); + it("does not fall back to hook pack for npm installs when a no-flag security scan blocks", async () => { + primeBlockedNpmPluginInstall({ + spec: "@acme/unsafe-plugin", + pluginId: "unsafe-plugin", + }); + + await expect(runPluginsCommand(["plugins", "install", "@acme/unsafe-plugin"])).rejects.toThrow( + "__exit__:1", + ); + + expect(installHooksFromNpmSpec).not.toHaveBeenCalled(); + expect(runtimeErrors.at(-1)).toContain('Plugin "unsafe-plugin" installation blocked'); + expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack"); + }); + it("does not fall back to hook pack for npm installs when security scan fails under dangerous force unsafe install", async () => { const cfg = {} as OpenClawConfig; const pluginInstallError = "plugin security scan failed"; diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index f708acf8484..64ae250a654 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -201,14 +201,10 @@ async function tryInstallHookPackFromNpmSpec(params: { return { ok: true }; } -function shouldExitOnForcedUnsafeInstall(params: { - forceUnsafeInstall: boolean; - code?: string; -}): boolean { +function isTerminalPluginInstallSecurityFailure(code?: string): boolean { return ( - params.forceUnsafeInstall && - (params.code === PLUGIN_INSTALL_ERROR_CODE.SECURITY_SCAN_BLOCKED || - params.code === PLUGIN_INSTALL_ERROR_CODE.SECURITY_SCAN_FAILED) + code === PLUGIN_INSTALL_ERROR_CODE.SECURITY_SCAN_BLOCKED || + code === PLUGIN_INSTALL_ERROR_CODE.SECURITY_SCAN_FAILED ); } @@ -372,8 +368,6 @@ export async function runPluginInstallCommand(params: { } const resolved = request.resolvedPath ?? request.normalizedSpec; - const forceUnsafeInstall = opts.dangerouslyForceUnsafeInstall === true; - if (fs.existsSync(resolved)) { if (opts.link) { const existing = cfg.plugins?.load?.paths ?? []; @@ -386,7 +380,7 @@ export async function runPluginInstallCommand(params: { logger: createPluginInstallLogger(), }); if (!probe.ok) { - if (shouldExitOnForcedUnsafeInstall({ forceUnsafeInstall, code: probe.code })) { + if (isTerminalPluginInstallSecurityFailure(probe.code)) { defaultRuntime.error(probe.error); return defaultRuntime.exit(1); } @@ -439,7 +433,7 @@ export async function runPluginInstallCommand(params: { logger: createPluginInstallLogger(), }); if (!result.ok) { - if (shouldExitOnForcedUnsafeInstall({ forceUnsafeInstall, code: result.code })) { + if (isTerminalPluginInstallSecurityFailure(result.code)) { defaultRuntime.error(result.error); return defaultRuntime.exit(1); } @@ -588,7 +582,7 @@ export async function runPluginInstallCommand(params: { logger: createPluginInstallLogger(), }); if (!result.ok) { - if (shouldExitOnForcedUnsafeInstall({ forceUnsafeInstall, code: result.code })) { + if (isTerminalPluginInstallSecurityFailure(result.code)) { defaultRuntime.error(result.error); return defaultRuntime.exit(1); }