refactor: share boundary open and gateway test helpers

This commit is contained in:
Peter Steinberger
2026-03-23 00:36:30 +00:00
parent b21bcf6eb6
commit 100d9a7a23
11 changed files with 140 additions and 91 deletions

View File

@@ -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 };
}

View File

@@ -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<void>((resolve) => {

View File

@@ -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<void>((resolve) => {
releaseBlockedReply = resolve;
});
const replySpy = vi.mocked(getReplyFromConfig);
replySpy.mockImplementationOnce(async (_ctx: unknown, opts?: GetReplyOptions) => {
mockGetReplyFromConfigOnce(async (_ctx, opts) => {
await new Promise<void>((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?" },

View File

@@ -298,6 +298,9 @@ export const piSdkMock = hoisted.piSdkMock;
export const cronIsolatedRun = hoisted.cronIsolatedRun;
export const agentCommand = hoisted.agentCommand;
export const getReplyFromConfig: Mock<GetReplyFromConfigFn> = hoisted.getReplyFromConfig;
export const mockGetReplyFromConfigOnce = (impl: GetReplyFromConfigFn) => {
getReplyFromConfig.mockImplementationOnce(impl);
};
export const sendWhatsAppMock = hoisted.sendWhatsAppMock;
export const testState = hoisted.testState;

View File

@@ -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");
});
});

View File

@@ -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<BoundaryFileOpenResult, { ok: false }>;
export type OpenBoundaryFileSyncParams = {
absolutePath: string;
rootPath: string;
@@ -89,6 +91,25 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda
});
}
export function matchBoundaryFileOpenFailure<T>(
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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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<string, SessionBindingRecord>();
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<typeof import("../infra/home-dir.js")>();

View File

@@ -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);

View File

@@ -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 {