diff --git a/src/gateway/control-ui.ts b/src/gateway/control-ui.ts index b3d65bd72b8..ece3c7f1337 100644 --- a/src/gateway/control-ui.ts +++ b/src/gateway/control-ui.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import type { IncomingMessage, ServerResponse } from "node:http"; import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; -import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; +import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isPackageProvenControlUiRootSync, resolveControlUiRootSync, @@ -271,10 +271,12 @@ function resolveSafeControlUiFile( rejectHardlinks, }); if (!opened.ok) { - if (opened.reason === "io") { - throw opened.error; - } - return null; + return matchBoundaryFileOpenFailure(opened, { + io: (failure) => { + throw failure.error; + }, + fallback: () => null, + }); } return { path: opened.path, fd: opened.fd }; } diff --git a/src/gateway/server.chat.gateway-server-chat-b.test.ts b/src/gateway/server.chat.gateway-server-chat-b.test.ts index c0945ccf288..d7b944df74d 100644 --- a/src/gateway/server.chat.gateway-server-chat-b.test.ts +++ b/src/gateway/server.chat.gateway-server-chat-b.test.ts @@ -8,6 +8,7 @@ import { connectOk, getReplyFromConfig, installGatewayTestHooks, + mockGetReplyFromConfigOnce, onceMessage, rpcReq, startServerWithClient, @@ -166,9 +167,8 @@ describe("gateway server chat", () => { await writeMainSessionStore(); testState.agentConfig = { blockStreamingDefault: "on" }; try { - spy.mockClear(); let capturedOpts: GetReplyOptions | undefined; - spy.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => { + mockGetReplyFromConfigOnce(async (_ctx, opts) => { capturedOpts = opts; return undefined; }); @@ -386,8 +386,7 @@ describe("gateway server chat", () => { await createSessionDir(); await writeMainSessionStore(); - spy.mockClear(); - spy.mockImplementationOnce(async (_ctx, opts) => { + mockGetReplyFromConfigOnce(async (_ctx, opts) => { opts?.onAgentRunStart?.(opts.runId ?? "idem-abort-1"); const signal = opts?.abortSignal; await new Promise((resolve) => { diff --git a/src/gateway/server.chat.gateway-server-chat.test.ts b/src/gateway/server.chat.gateway-server-chat.test.ts index 5759fae87bc..f42f8716390 100644 --- a/src/gateway/server.chat.gateway-server-chat.test.ts +++ b/src/gateway/server.chat.gateway-server-chat.test.ts @@ -3,14 +3,13 @@ import os from "node:os"; import path from "node:path"; import { describe, expect, test, vi } from "vitest"; import { WebSocket } from "ws"; -import type { GetReplyOptions } from "../auto-reply/types.js"; import { emitAgentEvent, registerAgentRunContext } from "../infra/agent-events.js"; import { extractFirstTextBlock } from "../shared/chat-message-content.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; import { connectOk, - getReplyFromConfig, installGatewayTestHooks, + mockGetReplyFromConfigOnce, onceMessage, rpcReq, testState, @@ -166,8 +165,7 @@ describe("gateway server chat", () => { const blockedReply = new Promise((resolve) => { releaseBlockedReply = resolve; }); - const replySpy = vi.mocked(getReplyFromConfig); - replySpy.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => { + mockGetReplyFromConfigOnce(async (_ctx, opts) => { await new Promise((resolve) => { let settled = false; const finish = () => { @@ -564,11 +562,10 @@ describe("gateway server chat", () => { test("routes /btw replies through side-result events without transcript injection", async () => { await withMainSessionStore(async () => { - const replyMock = vi.mocked(getReplyFromConfig); - replyMock.mockResolvedValueOnce({ + mockGetReplyFromConfigOnce(async () => ({ text: "323", btw: { question: "what is 17 * 19?" }, - }); + })); const sideResultPromise = onceMessage( ws, (o) => @@ -620,8 +617,7 @@ describe("gateway server chat", () => { test("routes block-streamed /btw replies through side-result events", async () => { await withMainSessionStore(async () => { - const replyMock = vi.mocked(getReplyFromConfig); - replyMock.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => { + mockGetReplyFromConfigOnce(async (_ctx, opts) => { await opts?.onBlockReply?.({ text: "first chunk", btw: { question: "what changed?" }, diff --git a/src/gateway/test-helpers.mocks.ts b/src/gateway/test-helpers.mocks.ts index 57012451046..d9856f7e9b3 100644 --- a/src/gateway/test-helpers.mocks.ts +++ b/src/gateway/test-helpers.mocks.ts @@ -298,6 +298,9 @@ export const piSdkMock = hoisted.piSdkMock; export const cronIsolatedRun = hoisted.cronIsolatedRun; export const agentCommand = hoisted.agentCommand; export const getReplyFromConfig: Mock = hoisted.getReplyFromConfig; +export const mockGetReplyFromConfigOnce = (impl: GetReplyFromConfigFn) => { + getReplyFromConfig.mockImplementationOnce(impl); +}; export const sendWhatsAppMock = hoisted.sendWhatsAppMock; export const testState = hoisted.testState; diff --git a/src/infra/boundary-file-read.test.ts b/src/infra/boundary-file-read.test.ts index 8829fec80b8..fb0e563d726 100644 --- a/src/infra/boundary-file-read.test.ts +++ b/src/infra/boundary-file-read.test.ts @@ -15,14 +15,19 @@ vi.mock("./safe-open-sync.js", () => ({ })); let canUseBoundaryFileOpen: typeof import("./boundary-file-read.js").canUseBoundaryFileOpen; +let matchBoundaryFileOpenFailure: typeof import("./boundary-file-read.js").matchBoundaryFileOpenFailure; let openBoundaryFile: typeof import("./boundary-file-read.js").openBoundaryFile; let openBoundaryFileSync: typeof import("./boundary-file-read.js").openBoundaryFileSync; describe("boundary-file-read", () => { beforeEach(async () => { vi.resetModules(); - ({ canUseBoundaryFileOpen, openBoundaryFile, openBoundaryFileSync } = - await import("./boundary-file-read.js")); + ({ + canUseBoundaryFileOpen, + matchBoundaryFileOpenFailure, + openBoundaryFile, + openBoundaryFileSync, + } = await import("./boundary-file-read.js")); resolveBoundaryPathSyncMock.mockReset(); resolveBoundaryPathMock.mockReset(); openVerifiedFileSyncMock.mockReset(); @@ -205,4 +210,31 @@ describe("boundary-file-read", () => { }); expect(openVerifiedFileSyncMock).not.toHaveBeenCalled(); }); + + it("matches boundary file failures by reason with fallback support", () => { + const missing = matchBoundaryFileOpenFailure( + { ok: false, reason: "path", error: new Error("missing") }, + { + path: () => "missing", + fallback: () => "fallback", + }, + ); + const io = matchBoundaryFileOpenFailure( + { ok: false, reason: "io", error: new Error("io") }, + { + io: () => "io", + fallback: () => "fallback", + }, + ); + const validation = matchBoundaryFileOpenFailure( + { ok: false, reason: "validation", error: new Error("blocked") }, + { + fallback: (failure) => failure.reason, + }, + ); + + expect(missing).toBe("missing"); + expect(io).toBe("io"); + expect(validation).toBe("validation"); + }); }); diff --git a/src/infra/boundary-file-read.ts b/src/infra/boundary-file-read.ts index 93ffef6deeb..2338ade6cfa 100644 --- a/src/infra/boundary-file-read.ts +++ b/src/infra/boundary-file-read.ts @@ -29,6 +29,8 @@ export type BoundaryFileOpenResult = | { ok: true; path: string; fd: number; stat: fs.Stats; rootRealPath: string } | { ok: false; reason: BoundaryFileOpenFailureReason; error?: unknown }; +export type BoundaryFileOpenFailure = Extract; + export type OpenBoundaryFileSyncParams = { absolutePath: string; rootPath: string; @@ -89,6 +91,25 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda }); } +export function matchBoundaryFileOpenFailure( + failure: BoundaryFileOpenFailure, + handlers: { + path?: (failure: BoundaryFileOpenFailure) => T; + validation?: (failure: BoundaryFileOpenFailure) => T; + io?: (failure: BoundaryFileOpenFailure) => T; + fallback: (failure: BoundaryFileOpenFailure) => T; + }, +): T { + switch (failure.reason) { + case "path": + return handlers.path ? handlers.path(failure) : handlers.fallback(failure); + case "validation": + return handlers.validation ? handlers.validation(failure) : handlers.fallback(failure); + case "io": + return handlers.io ? handlers.io(failure) : handlers.fallback(failure); + } +} + function openBoundaryFileResolved(params: { absolutePath: string; resolvedPath: string; diff --git a/src/plugins/bundle-manifest.ts b/src/plugins/bundle-manifest.ts index 3a3fed87158..8bf8b67bb45 100644 --- a/src/plugins/bundle-manifest.ts +++ b/src/plugins/bundle-manifest.ts @@ -1,6 +1,6 @@ import fs from "node:fs"; import path from "node:path"; -import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; +import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; import { DEFAULT_PLUGIN_ENTRY_CANDIDATES, PLUGIN_MANIFEST_FILENAME } from "./manifest.js"; import type { PluginBundleFormat } from "./types.js"; @@ -102,17 +102,19 @@ function loadBundleManifestFile(params: { rejectHardlinks: params.rejectHardlinks, }); if (!opened.ok) { - if (opened.reason === "path") { - if (params.allowMissing) { - return { ok: true, raw: {}, manifestPath }; - } - return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath }; - } - return { - ok: false, - error: `unsafe plugin manifest path: ${manifestPath} (${opened.reason})`, - manifestPath, - }; + return matchBoundaryFileOpenFailure(opened, { + path: () => { + if (params.allowMissing) { + return { ok: true, raw: {}, manifestPath }; + } + return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath }; + }, + fallback: (failure) => ({ + ok: false, + error: `unsafe plugin manifest path: ${manifestPath} (${failure.reason})`, + manifestPath, + }), + }); } try { const raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; diff --git a/src/plugins/bundle-mcp.ts b/src/plugins/bundle-mcp.ts index c4e2f0651bb..357961caf72 100644 --- a/src/plugins/bundle-mcp.ts +++ b/src/plugins/bundle-mcp.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; import { applyMergePatch } from "../config/merge-patch.js"; -import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; +import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; import { CLAUDE_BUNDLE_MANIFEST_RELATIVE_PATH, @@ -57,10 +57,18 @@ function readPluginJsonObject(params: { rejectHardlinks: true, }); if (!opened.ok) { - if (opened.reason === "path" && params.allowMissing) { - return { ok: true, raw: {} }; - } - return { ok: false, error: `unable to read ${params.relativePath}: ${opened.reason}` }; + return matchBoundaryFileOpenFailure(opened, { + path: () => { + if (params.allowMissing) { + return { ok: true, raw: {} }; + } + return { ok: false, error: `unable to read ${params.relativePath}: path` }; + }, + fallback: (failure) => ({ + ok: false, + error: `unable to read ${params.relativePath}: ${failure.reason}`, + }), + }); } try { const raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; diff --git a/src/plugins/conversation-binding.test.ts b/src/plugins/conversation-binding.test.ts index 1806976ca11..b02d6ec2fc3 100644 --- a/src/plugins/conversation-binding.test.ts +++ b/src/plugins/conversation-binding.test.ts @@ -7,34 +7,12 @@ import type { SessionBindingAdapter, SessionBindingRecord, } from "../infra/outbound/session-binding-service.js"; +import { createEmptyPluginRegistry } from "./registry-empty.js"; import type { PluginRegistry } from "./registry.js"; const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-plugin-binding-")); const approvalsPath = path.join(tempRoot, "plugin-binding-approvals.json"); -function createEmptyPluginRegistry(): PluginRegistry { - return { - plugins: [], - tools: [], - hooks: [], - typedHooks: [], - channels: [], - channelSetups: [], - providers: [], - speechProviders: [], - mediaUnderstandingProviders: [], - imageGenerationProviders: [], - webSearchProviders: [], - gatewayHandlers: {}, - httpRoutes: [], - cliRegistrars: [], - services: [], - commands: [], - conversationBindingResolvedHandlers: [], - diagnostics: [], - }; -} - const sessionBindingState = vi.hoisted(() => { const records = new Map(); let nextId = 1; @@ -105,9 +83,13 @@ const sessionBindingState = vi.hoisted(() => { }; }); -const pluginRuntimeState = vi.hoisted(() => ({ - registry: createEmptyPluginRegistry(), -})); +const pluginRuntimeState = vi.hoisted( + () => + ({ + // The runtime mock is initialized before imports; beforeEach installs the real shared stub. + registry: null as unknown as PluginRegistry, + }) satisfies { registry: PluginRegistry }, +); vi.mock("../infra/home-dir.js", async (importOriginal) => { const actual = await importOriginal(); diff --git a/src/plugins/discovery.ts b/src/plugins/discovery.ts index 113ce289308..d6f62299495 100644 --- a/src/plugins/discovery.ts +++ b/src/plugins/discovery.ts @@ -1,6 +1,6 @@ import fs from "node:fs"; import path from "node:path"; -import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; +import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { resolveUserPath } from "../utils.js"; import { detectBundleManifestFormat, loadBundleManifest } from "./bundle-manifest.js"; import { @@ -476,25 +476,25 @@ function resolvePackageEntrySource(params: { rejectHardlinks: params.rejectHardlinks ?? true, }); if (!opened.ok) { - if (opened.reason === "path") { - // File missing (ENOENT) — skip, not a security violation. - return null; - } - if (opened.reason === "io") { - // Filesystem error (EACCES, EMFILE, etc.) — warn but don't abort. - params.diagnostics.push({ - level: "warn", - message: `extension entry unreadable (I/O error): ${params.entryPath}`, - source: params.sourceLabel, - }); - return null; - } - params.diagnostics.push({ - level: "error", - message: `extension entry escapes package directory: ${params.entryPath}`, - source: params.sourceLabel, + return matchBoundaryFileOpenFailure(opened, { + path: () => null, + io: () => { + params.diagnostics.push({ + level: "warn", + message: `extension entry unreadable (I/O error): ${params.entryPath}`, + source: params.sourceLabel, + }); + return null; + }, + fallback: () => { + params.diagnostics.push({ + level: "error", + message: `extension entry escapes package directory: ${params.entryPath}`, + source: params.sourceLabel, + }); + return null; + }, }); - return null; } const safeSource = opened.path; fs.closeSync(opened.fd); diff --git a/src/plugins/manifest.ts b/src/plugins/manifest.ts index df6b0b8dd44..34e1dbee5a9 100644 --- a/src/plugins/manifest.ts +++ b/src/plugins/manifest.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; -import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; +import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; import type { PluginConfigUiHint, PluginKind } from "./types.js"; @@ -159,14 +159,18 @@ export function loadPluginManifest( rejectHardlinks, }); if (!opened.ok) { - if (opened.reason === "path") { - return { ok: false, error: `plugin manifest not found: ${manifestPath}`, manifestPath }; - } - return { - ok: false, - error: `unsafe plugin manifest path: ${manifestPath} (${opened.reason})`, - manifestPath, - }; + return matchBoundaryFileOpenFailure(opened, { + path: () => ({ + ok: false, + error: `plugin manifest not found: ${manifestPath}`, + manifestPath, + }), + fallback: (failure) => ({ + ok: false, + error: `unsafe plugin manifest path: ${manifestPath} (${failure.reason})`, + manifestPath, + }), + }); } let raw: unknown; try {