diff --git a/extensions/policy/src/cli.test.ts b/extensions/policy/src/cli.test.ts index 8b1b999fed30..eb9e68e5336c 100644 --- a/extensions/policy/src/cli.test.ts +++ b/extensions/policy/src/cli.test.ts @@ -1,81 +1,95 @@ // Policy tests cover cli plugin behavior. import { promises as fs } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { isAbsolute, join } from "node:path"; +import { Command } from "commander"; import { clearConfigCache } from "openclaw/plugin-sdk/runtime-config-snapshot"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { policyCheckCommand, policyCompareCommand, policyWatchCommand } from "./cli.js"; -import { resetPolicyDoctorChecksForTest } from "./doctor/register.js"; -import { - policyAttestationHash, - policyWorkspaceHash, - policyDocumentHash, - policyFindingsHash, -} from "./policy-state.js"; +import { registerPolicyCli } from "./cli.js"; +import { createPolicyAttestation, policyDocumentHash } from "./policy-state.js"; let workspaceDir: string; -async function runPolicyCheckJson(options: Parameters[0] = {}) { +type PolicyCheckCliOptions = { + readonly severityMin?: string; +}; + +type PolicyWatchCliOptions = { + readonly intervalMs?: string; +}; + +type PolicyCompareCliOptions = { + readonly baseline: string; + readonly policy?: string; +}; + +async function runPolicyCli(args: readonly string[]) { const output: string[] = []; - const exitCode = await policyCheckCommand( - { cwd: workspaceDir, json: true, ...options }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); - return { exitCode, parsed: JSON.parse(output.at(-1) ?? "{}"), output }; + const stdout = vi.spyOn(process.stdout, "write").mockImplementation(((chunk: unknown) => { + output.push(String(chunk)); + return true; + }) as typeof process.stdout.write); + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(((chunk: unknown) => { + output.push(String(chunk)); + return true; + }) as typeof process.stderr.write); + const previousExitCode = process.exitCode; + process.exitCode = undefined; + try { + const program = new Command().name("openclaw"); + registerPolicyCli(program); + await program.parseAsync(["policy", ...args], { from: "user" }); + const lastOutput = output.at(-1) ?? ""; + const parsed = /^[{[]/.test(lastOutput.trimStart()) ? JSON.parse(lastOutput) : {}; + return { exitCode: process.exitCode ?? 0, parsed, output }; + } finally { + process.exitCode = previousExitCode; + stdout.mockRestore(); + stderr.mockRestore(); + } } -async function runPolicyWatchJson(options: Parameters[0] = {}) { - const output: string[] = []; - const exitCode = await policyWatchCommand( - { cwd: workspaceDir, json: true, once: true, ...options }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - async sleep() { - throw new Error("policy watch should not sleep in --once mode"); - }, - }, - ); - return { exitCode, parsed: JSON.parse(output.at(-1) ?? "{}"), output }; +async function runPolicyCheckJson(options: PolicyCheckCliOptions = {}) { + return runPolicyCli([ + "check", + "--json", + ...(options.severityMin === undefined ? [] : ["--severity-min", options.severityMin]), + ]); } -async function runPolicyCompareJson(options: Parameters[0]) { - const output: string[] = []; - const exitCode = await policyCompareCommand( - { cwd: workspaceDir, json: true, ...options }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); - return { exitCode, parsed: JSON.parse(output.at(-1) ?? "{}"), output }; +async function runPolicyWatchJson(options: PolicyWatchCliOptions = {}) { + return runPolicyCli([ + "watch", + "--json", + "--once", + ...(options.intervalMs === undefined ? [] : ["--interval-ms", options.intervalMs]), + ]); +} + +function workspacePath(value: string): string { + return isAbsolute(value) ? value : join(workspaceDir, value); +} + +async function runPolicyCompareJson(options: PolicyCompareCliOptions) { + return runPolicyCli([ + "compare", + "--json", + "--baseline", + workspacePath(options.baseline), + ...(options.policy === undefined ? [] : ["--policy", workspacePath(options.policy)]), + ]); } describe("policy commands", () => { beforeEach(async () => { workspaceDir = await fs.mkdtemp(join(tmpdir(), "policy-cli-")); + vi.stubEnv("OPENCLAW_WORKSPACE_DIR", workspaceDir); }); afterEach(async () => { vi.unstubAllEnvs(); clearConfigCache(); await fs.rm(workspaceDir, { recursive: true, force: true }); - resetPolicyDoctorChecksForTest(); }); it("checks policy rules and emits an attestation", async () => { @@ -96,29 +110,18 @@ describe("policy commands", () => { modelRefs: [], network: [], }; - const workspaceHash = policyWorkspaceHash(evidence); - const findingsHash = policyFindingsHash([]); + const attestation = createPolicyAttestation({ + ok: true, + checkedAt: parsed.attestation.checkedAt, + policyPath: "policy.jsonc", + policyHash, + evidence, + findings: [], + }); expect(typeof parsed.attestation.checkedAt).toBe("string"); expect(parsed).toMatchObject({ ok: true, - attestation: { - checkedAt: parsed.attestation.checkedAt, - policy: { - path: "policy.jsonc", - hash: policyHash, - }, - workspace: { - scope: "policy", - hash: workspaceHash, - }, - findingsHash, - attestationHash: policyAttestationHash({ - ok: true, - policyHash, - workspaceHash, - findingsHash, - }), - }, + attestation, evidence, findings: [], }); @@ -134,22 +137,10 @@ describe("policy commands", () => { }), "utf-8", ); - const output: string[] = []; - - const exitCode = await policyCheckCommand( - { cwd: workspaceDir, json: true }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); + const { exitCode, parsed } = await runPolicyCheckJson(); expect(exitCode).toBe(0); - expect(JSON.parse(output.at(-1) ?? "{}")).toMatchObject({ + expect(parsed).toMatchObject({ ok: true, evidence: { channels: [], @@ -271,8 +262,24 @@ describe("policy commands", () => { const attestedFinding = { ...parsed.findings[0] }; expect(attestedFinding.policy).toBeDefined(); delete attestedFinding.policy; - expect(parsed.attestation.findingsHash).toBe(policyFindingsHash([attestedFinding])); - expect(parsed.attestation.findingsHash).not.toBe(policyFindingsHash(parsed.findings)); + const attestedOutput = createPolicyAttestation({ + ok: false, + checkedAt: parsed.attestation.checkedAt, + policyPath: "policy.jsonc", + policyHash: parsed.attestation.policy.hash, + evidence: parsed.evidence, + findings: [attestedFinding], + }); + const reportedOutput = createPolicyAttestation({ + ok: false, + checkedAt: parsed.attestation.checkedAt, + policyPath: "policy.jsonc", + policyHash: parsed.attestation.policy.hash, + evidence: parsed.evidence, + findings: parsed.findings, + }); + expect(parsed.attestation.findingsHash).toBe(attestedOutput.findingsHash); + expect(parsed.attestation.findingsHash).not.toBe(reportedOutput.findingsHash); }); it("attests underlying policy findings when the accepted attestation is stale", async () => { @@ -308,15 +315,16 @@ describe("policy commands", () => { expect(parsed.findings).toEqual([ expect.objectContaining({ checkId: "policy/attestation-hash-mismatch" }), ]); - expect(parsed.attestation.findingsHash).not.toBe(policyFindingsHash([])); - expect(parsed.attestation.attestationHash).toBe( - policyAttestationHash({ - ok: false, - policyHash: parsed.attestation.policy.hash, - workspaceHash: parsed.attestation.workspace.hash, - findingsHash: parsed.attestation.findingsHash, - }), - ); + const emptyOutput = createPolicyAttestation({ + ok: false, + checkedAt: parsed.attestation.checkedAt, + policyPath: "policy.jsonc", + policyHash: parsed.attestation.policy.hash, + evidence: parsed.evidence, + findings: [], + }); + expect(parsed.attestation.findingsHash).not.toBe(emptyOutput.findingsHash); + expect(parsed.attestation.attestationHash).toMatch(/^sha256:[a-f0-9]{64}$/); }); it("reports stale accepted attestations in policy watch", async () => { @@ -357,36 +365,14 @@ describe("policy commands", () => { }); it("rejects partial policy watch intervals before evaluating policy", async () => { - const output: string[] = []; - const exitCode = await policyWatchCommand( - { cwd: workspaceDir, json: true, once: true, intervalMs: "500ms" }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); + const { exitCode, output } = await runPolicyWatchJson({ intervalMs: "500ms" }); expect(exitCode).toBe(2); expect(output.join("\n")).toContain("--interval-ms must be an integer >= 250."); }); it("rejects sub-floor policy watch intervals before evaluating policy", async () => { - const output: string[] = []; - const exitCode = await policyWatchCommand( - { cwd: workspaceDir, json: true, once: true, intervalMs: "249" }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); + const { exitCode, output } = await runPolicyWatchJson({ intervalMs: "249" }); expect(exitCode).toBe(2); expect(output.join("\n")).toContain("--interval-ms must be an integer >= 250."); @@ -442,21 +428,11 @@ describe("policy commands", () => { }); it("rejects invalid severity thresholds", async () => { - const errors: string[] = []; - - const exitCode = await policyCheckCommand( - { cwd: workspaceDir, severityMin: "warnng" }, - { - writeStdout() {}, - error(value) { - errors.push(value); - }, - }, - ); + const { exitCode, output } = await runPolicyCheckJson({ severityMin: "warnng" }); expect(exitCode).toBe(2); - expect(errors).toEqual([ - "Invalid --severity-min value. Expected one of: info, warning, error.", + expect(output).toEqual([ + "Invalid --severity-min value. Expected one of: info, warning, error.\n", ]); }); @@ -932,19 +908,9 @@ describe("policy commands", () => { "utf-8", ); - const output: string[] = []; - const exitCode = await policyCompareCommand( - { baseline: join(workspaceDir, "baseline.policy.jsonc"), json: true }, - { - writeStdout(value) { - output.push(value); - }, - error(value) { - output.push(value); - }, - }, - ); - const parsed = JSON.parse(output.at(-1) ?? "{}"); + const { exitCode, parsed } = await runPolicyCompareJson({ + baseline: join(workspaceDir, "baseline.policy.jsonc"), + }); expect(exitCode).toBe(0); expect(parsed).toMatchObject({ diff --git a/extensions/policy/src/cli.ts b/extensions/policy/src/cli.ts index ffcad6f6872f..73fe3921ee78 100644 --- a/extensions/policy/src/cli.ts +++ b/extensions/policy/src/cli.ts @@ -101,7 +101,7 @@ export function registerPolicyCli(program: Command): void { }); } -export async function policyCompareCommand( +async function policyCompareCommand( options: PolicyCompareOptions, runtime: PolicyCommandRuntime = defaultRuntime, ): Promise { @@ -123,7 +123,7 @@ export async function policyCompareCommand( } } -export async function policyCheckCommand( +async function policyCheckCommand( options: PolicyCheckOptions, runtime: PolicyCommandRuntime = defaultRuntime, ): Promise { @@ -137,7 +137,7 @@ export async function policyCheckCommand( } } -export async function policyWatchCommand( +async function policyWatchCommand( options: PolicyWatchOptions, runtime: PolicyCommandRuntime = defaultRuntime, ): Promise { diff --git a/extensions/policy/src/doctor/register.base.test-utils.ts b/extensions/policy/src/doctor/register.base.test-utils.ts index c4f003e4ce85..2e2f90ad8909 100644 --- a/extensions/policy/src/doctor/register.base.test-utils.ts +++ b/extensions/policy/src/doctor/register.base.test-utils.ts @@ -1,7 +1,11 @@ // Imported by register.test.ts to keep its mocked suite in one Vitest module graph. import { promises as fs } from "node:fs"; import { join } from "node:path"; -import type { HealthCheck, OpenClawConfig } from "openclaw/plugin-sdk/health"; +import { + listHealthChecks, + type HealthCheck, + type OpenClawConfig, +} from "openclaw/plugin-sdk/health"; import { clearHealthChecksForTest } from "openclaw/plugin-sdk/plugin-test-runtime"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { @@ -9,17 +13,12 @@ import { createPolicyAttestation, policyDocumentHash, } from "../policy-state.js"; -import { - evaluatePolicy, - registerPolicyDoctorChecks, - resetPolicyDoctorChecksForTest, -} from "./register.js"; +import { evaluatePolicy, registerPolicyDoctorChecks } from "./register.js"; import { workspaceDir, cfgWithPolicy, ctx, repairCtx, - registerChecks, runPolicyChecks, runDeniedChannelRepair, runPolicyRepairCheck, @@ -245,16 +244,16 @@ describe("registerPolicyDoctorChecks", () => { ); }); - it("registers policy health checks once", () => { - const checks = registerChecks(); - const duplicateChecks: HealthCheck[] = []; - registerPolicyDoctorChecks({ - registerHealthCheck(check) { - duplicateChecks.push(check); + it("registers the complete policy health check set once per registry", () => { + const checks: HealthCheck[] = []; + const host = { + registerHealthCheck(check: HealthCheck) { + checks.push(check); }, - }); - - expect(checks.map((check) => check.id)).toEqual([ + }; + registerPolicyDoctorChecks(host); + registerPolicyDoctorChecks(host); + const expectedIds = [ "policy/policy-jsonc-missing", "policy/policy-jsonc-invalid", "policy/policy-hash-mismatch", @@ -319,8 +318,16 @@ describe("registerPolicyDoctorChecks", () => { "policy/tools-missing-sensitivity-token", "policy/tools-missing-owner", "policy/tools-unknown-sensitivity-token", - ]); - expect(duplicateChecks).toEqual([]); + ]; + expect(checks.map((check) => check.id)).toEqual(expectedIds); + + registerPolicyDoctorChecks(); + registerPolicyDoctorChecks(); + expect(listHealthChecks().map((check) => check.id)).toEqual(expectedIds); + + clearHealthChecksForTest(); + registerPolicyDoctorChecks(); + expect(listHealthChecks().map((check) => check.id)).toEqual(expectedIds); }); it("reports a missing policy file when the Policy plugin is enabled", async () => { @@ -916,7 +923,6 @@ describe("registerPolicyDoctorChecks", () => { "utf-8", ); clearHealthChecksForTest(); - resetPolicyDoctorChecksForTest(); const result = await runPolicyChecks(ctx(configPath, cfgWithPolicy())); diff --git a/extensions/policy/src/doctor/register.test-harness.ts b/extensions/policy/src/doctor/register.test-harness.ts index cb6e583ec3b7..924a69761ef3 100644 --- a/extensions/policy/src/doctor/register.test-harness.ts +++ b/extensions/policy/src/doctor/register.test-harness.ts @@ -11,7 +11,7 @@ import { type OpenClawConfig, } from "openclaw/plugin-sdk/health"; import { clearHealthChecksForTest } from "openclaw/plugin-sdk/plugin-test-runtime"; -import { registerPolicyDoctorChecks, resetPolicyDoctorChecksForTest } from "./register.js"; +import { registerPolicyDoctorChecks } from "./register.js"; export let workspaceDir: string; @@ -91,7 +91,6 @@ export async function runDeniedChannelRepair(repairCheckCtx: HealthRepairContext } export async function runPolicyRepairCheck(checkId: string, repairCheckCtx: HealthRepairContext) { - resetPolicyDoctorChecksForTest(); const check = registerChecks().find((entry) => entry.id === checkId); if (check?.detect === undefined || check.repair === undefined) { throw new Error(`${checkId} repair check was not registered`); @@ -106,7 +105,6 @@ export async function runPolicyRepairCheck(checkId: string, repairCheckCtx: Heal export const describe0BeforeEach0 = async () => { clearHealthChecksForTest(); - resetPolicyDoctorChecksForTest(); originalOpenClawHome = process.env.OPENCLAW_HOME; originalOpenClawStateDir = process.env.OPENCLAW_STATE_DIR; workspaceDir = await fs.mkdtemp(join(tmpdir(), "policy-doctor-")); @@ -140,5 +138,4 @@ export const describe0AfterEach1 = async () => { } await fs.rm(workspaceDir, { recursive: true, force: true }); clearHealthChecksForTest(); - resetPolicyDoctorChecksForTest(); }; diff --git a/extensions/policy/src/doctor/register.ts b/extensions/policy/src/doctor/register.ts index 464309eb8b11..996db6265b34 100644 --- a/extensions/policy/src/doctor/register.ts +++ b/extensions/policy/src/doctor/register.ts @@ -1,4 +1,5 @@ import { + getHealthCheck, registerHealthCheck as registerPluginHealthCheck, type HealthCheck, } from "openclaw/plugin-sdk/health"; @@ -11,32 +12,33 @@ import { workspaceRepairsEnabled, } from "./policy-runtime.js"; -let registered = false; +let policyDoctorChecks: readonly HealthCheck[] | undefined; +const registeredPolicyDoctorRegistrars = new WeakSet<(check: HealthCheck) => void>(); type PolicyDoctorRegistrationHost = { readonly registerHealthCheck: (check: HealthCheck) => void; }; export function registerPolicyDoctorChecks(host?: PolicyDoctorRegistrationHost): void { - if (registered) { + if (host !== undefined && registeredPolicyDoctorRegistrars.has(host.registerHealthCheck)) { return; } const registerHealthCheck = host?.registerHealthCheck ?? registerPluginHealthCheck; - for (const check of createPolicyDoctorChecks({ + policyDoctorChecks ??= createPolicyDoctorChecks({ channelIdsFromFindings, disableChannels, evaluatePolicy, findingsForCheck, workspaceRepairsDisabledResult, workspaceRepairsEnabled, - })) { + }); + for (const check of policyDoctorChecks) { + if (host === undefined && getHealthCheck(check.id) === check) { + continue; + } registerHealthCheck(check); } - registered = true; -} - -export function resetPolicyDoctorChecksForTest(): void { - registered = false; + registeredPolicyDoctorRegistrars.add(registerHealthCheck); } export { evaluatePolicy }; diff --git a/extensions/policy/src/policy-state.ts b/extensions/policy/src/policy-state.ts index 70c4da3df645..14b15e012dd4 100644 --- a/extensions/policy/src/policy-state.ts +++ b/extensions/policy/src/policy-state.ts @@ -259,15 +259,15 @@ export function policyDocumentHash(policy: unknown): string { return sha256(stableJson(policy)); } -export function policyWorkspaceHash(evidence: PolicyEvidence): string { +function policyWorkspaceHash(evidence: PolicyEvidence): string { return sha256(stableJson(evidence)); } -export function policyFindingsHash(findings: readonly unknown[]): string { +function policyFindingsHash(findings: readonly unknown[]): string { return sha256(stableJson(findings)); } -export function policyAttestationHash(input: { +function policyAttestationHash(input: { readonly ok: boolean; readonly policyHash?: string; readonly workspaceHash: string; diff --git a/scripts/deadcode-exports.baseline.mjs b/scripts/deadcode-exports.baseline.mjs index 28e4f2876b17..3e937c07daf1 100644 --- a/scripts/deadcode-exports.baseline.mjs +++ b/scripts/deadcode-exports.baseline.mjs @@ -433,13 +433,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/parallel/src/parallel-mcp-search.runtime.ts: extractMcpToolPayload", "extensions/parallel/src/parallel-mcp-search.runtime.ts: iterMcpMessages", "extensions/parallel/src/parallel-mcp-search.runtime.ts: selectMcpEnvelope", - "extensions/policy/src/cli.ts: policyCheckCommand", - "extensions/policy/src/cli.ts: policyCompareCommand", - "extensions/policy/src/cli.ts: policyWatchCommand", - "extensions/policy/src/doctor/register.ts: resetPolicyDoctorChecksForTest", - "extensions/policy/src/policy-state.ts: policyAttestationHash", - "extensions/policy/src/policy-state.ts: policyFindingsHash", - "extensions/policy/src/policy-state.ts: policyWorkspaceHash", "extensions/qa-channel/src/inbound.ts: isHttpMediaUrl", "extensions/qa-lab/src/agentic-parity-report.ts: computeQaAgenticParityMetrics", "extensions/qa-lab/src/agentic-parity-report.ts: QaParityLabelMismatchError",