From fdaaa1ba5304a77ecbc9faaa8ef13cc270caad62 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 20:13:55 -0700 Subject: [PATCH] =?UTF-8?q?refactor(validation):=20consolidate=20boundary?= =?UTF-8?q?=20guards=20into=20schemas=20=E2=80=94=20wave=202=20(#124961)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- config/assertion-safety-baseline.txt | 9 +- extensions/browser/package.json | 3 +- .../session-tab-registry.sqlite.test.ts | 12 +- .../browser/src/browser/session-tab-store.ts | 127 +++--- pnpm-lock.yaml | 3 + .../sessions/session-manager-codec.test.ts | 37 ++ src/agents/sessions/session-manager-codec.ts | 266 ++++++------- src/cron/task-run-detail.ts | 189 +++++---- src/infra/exec-approvals-config.test.ts | 31 ++ src/infra/exec-approvals-config.ts | 119 +++--- src/infra/state-migrations.acp-replay.ts | 100 ++--- src/memory-host-sdk/event-store.ts | 242 ++++++------ src/plugin-sdk/memory-host-events.test.ts | 38 ++ src/skills/workshop/store-record.test.ts | 50 +++ src/skills/workshop/store-record.ts | 363 +++++++----------- src/talk/client-voice-session-store.test.ts | 10 + src/talk/client-voice-session-store.ts | 113 +++--- 17 files changed, 907 insertions(+), 805 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 705d48c27a3a..796bc2e79213 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -112,7 +112,6 @@ extensions/browser/src/browser/server-context.tab-ops.ts 4 extensions/browser/src/browser/server-middleware.ts 2 extensions/browser/src/browser/session-tab-ephemeral-aliases.ts 4 extensions/browser/src/browser/session-tab-process-state.ts 4 -extensions/browser/src/browser/session-tab-store.ts 2 extensions/browser/src/browser/snapshot-delta-cache.ts 1 extensions/browser/src/browser/system-chrome-cookies.ts 1 extensions/browser/src/browser/system-profiles.ts 1 @@ -2866,7 +2865,7 @@ src/cron/store/quarantine.ts 1 src/cron/store/row-codec.ts 7 src/cron/store/state-codec.ts 3 src/cron/store/transaction-hooks.ts 1 -src/cron/task-run-detail.ts 6 +src/cron/task-run-detail.ts 2 src/cron/task-run-history.ts 1 src/daemon/arg-split.ts 2 src/daemon/launchd-install.ts 7 @@ -3479,7 +3478,7 @@ src/meeting-bot/session-speech-readiness.ts 1 src/meeting-bot/session-transcript-store.ts 1 src/meeting-bot/voice-call-gateway.ts 1 src/memory-host-sdk/dreaming.ts 3 -src/memory-host-sdk/event-store.ts 5 +src/memory-host-sdk/event-store.ts 2 src/memory-host-sdk/events.ts 1 src/model-catalog/bundled-catalog-stamp.ts 1 src/model-catalog/remote-overlay.ts 1 @@ -3840,7 +3839,7 @@ src/skills/workshop/experience-review.ts 2 src/skills/workshop/history-scan-transcript.ts 1 src/skills/workshop/history-scan.ts 2 src/skills/workshop/policy.ts 1 -src/skills/workshop/store-record.ts 6 +src/skills/workshop/store-record.ts 3 src/skills/workshop/store-sqlite-event.ts 1 src/skills/workshop/tool-policy-diagnostic.ts 1 src/snapshot/git-backup-codec.ts 10 @@ -3908,7 +3907,7 @@ src/talk/agent-consult-tool.ts 2 src/talk/agent-run-control-shared.ts 4 src/talk/client-voice-confirmation.ts 2 src/talk/client-voice-mutation-digest-owner.ts 1 -src/talk/client-voice-session-store.ts 5 +src/talk/client-voice-session-store.ts 3 src/talk/client-voice-session.ts 3 src/talk/consult-question.ts 2 src/talk/exact-speech-protocol.ts 1 diff --git a/extensions/browser/package.json b/extensions/browser/package.json index b50e55bfa63f..69452d56d148 100644 --- a/extensions/browser/package.json +++ b/extensions/browser/package.json @@ -9,7 +9,8 @@ "express": "5.2.1", "playwright-core": "1.62.1", "typebox": "1.3.6", - "ws": "8.21.1" + "ws": "8.21.1", + "zod": "4.4.3" }, "devDependencies": { "@openclaw/plugin-sdk": "workspace:*", diff --git a/extensions/browser/src/browser/session-tab-registry.sqlite.test.ts b/extensions/browser/src/browser/session-tab-registry.sqlite.test.ts index 6c7bc973ac5d..89f12322edc1 100644 --- a/extensions/browser/src/browser/session-tab-registry.sqlite.test.ts +++ b/extensions/browser/src/browser/session-tab-registry.sqlite.test.ts @@ -891,6 +891,16 @@ describe("durable session tab registry", () => { } satisfies DurableRecord; openStore().register("wrong-storage-key", validRecord); openStore().register("invalid-record", { version: 999, sessionKey: "agent:main:main" }); + openStore().register("partial-cleanup", { + ...validRecord, + nativeTargetId: "NATIVE-PARTIAL", + cleanupRequestedAt: 2_000, + }); + openStore().register("noncanonical-aliases", { + ...validRecord, + nativeTargetId: "NATIVE-ALIASES", + profileAliases: ["zeta", "alpha"], + }); const warnings: string[] = []; const registry = await freshRegistry("invalid"); @@ -901,7 +911,7 @@ describe("durable session tab registry", () => { }); expect(openStore().entries()).toEqual([]); expect(cdpMocks.closeTrackedCdpTarget).not.toHaveBeenCalled(); - expect(warnings).toHaveLength(2); + expect(warnings).toHaveLength(4); }); it("keeps non-durable tabs out of SQLite but shared across duplicate bundles", async () => { diff --git a/extensions/browser/src/browser/session-tab-store.ts b/extensions/browser/src/browser/session-tab-store.ts index 971bdde3c89f..f5b5470ac4b7 100644 --- a/extensions/browser/src/browser/session-tab-store.ts +++ b/extensions/browser/src/browser/session-tab-store.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import type { PluginRuntime } from "openclaw/plugin-sdk/runtime-store"; +import { z } from "zod"; import { getBrowserStateRuntime, getOptionalBrowserStateRuntime, @@ -13,21 +14,53 @@ import { const BROWSER_SESSION_TABS_NAMESPACE = "browser.session-tabs"; const BROWSER_SESSION_TABS_MAX_ENTRIES = 5_000; -export type BrowserSessionTabRecord = { - version: 1; - sessionKey: string; - nativeTargetId: string; - profile: string; - profileAliases?: string[]; - profileFingerprint: string; - browserInstanceFingerprint: string; - interactionTargetKind: "native" | "opaque"; - trackedAt: number; - lastUsedAt: number; - cleanupRequestedAt?: number; - cleanupAttemptToken?: string; - cleanupKind?: "lifecycle" | "sweep"; -}; +const browserSessionTimestampSchema = z.number().finite().nonnegative(); +const browserProfileAliasSchema = z + .string() + .min(1) + .refine((value) => value === value.trim().toLowerCase()); +const browserSessionTabRecordSchema = z + .looseObject({ + version: z.literal(1), + sessionKey: z.string().min(1), + nativeTargetId: z.string().min(1), + profile: z.string().min(1), + profileAliases: z.array(browserProfileAliasSchema).min(1).optional(), + profileFingerprint: z.string().min(1), + browserInstanceFingerprint: z.string().min(1), + interactionTargetKind: z.enum(["native", "opaque"]), + trackedAt: browserSessionTimestampSchema, + lastUsedAt: browserSessionTimestampSchema, + cleanupRequestedAt: browserSessionTimestampSchema.optional(), + cleanupAttemptToken: z.string().min(1).optional(), + cleanupKind: z.enum(["lifecycle", "sweep"]).optional(), + }) + .superRefine((record, context) => { + if (record.profileAliases) { + const canonical = [...new Set(record.profileAliases)].toSorted( + compareBrowserSessionTabProfileAliases, + ); + if ( + canonical.includes(record.profile) || + !canonical.every((entry, index) => entry === record.profileAliases?.[index]) + ) { + context.addIssue({ code: "custom", message: "profile aliases must be canonical" }); + } + } + const cleanupFieldCount = [ + record.cleanupRequestedAt, + record.cleanupAttemptToken, + record.cleanupKind, + ].filter((value) => value !== undefined).length; + if (cleanupFieldCount !== 0 && cleanupFieldCount !== 3) { + context.addIssue({ code: "custom", message: "cleanup fields must be all present or absent" }); + } + if (Object.hasOwn(record, "baseUrl") || Object.hasOwn(record, "interactionTargetId")) { + context.addIssue({ code: "custom", message: "retired browser tab fields are not allowed" }); + } + }); + +export type BrowserSessionTabRecord = z.infer; type BrowserSessionTabStoreRuntime = { state: Pick; @@ -92,73 +125,13 @@ export function browserSessionTabNativeIdentity( return `${record.sessionKey}\u0000${record.profile}\u0000${record.nativeTargetId}`; } -function isTimestamp(value: unknown): value is number { - return typeof value === "number" && Number.isFinite(value) && value >= 0; -} - export function compareBrowserSessionTabProfileAliases(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0; } -function isCanonicalProfileAliases( - value: unknown, - profile: unknown, -): value is string[] | undefined { - if (value === undefined) { - return true; - } - if ( - !Array.isArray(value) || - value.length === 0 || - value.some( - (entry) => typeof entry !== "string" || !entry || entry !== entry.trim().toLowerCase(), - ) - ) { - return false; - } - const canonical = [...new Set(value)].toSorted(compareBrowserSessionTabProfileAliases); - return ( - !canonical.includes(String(profile)) && - canonical.every((entry, index) => entry === value[index]) - ); -} - export function parseBrowserSessionTabRecord(value: unknown): BrowserSessionTabRecord | undefined { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return undefined; - } - const record = value as Record; - const cleanupFieldsValid = - (record.cleanupRequestedAt === undefined && - record.cleanupAttemptToken === undefined && - record.cleanupKind === undefined) || - (isTimestamp(record.cleanupRequestedAt) && - typeof record.cleanupAttemptToken === "string" && - record.cleanupAttemptToken.length > 0 && - (record.cleanupKind === "lifecycle" || record.cleanupKind === "sweep")); - if ( - record.version !== 1 || - typeof record.sessionKey !== "string" || - !record.sessionKey || - typeof record.nativeTargetId !== "string" || - !record.nativeTargetId || - typeof record.profile !== "string" || - !record.profile || - !isCanonicalProfileAliases(record.profileAliases, record.profile) || - typeof record.profileFingerprint !== "string" || - !record.profileFingerprint || - typeof record.browserInstanceFingerprint !== "string" || - !record.browserInstanceFingerprint || - (record.interactionTargetKind !== "native" && record.interactionTargetKind !== "opaque") || - !isTimestamp(record.trackedAt) || - !isTimestamp(record.lastUsedAt) || - !cleanupFieldsValid || - Object.hasOwn(record, "baseUrl") || - Object.hasOwn(record, "interactionTargetId") - ) { - return undefined; - } - return record as BrowserSessionTabRecord; + const parsed = browserSessionTabRecordSchema.safeParse(value); + return parsed.success ? parsed.data : undefined; } export function sameBrowserSessionTabRecord( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 144112b452e8..f6f1b513dffd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -538,6 +538,9 @@ importers: ws: specifier: 8.21.1 version: 8.21.1 + zod: + specifier: 4.4.3 + version: 4.4.3 devDependencies: '@openclaw/plugin-sdk': specifier: workspace:* diff --git a/src/agents/sessions/session-manager-codec.test.ts b/src/agents/sessions/session-manager-codec.test.ts index 67332a6d7b32..6cc0f0da89f4 100644 --- a/src/agents/sessions/session-manager-codec.test.ts +++ b/src/agents/sessions/session-manager-codec.test.ts @@ -1,4 +1,9 @@ import { describe, expect, it } from "vitest"; +import { + isIndexedSessionEntry, + parseOpaqueLeafEntry, + parseParentLinkedOpaqueEntry, +} from "./session-manager-codec.js"; import { CURRENT_SESSION_VERSION, SessionManager } from "./session-manager.js"; describe("session manager codec compatibility", () => { @@ -24,4 +29,36 @@ describe("session manager codec compatibility", () => { message: { role: "custom", customType: "hook", content: "persisted hook context" }, }); }); + + it.each([ + { + name: "message with malformed content", + entry: { type: "message", id: "m1", parentId: null, message: { role: "user" } }, + }, + { + name: "compaction without a kept entry", + entry: { type: "compaction", id: "c1", parentId: null, summary: "", tokensBefore: 1 }, + }, + { + name: "partial model change", + entry: { type: "model_change", id: "model1", parentId: null, provider: "openai" }, + }, + ])("rejects an indexed $name", ({ entry }) => { + expect(isIndexedSessionEntry(entry)).toBe(false); + }); + + it("parses opaque tree links without widening their variants", () => { + expect(parseParentLinkedOpaqueEntry({ type: "future", id: "f1", parentId: null })).toEqual({ + id: "f1", + parentId: null, + }); + expect(parseParentLinkedOpaqueEntry({ id: "untyped", parentId: "f1" })).toEqual({ + id: "untyped", + parentId: "f1", + }); + expect( + parseOpaqueLeafEntry({ type: "leaf", id: "leaf1", parentId: null, targetId: null }), + ).toEqual({ id: "leaf1", parentId: null, targetId: null }); + expect(parseOpaqueLeafEntry({ type: "leaf", id: "leaf1", parentId: null })).toBeUndefined(); + }); }); diff --git a/src/agents/sessions/session-manager-codec.ts b/src/agents/sessions/session-manager-codec.ts index 47ec25e40005..0a45e2794293 100644 --- a/src/agents/sessions/session-manager-codec.ts +++ b/src/agents/sessions/session-manager-codec.ts @@ -1,5 +1,6 @@ import { stripCompactionReplayCheckpointInPlace } from "@openclaw/ai/transports"; import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { z } from "zod"; import { selectSessionTranscriptLeafControlledPath } from "../../config/sessions/transcript-tree.js"; import { CURRENT_SESSION_VERSION } from "../../config/sessions/version.js"; import { logWarn } from "../../logger.js"; @@ -16,6 +17,123 @@ import type { SessionHeader, } from "./session-manager-types.js"; +const sessionEntryTypeSchema = z.enum([ + "message", + "thinking_level_change", + "model_change", + "compaction", + "reset", + "branch_summary", + "custom", + "custom_message", + "label", + "session_info", +]); +const readableContentSchema = z.union([z.string(), z.array(z.looseObject({ type: z.string() }))]); +const readableMessageSchema = z.discriminatedUnion("role", [ + z.looseObject({ role: z.literal("user"), content: readableContentSchema }), + z.looseObject({ role: z.literal("assistant"), content: readableContentSchema }), + z.looseObject({ + role: z.literal("toolResult"), + toolCallId: z.string(), + toolName: z.string(), + isError: z.boolean(), + content: z.array(z.unknown()), + }), + z.looseObject({ + role: z.literal("custom"), + customType: z.string(), + content: readableContentSchema, + }), + z.looseObject({ + role: z.literal("bashExecution"), + command: z.string(), + output: z.string(), + }), +]); +const indexedSessionEntryBaseShape = { + id: z.string().min(1), + parentId: z.union([z.string(), z.null()]).optional(), + timestamp: z.string().optional(), +}; +const indexedSessionEntrySchema = z.discriminatedUnion("type", [ + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("message"), + message: readableMessageSchema, + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("thinking_level_change"), + thinkingLevel: z.string().min(1), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("model_change"), + provider: z.string().min(1), + modelId: z.string().min(1), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("compaction"), + summary: z.string(), + firstKeptEntryId: z.string().min(1), + tokensBefore: z.custom((value) => typeof value === "number"), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("reset"), + reason: z.enum(["new", "reset", "idle", "daily", "cron-stale"]), + firstKeptEntryId: z.string().optional(), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("branch_summary"), + fromId: z.string(), + summary: z.string(), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("custom"), + customType: z.string().min(1), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("custom_message"), + customType: z.string().min(1), + content: readableContentSchema, + display: z.boolean(), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("label"), + targetId: z.string().min(1), + label: z.string().optional(), + }), + z.looseObject({ + ...indexedSessionEntryBaseShape, + type: z.literal("session_info"), + name: z.string().optional(), + }), +]); +const parentLinkedOpaqueEntrySchema = z.looseObject({ + type: z + .unknown() + .optional() + .refine((type) => type !== "session" && type !== "leaf"), + id: z.string().min(1), + parentId: z.union([z.string(), z.null()]), +}); +const opaqueLeafEntrySchema = z.looseObject({ + type: z.literal("leaf"), + id: z.string().min(1), + parentId: z.union([z.string(), z.null()]), + targetId: z.union([z.string(), z.null()]), + appendParentId: z.union([z.string(), z.null()]).optional(), + appendMode: z.literal("side").optional(), +}); +const sessionHeaderSchema = z.looseObject({ type: z.literal("session"), id: z.string() }); + export function isSessionContextMetadataEntry(entry: SessionEntry): boolean { return ( entry.type === "thinking_level_change" || @@ -205,113 +323,19 @@ export function normalizeLoadedFileEntry(entry: FileEntry): FileEntry { } function isSessionEntryType(type: unknown): boolean { - switch (type) { - case "message": - case "thinking_level_change": - case "model_change": - case "compaction": - case "reset": - case "branch_summary": - case "custom": - case "custom_message": - case "label": - case "session_info": - return true; - default: - return false; - } + return sessionEntryTypeSchema.safeParse(type).success; } export function isIndexedSessionEntry(entry: unknown): entry is SessionEntry { - if ( - !isRecord(entry) || - !isSessionEntryType(entry.type) || - typeof entry.id !== "string" || - entry.id.length === 0 || - (entry.parentId !== undefined && - entry.parentId !== null && - typeof entry.parentId !== "string") || - (entry.timestamp !== undefined && typeof entry.timestamp !== "string") - ) { - return false; - } - switch (entry.type) { - case "message": - return isReadableMessage(entry.message); - case "thinking_level_change": - return typeof entry.thinkingLevel === "string" && entry.thinkingLevel.length > 0; - case "model_change": - return ( - typeof entry.provider === "string" && - entry.provider.length > 0 && - typeof entry.modelId === "string" && - entry.modelId.length > 0 - ); - case "compaction": - return ( - typeof entry.summary === "string" && - typeof entry.firstKeptEntryId === "string" && - entry.firstKeptEntryId.length > 0 && - typeof entry.tokensBefore === "number" - ); - case "reset": - return ( - ["new", "reset", "idle", "daily", "cron-stale"].includes(String(entry.reason)) && - (entry.firstKeptEntryId === undefined || typeof entry.firstKeptEntryId === "string") - ); - case "branch_summary": - return typeof entry.fromId === "string" && typeof entry.summary === "string"; - case "custom": - return typeof entry.customType === "string" && entry.customType.length > 0; - case "custom_message": - return ( - typeof entry.customType === "string" && - entry.customType.length > 0 && - isReadableContent(entry.content) && - typeof entry.display === "boolean" - ); - case "label": - return ( - typeof entry.targetId === "string" && - entry.targetId.length > 0 && - (entry.label === undefined || typeof entry.label === "string") - ); - case "session_info": - return entry.name === undefined || typeof entry.name === "string"; - default: - return false; - } + return indexedSessionEntrySchema.safeParse(entry).success; } function isReadableContent(value: unknown): boolean { - return ( - typeof value === "string" || - (Array.isArray(value) && value.every((part) => isRecord(part) && typeof part.type === "string")) - ); + return readableContentSchema.safeParse(value).success; } function isReadableMessage(value: unknown): boolean { - if (!isRecord(value) || typeof value.role !== "string") { - return false; - } - switch (value.role) { - case "user": - case "assistant": - return isReadableContent(value.content); - case "toolResult": - return ( - typeof value.toolCallId === "string" && - typeof value.toolName === "string" && - typeof value.isError === "boolean" && - Array.isArray(value.content) - ); - case "custom": - return typeof value.customType === "string" && isReadableContent(value.content); - case "bashExecution": - return typeof value.command === "string" && typeof value.output === "string"; - default: - return false; - } + return readableMessageSchema.safeParse(value).success; } function isReadableLegacySessionEntry(value: unknown): value is FileEntry { @@ -345,17 +369,8 @@ function normalizePersistedLegacyHookMessage(value: unknown): unknown { export function parseParentLinkedOpaqueEntry( record: unknown, ): { id: string; parentId: string | null } | undefined { - if ( - !isRecord(record) || - record.type === "session" || - record.type === "leaf" || - typeof record.id !== "string" || - record.id.length === 0 || - (record.parentId !== null && typeof record.parentId !== "string") - ) { - return undefined; - } - return { id: record.id, parentId: record.parentId }; + const parsed = parentLinkedOpaqueEntrySchema.safeParse(record); + return parsed.success ? { id: parsed.data.id, parentId: parsed.data.parentId } : undefined; } export function parseOpaqueLeafEntry(record: unknown): @@ -367,26 +382,17 @@ export function parseOpaqueLeafEntry(record: unknown): appendMode?: "side"; } | undefined { - if ( - !isRecord(record) || - record.type !== "leaf" || - typeof record.id !== "string" || - record.id.length === 0 || - (record.parentId !== null && typeof record.parentId !== "string") || - (record.targetId !== null && typeof record.targetId !== "string") || - (record.appendParentId !== undefined && - record.appendParentId !== null && - typeof record.appendParentId !== "string") || - (record.appendMode !== undefined && record.appendMode !== "side") - ) { + const parsed = opaqueLeafEntrySchema.safeParse(record); + if (!parsed.success) { return undefined; } + const leaf = parsed.data; return { - id: record.id, - parentId: record.parentId, - targetId: record.targetId, - ...(record.appendParentId !== undefined ? { appendParentId: record.appendParentId } : {}), - ...(record.appendMode === "side" ? { appendMode: record.appendMode } : {}), + id: leaf.id, + parentId: leaf.parentId, + targetId: leaf.targetId, + ...(leaf.appendParentId !== undefined ? { appendParentId: leaf.appendParentId } : {}), + ...(leaf.appendMode === "side" ? { appendMode: leaf.appendMode } : {}), }; } @@ -398,14 +404,14 @@ export function partitionSessionFileEntries(entries: readonly FileEntry[]): { const fileEntries: FileEntry[] = []; const opaqueEntries: Array<{ index: number; record: unknown }> = []; const fileEntriesByOriginalIndex: Array = []; - const header = entries.find( - (entry) => isRecord(entry) && entry.type === "session" && typeof entry.id === "string", - ) as SessionHeader | undefined; + const header = entries.find((entry) => sessionHeaderSchema.safeParse(entry).success) as + | SessionHeader + | undefined; const acceptsLegacyEntries = (header?.version ?? 1) < CURRENT_SESSION_VERSION; let hasHeader = false; for (const [originalIndex, rawEntry] of entries.entries()) { const entry = normalizePersistedLegacyHookMessage(rawEntry) as FileEntry; - if (!hasHeader && isRecord(entry) && entry.type === "session" && typeof entry.id === "string") { + if (!hasHeader && sessionHeaderSchema.safeParse(entry).success) { fileEntries.push(entry); fileEntriesByOriginalIndex[originalIndex] = entry; hasHeader = true; diff --git a/src/cron/task-run-detail.ts b/src/cron/task-run-detail.ts index 41706c064fc6..6ada3b634fb7 100644 --- a/src/cron/task-run-detail.ts +++ b/src/cron/task-run-detail.ts @@ -7,6 +7,7 @@ import { } from "@openclaw/normalization-core/number-coercion"; import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { z } from "zod"; import { FAILOVER_REASONS, type FailoverReason, @@ -23,6 +24,88 @@ type CronRunStatus = import("./types.js").CronRunStatus; const CRON_TASK_DETAIL_KIND = "cron-run"; const CRON_FAILOVER_REASONS = new Set(FAILOVER_REASONS); +const cronRunStatusSchema = z.enum(["ok", "error", "skipped"]); +const cronDeliveryStatusSchema = z.enum(["delivered", "not-delivered", "unknown", "not-requested"]); +const optionalCronStringSchema = z.string().optional().catch(undefined); +const optionalNonBlankCronStringSchema = z + .string() + .refine((value) => value.trim().length > 0) + .optional() + .catch(undefined); +const optionalCronTimestampSchema = z + .unknown() + .optional() + .transform((value) => normalizeTimestamp(value)); +const optionalCronDurationSchema = z + .unknown() + .optional() + .transform((value) => asSafeIntegerInRange(value, { min: 0 })); +const optionalCronTokenCountSchema = z + .unknown() + .optional() + .transform((value) => asSafeIntegerInRange(value, { min: 0 })); +const cronUsageSchema = z + .looseObject({ + input_tokens: optionalCronTokenCountSchema, + output_tokens: optionalCronTokenCountSchema, + total_tokens: optionalCronTokenCountSchema, + cache_read_tokens: optionalCronTokenCountSchema, + cache_write_tokens: optionalCronTokenCountSchema, + }) + .transform((usage) => + Object.values(usage).some((tokenCount) => tokenCount !== undefined) ? usage : undefined, + ) + .optional() + .catch(undefined); +const cronFailureNotificationDeliverySchema = z + .looseObject({ + status: cronDeliveryStatusSchema, + delivered: z.boolean().optional().catch(undefined), + error: optionalCronStringSchema, + }) + .transform(({ status, delivered, error }) => ({ + status, + ...(delivered !== undefined ? { delivered } : {}), + ...(error !== undefined ? { error } : {}), + })) + .optional() + .catch(undefined); +const cronRunLogEntrySchema = z.looseObject({ + action: z.literal("finished"), + jobId: z.string().refine((value) => value.trim().length > 0), + ts: z + .unknown() + .transform((value) => normalizeTimestamp(value)) + .pipe(z.number()), + status: cronRunStatusSchema.optional().catch(undefined), + error: optionalCronStringSchema, + errorReason: z + .custom( + (value) => typeof value === "string" && CRON_FAILOVER_REASONS.has(value as FailoverReason), + ) + .optional() + .catch(undefined), + summary: optionalCronStringSchema, + runId: optionalNonBlankCronStringSchema, + diagnostics: z.unknown().optional(), + runAtMs: optionalCronTimestampSchema, + durationMs: optionalCronDurationSchema, + nextRunAtMs: optionalCronTimestampSchema, + triggerFired: z + .unknown() + .optional() + .transform((value) => (value === true ? true : undefined)), + model: optionalNonBlankCronStringSchema, + provider: optionalNonBlankCronStringSchema, + usage: cronUsageSchema, + delivered: z.boolean().optional().catch(undefined), + deliveryStatus: cronDeliveryStatusSchema.optional().catch(undefined), + deliveryError: optionalCronStringSchema, + failureNotificationDelivery: cronFailureNotificationDeliverySchema, + delivery: z.custom<{ [key: string]: JsonValue }>(isJsonObject).optional().catch(undefined), + sessionId: optionalNonBlankCronStringSchema, + sessionKey: optionalNonBlankCronStringSchema, +}); function toJsonValue(value: unknown): JsonValue | undefined { const serialized = JSON.stringify(value); @@ -38,33 +121,11 @@ function normalizeTimestamp(value: unknown): number | undefined { } export function isCronRunStatus(value: unknown): value is CronRunStatus { - return value === "ok" || value === "error" || value === "skipped"; + return cronRunStatusSchema.safeParse(value).success; } export function isCronDeliveryStatus(value: unknown): value is CronDeliveryStatus { - return ["delivered", "not-delivered", "unknown", "not-requested"].includes( - value as CronDeliveryStatus, - ); -} - -function normalizeUsage(value: unknown): CronRunLogEntry["usage"] { - if (!isJsonObject(value)) { - return undefined; - } - const usage = { - input_tokens: asSafeIntegerInRange(value.input_tokens, { min: 0 }), - output_tokens: asSafeIntegerInRange(value.output_tokens, { min: 0 }), - total_tokens: asSafeIntegerInRange(value.total_tokens, { min: 0 }), - cache_read_tokens: asSafeIntegerInRange(value.cache_read_tokens, { min: 0 }), - cache_write_tokens: asSafeIntegerInRange(value.cache_write_tokens, { min: 0 }), - }; - return Object.values(usage).some((tokenCount) => tokenCount !== undefined) ? usage : undefined; -} - -function normalizeCronRunLogErrorReason(value: unknown): FailoverReason | undefined { - return typeof value === "string" && CRON_FAILOVER_REASONS.has(value as FailoverReason) - ? (value as FailoverReason) - : undefined; + return cronDeliveryStatusSchema.safeParse(value).success; } /** Parses stored or migrated cron history while preserving the stable wire shape. */ @@ -73,85 +134,53 @@ export function parseCronRunLogEntryObject( opts?: { jobId?: string }, ): CronRunLogEntry | null { const jobId = normalizeOptionalString(opts?.jobId); - if (!obj || typeof obj !== "object") { - return null; - } - const entryObj = obj as Partial; - if (entryObj.action !== "finished") { - return null; - } - if (typeof entryObj.jobId !== "string" || entryObj.jobId.trim().length === 0) { - return null; - } - const ts = normalizeTimestamp(entryObj.ts); - if (ts === undefined) { + const parsed = cronRunLogEntrySchema.safeParse(obj); + if (!parsed.success) { return null; } + const entryObj = parsed.data; if (jobId && entryObj.jobId !== jobId) { return null; } - const normalizedError = typeof entryObj.error === "string" ? entryObj.error : undefined; - const normalizedProvider = - typeof entryObj.provider === "string" && entryObj.provider.trim() - ? entryObj.provider - : undefined; // Diagnostics are redacted at authoring; this read/migration path only normalizes stored shape. const entry: CronRunLogEntry = { - ts, + ts: entryObj.ts, jobId: entryObj.jobId, action: "finished", - status: isCronRunStatus(entryObj.status) ? entryObj.status : undefined, - error: normalizedError, - errorReason: normalizeCronRunLogErrorReason(entryObj.errorReason) ?? undefined, - summary: typeof entryObj.summary === "string" ? entryObj.summary : undefined, - runId: typeof entryObj.runId === "string" && entryObj.runId.trim() ? entryObj.runId : undefined, + status: entryObj.status, + error: entryObj.error, + errorReason: entryObj.errorReason, + summary: entryObj.summary, + runId: entryObj.runId, diagnostics: normalizeCronRunDiagnosticsCore(entryObj.diagnostics), - runAtMs: normalizeTimestamp(entryObj.runAtMs), - durationMs: asSafeIntegerInRange(entryObj.durationMs, { min: 0 }), - nextRunAtMs: normalizeTimestamp(entryObj.nextRunAtMs), - triggerFired: entryObj.triggerFired === true ? true : undefined, - model: typeof entryObj.model === "string" && entryObj.model.trim() ? entryObj.model : undefined, - provider: normalizedProvider, - usage: normalizeUsage(entryObj.usage), + runAtMs: entryObj.runAtMs, + durationMs: entryObj.durationMs, + nextRunAtMs: entryObj.nextRunAtMs, + triggerFired: entryObj.triggerFired, + model: entryObj.model, + provider: entryObj.provider, + usage: entryObj.usage, }; - if (typeof entryObj.delivered === "boolean") { + if (entryObj.delivered !== undefined) { entry.delivered = entryObj.delivered; } - if (isCronDeliveryStatus(entryObj.deliveryStatus)) { + if (entryObj.deliveryStatus !== undefined) { entry.deliveryStatus = entryObj.deliveryStatus; } - if (typeof entryObj.deliveryError === "string") { + if (entryObj.deliveryError !== undefined) { entry.deliveryError = entryObj.deliveryError; } - if ( - entryObj.failureNotificationDelivery && - typeof entryObj.failureNotificationDelivery === "object" - ) { - const failureNotificationDelivery = entryObj.failureNotificationDelivery as { - delivered?: unknown; - status?: unknown; - error?: unknown; - }; - if (isCronDeliveryStatus(failureNotificationDelivery.status)) { - entry.failureNotificationDelivery = { - status: failureNotificationDelivery.status, - ...(typeof failureNotificationDelivery.delivered === "boolean" - ? { delivered: failureNotificationDelivery.delivered } - : {}), - ...(typeof failureNotificationDelivery.error === "string" - ? { error: failureNotificationDelivery.error } - : {}), - }; - } + if (entryObj.failureNotificationDelivery !== undefined) { + entry.failureNotificationDelivery = entryObj.failureNotificationDelivery; } - if (isJsonObject(entryObj.delivery)) { + if (entryObj.delivery !== undefined) { entry.delivery = entryObj.delivery; } - if (typeof entryObj.sessionId === "string" && entryObj.sessionId.trim()) { + if (entryObj.sessionId !== undefined) { entry.sessionId = entryObj.sessionId; } - if (typeof entryObj.sessionKey === "string" && entryObj.sessionKey.trim()) { + if (entryObj.sessionKey !== undefined) { entry.sessionKey = entryObj.sessionKey; } return entry; diff --git a/src/infra/exec-approvals-config.test.ts b/src/infra/exec-approvals-config.test.ts index ed90840d833a..bea31a8bac9a 100644 --- a/src/infra/exec-approvals-config.test.ts +++ b/src/infra/exec-approvals-config.test.ts @@ -2,6 +2,7 @@ import { existsSync } from "node:fs"; import { describe, expect, it } from "vitest"; import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { tryParsePersistedExecApprovals } from "./exec-approvals-config.js"; import { makeExecApprovalsTempDir } from "./exec-approvals-test-helpers.js"; import { isSafeBinUsage, @@ -173,6 +174,36 @@ describe("exec approvals default agent migration", () => { }); }); +describe("persisted exec approvals schema", () => { + it("keeps legacy string allowlist entries while normalizing them", () => { + const parsed = tryParsePersistedExecApprovals( + JSON.stringify({ + version: 1, + agents: { main: { allowlist: ["ls", { pattern: "cat", source: "legacy" }] } }, + }), + ); + expect(parsed?.agents?.main?.allowlist?.[0]).toMatchObject({ pattern: "ls" }); + expect(parsed?.agents?.main?.allowlist?.[1]).toEqual( + expect.objectContaining({ pattern: "cat", source: undefined }), + ); + }); + + it.each([ + { name: "version", value: { version: 2 } }, + { name: "socket token", value: { version: 1, socket: { token: 42 } } }, + { name: "policy enum", value: { version: 1, defaults: { security: "none" } } }, + { + name: "allowlist metadata", + value: { + version: 1, + agents: { main: { allowlist: [{ pattern: "ls", lastUsedAt: "now" }] } }, + }, + }, + ])("rejects invalid persisted $name", ({ value }) => { + expect(tryParsePersistedExecApprovals(JSON.stringify(value))).toBeNull(); + }); +}); + describe("exec approvals invalid explicit policy fallback", () => { it("treats invalid explicit agent fields as masked and falls back to defaults instead of wildcard", () => { const resolved = resolveExecApprovalsFromFile({ diff --git a/src/infra/exec-approvals-config.ts b/src/infra/exec-approvals-config.ts index a48589f972be..51e32d1892cc 100644 --- a/src/infra/exec-approvals-config.ts +++ b/src/infra/exec-approvals-config.ts @@ -6,6 +6,7 @@ import { normalizeOptionalString, readStringValue, } from "@openclaw/normalization-core/string-coerce"; +import { z } from "zod"; import { DEFAULT_AGENT_ID } from "../routing/session-key.js"; import type { ExecApprovalsAgent, @@ -16,17 +17,51 @@ import type { } from "./exec-approvals-core.js"; import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; import { expandHomePrefix, resolveHomeRelativePath } from "./home-dir.js"; -import { isPlainObject } from "./plain-object.js"; const toStringOrUndefined = readStringValue; -function isExecSecurity(value: unknown): value is ExecSecurity { - return value === "allowlist" || value === "full" || value === "deny"; -} - -function isExecAsk(value: unknown): value is ExecAsk { - return value === "always" || value === "off" || value === "on-miss"; +const execSecuritySchema = z.enum(["allowlist", "full", "deny"]); +const execAskSchema = z.enum(["always", "off", "on-miss"]); +const persistedExecApprovalPolicySchema = z.looseObject({ + security: execSecuritySchema.optional(), + ask: execAskSchema.optional(), + askFallback: execSecuritySchema.optional(), + autoAllowSkills: z.boolean().optional(), +}); +function normalizePersistedAllowlistSource(value: string): "allow-always" | undefined { + return value === "allow-always" ? value : undefined; } +const persistedExecAllowlistEntrySchema = z + .union([ + z.string().refine((value) => value.trim().length > 0), + z.looseObject({ + pattern: z.string().refine((value) => value.trim().length > 0), + id: z.string().optional(), + source: z.string().transform(normalizePersistedAllowlistSource).optional(), + commandText: z.string().optional(), + argPattern: z.string().optional(), + lastUsedAt: z.number().finite().optional(), + lastUsedCommand: z.string().optional(), + lastResolvedPath: z.string().optional(), + }), + ]) + .transform( + (value): ExecAllowlistEntry => (typeof value === "string" ? { pattern: value } : value), + ); +const persistedExecApprovalsAgentSchema = persistedExecApprovalPolicySchema.extend({ + allowlist: z.array(persistedExecAllowlistEntrySchema).optional(), +}); +const persistedExecApprovalsSchema = z.looseObject({ + version: z.literal(1), + socket: z + .looseObject({ + path: z.string().optional(), + token: z.string().optional(), + }) + .optional(), + defaults: persistedExecApprovalPolicySchema.optional(), + agents: z.record(z.string(), persistedExecApprovalsAgentSchema).optional(), +}); export const DEFAULT_SECURITY: ExecSecurity = "full"; export const DEFAULT_ASK: ExecAsk = "off"; @@ -88,77 +123,13 @@ export function createFailClosedExecApprovalsFallback(): ExecApprovalsFile { }); } -function hasValidExecApprovalPolicyFields(value: unknown): value is Record { - if (!isPlainObject(value)) { - return false; - } - return ( - (value.security === undefined || isExecSecurity(value.security)) && - (value.ask === undefined || isExecAsk(value.ask)) && - (value.askFallback === undefined || isExecSecurity(value.askFallback)) && - (value.autoAllowSkills === undefined || typeof value.autoAllowSkills === "boolean") - ); -} - -function isValidPersistedExecAllowlistEntry(value: unknown): boolean { - if (typeof value === "string") { - return value.trim().length > 0; - } - if (!isPlainObject(value) || typeof value.pattern !== "string" || !value.pattern.trim()) { - return false; - } - return ( - (value.id === undefined || typeof value.id === "string") && - (value.source === undefined || typeof value.source === "string") && - (value.commandText === undefined || typeof value.commandText === "string") && - (value.argPattern === undefined || typeof value.argPattern === "string") && - (value.lastUsedAt === undefined || - (typeof value.lastUsedAt === "number" && Number.isFinite(value.lastUsedAt))) && - (value.lastUsedCommand === undefined || typeof value.lastUsedCommand === "string") && - (value.lastResolvedPath === undefined || typeof value.lastResolvedPath === "string") - ); -} - -function isValidPersistedExecApprovals(value: unknown): value is ExecApprovalsFile { - if (!isPlainObject(value) || value.version !== 1) { - return false; - } - if (value.socket !== undefined) { - if ( - !isPlainObject(value.socket) || - (value.socket.path !== undefined && typeof value.socket.path !== "string") || - (value.socket.token !== undefined && typeof value.socket.token !== "string") - ) { - return false; - } - } - if (value.defaults !== undefined && !hasValidExecApprovalPolicyFields(value.defaults)) { - return false; - } - if (value.agents !== undefined) { - if (!isPlainObject(value.agents)) { - return false; - } - for (const agent of Object.values(value.agents)) { - if ( - !hasValidExecApprovalPolicyFields(agent) || - (agent.allowlist !== undefined && - (!Array.isArray(agent.allowlist) || - !agent.allowlist.every(isValidPersistedExecAllowlistEntry))) - ) { - return false; - } - } - } - return true; -} - /** Parse only structurally valid persisted approvals without inventing fallback policy. */ export function tryParsePersistedExecApprovals(raw: string): ExecApprovalsFile | null { try { const parsed = JSON.parse(raw) as unknown; - if (isValidPersistedExecApprovals(parsed)) { - return normalizeExecApprovalsInternal(parsed); + const result = persistedExecApprovalsSchema.safeParse(parsed); + if (result.success) { + return normalizeExecApprovalsInternal(result.data); } } catch { // A partial Windows fallback write is existing state, not a missing policy. diff --git a/src/infra/state-migrations.acp-replay.ts b/src/infra/state-migrations.acp-replay.ts index 6af77142f6d2..9a32ec0c38d4 100644 --- a/src/infra/state-migrations.acp-replay.ts +++ b/src/infra/state-migrations.acp-replay.ts @@ -6,9 +6,9 @@ import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import { isDeepStrictEqual } from "node:util"; import type { SessionUpdate } from "@agentclientprotocol/sdk"; +import { z } from "zod"; import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js"; import { runOpenClawStateWriteTransaction } from "../state/openclaw-state-db.js"; -import { isRecord } from "../utils.js"; import { withFileLock } from "./file-lock.js"; import { executeSqliteQuerySync, @@ -63,6 +63,36 @@ type AcpReplayMigrationDatabase = Pick< "acp_replay_events" | "acp_replay_sessions" >; +const legacyAcpReplayUpdateSchema = z.looseObject({ sessionUpdate: z.string() }); +const legacyAcpReplayEventSchema = z.looseObject({ + seq: z + .number() + .refine(Number.isInteger) + .refine((value) => value >= 1), + at: z.number().finite(), + sessionId: z.string(), + sessionKey: z.string(), + runId: z.unknown().optional(), + update: legacyAcpReplayUpdateSchema, +}); +const legacyAcpReplaySessionSchema = z.looseObject({ + sessionId: z.string(), + sessionKey: z.string(), + cwd: z.string(), + complete: z.boolean(), + createdAt: z.number().finite(), + updatedAt: z.number().finite(), + nextSeq: z + .number() + .refine(Number.isInteger) + .refine((value) => value >= 1), + events: z.array(z.unknown()), +}); +const legacyAcpReplayLedgerSchema = z.looseObject({ + version: z.literal(LEGACY_LEDGER_VERSION), + sessions: z.record(z.string(), z.unknown()), +}); + function resolveLegacyAcpReplayLedgerPath(stateDir: string): string { return path.join(stateDir, "acp", "event-ledger.json"); } @@ -87,76 +117,54 @@ export function detectLegacyAcpReplayLedger(params: { } function parseLegacyEvent(raw: unknown, sessionId: string): LegacyAcpReplayEvent { - if (!isRecord(raw) || !isRecord(raw.update)) { + const parsed = legacyAcpReplayEventSchema.safeParse(raw); + if (!parsed.success || parsed.data.sessionId !== sessionId) { throw new Error(`legacy ACP replay session ${sessionId} contains an invalid event`); } - if ( - typeof raw.seq !== "number" || - !Number.isInteger(raw.seq) || - raw.seq < 1 || - typeof raw.at !== "number" || - !Number.isFinite(raw.at) || - raw.sessionId !== sessionId || - typeof raw.sessionKey !== "string" || - typeof raw.update.sessionUpdate !== "string" - ) { - throw new Error(`legacy ACP replay session ${sessionId} contains an invalid event`); - } - if (raw.runId !== undefined && (typeof raw.runId !== "string" || raw.runId.length === 0)) { + const event = parsed.data; + if (event.runId !== undefined && (typeof event.runId !== "string" || event.runId.length === 0)) { throw new Error(`legacy ACP replay session ${sessionId} contains an invalid run id`); } return { - seq: raw.seq, - at: raw.at, + seq: event.seq, + at: event.at, sessionId, - sessionKey: raw.sessionKey, - ...(typeof raw.runId === "string" ? { runId: raw.runId } : {}), - update: structuredClone(raw.update) as SessionUpdate, + sessionKey: event.sessionKey, + ...(typeof event.runId === "string" ? { runId: event.runId } : {}), + update: structuredClone(event.update) as SessionUpdate, }; } function parseLegacySession(raw: unknown, expectedSessionId: string): LegacyAcpReplaySession { - if ( - !isRecord(raw) || - raw.sessionId !== expectedSessionId || - typeof raw.sessionKey !== "string" || - typeof raw.cwd !== "string" || - typeof raw.complete !== "boolean" || - typeof raw.createdAt !== "number" || - !Number.isFinite(raw.createdAt) || - typeof raw.updatedAt !== "number" || - !Number.isFinite(raw.updatedAt) || - typeof raw.nextSeq !== "number" || - !Number.isInteger(raw.nextSeq) || - raw.nextSeq < 1 || - !Array.isArray(raw.events) - ) { + const parsed = legacyAcpReplaySessionSchema.safeParse(raw); + if (!parsed.success || parsed.data.sessionId !== expectedSessionId) { throw new Error(`legacy ACP replay session ${expectedSessionId} is invalid`); } - const events = raw.events.map((event) => parseLegacyEvent(event, expectedSessionId)); + const session = parsed.data; + const events = session.events.map((event) => parseLegacyEvent(event, expectedSessionId)); const sequences = new Set(events.map((event) => event.seq)); const maxSeq = events.reduce((max, event) => Math.max(max, event.seq), 0); - if (sequences.size !== events.length || raw.nextSeq <= maxSeq) { + if (sequences.size !== events.length || session.nextSeq <= maxSeq) { throw new Error(`legacy ACP replay session ${expectedSessionId} has invalid sequencing`); } return { sessionId: expectedSessionId, - sessionKey: raw.sessionKey, - cwd: raw.cwd, - complete: raw.complete, - createdAt: raw.createdAt, - updatedAt: raw.updatedAt, - nextSeq: raw.nextSeq, + sessionKey: session.sessionKey, + cwd: session.cwd, + complete: session.complete, + createdAt: session.createdAt, + updatedAt: session.updatedAt, + nextSeq: session.nextSeq, events: events.toSorted((left, right) => left.seq - right.seq), }; } function parseLegacyLedger(raw: string): LegacyAcpReplaySession[] { - const parsed = JSON.parse(raw) as unknown; - if (!isRecord(parsed) || parsed.version !== LEGACY_LEDGER_VERSION || !isRecord(parsed.sessions)) { + const parsed = legacyAcpReplayLedgerSchema.safeParse(JSON.parse(raw) as unknown); + if (!parsed.success) { throw new Error("legacy ACP replay ledger must be a version 1 JSON object"); } - return Object.entries(parsed.sessions).map(([sessionId, session]) => + return Object.entries(parsed.data.sessions).map(([sessionId, session]) => parseLegacySession(session, sessionId), ); } diff --git a/src/memory-host-sdk/event-store.ts b/src/memory-host-sdk/event-store.ts index a495644b3e62..bfa8cf4405b6 100644 --- a/src/memory-host-sdk/event-store.ts +++ b/src/memory-host-sdk/event-store.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { z } from "zod"; import { resolveWorkspaceStateIdentity } from "../agents/workspace-state-store.js"; import { pluginStateEntriesInKeyRange, @@ -35,6 +35,95 @@ export type PersistedMemoryHostEvent = { createdAt: number; }; +const memoryHostFiniteNumberSchema = z.number().finite(); +const memoryHostRecallResultSchema = z.looseObject({ + path: z.string(), + startLine: memoryHostFiniteNumberSchema, + endLine: memoryHostFiniteNumberSchema, + score: memoryHostFiniteNumberSchema, +}); +const memoryHostSkippedRecallResultSchema = memoryHostRecallResultSchema.extend({ + reason: z.literal("non-short-term-memory-path"), +}); +const boundedRecallResultsSchema = z.array(z.unknown()).transform((values, context) => { + const parsed = z + .array(memoryHostRecallResultSchema) + .safeParse(values.slice(0, MAX_MEMORY_HOST_EVENT_ITEMS)); + if (!parsed.success) { + context.addIssue({ code: "custom", message: "invalid recall result" }); + return z.NEVER; + } + return { items: parsed.data, truncated: values.length > MAX_MEMORY_HOST_EVENT_ITEMS }; +}); +const boundedSkippedRecallResultsSchema = z.array(z.unknown()).transform((values, context) => { + const parsed = z + .array(memoryHostSkippedRecallResultSchema) + .safeParse(values.slice(0, MAX_MEMORY_HOST_EVENT_ITEMS)); + if (!parsed.success) { + context.addIssue({ code: "custom", message: "invalid skipped recall result" }); + return z.NEVER; + } + return { items: parsed.data, truncated: values.length > MAX_MEMORY_HOST_EVENT_ITEMS }; +}); +const memoryHostPromotionCandidateSchema = z.looseObject({ + key: z.string(), + path: z.string(), + startLine: memoryHostFiniteNumberSchema, + endLine: memoryHostFiniteNumberSchema, + score: memoryHostFiniteNumberSchema, + recallCount: memoryHostFiniteNumberSchema, +}); +const boundedPromotionCandidatesSchema = z.array(z.unknown()).transform((values, context) => { + const parsed = z + .array(memoryHostPromotionCandidateSchema) + .safeParse(values.slice(0, MAX_MEMORY_HOST_EVENT_ITEMS)); + if (!parsed.success) { + context.addIssue({ code: "custom", message: "invalid promotion candidate" }); + return z.NEVER; + } + return { items: parsed.data, truncated: values.length > MAX_MEMORY_HOST_EVENT_ITEMS }; +}); +const memoryHostEventRecordSchema = z.discriminatedUnion("type", [ + z.looseObject({ + type: z.literal("memory.recall.recorded"), + timestamp: z.string(), + storageTruncated: z.unknown().optional(), + query: z.string(), + resultCount: memoryHostFiniteNumberSchema, + results: boundedRecallResultsSchema, + }), + z.looseObject({ + type: z.literal("memory.recall.skipped"), + timestamp: z.string(), + storageTruncated: z.unknown().optional(), + query: z.string(), + reason: z.literal("non-short-term-memory-path"), + eligibleResultCount: memoryHostFiniteNumberSchema, + skippedResultCount: memoryHostFiniteNumberSchema, + results: boundedSkippedRecallResultsSchema, + }), + z.looseObject({ + type: z.literal("memory.promotion.applied"), + timestamp: z.string(), + storageTruncated: z.unknown().optional(), + memoryPath: z.string(), + applied: memoryHostFiniteNumberSchema, + candidates: boundedPromotionCandidatesSchema, + }), + z.looseObject({ + type: z.literal("memory.dream.completed"), + timestamp: z.string(), + storageTruncated: z.unknown().optional(), + phase: z.enum(["light", "deep", "rem"]), + outcome: z.enum(["completed", "failed"]).optional(), + error: z.string().optional(), + inlinePath: z.string().optional(), + reportPath: z.string().optional(), + lineCount: memoryHostFiniteNumberSchema, + storageMode: z.enum(["inline", "separate", "both"]), + }), +]); + function normalizeMemoryHostWorkspaceKey(workspaceDir: string): string { // Workspace aliases must share one event/cursor namespace. Otherwise two // configured paths to the same workspace can publish conflicting exports. @@ -86,77 +175,41 @@ function truncateUtf8(value: string, maxBytes: number): { value: string; truncat return { value: `${value.slice(0, end)}…`, truncated: true }; } -function isFiniteNumber(value: unknown): value is number { - return typeof value === "number" && Number.isFinite(value); -} - /** Validate and bound one diagnostic event before storing it in plugin state. */ export function normalizeMemoryHostEventRecordForStorage( value: unknown, ): MemoryHostEventRecord | null { - if (!isRecord(value) || typeof value.type !== "string" || typeof value.timestamp !== "string") { + const parsed = memoryHostEventRecordSchema.safeParse(value); + if (!parsed.success) { return null; } - const timestamp = truncateUtf8(value.timestamp, 128); - let truncated = timestamp.truncated || value.storageTruncated === true; + const event = parsed.data; + const timestamp = truncateUtf8(event.timestamp, 128); + let truncated = timestamp.truncated || event.storageTruncated === true; - if (value.type === "memory.recall.recorded" || value.type === "memory.recall.skipped") { - if ( - typeof value.query !== "string" || - !Array.isArray(value.results) || - (value.type === "memory.recall.recorded" - ? !isFiniteNumber(value.resultCount) - : !isFiniteNumber(value.skippedResultCount)) - ) { - return null; - } - if ( - value.type === "memory.recall.skipped" && - (value.reason !== "non-short-term-memory-path" || - !isFiniteNumber(value.eligibleResultCount) || - !isFiniteNumber(value.skippedResultCount)) - ) { - return null; - } - const query = truncateUtf8(value.query, MAX_MEMORY_HOST_EVENT_TEXT_BYTES); - truncated ||= query.truncated || value.results.length > MAX_MEMORY_HOST_EVENT_ITEMS; - const results: Array<{ - path: string; - startLine: number; - endLine: number; - score: number; - reason?: "non-short-term-memory-path"; - }> = []; - for (const result of value.results.slice(0, MAX_MEMORY_HOST_EVENT_ITEMS)) { - if ( - !isRecord(result) || - typeof result.path !== "string" || - !isFiniteNumber(result.startLine) || - !isFiniteNumber(result.endLine) || - !isFiniteNumber(result.score) || - (value.type === "memory.recall.skipped" && result.reason !== "non-short-term-memory-path") - ) { - return null; - } + if (event.type === "memory.recall.recorded" || event.type === "memory.recall.skipped") { + const query = truncateUtf8(event.query, MAX_MEMORY_HOST_EVENT_TEXT_BYTES); + truncated ||= query.truncated || event.results.truncated; + const results = event.results.items.map((result) => { const resultPath = truncateUtf8(result.path, MAX_MEMORY_HOST_EVENT_PATH_BYTES); truncated ||= resultPath.truncated; - results.push({ + return { path: resultPath.value, startLine: result.startLine, endLine: result.endLine, score: result.score, - ...(value.type === "memory.recall.skipped" + ...(event.type === "memory.recall.skipped" ? { reason: "non-short-term-memory-path" as const } : {}), - }); - } + }; + }); const normalized = - value.type === "memory.recall.recorded" + event.type === "memory.recall.recorded" ? { type: "memory.recall.recorded" as const, timestamp: timestamp.value, query: query.value, - resultCount: value.resultCount as number, + resultCount: event.resultCount, results: results.map((result) => ({ path: result.path, startLine: result.startLine, @@ -170,8 +223,8 @@ export function normalizeMemoryHostEventRecordForStorage( timestamp: timestamp.value, query: query.value, reason: "non-short-term-memory-path" as const, - eligibleResultCount: value.eligibleResultCount as number, - skippedResultCount: value.skippedResultCount as number, + eligibleResultCount: event.eligibleResultCount, + skippedResultCount: event.skippedResultCount, results: results.map((result) => ({ path: result.path, startLine: result.startLine, @@ -186,53 +239,27 @@ export function normalizeMemoryHostEventRecordForStorage( : { ...normalized, results: [], storageTruncated: true as const }; } - if (value.type === "memory.promotion.applied") { - if ( - typeof value.memoryPath !== "string" || - !isFiniteNumber(value.applied) || - !Array.isArray(value.candidates) - ) { - return null; - } - const memoryPath = truncateUtf8(value.memoryPath, MAX_MEMORY_HOST_EVENT_PATH_BYTES); - truncated ||= memoryPath.truncated || value.candidates.length > MAX_MEMORY_HOST_EVENT_ITEMS; - const candidates: Array<{ - key: string; - path: string; - startLine: number; - endLine: number; - score: number; - recallCount: number; - }> = []; - for (const candidate of value.candidates.slice(0, MAX_MEMORY_HOST_EVENT_ITEMS)) { - if ( - !isRecord(candidate) || - typeof candidate.key !== "string" || - typeof candidate.path !== "string" || - !isFiniteNumber(candidate.startLine) || - !isFiniteNumber(candidate.endLine) || - !isFiniteNumber(candidate.score) || - !isFiniteNumber(candidate.recallCount) - ) { - return null; - } + if (event.type === "memory.promotion.applied") { + const memoryPath = truncateUtf8(event.memoryPath, MAX_MEMORY_HOST_EVENT_PATH_BYTES); + truncated ||= memoryPath.truncated || event.candidates.truncated; + const candidates = event.candidates.items.map((candidate) => { const key = truncateUtf8(candidate.key, MAX_MEMORY_HOST_EVENT_PATH_BYTES); const candidatePath = truncateUtf8(candidate.path, MAX_MEMORY_HOST_EVENT_PATH_BYTES); truncated ||= key.truncated || candidatePath.truncated; - candidates.push({ + return { key: key.value, path: candidatePath.value, startLine: candidate.startLine, endLine: candidate.endLine, score: candidate.score, recallCount: candidate.recallCount, - }); - } + }; + }); const normalized = { type: "memory.promotion.applied" as const, timestamp: timestamp.value, memoryPath: memoryPath.value, - applied: value.applied, + applied: event.applied, candidates, ...(truncated ? { storageTruncated: true as const } : {}), }; @@ -241,42 +268,27 @@ export function normalizeMemoryHostEventRecordForStorage( : { ...normalized, candidates: [], storageTruncated: true as const }; } - if (value.type === "memory.dream.completed") { - if ( - (value.phase !== "light" && value.phase !== "deep" && value.phase !== "rem") || - (value.outcome !== undefined && - value.outcome !== "completed" && - value.outcome !== "failed") || - (value.error !== undefined && typeof value.error !== "string") || - (value.inlinePath !== undefined && typeof value.inlinePath !== "string") || - (value.reportPath !== undefined && typeof value.reportPath !== "string") || - !isFiniteNumber(value.lineCount) || - (value.storageMode !== "inline" && - value.storageMode !== "separate" && - value.storageMode !== "both") - ) { - return null; - } - const error = value.error - ? truncateUtf8(value.error, MAX_MEMORY_HOST_EVENT_TEXT_BYTES) + if (event.type === "memory.dream.completed") { + const error = event.error + ? truncateUtf8(event.error, MAX_MEMORY_HOST_EVENT_TEXT_BYTES) : undefined; - const inlinePath = value.inlinePath - ? truncateUtf8(value.inlinePath, MAX_MEMORY_HOST_EVENT_PATH_BYTES) + const inlinePath = event.inlinePath + ? truncateUtf8(event.inlinePath, MAX_MEMORY_HOST_EVENT_PATH_BYTES) : undefined; - const reportPath = value.reportPath - ? truncateUtf8(value.reportPath, MAX_MEMORY_HOST_EVENT_PATH_BYTES) + const reportPath = event.reportPath + ? truncateUtf8(event.reportPath, MAX_MEMORY_HOST_EVENT_PATH_BYTES) : undefined; truncated ||= Boolean(error?.truncated || inlinePath?.truncated || reportPath?.truncated); return { - type: value.type, + type: event.type, timestamp: timestamp.value, - phase: value.phase, - ...(value.outcome ? { outcome: value.outcome } : {}), + phase: event.phase, + ...(event.outcome ? { outcome: event.outcome } : {}), ...(error ? { error: error.value } : {}), ...(inlinePath ? { inlinePath: inlinePath.value } : {}), ...(reportPath ? { reportPath: reportPath.value } : {}), - lineCount: value.lineCount, - storageMode: value.storageMode, + lineCount: event.lineCount, + storageMode: event.storageMode, ...(truncated ? { storageTruncated: true } : {}), }; } diff --git a/src/plugin-sdk/memory-host-events.test.ts b/src/plugin-sdk/memory-host-events.test.ts index fa9fff7b412f..e88e142151a6 100644 --- a/src/plugin-sdk/memory-host-events.test.ts +++ b/src/plugin-sdk/memory-host-events.test.ts @@ -6,6 +6,7 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { listStoredMemoryHostEvents, + normalizeMemoryHostEventRecordForStorage, setMaxMemoryHostEventsForTests, } from "../memory-host-sdk/event-store.js"; import { resetPluginStateStoreForTests } from "../plugin-state/plugin-state-store.js"; @@ -235,6 +236,43 @@ describe("memory host event journal helpers", () => { expect(Buffer.byteLength(JSON.stringify(event), "utf8")).toBeLessThanOrEqual(8 * 1024); }); + it("validates only the retained prefix of an oversized event", () => { + const result = { + path: "memory/2026-04-05.md", + startLine: 1, + endLine: 2, + score: 0.9, + }; + const normalized = normalizeMemoryHostEventRecordForStorage({ + type: "memory.recall.recorded", + timestamp: "2026-04-05T12:00:00.000Z", + query: "bounded tail", + resultCount: 11, + results: [...Array.from({ length: 10 }, () => result), { path: 42 }], + }); + + expect(normalized).toMatchObject({ storageTruncated: true }); + expect(normalized?.type === "memory.recall.recorded" ? normalized.results : []).toHaveLength( + 10, + ); + }); + + it.each([ + { name: "unknown event type", value: { type: "memory.unknown", timestamp: "now" } }, + { + name: "malformed retained result", + value: { + type: "memory.recall.recorded", + timestamp: "now", + query: "invalid", + resultCount: 1, + results: [{ path: 42 }], + }, + }, + ])("rejects $name", ({ value }) => { + expect(normalizeMemoryHostEventRecordForStorage(value)).toBeNull(); + }); + it("rotates old events without evicting the workspace sequence cursor", async () => { const workspaceDir = await createTempDir("memory-host-events-rotation-"); const env = { ...process.env, OPENCLAW_STATE_DIR: workspaceDir }; diff --git a/src/skills/workshop/store-record.test.ts b/src/skills/workshop/store-record.test.ts index d0d140637fb7..5bc4cbd33c21 100644 --- a/src/skills/workshop/store-record.test.ts +++ b/src/skills/workshop/store-record.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + parseSkillProposalEvaluation, parseSkillProposalRecord, parseSkillProposalRollback, validateSkillProposalRecord, @@ -74,6 +75,16 @@ const shippedRollback = { ], } as const; +const validEvaluation = { + id: "evaluation-1", + proposedVersion: "v1", + revisionHash: "a".repeat(64), + trigger: "manual", + startedAt: "2026-07-29T00:00:00.000Z", + completedAt: "2026-07-29T00:00:01.000Z", + outcomes: [], +} as const; + describe("Skill Workshop persisted record validation", () => { it("accepts the shipped v1 proposal and rollback shapes unchanged", () => { expect(validateSkillProposalRecord(shippedProposal)).toEqual({ @@ -104,4 +115,43 @@ describe("Skill Workshop persisted record validation", () => { }, }); }); + + it.each([ + { + name: "duplicate normalized support paths", + value: { + ...shippedProposal, + supportFiles: [ + shippedProposal.supportFiles[0], + { ...shippedProposal.supportFiles[0], path: "references/./proof.md" }, + ], + }, + }, + { + name: "invalid nested evaluation findings", + value: { + ...shippedProposal, + evaluation: { + ...validEvaluation, + outcomes: [ + { + evaluatorId: "reviewer", + pluginId: "review-plugin", + status: "completed", + result: { + findings: [{ ruleId: "", severity: "info", message: "missing rule id" }], + }, + }, + ], + }, + }, + }, + ])("rejects $name", ({ value }) => { + expect(parseSkillProposalRecord(value)).toBeNull(); + }); + + it("keeps evaluation validation at the persisted boundary", () => { + expect(parseSkillProposalEvaluation(validEvaluation)).toBe(validEvaluation); + expect(parseSkillProposalEvaluation({ ...validEvaluation, targetTreeSha256: 42 })).toBeNull(); + }); }); diff --git a/src/skills/workshop/store-record.ts b/src/skills/workshop/store-record.ts index 862ebd4e7c53..029c86c77f08 100644 --- a/src/skills/workshop/store-record.ts +++ b/src/skills/workshop/store-record.ts @@ -1,10 +1,5 @@ -import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { err, ok, type Result } from "@openclaw/normalization-core/result"; -import type { - PluginHookSkillEvaluationFinding, - PluginHookSkillProposalEvaluateResult, - PluginHookSkillProposalEvaluationOutcome, -} from "../../plugins/hook-types.js"; +import { z } from "zod"; import { MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES, normalizeWorkspaceSkillSupportPath, @@ -16,7 +11,6 @@ import { type SkillProposalEvaluation, type SkillProposalRecord, type SkillProposalRollback, - type SkillProposalSupportFile, } from "./types.js"; export const PROPOSAL_DRAFT_FILE = "PROPOSAL.md"; @@ -29,6 +23,143 @@ type SkillProposalRecordValidationError = { message: string; }; +const sha256Schema = z.string().regex(/^[a-f0-9]{64}$/i); +const skillProposalFindingSchema = z.looseObject({ + ruleId: z.string().min(1).max(256), + severity: z.enum(["info", "warn", "critical"]), + message: z.string().min(1).max(4_000), + file: z.string().max(1_024).optional(), + line: z + .number() + .refine(Number.isSafeInteger) + .refine((value) => value >= 1) + .optional(), +}); +const skillProposalMetricsSchema = z + .record(z.string(), z.union([z.string().max(4_000), z.number().finite(), z.boolean()])) + .superRefine((metrics, context) => { + const keys = Object.keys(metrics); + if (keys.length > 64 || keys.some((key) => key.length === 0 || key.length > 128)) { + context.addIssue({ code: "custom", message: "invalid evaluation metric keys" }); + } + }); +const skillProposalEvaluationResultSchema = z.looseObject({ + summary: z.string().max(8_000).optional(), + evaluatorVersion: z.string().max(128).optional(), + mode: z.string().max(128).optional(), + decision: z.enum(["pass", "revise", "block"]).optional(), + decisionReason: z.string().max(2_000).optional(), + findings: z.array(skillProposalFindingSchema).max(200).optional(), + metrics: skillProposalMetricsSchema.optional(), +}); +const skillProposalEvaluationOutcomeBaseShape = { + evaluatorId: z.string().min(1).max(128), + pluginId: z.string().min(1).max(128), + pluginVersion: z.string().max(128).optional(), +}; +const skillProposalEvaluationOutcomeSchema = z.discriminatedUnion("status", [ + z.looseObject({ ...skillProposalEvaluationOutcomeBaseShape, status: z.literal("skipped") }), + z.looseObject({ + ...skillProposalEvaluationOutcomeBaseShape, + status: z.literal("error"), + error: z.string().max(2_000), + }), + z.looseObject({ + ...skillProposalEvaluationOutcomeBaseShape, + status: z.literal("completed"), + result: skillProposalEvaluationResultSchema, + }), +]); +const skillProposalEvaluationSchema = z.looseObject({ + id: z.string().min(1).max(128), + proposedVersion: z.string(), + revisionHash: sha256Schema, + trigger: z.enum(["manual", "apply"]), + startedAt: z.string(), + completedAt: z.string(), + correlationId: z + .string() + .min(1) + .refine((value) => Array.from(value).length <= 256) + .optional(), + targetTreeSha256: sha256Schema.optional(), + outcomes: z.array(skillProposalEvaluationOutcomeSchema).max(64), +}); +const skillProposalSupportFileSchema = z.looseObject({ + path: z.string(), + hash: sha256Schema, + sizeBytes: z + .number() + .refine(Number.isSafeInteger) + .refine((value) => value >= 0 && value <= MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES), + targetExisted: z.boolean().optional(), + targetContentHash: sha256Schema.optional(), +}); +const skillProposalSupportFilesSchema = z + .array(skillProposalSupportFileSchema) + .max(MAX_PROPOSAL_SUPPORT_FILES) + .superRefine((files, context) => { + const seen = new Set(); + for (const [index, file] of files.entries()) { + let normalized: string; + try { + normalized = normalizeWorkspaceSkillSupportPath(file.path); + } catch { + context.addIssue({ + code: "custom", + message: "invalid support path", + path: [index, "path"], + }); + continue; + } + if (seen.has(normalized)) { + context.addIssue({ + code: "custom", + message: "duplicate support path", + path: [index, "path"], + }); + } + seen.add(normalized); + } + }); +const skillProposalRecordSchema = z + .looseObject({ + schema: z.literal(SKILL_WORKSHOP_SCHEMA), + id: z.string().regex(PROPOSAL_ID_PATTERN), + kind: z.enum(["create", "update"]), + status: z.enum(["pending", "applied", "rejected", "quarantined", "stale"]), + title: z.string(), + description: z.string(), + createdAt: z.string(), + updatedAt: z.string(), + autonomousCapture: z.literal(true).optional(), + draftHash: z.string(), + draftFile: z.literal(PROPOSAL_DRAFT_FILE), + origin: z.unknown().optional(), + originRunIds: z.unknown().optional(), + originRunMutationCounts: z.unknown().optional(), + supportFiles: skillProposalSupportFilesSchema.optional(), + evaluation: skillProposalEvaluationSchema.optional(), + target: z.looseObject({ + skillName: z.string(), + skillKey: z.string(), + skillDir: z.string(), + skillFile: z.string(), + }), + scan: z.custom((value) => value !== null && typeof value === "object"), + }) + .refine(hasValidProposalOriginProvenance); +const skillProposalRollbackSchema = z.looseObject({ + schema: z.literal(SKILL_WORKSHOP_ROLLBACK_SCHEMA), + proposalId: z.string().regex(PROPOSAL_ID_PATTERN), + writtenAt: z.string(), + targetSkillFile: z.string(), + action: z.enum(["create", "update"]), + previousContentHash: sha256Schema.optional(), + previousContent: z.string().optional(), + supportFiles: z.array(z.unknown()).optional(), +}); + export function assertSkillProposalEvaluationWithinLimit( evaluation: SkillProposalEvaluation, ): void { @@ -49,37 +180,10 @@ export function assertProposalId(proposalId: string): void { export function validateSkillProposalRecord( raw: unknown, ): Result { - if (!isRecord(raw)) { + if (!skillProposalRecordSchema.safeParse(raw).success) { return invalidMetadata("proposal"); } - const record = raw as SkillProposalRecord; - if ( - record.schema !== SKILL_WORKSHOP_SCHEMA || - !PROPOSAL_ID_PATTERN.test(record.id) || - (record.kind !== "create" && record.kind !== "update") || - !["pending", "applied", "rejected", "quarantined", "stale"].includes(record.status) || - typeof record.title !== "string" || - typeof record.description !== "string" || - typeof record.createdAt !== "string" || - typeof record.updatedAt !== "string" || - (record.autonomousCapture !== undefined && !record.autonomousCapture) || - typeof record.draftHash !== "string" || - record.draftFile !== PROPOSAL_DRAFT_FILE || - !hasValidProposalOriginProvenance(record) || - !isValidSupportFileList(record.supportFiles) || - (record.evaluation !== undefined && !parseSkillProposalEvaluation(record.evaluation)) || - !record.target || - typeof record.target !== "object" || - typeof record.target.skillName !== "string" || - typeof record.target.skillKey !== "string" || - typeof record.target.skillDir !== "string" || - typeof record.target.skillFile !== "string" || - !record.scan || - typeof record.scan !== "object" - ) { - return invalidMetadata("proposal"); - } - return ok(record); + return ok(raw as SkillProposalRecord); } export function parseSkillProposalRecord(raw: unknown): SkillProposalRecord | null { @@ -88,195 +192,18 @@ export function parseSkillProposalRecord(raw: unknown): SkillProposalRecord | nu } export function parseSkillProposalEvaluation(raw: unknown): SkillProposalEvaluation | null { - if (!isRecord(raw)) { - return null; - } - const value = raw as SkillProposalEvaluation; - if ( - typeof value.id === "string" && - value.id.length > 0 && - value.id.length <= 128 && - typeof value.proposedVersion === "string" && - typeof value.revisionHash === "string" && - /^[a-f0-9]{64}$/i.test(value.revisionHash) && - (value.trigger === "manual" || value.trigger === "apply") && - typeof value.startedAt === "string" && - typeof value.completedAt === "string" && - (value.correlationId === undefined || - (typeof value.correlationId === "string" && - value.correlationId.length > 0 && - Array.from(value.correlationId).length <= 256)) && - (value.targetTreeSha256 === undefined || - (typeof value.targetTreeSha256 === "string" && - /^[a-f0-9]{64}$/i.test(value.targetTreeSha256))) && - Array.isArray(value.outcomes) && - value.outcomes.length <= 64 && - value.outcomes.every(isValidEvaluationOutcome) - ) { - return value; - } - return null; -} - -function isValidEvaluationOutcome( - value: unknown, -): value is PluginHookSkillProposalEvaluationOutcome { - if (!isRecord(value)) { - return false; - } - const outcome = value as PluginHookSkillProposalEvaluationOutcome; - if ( - typeof outcome.evaluatorId !== "string" || - outcome.evaluatorId.length === 0 || - outcome.evaluatorId.length > 128 || - typeof outcome.pluginId !== "string" || - outcome.pluginId.length === 0 || - outcome.pluginId.length > 128 || - (outcome.pluginVersion !== undefined && - (typeof outcome.pluginVersion !== "string" || outcome.pluginVersion.length > 128)) - ) { - return false; - } - if (outcome.status === "skipped") { - return true; - } - if (outcome.status === "error") { - return typeof outcome.error === "string" && outcome.error.length <= 2_000; - } - return outcome.status === "completed" && isValidEvaluationResult(outcome.result); -} - -function isValidEvaluationResult(value: unknown): value is PluginHookSkillProposalEvaluateResult { - if (!isRecord(value)) { - return false; - } - const result = value as PluginHookSkillProposalEvaluateResult; - return ( - (result.summary === undefined || - (typeof result.summary === "string" && result.summary.length <= 8_000)) && - (result.evaluatorVersion === undefined || - (typeof result.evaluatorVersion === "string" && result.evaluatorVersion.length <= 128)) && - (result.mode === undefined || (typeof result.mode === "string" && result.mode.length <= 128)) && - (result.decision === undefined || ["pass", "revise", "block"].includes(result.decision)) && - (result.decisionReason === undefined || - (typeof result.decisionReason === "string" && result.decisionReason.length <= 2_000)) && - isValidEvaluationFindings(result.findings) && - isValidEvaluationMetrics(result.metrics) - ); -} - -function isValidEvaluationFindings(value: PluginHookSkillEvaluationFinding[] | undefined): boolean { - if (value === undefined) { - return true; - } - return ( - Array.isArray(value) && - value.length <= 200 && - value.every( - (finding) => - finding && - typeof finding === "object" && - typeof finding.ruleId === "string" && - finding.ruleId.length > 0 && - finding.ruleId.length <= 256 && - ["info", "warn", "critical"].includes(finding.severity) && - typeof finding.message === "string" && - finding.message.length > 0 && - finding.message.length <= 4_000 && - (finding.file === undefined || - (typeof finding.file === "string" && finding.file.length <= 1_024)) && - (finding.line === undefined || (Number.isSafeInteger(finding.line) && finding.line >= 1)), - ) - ); -} - -function isValidEvaluationMetrics( - value: Record | undefined, -): boolean { - if (value === undefined) { - return true; - } - if (!isRecord(value)) { - return false; - } - const entries = Object.entries(value); - return ( - entries.length <= 64 && - entries.every( - ([key, metric]) => - key.length > 0 && - key.length <= 128 && - ((typeof metric === "string" && metric.length <= 4_000) || - (typeof metric === "number" && Number.isFinite(metric)) || - typeof metric === "boolean"), - ) - ); -} - -function isValidSupportFileList(value: unknown): boolean { - if (value === undefined) { - return true; - } - if (!Array.isArray(value) || value.length > MAX_PROPOSAL_SUPPORT_FILES) { - return false; - } - const seen = new Set(); - for (const item of value) { - if (!isRecord(item)) { - return false; - } - const file = item as SkillProposalSupportFile; - if ( - typeof file.path !== "string" || - typeof file.hash !== "string" || - !/^[a-f0-9]{64}$/i.test(file.hash) || - typeof file.sizeBytes !== "number" || - !Number.isSafeInteger(file.sizeBytes) || - file.sizeBytes < 0 || - file.sizeBytes > MAX_WORKSPACE_SKILL_SUPPORT_FILE_BYTES || - (file.targetExisted !== undefined && typeof file.targetExisted !== "boolean") || - (file.targetContentHash !== undefined && - (typeof file.targetContentHash !== "string" || - !/^[a-f0-9]{64}$/i.test(file.targetContentHash))) - ) { - return false; - } - let normalized: string; - try { - normalized = normalizeWorkspaceSkillSupportPath(file.path); - } catch { - return false; - } - if (seen.has(normalized)) { - return false; - } - seen.add(normalized); - } - return true; + return skillProposalEvaluationSchema.safeParse(raw).success + ? (raw as SkillProposalEvaluation) + : null; } export function validateSkillProposalRollback( raw: unknown, ): Result { - if (!isRecord(raw)) { + if (!skillProposalRollbackSchema.safeParse(raw).success) { return invalidMetadata("rollback"); } - const rollback = raw as SkillProposalRollback; - if ( - rollback.schema !== SKILL_WORKSHOP_ROLLBACK_SCHEMA || - !PROPOSAL_ID_PATTERN.test(rollback.proposalId) || - typeof rollback.writtenAt !== "string" || - typeof rollback.targetSkillFile !== "string" || - (rollback.action !== "create" && rollback.action !== "update") || - (rollback.previousContentHash !== undefined && - (typeof rollback.previousContentHash !== "string" || - !/^[a-f0-9]{64}$/i.test(rollback.previousContentHash))) || - (rollback.previousContent !== undefined && typeof rollback.previousContent !== "string") || - (rollback.supportFiles !== undefined && !Array.isArray(rollback.supportFiles)) - ) { - return invalidMetadata("rollback"); - } - return ok(rollback); + return ok(raw as SkillProposalRollback); } export function parseSkillProposalRollback(raw: unknown): SkillProposalRollback | null { diff --git a/src/talk/client-voice-session-store.test.ts b/src/talk/client-voice-session-store.test.ts index 2f947e92b596..8436cba310aa 100644 --- a/src/talk/client-voice-session-store.test.ts +++ b/src/talk/client-voice-session-store.test.ts @@ -55,4 +55,14 @@ describe("client voice session store", () => { ), ).toBeUndefined(); }); + + it.each([ + { name: "version", patch: { version: 2 } }, + { name: "origin", patch: { origin: "server" } }, + { name: "provider", patch: { provider: " " } }, + { name: "updated timestamp", patch: { updatedAt: "later" } }, + ])("rejects an invalid $name", ({ patch }) => { + const value = JSON.parse(storedRecord([])) as Record; + expect(parseStoredVoiceSessionRecord(JSON.stringify({ ...value, ...patch }))).toBeUndefined(); + }); }); diff --git a/src/talk/client-voice-session-store.ts b/src/talk/client-voice-session-store.ts index faa2fdf7fa60..cea7d3fd8cdd 100644 --- a/src/talk/client-voice-session-store.ts +++ b/src/talk/client-voice-session-store.ts @@ -1,3 +1,4 @@ +import { z } from "zod"; /** SQLite-backed persistence for durable per-agent Talk voice-call records. */ import { openOpenClawAgentDatabase, @@ -48,65 +49,61 @@ export type ClientVoiceRunBinding = { const TRANSCRIPT_FAILURE_KEY_PATTERN = /^[0-9a-f]{64}$/; +const clientVoiceToolEffectSchema = z.looseObject({ + runId: z.string(), + toolName: z.string(), + startedAt: z.number(), + status: z.enum(["started", "succeeded", "failed", "cancelled", "blocked"]), +}); + +const clientVoiceSessionRecordSchema = z.looseObject({ + version: z.literal(VOICE_SESSION_RECORD_VERSION), + voiceSessionId: z.string(), + agentId: z.string(), + sessionKey: z.string(), + provider: z + .string() + .refine((value) => value.trim().length > 0) + .transform((value) => value.trim()) + .optional(), + origin: z.enum(["client", "relay"]), + status: z.enum(["open", "closed"]), + createdAt: z.number(), + updatedAt: z.number(), + consultRunIds: z + .unknown() + .optional() + .transform((value) => + Array.isArray(value) + ? value.filter((entry): entry is string => typeof entry === "string") + : [], + ), + effects: z + .unknown() + .optional() + .transform((value) => + Array.isArray(value) + ? value.flatMap((entry) => { + const parsed = clientVoiceToolEffectSchema.safeParse(entry); + return parsed.success ? [parsed.data] : []; + }) + : [], + ), + transcriptFailureKeys: z + .unknown() + .optional() + .transform((value) => value ?? []) + .pipe( + z + .array(z.string().regex(TRANSCRIPT_FAILURE_KEY_PATTERN)) + .max(VOICE_TRANSCRIPT_MAX_UNRESOLVED) + .refine((keys) => new Set(keys).size === keys.length), + ), +}); + function parseVoiceSessionRecord(value: unknown): ClientVoiceSessionRecord | undefined { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return undefined; - } - const record = value as Partial; - if ( - record.version !== VOICE_SESSION_RECORD_VERSION || - typeof record.voiceSessionId !== "string" || - typeof record.agentId !== "string" || - typeof record.sessionKey !== "string" || - (record.provider !== undefined && - (typeof record.provider !== "string" || !record.provider.trim())) || - (record.origin !== "client" && record.origin !== "relay") || - (record.status !== "open" && record.status !== "closed") || - typeof record.createdAt !== "number" || - typeof record.updatedAt !== "number" - ) { - return undefined; - } - const consultRunIds = Array.isArray(record.consultRunIds) - ? record.consultRunIds.filter((entry): entry is string => typeof entry === "string") - : []; - const effects = Array.isArray(record.effects) - ? record.effects.filter((entry): entry is ClientVoiceToolEffect => { - if (!entry || typeof entry !== "object") { - return false; - } - const effect = entry as Partial; - return ( - typeof effect.runId === "string" && - typeof effect.toolName === "string" && - typeof effect.startedAt === "number" && - (effect.status === "started" || - effect.status === "succeeded" || - effect.status === "failed" || - effect.status === "cancelled" || - effect.status === "blocked") - ); - }) - : []; - const transcriptFailureKeys = record.transcriptFailureKeys ?? []; - if ( - !Array.isArray(transcriptFailureKeys) || - transcriptFailureKeys.length > VOICE_TRANSCRIPT_MAX_UNRESOLVED || - transcriptFailureKeys.some( - (entry) => typeof entry !== "string" || !TRANSCRIPT_FAILURE_KEY_PATTERN.test(entry), - ) || - new Set(transcriptFailureKeys).size !== transcriptFailureKeys.length - ) { - return undefined; - } - const provider = record.provider?.trim(); - return { - ...record, - ...(provider ? { provider } : {}), - consultRunIds, - effects, - transcriptFailureKeys, - } as ClientVoiceSessionRecord; + const parsed = clientVoiceSessionRecordSchema.safeParse(value); + return parsed.success ? (parsed.data as ClientVoiceSessionRecord) : undefined; } export function parseStoredVoiceSessionRecord(