From 30cb20643851b8bcfebe32a69f58694103bfb5ec Mon Sep 17 00:00:00 2001 From: Eve Date: Fri, 8 May 2026 13:21:43 +0800 Subject: [PATCH] test: harden decision record storage slice --- ...governance-plugin-status-plain-language.md | 2 +- plugins/reporting-governance/package.json | 7 +- .../src/storage/decision-artifact.mjs | 55 ++++- .../src/storage/index.mjs | 1 + ...ecision-store-runtime.integration.test.mjs | 209 ++++++++++++++++++ .../test/decision-store.test.mjs | 177 +++++++++++++++ 6 files changed, 440 insertions(+), 11 deletions(-) create mode 100644 plugins/reporting-governance/test/decision-store-runtime.integration.test.mjs diff --git a/docs/roadmaps/reporting-governance-plugin-status-plain-language.md b/docs/roadmaps/reporting-governance-plugin-status-plain-language.md index 20aa999..9754539 100644 --- a/docs/roadmaps/reporting-governance-plugin-status-plain-language.md +++ b/docs/roadmaps/reporting-governance-plugin-status-plain-language.md @@ -3,7 +3,7 @@ 這份文件是給**不想先看程式碼的人**看的。 一句話先講白： -**這個 plugin 在做的事，是把「代理有沒有老實回報、有没有卡住不講、有没有把該交接的事真的交出去」這些規則，從口頭要求，慢慢做成可驗證的程式與紀錄。** +**這個 plugin 在做的事，是把「代理有沒有老實回報、有沒有卡住不講、有沒有把該交接的事真的交出去」這些規則，從口頭要求，慢慢做成可驗證的程式與紀錄。** --- diff --git a/plugins/reporting-governance/package.json b/plugins/reporting-governance/package.json index 208ad2e..99b9592 100644 --- a/plugins/reporting-governance/package.json +++ b/plugins/reporting-governance/package.json @@ -1,7 +1,6 @@ { "name": "@openclaw/plugin-reporting-governance", "version": "0.1.0-mainline", - "private": true, "type": "module", "description": "Reporting governance plugin package skeleton with capability descriptors and OpenClaw reference adapter boundaries.", "exports": { @@ -14,10 +13,10 @@ "./adapters/orchestrator": "./src/adapters/orchestrator.mjs" }, "scripts": { - "test": "node --test test/package-structure.test.mjs test/policy-evaluator.test.mjs test/compatibility-preflight.test.mjs test/profile-artifact.test.mjs test/profile-generator.test.mjs test/decision-runner.test.mjs test/decision-store.test.mjs test/governance-contract.integration.test.mjs test/watchdog-chain.integration.test.mjs test/runtime-integrated.integration.test.mjs test/exports-boundary.integration.test.mjs" + "test": "node --test test/package-structure.test.mjs test/policy-evaluator.test.mjs test/compatibility-preflight.test.mjs test/profile-artifact.test.mjs test/profile-generator.test.mjs test/decision-runner.test.mjs test/decision-store.test.mjs test/decision-store-runtime.integration.test.mjs test/governance-contract.integration.test.mjs test/watchdog-chain.integration.test.mjs test/runtime-integrated.integration.test.mjs test/exports-boundary.integration.test.mjs" }, "dependencies": { - "ajv": "^8.20.0", - "yaml": "^2.8.4" + "ajv": "^8.17.1", + "yaml": "^2.8.0" } } diff --git a/plugins/reporting-governance/src/storage/decision-artifact.mjs b/plugins/reporting-governance/src/storage/decision-artifact.mjs index a224bb8..6b75305 100644 --- a/plugins/reporting-governance/src/storage/decision-artifact.mjs +++ b/plugins/reporting-governance/src/storage/decision-artifact.mjs @@ -18,10 +18,24 @@ function assertObjectRecord(value, label) { } function sanitizeFileSegment(value, fallback) { - const normalized = String(value ?? '').trim().replace(/[^a-zA-Z0-9._-]+/g, '-').replace(/^-+|-+$/g, ''); + const normalized = String(value ?? '') + .trim() + .replace(/[^a-zA-Z0-9._-]+/g, '-') + .replace(/^-+|-+$/g, '') + .replace(/^(?:\.+-*)+/, '') + .replace(/(?:-+\.)+$/g, '') + .replace(/^-+|-+$/g, ''); return normalized || fallback; } +function assertIsoTimestamp(value, label) { + const normalized = assertNonEmptyString(value, label); + if (Number.isNaN(Date.parse(normalized))) { + throw new Error(`${label} must be a valid ISO-8601 timestamp`); + } + return normalized; +} + export function validateDecisionRecordArtifact(artifact) { if (!artifact || typeof artifact !== 'object' || Array.isArray(artifact)) { throw new Error('decision record artifact must be an object'); @@ -37,14 +51,43 @@ export function validateDecisionRecordArtifact(artifact) { const spec = assertObjectRecord(artifact.spec, 'decision record artifact spec'); const decision = assertObjectRecord(spec.decision, 'decision record artifact spec.decision'); const receipt = assertObjectRecord(spec.receipt, 'decision record artifact spec.receipt'); + const source = spec.source == null ? null : assertObjectRecord(spec.source, 'decision record artifact spec.source'); - assertNonEmptyString(metadata.recorded_at, 'decision record artifact metadata.recorded_at'); - assertNonEmptyString(metadata.policy_id, 'decision record artifact metadata.policy_id'); - assertNonEmptyString(metadata.decision, 'decision record artifact metadata.decision'); - assertNonEmptyString(decision.policy_id, 'decision record artifact spec.decision.policy_id'); - assertNonEmptyString(decision.decision, 'decision record artifact spec.decision.decision'); + const recordedAt = assertIsoTimestamp(metadata.recorded_at, 'decision record artifact metadata.recorded_at'); + const metadataPolicyId = assertNonEmptyString(metadata.policy_id, 'decision record artifact metadata.policy_id'); + const metadataDecision = assertNonEmptyString(metadata.decision, 'decision record artifact metadata.decision'); + const decisionPolicyId = assertNonEmptyString(decision.policy_id, 'decision record artifact spec.decision.policy_id'); + const decisionName = assertNonEmptyString(decision.decision, 'decision record artifact spec.decision.decision'); assertNonEmptyString(receipt.delivery_state, 'decision record artifact spec.receipt.delivery_state'); + if (metadataPolicyId !== decisionPolicyId) { + throw new Error('decision record artifact policy_id mismatch between metadata and spec.decision'); + } + if (metadataDecision !== decisionName) { + throw new Error('decision record artifact decision mismatch between metadata and spec.decision'); + } + if (source?.event_id != null) { + assertNonEmptyString(source.event_id, 'decision record artifact spec.source.event_id'); + } + if (source?.task_id != null) { + const sourceTaskId = assertNonEmptyString(source.task_id, 'decision record artifact spec.source.task_id'); + if (metadata.task_id != null && assertNonEmptyString(metadata.task_id, 'decision record artifact metadata.task_id') !== sourceTaskId) { + throw new Error('decision record artifact task_id mismatch between metadata and spec.source'); + } + } + if (source?.correlation_id != null) { + const sourceCorrelationId = assertNonEmptyString(source.correlation_id, 'decision record artifact spec.source.correlation_id'); + if (metadata.correlation_id != null && assertNonEmptyString(metadata.correlation_id, 'decision record artifact metadata.correlation_id') !== sourceCorrelationId) { + throw new Error('decision record artifact correlation_id mismatch between metadata and spec.source'); + } + } + + metadata.recorded_at = recordedAt; + metadata.policy_id = metadataPolicyId; + metadata.decision = metadataDecision; + decision.policy_id = decisionPolicyId; + decision.decision = decisionName; + return artifact; } diff --git a/plugins/reporting-governance/src/storage/index.mjs b/plugins/reporting-governance/src/storage/index.mjs index b602fec..20d704d 100644 --- a/plugins/reporting-governance/src/storage/index.mjs +++ b/plugins/reporting-governance/src/storage/index.mjs @@ -12,5 +12,6 @@ export { createDecisionRecordArtifact, createDecisionRecordFileName, validateDecisionRecordArtifact, + __testables, } from './decision-artifact.mjs'; export { createFileDecisionStore } from './decision-store.mjs'; diff --git a/plugins/reporting-governance/test/decision-store-runtime.integration.test.mjs b/plugins/reporting-governance/test/decision-store-runtime.integration.test.mjs new file mode 100644 index 0000000..feeb8fd --- /dev/null +++ b/plugins/reporting-governance/test/decision-store-runtime.integration.test.mjs @@ -0,0 +1,209 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { executeRuntimeIntegratedGovernance, createFileDecisionStore } from '../src/index.mjs'; +import capabilityDescriptor from '../capabilities/openclaw-watchdog-reference.json' with { type: 'json' }; + +const packageRoot = path.resolve(import.meta.dirname, '..'); +const repoRoot = path.resolve(packageRoot, '..', '..'); + +const noSilencePack = { + metadata: { id: 'no-silence', severity_default: 'high' }, + spec: { + evaluation_mode: 'any_rule_match', + rules: [ + { + id: 'no-silence.missed-checkpoint', + title: 'Missed checkpoint requires visible recovery', + triggers: { event_types: ['silence_timeout'] }, + conditions: { + all: [ + { fact: 'checkpoint.is_overdue', equals: true } + ] + }, + decision_output: { + decision: 'force_checkpoint', + severity: 'high', + reason: 'checkpoint overdue triggered forced operator-visible recovery', + required_actions: [ + { action: 'notify_operator', target: 'operator_channel', mandatory: true }, + { action: 'emit_event', target: 'event_stream', mandatory: true } + ], + operator_notice: { + required: true, + channel: 'telegram', + urgency: 'high', + message: 'Required update: checkpoint overdue.', + deadline: '2026-01-01T00:00:00.000Z' + } + } + } + ] + } +}; + +const strictProfileArtifact = { + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + metadata: { + id: 'strict-manager-mode', + runtime: 'openclaw', + compatibility_mode: 'strict_envelope', + }, + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + policies: { + overrides: { + checkpoints: { overdueAction: 'force_checkpoint' } + } + }, + notifications: { + operatorVisibleRecoveryRequired: true + }, + bindings: { + runtime: 'openclaw', + entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs', + scripts: { + watchdog: 'scripts/long_task_watchdog.mjs', + dispatcher: 'scripts/operator_notify_dispatcher.mjs', + bridgeSupervisor: 'scripts/operator_notify_bridge_supervisor.mjs', + senderBinding: 'scripts/operator_notify_sender_binding.mjs', + orchestrator: 'scripts/watchdog_auto_notify_orchestrator.mjs' + }, + artifact_roots: { + queueItems: 'state/operator-notify-queue' + } + } + }, + capability_expectations: { + required: [ + 'emit_canonical_events', + 'evaluate_watchdog_overdue', + 'create_queue_items', + 'create_spool_handoff', + 'write_bridge_receipts' + ], + preferred: ['direct_sender_binding', 'final_delivery_ack'] + } +}; + +function createFixtureRoot() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'reporting-governance-decision-runtime-')); +} + +function mkdirs(root, names) { + for (const name of names) { + fs.mkdirSync(path.join(root, name), { recursive: true }); + } +} + +function writeState(root) { + const statePath = path.join(root, 'watchdog-state.json'); + fs.writeFileSync(statePath, `${JSON.stringify({ + version: 1, + watchdogs: [ + { + id: 'reporting-governance-plugin-watchdog', + task: 'reporting-governance plugin spec development', + status: 'active', + ownerSessionKey: 'agent:coder:main', + reportChannel: 'telegram', + reportTarget: '864811879', + intervalMinutes: 10, + lastMilestoneAt: '2026-05-07T08:00:00.000Z', + lastAlertAt: null, + }, + ], + }, null, 2)}\n`, 'utf8'); + return statePath; +} + +function readSingleJson(dirPath) { + const files = fs.readdirSync(dirPath).filter((name) => name.endsWith('.json')).sort(); + assert.equal(files.length, 1, `expected exactly one json file in ${dirPath}`); + return JSON.parse(fs.readFileSync(path.join(dirPath, files[0]), 'utf8')); +} + +function createBaseArgs() { + return { + event: { + type: 'silence_timeout', + payload: { + checkpoint_overdue: true, + } + }, + evidence: [ + { id: 'ev-watchdog', quality: 'moderate', is_new: true } + ], + capabilityDescriptor, + policyPacks: [noSilencePack], + context: { + signals: ['checkpoint_overdue'], + }, + profile: strictProfileArtifact, + packageVersion: '0.1.0-mainline', + repoRootOverride: repoRoot, + }; +} + +test('decision record integrates planning output with runtime receipts and queue artifacts', () => { + const root = createFixtureRoot(); + try { + mkdirs(root, ['evidence', 'events', 'queue', 'spool', 'receipts', 'repo']); + const statePath = writeState(root); + const fakeRepoRoot = path.join(root, 'repo'); + fs.mkdirSync(path.join(fakeRepoRoot, 'state', 'decisions'), { recursive: true }); + + const result = executeRuntimeIntegratedGovernance({ + ...createBaseArgs(), + runtime: { + state: statePath, + evidenceDir: path.join(root, 'evidence'), + eventDir: path.join(root, 'events'), + queueDir: path.join(root, 'queue'), + spoolDir: path.join(root, 'spool'), + receiptDir: path.join(root, 'receipts'), + senderCommand: `node -e "process.stdout.write(JSON.stringify({state:'sent'}))"`, + writeState: true, + now: '2026-05-07T08:20:00.000Z', + }, + }); + + assert.equal(result.contract.decision, 'force_checkpoint'); + assert.equal(result.planning.receipt.delivery_state, 'acked'); + + const queueItem = readSingleJson(path.join(root, 'queue')); + const receipt = readSingleJson(path.join(root, 'receipts')); + const runtimeEventRef = queueItem.evidence_refs.find((ref) => ref.label === 'watchdog_event'); + + const store = createFileDecisionStore({ + decisionsDir: path.join(fakeRepoRoot, 'state', 'decisions'), + repoRootOverride: fakeRepoRoot, + }); + + const written = store.write({ + decision: result.planning.decision, + receipt: result.planning.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + source: { + task_id: queueItem.governance.task_id, + correlation_id: queueItem.governance.correlation_id, + event_id: runtimeEventRef.path, + }, + }); + + const loaded = store.load(written.artifactPath); + assert.equal(loaded.artifact.metadata.policy_id, 'no-silence.missed-checkpoint'); + assert.equal(loaded.artifact.spec.receipt.delivery_state, 'acked'); + assert.equal(loaded.artifact.metadata.task_id, 'reporting-governance-plugin-watchdog'); + assert.equal(loaded.artifact.metadata.correlation_id, 'watchdog:reporting-governance-plugin-watchdog'); + assert.equal(loaded.artifact.spec.source.event_id, runtimeEventRef.path); + assert.equal(receipt.state, loaded.artifact.spec.receipt.delivery_state); + assert.equal(queueItem.governance.decision, loaded.artifact.spec.decision.decision); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/plugins/reporting-governance/test/decision-store.test.mjs b/plugins/reporting-governance/test/decision-store.test.mjs index 3f2cea3..b904e1e 100644 --- a/plugins/reporting-governance/test/decision-store.test.mjs +++ b/plugins/reporting-governance/test/decision-store.test.mjs @@ -9,6 +9,7 @@ import { createDecisionRecordFileName, createFileDecisionStore, validateDecisionRecordArtifact, + __testables as decisionArtifactTestables, } from '../src/storage/index.mjs'; import { planDecisionExecution } from '../src/core/decision-runner.mjs'; @@ -78,6 +79,70 @@ test('decision artifact validates minimal package-owned contract', () => { assert.equal(validateDecisionRecordArtifact(artifact), artifact); }); +test('decision artifact validator rejects malformed top-level or nested fields', () => { + assert.throws(() => validateDecisionRecordArtifact(null), /decision record artifact must be an object/); + + const planned = createPlannedDecision(); + const artifact = createDecisionRecordArtifact({ + decision: planned.decision, + receipt: planned.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + }); + + const wrongKind = structuredClone(artifact); + wrongKind.kind = 'NotDecisionRecordArtifact'; + assert.throws(() => validateDecisionRecordArtifact(wrongKind), /kind must be DecisionRecordArtifact/); + + const missingReceipt = structuredClone(artifact); + delete missingReceipt.spec.receipt; + assert.throws(() => validateDecisionRecordArtifact(missingReceipt), /spec\.receipt must be an object record/); + + const blankDeliveryState = structuredClone(artifact); + blankDeliveryState.spec.receipt.delivery_state = ' '; + assert.throws(() => validateDecisionRecordArtifact(blankDeliveryState), /spec\.receipt\.delivery_state must be a non-empty string/); +}); + +test('decision artifact validator enforces metadata and spec consistency', () => { + const planned = createPlannedDecision(); + const artifact = createDecisionRecordArtifact({ + decision: planned.decision, + receipt: planned.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + source: { + task_id: 'task-reporting-governance', + correlation_id: 'corr-001', + }, + }); + + const mismatchedPolicy = structuredClone(artifact); + mismatchedPolicy.metadata.policy_id = 'different-policy'; + assert.throws(() => validateDecisionRecordArtifact(mismatchedPolicy), /policy_id mismatch between metadata and spec\.decision/); + + const mismatchedDecision = structuredClone(artifact); + mismatchedDecision.metadata.decision = 'rewrite'; + assert.throws(() => validateDecisionRecordArtifact(mismatchedDecision), /decision mismatch between metadata and spec\.decision/); + + const mismatchedTask = structuredClone(artifact); + mismatchedTask.metadata.task_id = 'other-task'; + assert.throws(() => validateDecisionRecordArtifact(mismatchedTask), /task_id mismatch between metadata and spec\.source/); + + const mismatchedCorrelation = structuredClone(artifact); + mismatchedCorrelation.metadata.correlation_id = 'corr-other'; + assert.throws(() => validateDecisionRecordArtifact(mismatchedCorrelation), /correlation_id mismatch between metadata and spec\.source/); +}); + +test('decision artifact validator rejects malformed recorded_at timestamp', () => { + const planned = createPlannedDecision(); + const artifact = createDecisionRecordArtifact({ + decision: planned.decision, + receipt: planned.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + }); + + artifact.metadata.recorded_at = 'not-a-timestamp'; + assert.throws(() => validateDecisionRecordArtifact(artifact), /recorded_at must be a valid ISO-8601 timestamp/); +}); + test('decision artifact filename is stable and readable', () => { const planned = createPlannedDecision(); const artifact = createDecisionRecordArtifact({ @@ -90,6 +155,45 @@ test('decision artifact filename is stable and readable', () => { assert.match(fileName, /^2026-05-08T04-00-00-000Z-no-silence\.missed-checkpoint-force_checkpoint-dec_[a-f0-9-]+\.json$/); }); +test('decision artifact filename sanitizes unsafe policy and decision segments', () => { + const artifact = { + kind: 'DecisionRecordArtifact', + apiVersion: 'reporting-governance/v1alpha1', + metadata: { + record_id: 'dec_test', + recorded_at: '2026-05-08T04:00:00.000Z', + policy_id: '../policy with spaces?', + decision: 'force checkpoint!', + correlation_id: null, + task_id: null, + event_id: null, + }, + spec: { + decision: { + policy_id: '../policy with spaces?', + decision: 'force checkpoint!', + }, + receipt: { + delivery_state: 'pending_external_send', + }, + source: { + event_id: null, + task_id: null, + correlation_id: null, + }, + }, + }; + + const fileName = createDecisionRecordFileName(artifact); + assert.equal(fileName, '2026-05-08T04-00-00-000Z-policy-with-spaces-force-checkpoint-dec_test.json'); +}); + +test('sanitizeFileSegment collapses malicious or blank segments to safe file names', () => { + assert.equal(decisionArtifactTestables.sanitizeFileSegment('../../etc/passwd', 'fallback'), 'etc-passwd'); + assert.equal(decisionArtifactTestables.sanitizeFileSegment(' ', 'fallback'), 'fallback'); + assert.equal(decisionArtifactTestables.sanitizeFileSegment('policy:force/checkpoint', 'fallback'), 'policy-force-checkpoint'); +}); + test('file decision store writes and reloads a validated decision artifact inside repo root', async (t) => { const sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'reporting-governance-decision-store-')); t.after(() => fs.rmSync(sandbox, { recursive: true, force: true })); @@ -122,6 +226,79 @@ test('file decision store writes and reloads a validated decision artifact insid assert.equal(loaded.artifact.spec.receipt.delivery_state, 'pending_external_send'); }); +test('file decision store load rejects corrupted or malformed artifacts', async (t) => { + const sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'reporting-governance-decision-store-')); + t.after(() => fs.rmSync(sandbox, { recursive: true, force: true })); + + const fakeRepoRoot = path.join(sandbox, 'repo'); + const decisionsDir = path.join(fakeRepoRoot, 'state', 'decisions'); + fs.mkdirSync(decisionsDir, { recursive: true }); + + const store = createFileDecisionStore({ + decisionsDir, + repoRootOverride: fakeRepoRoot, + }); + + const malformedJsonPath = path.join(decisionsDir, 'broken.json'); + fs.writeFileSync(malformedJsonPath, '{not-json\n', 'utf8'); + assert.throws(() => store.load(malformedJsonPath)); + + const corruptedArtifactPath = path.join(decisionsDir, 'corrupted.json'); + fs.writeFileSync(corruptedArtifactPath, `${JSON.stringify({ + kind: 'DecisionRecordArtifact', + apiVersion: 'reporting-governance/v1alpha1', + metadata: { + recorded_at: '', + policy_id: 'no-silence.missed-checkpoint', + decision: 'force_checkpoint', + }, + spec: { + decision: { + policy_id: 'no-silence.missed-checkpoint', + decision: 'force_checkpoint', + }, + receipt: { + delivery_state: 'pending_external_send', + }, + source: { + event_id: null, + task_id: null, + correlation_id: null, + }, + }, + }, null, 2)}\n`, 'utf8'); + assert.throws(() => store.load(corruptedArtifactPath), /metadata\.recorded_at must be a non-empty string/); +}); + +test('file decision store produces distinct filenames for same decision written twice', async (t) => { + const sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'reporting-governance-decision-store-')); + t.after(() => fs.rmSync(sandbox, { recursive: true, force: true })); + + const fakeRepoRoot = path.join(sandbox, 'repo'); + fs.mkdirSync(fakeRepoRoot, { recursive: true }); + + const planned = createPlannedDecision(); + const store = createFileDecisionStore({ + decisionsDir: path.join(fakeRepoRoot, 'state', 'decisions'), + repoRootOverride: fakeRepoRoot, + }); + + const first = store.write({ + decision: planned.decision, + receipt: planned.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + }); + const second = store.write({ + decision: planned.decision, + receipt: planned.receipt, + recordedAt: '2026-05-08T04:00:00.000Z', + }); + + assert.notEqual(first.artifact.metadata.record_id, second.artifact.metadata.record_id); + assert.notEqual(path.basename(first.artifactPath), path.basename(second.artifactPath)); + assert.equal(fs.readdirSync(path.join(fakeRepoRoot, 'state', 'decisions')).filter((name) => name.endsWith('.json')).length, 2); +}); + test('file decision store rejects decision directory escaping repo root', () => { assert.throws( () => createFileDecisionStore({