From 7cf4e84584809b53c1897febff145fc553b0cc0b Mon Sep 17 00:00:00 2001 From: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:08:51 +1000 Subject: [PATCH] fix(security): bind install warning approvals --- docs/gateway/protocol.md | 2 +- docs/tools/plugin.md | 18 +- docs/tools/skills-config.md | 9 +- .../install-policy-warning-error-details.ts | 2 + .../src/plugins-validators.test.ts | 6 +- .../gateway-protocol/src/schema/plugins.ts | 4 +- ...all-policy-warning-acknowledgement.test.ts | 18 ++ ...stall-policy-warning-error-details.test.ts | 1 + src/gateway/server-methods/plugins.test.ts | 169 ++++++++++++++++-- src/gateway/server-methods/plugins.ts | 122 ++++++++++++- src/plugins/clawhub.test.ts | 1 - src/plugins/clawhub.ts | 2 +- .../install-security-scan.runtime.test.ts | 6 + src/plugins/install-security-scan.runtime.ts | 1 + src/plugins/install-security-scan.types.ts | 1 + src/plugins/management-service.test.ts | 8 +- src/plugins/management-service.ts | 71 ++++++-- .../test-helpers/install-policy-warning.ts | 25 +++ 18 files changed, 406 insertions(+), 60 deletions(-) diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index dd63d6cfa063..83d39f1609e6 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -551,7 +551,7 @@ methods. Treat this as feature discovery, not a full enumeration of - `plugins.list` (`operator.read`) returns the installed plugin inventory plus locally curated official picks, diagnostics, and whether the current install mode allows mutations. - `plugins.search` (`operator.read`) searches installable ClawHub code-plugin and bundle-plugin families. Pass non-empty `query` and optional `limit` from 1 to 100. - - `plugins.install` (`operator.admin`) installs either an official catalog entry with `{ source: "official", pluginId, acknowledgeInstallPolicyWarning? }` or a ClawHub package with `{ source: "clawhub", packageName, version?, acknowledgeClawHubRisk?, acknowledgeInstallPolicyWarning? }`. ClawHub installs preserve Gateway trust, integrity, and install-policy checks. When install policy warns, the request fails before commit with structured `error.details` containing `installPolicyCode`, target metadata, `reason`, and optional `findings`. After showing those details, a client may retry once with `acknowledgeInstallPolicyWarning: true`; that approval is consumed by the first warning and policy re-evaluates the staged source before continuing. A block or later warning remains terminal for that request and is returned with its own structured details. Successful installs require a Gateway restart. + - `plugins.install` (`operator.admin`) installs either an official catalog entry with `{ source: "official", pluginId, installPolicyWarningAcknowledgement? }` or a ClawHub package with `{ source: "clawhub", packageName, version?, acknowledgeClawHubRisk?, installPolicyWarningAcknowledgement? }`. ClawHub installs preserve Gateway trust, integrity, and install-policy checks. When install policy warns, the request fails before commit with structured `error.details` containing `installPolicyCode`, target metadata, `reason`, optional `findings`, and a server-issued `acknowledgementToken`. After showing those details, a client may retry with that token as `installPolicyWarningAcknowledgement`. The Gateway consumes the token once and only for the same install request and resolved artifact; policy then re-evaluates the staged source and continues only when the warning is unchanged. A block or changed/later warning remains terminal for that request and is returned with its own structured details and token. Successful installs require a Gateway restart. - `plugins.setEnabled` (`operator.admin`) changes one installed plugin's enabled policy with `{ pluginId, enabled }`. The response includes the updated catalog entry, restart metadata, and any slot-selection warnings. - `plugins.uninstall` (`operator.admin`) removes one externally installed plugin with `{ pluginId }`: config references, the install record, and managed files. Bundled plugins cannot be uninstalled, only disabled. The response lists the removal actions and always requires a Gateway restart. diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index fe0cb1a03550..5c4efb896aff 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -154,12 +154,13 @@ non-interactive direct CLI commands can use `--acknowledge-install-policy-warnin That flag is consumed by the first warning in one command; a later warning fails closed and requires interactive review. Gateway `plugins.install` clients receive structured warning details and may -make one explicit retry with `acknowledgeInstallPolicyWarning: true`. That -approval is consumed by the first warning. OpenClaw evaluates that same staged -scan again before allowing the acknowledged warning to continue. A block from -that evaluation, or a warning from any later package or dependency scan, stops -the request before commit and returns its own details. Other Gateway-backed and -automatic installs remain blocked on warnings because they have no +make one explicit retry with the returned `acknowledgementToken` as +`installPolicyWarningAcknowledgement`. The Gateway consumes that server-issued +token once and only for the same install request and resolved artifact. OpenClaw +re-evaluates the staged source and continues only when the warning is unchanged. +A block, a changed warning, or a warning from a later package or dependency scan +stops the request before commit and returns its own details. Other Gateway-backed +and automatic installs remain blocked on warnings because they have no operator-confirmation flow. When an equivalent direct plugin or skill command exists, use that command to review and approve the warning. Otherwise, change `security.installPolicy` to return `allow` for the reviewed request, then retry @@ -167,8 +168,9 @@ the managed flow. Neither `--force` nor the deprecated plugin install/update flag `--dangerously-force-unsafe-install` approves a policy warning. Plugin `before_install` hooks run later, and only in OpenClaw processes where plugin hooks are loaded, so use `security.installPolicy` for operator-owned install -decisions instead. The flag does not override a block or policy failure. -It also does not bypass `before_install` hook blocks. +decisions instead. `--acknowledge-install-policy-warning` does not override a +block or policy failure. Neither acknowledgement nor the deprecated flag +bypasses `before_install` hook blocks. See [Skills config](/tools/skills-config#operator-install-policy-securityinstallpolicy) for the shared `security.installPolicy` exec schema used by both skills and diff --git a/docs/tools/skills-config.md b/docs/tools/skills-config.md index a7ef8468b4d5..c13f602f43b6 100644 --- a/docs/tools/skills-config.md +++ b/docs/tools/skills-config.md @@ -202,10 +202,11 @@ every approved warning is re-evaluated before continuing. The flag is consumed by the first warning in one command; a later warning fails closed and requires interactive review. Gateway `plugins.install` clients receive structured warning details and may -make one explicit retry with `acknowledgeInstallPolicyWarning: true`. The first -warning consumes that approval. OpenClaw evaluates that same staged scan again -before allowing the acknowledged warning to continue. A block from that -evaluation, or a warning from any later package or dependency scan, stops the +make one explicit retry with the returned `acknowledgementToken` as +`installPolicyWarningAcknowledgement`. The Gateway consumes that server-issued +token once and only for the same install request and resolved artifact. OpenClaw +re-evaluates the staged source and continues only when the warning is unchanged. A block, a +changed warning, or a warning from a later package or dependency scan stops the request before commit and returns its own details. Other Gateway-backed and automatic installs remain blocked on warnings because they have no operator-confirmation flow. Use an equivalent direct plugin or skill diff --git a/packages/gateway-protocol/src/install-policy-warning-error-details.ts b/packages/gateway-protocol/src/install-policy-warning-error-details.ts index 7241a11d2eb8..4da07ceccea4 100644 --- a/packages/gateway-protocol/src/install-policy-warning-error-details.ts +++ b/packages/gateway-protocol/src/install-policy-warning-error-details.ts @@ -19,6 +19,7 @@ export type InstallPolicyWarningErrorDetails = { targetType: "skill" | "plugin"; requestMode: "install" | "update"; reason: string; + acknowledgementToken: string; findings?: InstallPolicyWarningErrorFinding[]; }; @@ -37,6 +38,7 @@ const installPolicyWarningErrorDetailsSchema = z.object({ targetType: z.enum(["skill", "plugin"]), requestMode: z.enum(["install", "update"]), reason: z.string().trim().min(1), + acknowledgementToken: z.string().trim().min(1), findings: z.array(installPolicyWarningFindingSchema).optional(), }); diff --git a/packages/gateway-protocol/src/plugins-validators.test.ts b/packages/gateway-protocol/src/plugins-validators.test.ts index e96d34107d7f..881b67400960 100644 --- a/packages/gateway-protocol/src/plugins-validators.test.ts +++ b/packages/gateway-protocol/src/plugins-validators.test.ts @@ -31,14 +31,14 @@ describe("plugin lifecycle protocol validators", () => { packageName: "memory-plus", version: "2.1.0", acknowledgeClawHubRisk: true, - acknowledgeInstallPolicyWarning: true, + installPolicyWarningAcknowledgement: "approval-token", }), ).toBe(true); expect( validatePluginsInstallParams({ source: "official", pluginId: "workboard", - acknowledgeInstallPolicyWarning: true, + installPolicyWarningAcknowledgement: "approval-token", }), ).toBe(true); expect( @@ -52,7 +52,7 @@ describe("plugin lifecycle protocol validators", () => { validatePluginsInstallParams({ source: "official", pluginId: "workboard", - acknowledgeInstallPolicyWarning: "yes", + installPolicyWarningAcknowledgement: true, }), ).toBe(false); }); diff --git a/packages/gateway-protocol/src/schema/plugins.ts b/packages/gateway-protocol/src/schema/plugins.ts index 6021c9a5dade..c657d635afed 100644 --- a/packages/gateway-protocol/src/schema/plugins.ts +++ b/packages/gateway-protocol/src/schema/plugins.ts @@ -173,12 +173,12 @@ export const PluginsInstallParamsSchema = Type.Union([ packageName: NonEmptyString, version: Type.Optional(NonEmptyString), acknowledgeClawHubRisk: Type.Optional(Type.Boolean()), - acknowledgeInstallPolicyWarning: Type.Optional(Type.Boolean()), + installPolicyWarningAcknowledgement: Type.Optional(NonEmptyString), }), closedObject({ source: Type.Literal("official"), pluginId: NonEmptyString, - acknowledgeInstallPolicyWarning: Type.Optional(Type.Boolean()), + installPolicyWarningAcknowledgement: Type.Optional(NonEmptyString), }), ]); diff --git a/src/cli/install-policy-warning-acknowledgement.test.ts b/src/cli/install-policy-warning-acknowledgement.test.ts index bb4dd4f6188b..d451ee5db84f 100644 --- a/src/cli/install-policy-warning-acknowledgement.test.ts +++ b/src/cli/install-policy-warning-acknowledgement.test.ts @@ -47,6 +47,12 @@ describe("resolveInstallPolicyWarningAcknowledgementCliOptions", () => { targetName: "demo\npkg", targetType: "plugin", requestMode: fixture.requestMode, + warning: { + targetName: "demo\npkg", + targetType: "plugin", + requestMode: fixture.requestMode, + reason: "Review required", + }, }), ).resolves.toEqual({ status: "approved" }); @@ -66,6 +72,12 @@ describe("resolveInstallPolicyWarningAcknowledgementCliOptions", () => { targetName: "demo", targetType: "skill", requestMode: "install", + warning: { + targetName: "demo", + targetType: "skill", + requestMode: "install", + reason: "Review required", + }, }), ).resolves.toEqual({ status: "declined" }); }); @@ -104,6 +116,12 @@ describe("resolveInstallPolicyWarningAcknowledgementCliOptions", () => { targetName: "demo", targetType: "plugin", requestMode: "install", + warning: { + targetName: "demo", + targetType: "plugin", + requestMode: "install", + reason: "Review required", + }, }), ).resolves.toEqual({ status: "approved" }); await expect( diff --git a/src/gateway/install-policy-warning-error-details.test.ts b/src/gateway/install-policy-warning-error-details.test.ts index ff1867a58c80..731616e3c38d 100644 --- a/src/gateway/install-policy-warning-error-details.test.ts +++ b/src/gateway/install-policy-warning-error-details.test.ts @@ -11,6 +11,7 @@ describe("install policy warning error details", () => { targetType: "plugin", requestMode: "install", reason: "Scanner found behavior that needs review", + acknowledgementToken: "approval-token", findings: [ { ruleId: "dynamic-eval", diff --git a/src/gateway/server-methods/plugins.test.ts b/src/gateway/server-methods/plugins.test.ts index 58dc235cc92a..99fa4596c1ad 100644 --- a/src/gateway/server-methods/plugins.test.ts +++ b/src/gateway/server-methods/plugins.test.ts @@ -3,6 +3,7 @@ import { expectDefined } from "@openclaw/normalization-core"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { InstallPolicyWarningDetails } from "../../plugins/install-security-scan.types.js"; +import type { ManagedPluginSourceInstallRequest } from "../../plugins/management-service.js"; const managementMocks = vi.hoisted(() => { class ManagedPluginLifecycleError extends Error { @@ -11,6 +12,7 @@ const managementMocks = vi.hoisted(() => { readonly version?: string; readonly warning?: string; readonly installPolicyWarning?: InstallPolicyWarningDetails; + readonly installPolicyResolvedRequest?: ManagedPluginSourceInstallRequest; constructor( message: string, @@ -20,6 +22,7 @@ const managementMocks = vi.hoisted(() => { version?: string; warning?: string; installPolicyWarning?: InstallPolicyWarningDetails; + installPolicyResolvedRequest?: ManagedPluginSourceInstallRequest; }, ) { super(message); @@ -28,6 +31,7 @@ const managementMocks = vi.hoisted(() => { this.version = details?.version; this.warning = details?.warning; this.installPolicyWarning = details?.installPolicyWarning; + this.installPolicyResolvedRequest = details?.installPolicyResolvedRequest; } } return { @@ -304,24 +308,18 @@ describe("plugin management Gateway handlers", () => { }); }); - it("forwards explicit install-policy warning acknowledgement", async () => { - managementMocks.install.mockResolvedValue({ - plugin: { ...workboard, id: "diffs", name: "Diffs", enabled: true, state: "enabled" }, - }); - - await callHandler("plugins.install", { + it("rejects an install-policy acknowledgement that the Gateway did not issue", async () => { + const result = await callHandler("plugins.install", { source: "official", pluginId: "diffs", - acknowledgeInstallPolicyWarning: true, + installPolicyWarningAcknowledgement: "not-issued", }); - expect(managementMocks.install).toHaveBeenCalledWith({ - request: { - source: "official", - pluginId: "diffs", - acknowledgeInstallPolicyWarning: true, - }, + expect(result.error).toMatchObject({ + code: "INVALID_REQUEST", + message: expect.stringContaining("does not match this plugin"), }); + expect(managementMocks.install).not.toHaveBeenCalled(); }); it("returns structured ClawHub acknowledgement details", async () => { @@ -354,6 +352,10 @@ describe("plugin management Gateway handlers", () => { it("returns structured install-policy warning details", async () => { managementMocks.install.mockRejectedValue( new managementMocks.ManagedPluginLifecycleError("Install requires approval", { + installPolicyResolvedRequest: { + source: "clawhub", + spec: "clawhub:community/plugin@1.0.0", + }, installPolicyWarning: { targetName: "demo-plugin", targetType: "plugin", @@ -397,6 +399,147 @@ describe("plugin management Gateway handlers", () => { ], }, }); + + const error = result.error as { details?: { acknowledgementToken?: unknown } }; + const acknowledgementToken = expectDefined( + error.details?.acknowledgementToken, + "expected install-policy acknowledgement token", + ); + expect(acknowledgementToken).toEqual(expect.any(String)); + + managementMocks.install.mockRejectedValueOnce( + new managementMocks.ManagedPluginLifecycleError("Warning changed", { + installPolicyResolvedRequest: { + source: "clawhub", + spec: "clawhub:community/plugin@1.0.0", + }, + installPolicyWarning: { + targetName: "demo-plugin", + targetType: "plugin", + requestMode: "install", + reason: "Scanner found a different issue", + }, + }), + ); + const changed = await callHandler("plugins.install", { + source: "clawhub", + packageName: "community/plugin", + installPolicyWarningAcknowledgement: acknowledgementToken, + }); + + expect(changed.error).toMatchObject({ + details: { reason: "Scanner found a different issue" }, + }); + expect(managementMocks.install).toHaveBeenLastCalledWith({ + request: { + source: "clawhub", + packageName: "community/plugin", + installPolicyWarningAcknowledgement: { + resolvedRequest: { + source: "clawhub", + spec: "clawhub:community/plugin@1.0.0", + }, + warning: { + targetName: "demo-plugin", + targetType: "plugin", + requestMode: "install", + reason: "Scanner found behavior that needs review", + findings: [ + { + ruleId: "dynamic-eval", + severity: "warn", + message: "Dynamic code execution", + file: "index.js", + line: 12, + }, + ], + }, + }, + }, + }); + + const changedError = changed.error as { details?: { acknowledgementToken?: unknown } }; + const changedAcknowledgementToken = expectDefined( + changedError.details?.acknowledgementToken, + "expected changed-warning acknowledgement token", + ); + expect(changedAcknowledgementToken).not.toBe(acknowledgementToken); + + managementMocks.install.mockResolvedValue({ + plugin: { ...workboard, id: "diffs", name: "Diffs", enabled: true, state: "enabled" }, + }); + const approved = await callHandler("plugins.install", { + source: "clawhub", + packageName: "community/plugin", + installPolicyWarningAcknowledgement: changedAcknowledgementToken, + }); + expect(approved.ok).toBe(true); + expect(managementMocks.install).toHaveBeenLastCalledWith({ + request: { + source: "clawhub", + packageName: "community/plugin", + installPolicyWarningAcknowledgement: { + resolvedRequest: { + source: "clawhub", + spec: "clawhub:community/plugin@1.0.0", + }, + warning: { + targetName: "demo-plugin", + targetType: "plugin", + requestMode: "install", + reason: "Scanner found a different issue", + }, + }, + }, + }); + + const replay = await callHandler("plugins.install", { + source: "clawhub", + packageName: "community/plugin", + installPolicyWarningAcknowledgement: changedAcknowledgementToken, + }); + expect(replay.error).toMatchObject({ code: "INVALID_REQUEST" }); + expect(managementMocks.install).toHaveBeenCalledTimes(3); + }); + + it("binds an install-policy acknowledgement to the request that received it", async () => { + managementMocks.install.mockRejectedValue( + new managementMocks.ManagedPluginLifecycleError("Install requires approval", { + installPolicyResolvedRequest: { + source: "official", + spec: "@openclaw/diffs@1.0.0", + pluginId: "diffs", + mode: "install", + }, + installPolicyWarning: { + targetName: "diffs", + targetType: "plugin", + requestMode: "install", + reason: "Review required", + }, + }), + ); + const warning = await callHandler("plugins.install", { + source: "official", + pluginId: "diffs", + }); + const error = warning.error as { details?: { acknowledgementToken?: unknown } }; + const acknowledgementToken = expectDefined( + error.details?.acknowledgementToken, + "expected install-policy acknowledgement token", + ); + + const mismatch = await callHandler("plugins.install", { + source: "official", + pluginId: "workboard", + installPolicyWarningAcknowledgement: acknowledgementToken, + }); + + expect(mismatch.error).toMatchObject({ + code: "INVALID_REQUEST", + message: expect.stringContaining("does not match this plugin"), + }); + expect(managementMocks.install).toHaveBeenCalledOnce(); }); it("classifies ClawHub security outages as unavailable", async () => { diff --git a/src/gateway/server-methods/plugins.ts b/src/gateway/server-methods/plugins.ts index 837b634bba8c..00fd1876e5dc 100644 --- a/src/gateway/server-methods/plugins.ts +++ b/src/gateway/server-methods/plugins.ts @@ -1,4 +1,5 @@ // Gateway control-plane handlers for cold plugin catalog and lifecycle operations. +import { randomUUID } from "node:crypto"; import { buildClawHubTrustErrorDetails, INSTALL_POLICY_WARNING_ACKNOWLEDGEMENT_REQUIRED, @@ -11,9 +12,11 @@ import { validatePluginsSearchParams, validatePluginsSetEnabledParams, validatePluginsUninstallParams, + type PluginsInstallParams, } from "../../../packages/gateway-protocol/src/index.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { searchInstallablePluginPackages } from "../../plugins/catalog-search.js"; +import type { InstallPolicyWarningDetails } from "../../plugins/install-security-scan.types.js"; import { formatManagedPluginLifecycleError, installManagedPlugin, @@ -21,6 +24,8 @@ import { ManagedPluginLifecycleError, setManagedPluginEnabled, uninstallManagedPlugin, + type ManagedPluginInstallRequest, + type ManagedPluginSourceInstallRequest, } from "../../plugins/management-service.js"; import { buildGatewayReloadPlan } from "../config-reload-plan.js"; import { resolveGatewayReloadSettings } from "../config-reload-settings.js"; @@ -28,6 +33,103 @@ import { readInstallPolicyWarningErrorDetails } from "../install-policy-warning- import type { GatewayRequestHandlers } from "./types.js"; import { assertValidParams } from "./validation.js"; +const INSTALL_POLICY_ACKNOWLEDGEMENT_TTL_MS = 5 * 60_000; +const MAX_INSTALL_POLICY_ACKNOWLEDGEMENTS = 256; + +type InstallPolicyAcknowledgement = { + expiresAt: number; + requestKey: string; + resolvedRequest: ManagedPluginSourceInstallRequest; + warning: InstallPolicyWarningDetails; +}; + +const installPolicyAcknowledgements = new Map(); + +function installPolicyRequestKey(request: PluginsInstallParams): string { + return request.source === "clawhub" + ? JSON.stringify({ + source: request.source, + packageName: request.packageName, + version: request.version ?? null, + acknowledgeClawHubRisk: request.acknowledgeClawHubRisk ?? false, + }) + : JSON.stringify({ source: request.source, pluginId: request.pluginId }); +} + +function pruneInstallPolicyAcknowledgements(now: number): void { + for (const [token, acknowledgement] of installPolicyAcknowledgements) { + if (acknowledgement.expiresAt <= now) { + installPolicyAcknowledgements.delete(token); + } + } + while (installPolicyAcknowledgements.size >= MAX_INSTALL_POLICY_ACKNOWLEDGEMENTS) { + const oldestToken = installPolicyAcknowledgements.keys().next().value; + if (typeof oldestToken !== "string") { + break; + } + installPolicyAcknowledgements.delete(oldestToken); + } +} + +function issueInstallPolicyAcknowledgement(params: { + request: PluginsInstallParams; + resolvedRequest: ManagedPluginSourceInstallRequest; + warning: InstallPolicyWarningDetails; +}): string { + const now = Date.now(); + pruneInstallPolicyAcknowledgements(now); + const token = randomUUID(); + installPolicyAcknowledgements.set(token, { + expiresAt: now + INSTALL_POLICY_ACKNOWLEDGEMENT_TTL_MS, + requestKey: installPolicyRequestKey(params.request), + resolvedRequest: params.resolvedRequest, + warning: params.warning, + }); + return token; +} + +function consumeInstallPolicyAcknowledgement( + request: PluginsInstallParams, +): Pick | undefined { + const token = request.installPolicyWarningAcknowledgement; + if (!token) { + return undefined; + } + const acknowledgement = installPolicyAcknowledgements.get(token); + installPolicyAcknowledgements.delete(token); + if ( + !acknowledgement || + acknowledgement.expiresAt <= Date.now() || + acknowledgement.requestKey !== installPolicyRequestKey(request) + ) { + throw new ManagedPluginLifecycleError( + "Install policy approval expired or does not match this plugin. Review the current warning and try again.", + ); + } + return { + resolvedRequest: acknowledgement.resolvedRequest, + warning: acknowledgement.warning, + }; +} + +function managedInstallRequest(params: PluginsInstallParams): ManagedPluginInstallRequest { + const installPolicyWarningAcknowledgement = consumeInstallPolicyAcknowledgement(params); + if (params.source === "clawhub") { + return { + source: params.source, + packageName: params.packageName, + ...(params.version ? { version: params.version } : {}), + ...(params.acknowledgeClawHubRisk ? { acknowledgeClawHubRisk: true } : {}), + ...(installPolicyWarningAcknowledgement ? { installPolicyWarningAcknowledgement } : {}), + }; + } + return { + source: params.source, + pluginId: params.pluginId, + ...(installPolicyWarningAcknowledgement ? { installPolicyWarningAcknowledgement } : {}), + }; +} + function pluginPolicyRestartRequired(params: { config: OpenClawConfig; changedPaths: readonly string[]; @@ -120,7 +222,7 @@ export const pluginsHandlers: GatewayRequestHandlers = { return; } try { - const result = await installManagedPlugin({ request: params }); + const result = await installManagedPlugin({ request: managedInstallRequest(params) }); respond( true, { @@ -144,12 +246,18 @@ export const pluginsHandlers: GatewayRequestHandlers = { ...(lifecycleError.warning ? { warning: lifecycleError.warning } : {}), }) : undefined; - const installPolicyDetails = lifecycleError?.installPolicyWarning - ? readInstallPolicyWarningErrorDetails({ - installPolicyCode: INSTALL_POLICY_WARNING_ACKNOWLEDGEMENT_REQUIRED, - ...lifecycleError.installPolicyWarning, - }) - : undefined; + const installPolicyDetails = + lifecycleError?.installPolicyWarning && lifecycleError.installPolicyResolvedRequest + ? readInstallPolicyWarningErrorDetails({ + installPolicyCode: INSTALL_POLICY_WARNING_ACKNOWLEDGEMENT_REQUIRED, + ...lifecycleError.installPolicyWarning, + acknowledgementToken: issueInstallPolicyAcknowledgement({ + request: params, + resolvedRequest: lifecycleError.installPolicyResolvedRequest, + warning: lifecycleError.installPolicyWarning, + }), + }) + : undefined; const details = installPolicyDetails ?? trustDetails; respond( false, diff --git a/src/plugins/clawhub.test.ts b/src/plugins/clawhub.test.ts index e217de653f07..a6365b03eb9e 100644 --- a/src/plugins/clawhub.test.ts +++ b/src/plugins/clawhub.test.ts @@ -284,7 +284,6 @@ type ArchiveInstallCall = { requestedSpecifier?: string; source?: { kind?: string; authority?: string; mutable?: boolean; network?: boolean }; }; - onInstallPolicyWarning?: () => Promise; trustedSourceLinkedOfficialInstall?: boolean; }; diff --git a/src/plugins/clawhub.ts b/src/plugins/clawhub.ts index da14de30fc50..ed1c693694c8 100644 --- a/src/plugins/clawhub.ts +++ b/src/plugins/clawhub.ts @@ -1466,7 +1466,7 @@ export async function installPluginFromClawHub( }, }); if (!installResult.ok) { - return installResult; + return { ...installResult, version: versionState.version }; } const pkg = detail.package!; diff --git a/src/plugins/install-security-scan.runtime.test.ts b/src/plugins/install-security-scan.runtime.test.ts index 9f1a6ad20a36..e3e4bf2d31e3 100644 --- a/src/plugins/install-security-scan.runtime.test.ts +++ b/src/plugins/install-security-scan.runtime.test.ts @@ -344,6 +344,12 @@ describe("legacy file install scan compatibility", () => { targetName: "payload", targetType: "plugin", requestMode: "install", + warning: { + targetName: "payload", + targetType: "plugin", + requestMode: "install", + reason: "review this plugin", + }, }); expect(onInstallPolicyWarning).toHaveBeenCalledTimes(1); expect(runInstallPolicyMock).toHaveBeenCalledTimes(2); diff --git a/src/plugins/install-security-scan.runtime.ts b/src/plugins/install-security-scan.runtime.ts index 6092cac0b817..2506d0156ff5 100644 --- a/src/plugins/install-security-scan.runtime.ts +++ b/src/plugins/install-security-scan.runtime.ts @@ -853,6 +853,7 @@ async function runOperatorInstallPolicy(params: { targetName: params.targetName, targetType: params.targetType, requestMode: params.requestMode, + warning: installPolicyWarning, }); if (acknowledgement.status === "approved") { const reevaluated = await evaluatePolicy(); diff --git a/src/plugins/install-security-scan.types.ts b/src/plugins/install-security-scan.types.ts index a447239f7ddf..4480d3baef05 100644 --- a/src/plugins/install-security-scan.types.ts +++ b/src/plugins/install-security-scan.types.ts @@ -14,6 +14,7 @@ export type InstallPolicyWarningAcknowledgementRequest = { targetName: string; targetType: "skill" | "plugin"; requestMode: "install" | "update"; + warning: InstallPolicyWarningDetails; }; type InstallPolicyWarningAcknowledgementResult = diff --git a/src/plugins/management-service.test.ts b/src/plugins/management-service.test.ts index 0e0974063590..c213319404a3 100644 --- a/src/plugins/management-service.test.ts +++ b/src/plugins/management-service.test.ts @@ -1,6 +1,9 @@ import { expectDefined } from "@openclaw/normalization-core"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { expectOneShotInstallPolicyWarningAcknowledgement } from "./test-helpers/install-policy-warning.js"; +import { + expectOneShotInstallPolicyWarningAcknowledgement, + officialDiffsWarningRequest, +} from "./test-helpers/install-policy-warning.js"; const mocks = vi.hoisted(() => ({ applyUninstall: vi.fn(), @@ -785,7 +788,7 @@ describe("plugin management service", () => { ); await installManagedPlugin({ - request: { source: "official", pluginId: "diffs", acknowledgeInstallPolicyWarning: true }, + request: officialDiffsWarningRequest, env: {}, }); @@ -794,7 +797,6 @@ describe("plugin management service", () => { spec: "clawhub:@openclaw/diffs@2026.6.11", expectedPluginId: "diffs", expectedIntegrity: `sha256-${Buffer.from("a".repeat(64), "hex").toString("base64")}`, - onInstallPolicyWarning: expect.any(Function), }), ); await expectOneShotInstallPolicyWarningAcknowledgement(mocks.clawhubInstall); diff --git a/src/plugins/management-service.ts b/src/plugins/management-service.ts index 9c5d38676b30..faea905b0767 100644 --- a/src/plugins/management-service.ts +++ b/src/plugins/management-service.ts @@ -1,5 +1,6 @@ // Structured plugin catalog and lifecycle operations shared by Gateway-facing surfaces. import path from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { asSafeIntegerInRange } from "@openclaw/normalization-core/number-coercion"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { uniqueStrings } from "@openclaw/normalization-core/string-normalization"; @@ -117,18 +118,24 @@ type ManagedPluginCatalog = { mutationAllowed: boolean; }; -type ManagedPluginInstallRequest = +export type ManagedPluginInstallRequest = | { source: "clawhub"; packageName: string; version?: string; acknowledgeClawHubRisk?: boolean; - acknowledgeInstallPolicyWarning?: boolean; + installPolicyWarningAcknowledgement?: { + warning: InstallPolicyWarningDetails; + resolvedRequest: ManagedPluginSourceInstallRequest; + }; } | { source: "official"; pluginId: string; - acknowledgeInstallPolicyWarning?: boolean; + installPolicyWarningAcknowledgement?: { + warning: InstallPolicyWarningDetails; + resolvedRequest: ManagedPluginSourceInstallRequest; + }; }; export type ManagedPluginSourceInstallRequest = @@ -223,6 +230,7 @@ export class ManagedPluginLifecycleError extends Error { readonly version?: string; readonly warning?: string; readonly installPolicyWarning?: InstallPolicyWarningDetails; + readonly installPolicyResolvedRequest?: ManagedPluginSourceInstallRequest; constructor( message: string, @@ -232,6 +240,7 @@ export class ManagedPluginLifecycleError extends Error { version?: string; warning?: string; installPolicyWarning?: InstallPolicyWarningDetails; + installPolicyResolvedRequest?: ManagedPluginSourceInstallRequest; cause?: unknown; }, ) { @@ -242,6 +251,7 @@ export class ManagedPluginLifecycleError extends Error { this.version = details?.version; this.warning = details?.warning; this.installPolicyWarning = details?.installPolicyWarning; + this.installPolicyResolvedRequest = details?.installPolicyResolvedRequest; } } @@ -990,13 +1000,27 @@ function buildClawHubSpec(packageName: string, version?: string): string { return `clawhub:${packageName}${version ? `@${version}` : ""}`; } -function throwInstallFailure(result: { - error: string; - code?: string; - version?: string; - warning?: string; - installPolicyWarning?: InstallPolicyWarningDetails; -}): never { +function pinInstallPolicyResolvedRequest( + request: ManagedPluginSourceInstallRequest, + version: string | undefined, +): ManagedPluginSourceInstallRequest { + if (request.source !== "clawhub" || !version) { + return request; + } + const parsed = parseClawHubPluginSpec(request.spec); + return parsed ? { ...request, spec: buildClawHubSpec(parsed.name, version) } : request; +} + +function throwInstallFailure( + result: { + error: string; + code?: string; + version?: string; + warning?: string; + installPolicyWarning?: InstallPolicyWarningDetails; + }, + resolvedRequest?: ManagedPluginSourceInstallRequest, +): never { const unavailable = !result.code || result.code === CLAWHUB_INSTALL_ERROR_CODE.ARTIFACT_UNAVAILABLE || @@ -1008,6 +1032,14 @@ function throwInstallFailure(result: { version: result.version, warning: result.warning, installPolicyWarning: result.installPolicyWarning, + ...(result.installPolicyWarning && resolvedRequest + ? { + installPolicyResolvedRequest: pinInstallPolicyResolvedRequest( + resolvedRequest, + result.version, + ), + } + : {}), cause: result, }); } @@ -1085,6 +1117,7 @@ function throwPersistenceFailureWithCleanupWarnings(error: unknown, warnings: st version: error.version, warning: [error.warning, cleanupWarning].filter(Boolean).join("\n"), installPolicyWarning: error.installPolicyWarning, + installPolicyResolvedRequest: error.installPolicyResolvedRequest, cause: error, }); } @@ -1435,10 +1468,11 @@ export async function installManagedPlugin(params: { const warnings: string[] = []; const installLogger = createInstallLogger(warnings); let installPolicyWarningAcknowledgementAvailable = Boolean( - params.request.acknowledgeInstallPolicyWarning, + params.request.installPolicyWarningAcknowledgement, ); const request = - params.request.source === "clawhub" + params.request.installPolicyWarningAcknowledgement?.resolvedRequest ?? + (params.request.source === "clawhub" ? resolveManagedClawHubInstallRequest({ request: params.request, officialEntries: officialCatalog.entries, @@ -1446,22 +1480,25 @@ export async function installManagedPlugin(params: { : resolveManagedOfficialInstallRequest({ request: params.request, officialEntries: officialCatalog.entries, - }); + })); const installed = await installManagedPluginSource({ request, snapshot, env, logger: installLogger, persistenceLogger: installLogger, - ...(params.request.acknowledgeInstallPolicyWarning + ...(params.request.installPolicyWarningAcknowledgement ? { safetyOverrides: { - onInstallPolicyWarning: async () => { + onInstallPolicyWarning: async ({ warning }) => { if (!installPolicyWarningAcknowledgementAvailable) { return false; } installPolicyWarningAcknowledgementAvailable = false; - return true; + return isDeepStrictEqual( + warning, + params.request.installPolicyWarningAcknowledgement?.warning, + ); }, }, } @@ -1471,7 +1508,7 @@ export async function installManagedPlugin(params: { runtime: createSilentRuntime(), }); if (!installed.ok) { - return throwInstallFailure(installed); + return throwInstallFailure(installed, request); } const catalog = await listManagedPlugins({ config: installed.config, diff --git a/src/plugins/test-helpers/install-policy-warning.ts b/src/plugins/test-helpers/install-policy-warning.ts index cbb6cea00de3..b509e28c0613 100644 --- a/src/plugins/test-helpers/install-policy-warning.ts +++ b/src/plugins/test-helpers/install-policy-warning.ts @@ -8,6 +8,25 @@ type InstallPolicyWarningCall = { ) => Promise; }; +export const officialDiffsWarningRequest = { + source: "official", + pluginId: "diffs", + installPolicyWarningAcknowledgement: { + resolvedRequest: { + source: "clawhub", + spec: "clawhub:@openclaw/diffs@2026.6.11", + expectedPluginId: "diffs", + expectedIntegrity: `sha256-${Buffer.from("a".repeat(64), "hex").toString("base64")}`, + }, + warning: { + targetName: "diffs", + targetType: "plugin", + requestMode: "install", + reason: "Review this warning", + }, + }, +} as const; + export async function expectOneShotInstallPolicyWarningAcknowledgement(mock: { mock: { calls: unknown[][] }; }): Promise { @@ -21,6 +40,12 @@ export async function expectOneShotInstallPolicyWarningAcknowledgement(mock: { targetName: "diffs", targetType: "plugin", requestMode: "install", + warning: { + targetName: "diffs", + targetType: "plugin", + requestMode: "install", + reason: "Review this warning", + }, }; expect(await acknowledge(request)).toBe(true); expect(await acknowledge(request)).toBe(false);