mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(config): bound external catalog file reads with size limit (#108200)
* fix(config): bound external catalog file reads with size limit
Replace unbounded fs.readFileSync with readRegularFileSync capped at
16 MiB (MAX_EXTERNAL_CATALOG_BYTES) to prevent memory exhaustion from
oversized or malicious plugin catalog files.
Resolve symlinks via fs.realpathSync before the bounded read so
symlinked catalog files keep working — matching the marketplace.ts
pattern for bounded manifest reads.
* fix(config): add diagnostic when oversized catalog is skipped
Log a warning when an external catalog file exceeds the 16 MiB limit
so operators know a configured file was skipped. Add regression test
verifying the oversized catalog is skipped and selection continues.
* fix(config): fix false-positive oversized catalog test
Replace the broken spy-on-object-literal mock with a real sparse file
that genuinely triggers readRegularFileSync rejection via stat.size.
The previous vi.spyOn({ readRegularFileSync }, ...) intercepted a
throwaway object, never the actual module import, so the test passed
regardless of whether the fix was applied or not.
Also fix the no-unused-expressions lint error on the env-primary
assertion that used a discarded ternary (? undefined : undefined)
instead of a proper expect assertion.
This commit is contained in:
@@ -2,6 +2,13 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const logWarnSpy = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../logging/subsystem.js", () => ({
|
||||
createSubsystemLogger: () => ({ warn: logWarnSpy }),
|
||||
}));
|
||||
|
||||
import {
|
||||
applyPluginAutoEnable,
|
||||
materializePluginAutoEnableCandidates,
|
||||
@@ -33,6 +40,7 @@ beforeEach(() => {
|
||||
|
||||
afterEach(() => {
|
||||
resetPluginAutoEnableTestState();
|
||||
logWarnSpy.mockClear();
|
||||
});
|
||||
|
||||
describe("applyPluginAutoEnable channels", () => {
|
||||
@@ -140,7 +148,7 @@ describe("applyPluginAutoEnable channels", () => {
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
const readFileSpy = vi.spyOn(fs, "readFileSync");
|
||||
const realpathSpy = vi.spyOn(fs, "realpathSync");
|
||||
|
||||
try {
|
||||
materializePluginAutoEnableCandidates({
|
||||
@@ -164,15 +172,126 @@ describe("applyPluginAutoEnable channels", () => {
|
||||
});
|
||||
|
||||
expect(
|
||||
readFileSpy.mock.calls.filter(([filePath]) =>
|
||||
realpathSpy.mock.calls.filter(([filePath]) =>
|
||||
String(filePath).endsWith("plugins/catalog.json"),
|
||||
),
|
||||
).toHaveLength(2);
|
||||
} finally {
|
||||
readFileSpy.mockRestore();
|
||||
realpathSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("reads external catalog files through a symlink", () => {
|
||||
const stateDir = makeTempDir();
|
||||
const pluginsDir = path.join(stateDir, "plugins");
|
||||
fs.mkdirSync(pluginsDir, { recursive: true });
|
||||
const realPath = path.join(stateDir, "real-catalog.json");
|
||||
fs.writeFileSync(
|
||||
realPath,
|
||||
JSON.stringify({
|
||||
entries: [
|
||||
{
|
||||
name: "@openclaw/env-secondary",
|
||||
openclaw: {
|
||||
channel: {
|
||||
id: "env-secondary",
|
||||
label: "Env Secondary",
|
||||
selectionLabel: "Env Secondary",
|
||||
docsPath: "/channels/env-secondary",
|
||||
blurb: "Env secondary entry",
|
||||
preferOver: ["env-primary"],
|
||||
},
|
||||
install: { npmSpec: "@openclaw/env-secondary" },
|
||||
},
|
||||
},
|
||||
],
|
||||
}),
|
||||
"utf-8",
|
||||
);
|
||||
const catalogPath = path.join(pluginsDir, "catalog.json");
|
||||
fs.symlinkSync(realPath, catalogPath);
|
||||
|
||||
const result = materializePluginAutoEnableCandidates({
|
||||
config: {
|
||||
channels: {
|
||||
"env-primary": { token: "primary" },
|
||||
"env-secondary": { token: "secondary" },
|
||||
},
|
||||
},
|
||||
candidates: [
|
||||
{
|
||||
pluginId: "env-primary",
|
||||
kind: "channel-configured" as const,
|
||||
channelId: "env-primary",
|
||||
},
|
||||
{
|
||||
pluginId: "env-secondary",
|
||||
kind: "channel-configured" as const,
|
||||
channelId: "env-secondary",
|
||||
},
|
||||
],
|
||||
env: {
|
||||
...makeIsolatedEnv(),
|
||||
OPENCLAW_STATE_DIR: stateDir,
|
||||
OPENCLAW_BUNDLED_PLUGINS_DIR: "/nonexistent/bundled/plugins",
|
||||
},
|
||||
manifestRegistry: makeRegistry([]),
|
||||
});
|
||||
|
||||
expect(result.config.plugins?.entries?.["env-secondary"]?.enabled).toBe(true);
|
||||
expect(result.config.plugins?.entries?.["env-primary"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("warns when an oversized catalog is skipped and continues selection", () => {
|
||||
const stateDir = makeTempDir();
|
||||
const catalogPath = path.join(stateDir, "plugins", "catalog.json");
|
||||
fs.mkdirSync(path.dirname(catalogPath), { recursive: true });
|
||||
// Create a sparse file whose stat.size exceeds the 16 MiB cap without
|
||||
// allocating actual disk blocks — the bounded read rejects it by size
|
||||
// before loading content into memory.
|
||||
const fd = fs.openSync(catalogPath, "w");
|
||||
try {
|
||||
fs.writeSync(fd, "{}\n");
|
||||
fs.ftruncateSync(fd, 17 * 1024 * 1024);
|
||||
} finally {
|
||||
fs.closeSync(fd);
|
||||
}
|
||||
|
||||
const result = materializePluginAutoEnableCandidates({
|
||||
config: {
|
||||
channels: {
|
||||
"env-primary": { token: "primary" },
|
||||
"env-secondary": { token: "secondary" },
|
||||
},
|
||||
},
|
||||
candidates: [
|
||||
{
|
||||
pluginId: "env-primary",
|
||||
kind: "channel-configured" as const,
|
||||
channelId: "env-primary",
|
||||
},
|
||||
{
|
||||
pluginId: "env-secondary",
|
||||
kind: "channel-configured" as const,
|
||||
channelId: "env-secondary",
|
||||
},
|
||||
],
|
||||
env: {
|
||||
...makeIsolatedEnv(),
|
||||
OPENCLAW_STATE_DIR: stateDir,
|
||||
OPENCLAW_BUNDLED_PLUGINS_DIR: "/nonexistent/bundled/plugins",
|
||||
},
|
||||
manifestRegistry: makeRegistry([]),
|
||||
});
|
||||
|
||||
// Selection continues: env-secondary is still auto-enabled.
|
||||
expect(result.config.plugins?.entries?.["env-secondary"]?.enabled).toBe(true);
|
||||
// Warning was logged for the oversized catalog.
|
||||
expect(logWarnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("skipping oversized external catalog file"),
|
||||
);
|
||||
});
|
||||
|
||||
describe("third-party channel plugins", () => {
|
||||
it("activates external channel plugins under plugins.entries when plugin id matches channel id", () => {
|
||||
const result = materializePluginAutoEnableCandidates({
|
||||
|
||||
@@ -4,11 +4,17 @@ import path from "node:path";
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
|
||||
import { findChatChannelMeta, normalizeChatChannelId } from "../channels/registry.js";
|
||||
import { readRegularFileSync } from "../infra/regular-file.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import type { PluginManifestRegistry } from "../plugins/manifest-registry.js";
|
||||
import { isRecord, resolveConfigDir, resolveUserPath } from "../utils.js";
|
||||
import type { PluginAutoEnableCandidate } from "./plugin-auto-enable.types.js";
|
||||
import type { OpenClawConfig } from "./types.openclaw.js";
|
||||
|
||||
/** Maximum bytes to read from an external catalog file before rejecting it. */
|
||||
const MAX_EXTERNAL_CATALOG_BYTES = 16 * 1024 * 1024;
|
||||
const log = createSubsystemLogger("config/plugin-catalog");
|
||||
|
||||
type ExternalCatalogChannelEntry = {
|
||||
id: string;
|
||||
preferOver: string[];
|
||||
@@ -78,15 +84,30 @@ function resolveExternalCatalogPreferOver(channelId: string, env: NodeJS.Process
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
const payload = JSON.parse(fs.readFileSync(resolved, "utf-8")) as unknown;
|
||||
// Resolve symlinks so a catalog file that points to a regular file
|
||||
// keeps working while the bounded regular-file read still rejects
|
||||
// directories, FIFOs, and oversized targets.
|
||||
const resolvedRealPath = fs.realpathSync(resolved);
|
||||
const { buffer } = readRegularFileSync({
|
||||
filePath: resolvedRealPath,
|
||||
maxBytes: MAX_EXTERNAL_CATALOG_BYTES,
|
||||
});
|
||||
const payload = JSON.parse(buffer.toString("utf-8")) as unknown;
|
||||
const channel = parseExternalCatalogChannelEntries(payload).find(
|
||||
(entry) => entry.id === channelId,
|
||||
);
|
||||
if (channel) {
|
||||
return channel.preferOver;
|
||||
}
|
||||
} catch {
|
||||
// Ignore invalid catalog files.
|
||||
} catch (err) {
|
||||
// Surface oversized catalogs so operators know a configured file was
|
||||
// skipped — unlike parse or permission errors which mean the file is
|
||||
// genuinely unusable.
|
||||
if (err instanceof Error && err.message.startsWith("File exceeds")) {
|
||||
log.warn(
|
||||
`skipping oversized external catalog file (max ${MAX_EXTERNAL_CATALOG_BYTES} bytes): ${resolved}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
return [];
|
||||
|
||||
Reference in New Issue
Block a user