ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-24 03:15:46 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): fingerprint full policy warnings

Browse Source
This commit is contained in:
jesse-merhi
2026-08-11 04:18:35 +10:00
parent 54368acb05
commit e5bb6e955b
4 changed files with 135 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/plugins/install-security-scan.runtime.test.ts
+49 -9
View File
@@ -325,8 +325,12 @@ describe("legacy file install scan compatibility", () => {
it("continues after one acknowledgement and a fresh evaluation of the same warning", async () => {
const onInstallPolicyWarning = vi.fn().mockResolvedValue({ status: "approved" });
runInstallPolicyMock
.mockResolvedValueOnce({ warning: { reason: "review this plugin" } })
.mockResolvedValueOnce({ warning: { reason: "review this plugin" } });
.mockResolvedValueOnce({
warning: { reason: "review this plugin", fingerprint: "warning-a" },
})
.mockResolvedValueOnce({
warning: { reason: "review this plugin", fingerprint: "warning-a" },
});
const result = await scanFileInstallSourceRuntime({
filePath: "/tmp/payload.js",
@@ -347,9 +351,11 @@ describe("legacy file install scan compatibility", () => {
it("requires approval again when policy re-evaluation returns a changed warning", async () => {
const onInstallPolicyWarning = vi.fn().mockResolvedValue({ status: "approved" });
runInstallPolicyMock
.mockResolvedValueOnce({ warning: { reason: "review this plugin" } })
.mockResolvedValueOnce({
warning: { reason: "review the new finding" },
warning: { reason: "review this plugin", fingerprint: "warning-a" },
})
.mockResolvedValueOnce({
warning: { reason: "review the new finding", fingerprint: "warning-b" },
findings: [
{
ruleId: "changed-warning",
@@ -376,9 +382,41 @@ describe("legacy file install scan compatibility", () => {
expect(runInstallPolicyMock).toHaveBeenCalledTimes(2);
});
it("requires approval again when the full warning changes behind identical display values", async () => {
const onInstallPolicyWarning = vi.fn().mockResolvedValue({ status: "approved" });
const displayedWarning = {
warning: { reason: "same bounded reason", fingerprint: "full-warning-a" },
findings: [
{
ruleId: "same-bounded-finding",
severity: "warn" as const,
message: "same bounded message",
},
],
};
runInstallPolicyMock.mockResolvedValueOnce(displayedWarning).mockResolvedValueOnce({
...displayedWarning,
warning: { ...displayedWarning.warning, fingerprint: "full-warning-b" },
});
const result = await scanFileInstallSourceRuntime({
filePath: "/tmp/payload.js",
logger: {},
onInstallPolicyWarning,
pluginId: "payload",
});
expect(result?.blocked?.reason).toContain("The policy warning changed after approval.");
expect(result?.blocked?.reason).toContain("same bounded reason");
expect(onInstallPolicyWarning).toHaveBeenCalledTimes(1);
expect(runInstallPolicyMock).toHaveBeenCalledTimes(2);
});
it("keeps a block from policy re-evaluation terminal", async () => {
runInstallPolicyMock
.mockResolvedValueOnce({ warning: { reason: "review this plugin" } })
.mockResolvedValueOnce({
warning: { reason: "review this plugin", fingerprint: "warning-a" },
})
.mockResolvedValueOnce({
blocked: { code: "security_scan_blocked", reason: "now blocked" },
});
@@ -395,7 +433,7 @@ describe("legacy file install scan compatibility", () => {
it("keeps the deprecated unsafe flag inert when policy warns", async () => {
runInstallPolicyMock.mockResolvedValue({
warning: { reason: "review this plugin" },
warning: { reason: "review this plugin", fingerprint: "warning-a" },
});
const result = await scanFileInstallSourceRuntime({
@@ -424,7 +462,9 @@ describe("legacy file install scan compatibility", () => {
it("keeps a block from acknowledged policy re-evaluation terminal", async () => {
runInstallPolicyMock
.mockResolvedValueOnce({ warning: { reason: "review this plugin" } })
.mockResolvedValueOnce({
warning: { reason: "review this plugin", fingerprint: "warning-a" },
})
.mockResolvedValueOnce({
blocked: { code: "security_scan_blocked", reason: "now blocked" },
});
@@ -442,7 +482,7 @@ describe("legacy file install scan compatibility", () => {
it("distinguishes an exhausted noninteractive approval from cancellation", async () => {
runInstallPolicyMock.mockResolvedValue({
warning: { reason: "review the dependency warning" },
warning: { reason: "review the dependency warning", fingerprint: "warning-a" },
findings: [
{
ruleId: "dependency-warning",
@@ -473,7 +513,7 @@ describe("legacy file install scan compatibility", () => {
it("renders warning details as one readable review notice", async () => {
const warnings: string[] = [];
runInstallPolicyMock.mockResolvedValue({
warning: { reason: "review this plugin" },
warning: { reason: "review this plugin", fingerprint: "warning-a" },
findings: [{ ruleId: "context", severity: "info", message: "Informational context." }],
});
src/plugins/install-security-scan.runtime.ts
+1 -4
View File
@@ -1,7 +1,6 @@
// Runtime bridge for plugin install security scanning.
import fs from "node:fs/promises";
import path from "node:path";
import { isDeepStrictEqual } from "node:util";
import { sanitizeTerminalText } from "../../packages/terminal-core/src/safe-text.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { formatErrorMessage } from "../infra/errors.js";
@@ -776,9 +775,7 @@ async function runOperatorInstallPolicy(params: {
});
}
if (reevaluated?.warning) {
const warningUnchanged =
reevaluated.warning.reason === result.warning.reason &&
isDeepStrictEqual(reevaluated.findings ?? [], result.findings ?? []);
const warningUnchanged = reevaluated.warning.fingerprint === result.warning.fingerprint;
if (!warningUnchanged) {
return {
blocked: {
src/security/install-policy.test.ts
+51 -1
View File
@@ -435,7 +435,10 @@ describe("runInstallPolicy", () => {
});
expect(result).toEqual({
warning: { reason: "review this source" },
warning: {
reason: "review this source",
fingerprint: expect.stringMatching(/^[a-f0-9]{64}$/),
},
findings: [
{
ruleId: "manual-review",
@@ -447,6 +450,53 @@ describe("runInstallPolicy", () => {
expect(debugLogs.filter((message) => message.endsWith(": warned"))).toHaveLength(1);
});
it("fingerprints warning reason changes beyond the display limit", async () => {
const sharedPrefix = "r".repeat(1000);
const runWarning = async (reason: string) =>
await runInstallPolicy({
config: configWithPolicy(scriptPath, {
POLICY_RESPONSE: JSON.stringify({ protocolVersion: 1, decision: "warn", reason }),
}),
request: baseRequest(sourceDir),
});
const first = await runWarning(`${sharedPrefix}-first`);
const second = await runWarning(`${sharedPrefix}-second`);
expect(first?.warning?.reason).toBe(second?.warning?.reason);
expect(first?.warning?.fingerprint).not.toBe(second?.warning?.fingerprint);
});
it("fingerprints warning findings beyond the display limit", async () => {
const visibleFindings = Array.from({ length: 100 }, (_, index) => ({
ruleId: `visible-${String(index)}`,
severity: "warn",
message: `Visible finding ${String(index)}`,
}));
const runWarning = async (extraMessage: string) =>
await runInstallPolicy({
config: configWithPolicy(scriptPath, {
POLICY_RESPONSE: JSON.stringify({
protocolVersion: 1,
decision: "warn",
reason: "review all findings",
findings: [
...visibleFindings,
{ ruleId: "hidden-100", severity: "warn", message: extraMessage },
],
}),
}),
request: baseRequest(sourceDir),
});
const first = await runWarning("First hidden finding");
const second = await runWarning("Second hidden finding");
expect(first?.findings).toEqual(second?.findings);
expect(first?.findings).toHaveLength(100);
expect(first?.warning?.fingerprint).not.toBe(second?.warning?.fingerprint);
});
it.each([
{ label: "missing", reason: undefined },
{ label: "empty", reason: " " },
src/security/install-policy.ts
+34 -15
View File
@@ -1,3 +1,4 @@
import { createHash } from "node:crypto";
// Checks install policy constraints for package and plugin operations.
import fs from "node:fs/promises";
import path from "node:path";
@@ -122,7 +123,7 @@ type InstallPolicyResult =
| { blocked?: undefined; warning?: undefined; findings?: InstallPolicyFinding[] }
| {
blocked?: undefined;
warning: { reason: string };
warning: { reason: string; fingerprint: string };
findings?: InstallPolicyFinding[];
}
| {
@@ -359,11 +360,7 @@ const installPolicyResponseEnvelopeSchema = z.object({
const installPolicyReasonSchema = z.string().trim().min(1);
const findingTextSchema = z
.string()
.trim()
.min(1)
.transform((value) => truncateText(value, MAX_FINDING_TEXT_CHARS));
const findingTextSchema = z.string().trim().min(1);
const optionalFindingTextSchema = findingTextSchema.optional().catch(undefined);
@@ -421,13 +418,6 @@ function blockedByPolicy(reason: string, findings?: InstallPolicyFinding[]): Ins
};
}
function warnedByPolicy(reason: string, findings?: InstallPolicyFinding[]): InstallPolicyResult {
return {
warning: { reason: truncateText(reason, MAX_REASON_CHARS) },
...(findings?.length ? { findings } : {}),
};
}
function isTargetEnabled(params: {
policy: NonNullable<SecurityConfig["installPolicy"]>;
targetType: InstallPolicyTarget;
@@ -527,6 +517,25 @@ function normalizeFinding(value: unknown): InstallPolicyFinding | null {
return parsed.success ? parsed.data : null;
}
function truncateFinding(finding: InstallPolicyFinding): InstallPolicyFinding {
return {
ruleId: truncateText(finding.ruleId, MAX_FINDING_TEXT_CHARS),
severity: finding.severity,
message: truncateText(finding.message, MAX_FINDING_TEXT_CHARS),
...(finding.file ? { file: truncateText(finding.file, MAX_FINDING_TEXT_CHARS) } : {}),
...(finding.line !== undefined ? { line: finding.line } : {}),
...(finding.evidence
? { evidence: truncateText(finding.evidence, MAX_FINDING_TEXT_CHARS) }
: {}),
};
}
function fingerprintWarning(reason: string, findings: InstallPolicyFinding[]): string {
// Presentation is truncated and capped; approval must bind the complete
// validated warning so hidden suffix or overflow changes still fail closed.
return createHash("sha256").update(JSON.stringify({ reason, findings })).digest("hex");
}
function formatPolicyResponseEnvelopeError(error: z.ZodError): string {
const invalidPath = error.issues[0]?.path[0];
return invalidPath === undefined
@@ -552,10 +561,14 @@ function parsePolicyResponse(stdout: string): InstallPolicyResult {
if (!response.success) {
return blockedByFailure(formatPolicyResponseEnvelopeError(response.error));
}
const fullFindings = (response.data.findings ?? [])
.map(normalizeFinding)
.filter((finding): finding is InstallPolicyFinding => finding !== null);
const normalizedFindings = (response.data.findings ?? [])
.slice(0, MAX_FINDINGS)
.map(normalizeFinding)
.filter((finding): finding is InstallPolicyFinding => finding !== null);
.filter((finding): finding is InstallPolicyFinding => finding !== null)
.map(truncateFinding);
if (response.data.decision === "allow") {
return normalizedFindings.length > 0 ? { findings: normalizedFindings } : {};
}
@@ -566,7 +579,13 @@ function parsePolicyResponse(stdout: string): InstallPolicyResult {
);
}
if (response.data.decision === "warn") {
return warnedByPolicy(reason.data, normalizedFindings);
return {
warning: {
reason: truncateText(reason.data, MAX_REASON_CHARS),
fingerprint: fingerprintWarning(reason.data, fullFindings),
},
...(normalizedFindings.length > 0 ? { findings: normalizedFindings } : {}),
};
}
return blockedByPolicy(reason.data, normalizedFindings);
}