From 254db642956f25eb40b11a62e853fc1e5b647f9f Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Tue, 11 Aug 2026 03:25:28 +1000 Subject: [PATCH] fix(gateway): preserve unapproved policy warnings --- ...all-policy-warning-acknowledgement.test.ts | 45 +++++++++---------- .../install-security-scan.runtime.test.ts | 13 ++++++ src/plugins/install-security-scan.runtime.ts | 12 +++-- src/plugins/install-security-scan.types.ts | 2 +- .../management-service-install-policy.test.ts | 35 +++++++++++---- src/plugins/management-service.ts | 4 +- .../test-helpers/install-policy-warning.ts | 10 +++-- 7 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/cli/install-policy-warning-acknowledgement.test.ts b/src/cli/install-policy-warning-acknowledgement.test.ts index c28e5f9f5959..b059a8e3c695 100644 --- a/src/cli/install-policy-warning-acknowledgement.test.ts +++ b/src/cli/install-policy-warning-acknowledgement.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import type { InstallPolicyWarningAcknowledgementRequest } from "../plugins/install-security-scan.types.js"; import { resolveInstallPolicyWarningAcknowledgementCliOptions } from "./install-policy-warning-acknowledgement.ts"; const promptTextMock = vi.hoisted(() => vi.fn()); @@ -120,32 +121,30 @@ describe("resolveInstallPolicyWarningAcknowledgementCliOptions", () => { const options = resolveInstallPolicyWarningAcknowledgementCliOptions({ acknowledgeInstallPolicyWarning: true, }); - - await expect( - options.onInstallPolicyWarning?.({ + const warningRequest: InstallPolicyWarningAcknowledgementRequest = { + targetName: "demo", + targetType: "plugin", + requestMode: "install", + scan: { + requestKind: "plugin-npm", + originType: "plugin-npm", + pluginContentType: "package", + }, + warning: { targetName: "demo", targetType: "plugin", requestMode: "install", - scan: { - requestKind: "plugin-npm", - originType: "plugin-npm", - pluginContentType: "package", - }, - warning: { - targetName: "demo", - targetType: "plugin", - requestMode: "install", - reason: "Review required", - }, - }), - ).resolves.toEqual({ status: "approved" }); - await expect( - options.onInstallPolicyWarning?.({ - targetName: "demo-dependency", - targetType: "plugin", - requestMode: "install", - }), - ).resolves.toEqual({ status: "unavailable", reason: "approval-exhausted" }); + reason: "Review required", + }, + }; + + await expect(options.onInstallPolicyWarning?.(warningRequest)).resolves.toEqual({ + status: "approved", + }); + await expect(options.onInstallPolicyWarning?.(warningRequest)).resolves.toEqual({ + status: "unavailable", + reason: "approval-exhausted", + }); expect(promptTextMock).not.toHaveBeenCalled(); }); }); diff --git a/src/plugins/install-security-scan.runtime.test.ts b/src/plugins/install-security-scan.runtime.test.ts index bc8ec38d00ac..9efa9eba6800 100644 --- a/src/plugins/install-security-scan.runtime.test.ts +++ b/src/plugins/install-security-scan.runtime.test.ts @@ -568,6 +568,19 @@ describe("legacy file install scan compatibility", () => { expect(result?.blocked?.reason).toContain("The noninteractive approval was already used."); expect(result?.blocked?.reason).toContain("Review this warning and rerun interactively."); expect(result?.blocked?.reason).not.toContain("Install cancelled"); + expect(result?.blocked?.installPolicyWarning).toMatchObject({ + scan: { + requestKind: "plugin-file", + originType: "plugin-file", + pluginContentType: "file", + }, + warning: { + targetName: "payload", + targetType: "plugin", + requestMode: "install", + reason: "review the dependency warning", + }, + }); expect(runInstallPolicyMock).toHaveBeenCalledTimes(1); }); diff --git a/src/plugins/install-security-scan.runtime.ts b/src/plugins/install-security-scan.runtime.ts index c2dd31125398..19b156361fe5 100644 --- a/src/plugins/install-security-scan.runtime.ts +++ b/src/plugins/install-security-scan.runtime.ts @@ -924,13 +924,17 @@ async function runOperatorInstallPolicy(params: { return { blocked: { code: "security_scan_blocked", + installPolicyWarning: warningOccurrence, reason: formatInstallPolicyNotice({ decision: "warn", findings: result.findings, - guidance: [ - "The noninteractive approval was already used.", - "Review this warning and rerun interactively.", - ], + guidance: + acknowledgement.reason === "approval-exhausted" + ? [ + "The noninteractive approval was already used.", + "Review this warning and rerun interactively.", + ] + : ["This warning has not been approved.", "Review it and try again."], reason: result.warning.reason, targetName: params.targetName, targetType: params.targetType, diff --git a/src/plugins/install-security-scan.types.ts b/src/plugins/install-security-scan.types.ts index d79a9af0239b..ad313284e862 100644 --- a/src/plugins/install-security-scan.types.ts +++ b/src/plugins/install-security-scan.types.ts @@ -34,7 +34,7 @@ type InstallPolicyWarningAcknowledgementResult = | { status: "declined" } | { status: "unavailable"; - reason: "approval-exhausted"; + reason: "approval-exhausted" | "warning-not-approved"; }; /** Overrides that intentionally loosen install safety policy for trusted/operator paths. */ diff --git a/src/plugins/management-service-install-policy.test.ts b/src/plugins/management-service-install-policy.test.ts index 5f76952eb4c4..e6eb3311588b 100644 --- a/src/plugins/management-service-install-policy.test.ts +++ b/src/plugins/management-service-install-policy.test.ts @@ -1,6 +1,10 @@ import { expectDefined } from "@openclaw/normalization-core"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { InstallPolicyWarningOccurrence } from "./install-security-scan.types.js"; +import type { + InstallPolicyWarningAcknowledgementRequest, + InstallPolicyWarningAcknowledgementResult, + InstallPolicyWarningOccurrence, +} from "./install-security-scan.types.js"; import { expectOneShotInstallPolicyWarningAcknowledgement, officialDiffsWarningRequest, @@ -239,17 +243,32 @@ describe("plugin management install-policy acknowledgements", () => { const acknowledge = expectDefined( ( call[0] as { - onInstallPolicyWarning?: (request: { - scan: typeof packageWarning.scan; - warning: typeof packageWarning.warning; - }) => Promise; + onInstallPolicyWarning?: ( + request: InstallPolicyWarningAcknowledgementRequest, + ) => Promise; } ).onInstallPolicyWarning, "install-policy acknowledgement callback", ); - expect(await acknowledge(dependencyWarning)).toBe(false); - expect(await acknowledge(packageWarning)).toBe(true); - expect(await acknowledge(packageWarning)).toBe(false); + expect( + await acknowledge({ + targetName: dependencyWarning.warning.targetName, + targetType: dependencyWarning.warning.targetType, + requestMode: dependencyWarning.warning.requestMode, + ...dependencyWarning, + }), + ).toEqual({ status: "unavailable", reason: "warning-not-approved" }); + const packageRequest: InstallPolicyWarningAcknowledgementRequest = { + targetName: packageWarning.warning.targetName, + targetType: packageWarning.warning.targetType, + requestMode: packageWarning.warning.requestMode, + ...packageWarning, + }; + expect(await acknowledge(packageRequest)).toEqual({ status: "approved" }); + expect(await acknowledge(packageRequest)).toEqual({ + status: "unavailable", + reason: "warning-not-approved", + }); }); it("pins reviewed npm warnings to the first resolved version and integrity", async () => { diff --git a/src/plugins/management-service.ts b/src/plugins/management-service.ts index 45806eef9a66..c204b8ff7214 100644 --- a/src/plugins/management-service.ts +++ b/src/plugins/management-service.ts @@ -1540,13 +1540,13 @@ export async function installManagedPlugin(params: { isDeepStrictEqual(warning, approved.warning), ); if (warningIndex < 0) { - return false; + return { status: "unavailable", reason: "warning-not-approved" }; } const [approved] = remainingInstallPolicyWarnings.splice(warningIndex, 1); if (approved) { acknowledgedInstallPolicyWarnings.push(approved); } - return true; + return { status: "approved" }; }, }, } diff --git a/src/plugins/test-helpers/install-policy-warning.ts b/src/plugins/test-helpers/install-policy-warning.ts index c0f885be1218..a2224fcabc35 100644 --- a/src/plugins/test-helpers/install-policy-warning.ts +++ b/src/plugins/test-helpers/install-policy-warning.ts @@ -2,6 +2,7 @@ import { expectDefined } from "@openclaw/normalization-core"; import { expect } from "vitest"; import type { InstallPolicyWarningAcknowledgementRequest, + InstallPolicyWarningAcknowledgementResult, InstallPolicyWarningOccurrence, } from "../install-security-scan.types.js"; @@ -22,7 +23,7 @@ export const officialDiffsWarningOccurrence: InstallPolicyWarningOccurrence = { type InstallPolicyWarningCall = { onInstallPolicyWarning?: ( request: InstallPolicyWarningAcknowledgementRequest, - ) => Promise; + ) => Promise; }; export const officialDiffsWarningRequest = { @@ -54,6 +55,9 @@ export async function expectOneShotInstallPolicyWarningAcknowledgement(mock: { requestMode: "install", ...officialDiffsWarningOccurrence, }; - expect(await acknowledge(request)).toBe(true); - expect(await acknowledge(request)).toBe(false); + expect(await acknowledge(request)).toEqual({ status: "approved" }); + expect(await acknowledge(request)).toEqual({ + status: "unavailable", + reason: "warning-not-approved", + }); }