Skip to content

Commit bdfb408

Browse files
pgondhi987pgondhi987
authored
fix(plugins): restrict bundled plugin dir resolution to trusted package roots (#73275)
* fix: address issue * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address codex review feedback * fix: address codex review feedback * fix: address codex review feedback * fix: address PR review feedback * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address review feedback * docs: add changelog entry for PR merge
1 parent 230f712 commit bdfb408

7 files changed

Lines changed: 289 additions & 58 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
1212

1313
### Fixes
1414

15+
- fix(plugins): restrict bundled plugin dir resolution to trusted package roots. (#73275) Thanks @pgondhi987.
1516
- fix(security): prevent workspace PATH injection via service env and trash helpers. (#73264) Thanks @pgondhi987.
1617
- Active Memory: allow `allowedChatTypes` to include explicit portal/webchat sessions and classify `agent:...:explicit:...` session keys before opaque session ids can shadow the chat type. Fixes #65775. (#66285) Thanks @Lidang-Jiang.
1718
- Active Memory: allow the hidden recall sub-agent to use both `memory_recall` and the legacy `memory_search`/`memory_get` memory tool contract, so bundled `memory-lancedb` recall works without breaking the default `memory-core` path. Fixes #73502. (#73584) Thanks @Takhoffman.

‎extensions/signal/src/install-signal-cli.ts‎

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { createWriteStream } from "node:fs";
22
import fs from "node:fs/promises";
3-
import { request } from "node:https";
43
import path from "node:path";
4+
import { Readable, Transform } from "node:stream";
55
import { pipeline } from "node:stream/promises";
66
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
77
import { runPluginCommandWithTimeout } from "openclaw/plugin-sdk/run-command";
88
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
99
import { CONFIG_DIR, extractArchive, resolveBrewExecutable } from "openclaw/plugin-sdk/setup-tools";
10+
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
1011
import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path";
1112
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
1213

@@ -25,6 +26,8 @@ type ReleaseResponse = {
2526
assets?: ReleaseAsset[];
2627
};
2728

29+
const MAX_SIGNAL_CLI_ARCHIVE_BYTES = 256 * 1024 * 1024;
30+
2831
export type SignalInstallResult = {
2932
ok: boolean;
3033
cliPath?: string;
@@ -46,6 +49,20 @@ export function looksLikeArchive(name: string): boolean {
4649
return name.endsWith(".tar.gz") || name.endsWith(".tgz") || name.endsWith(".zip");
4750
}
4851

52+
function isNodeReadableStream(value: unknown): value is Readable {
53+
return Boolean(value && typeof (value as { pipe?: unknown }).pipe === "function");
54+
}
55+
56+
function chunkByteLength(chunk: unknown): number {
57+
if (typeof chunk === "string") {
58+
return Buffer.byteLength(chunk);
59+
}
60+
if (chunk instanceof Uint8Array) {
61+
return chunk.byteLength;
62+
}
63+
return Buffer.byteLength(String(chunk));
64+
}
65+
4966
/**
5067
* Pick a native release asset from the official GitHub releases.
5168
*
@@ -95,28 +112,37 @@ export function pickAsset(
95112
}
96113

97114
async function downloadToFile(url: string, dest: string, maxRedirects = 5): Promise<void> {
98-
await new Promise<void>((resolve, reject) => {
99-
const req = request(url, (res) => {
100-
if (res.statusCode && res.statusCode >= 300 && res.statusCode < 400) {
101-
const location = res.headers.location;
102-
if (!location || maxRedirects <= 0) {
103-
reject(new Error("Redirect loop or missing Location header"));
115+
const { response, release } = await fetchWithSsrFGuard({
116+
url,
117+
maxRedirects,
118+
requireHttps: true,
119+
capture: false,
120+
auditContext: "signal-cli-install-archive",
121+
});
122+
try {
123+
if (!response.ok || !response.body) {
124+
throw new Error(`HTTP ${response.status || "?"} downloading file`);
125+
}
126+
127+
let totalBytes = 0;
128+
const body = response.body;
129+
const readable = isNodeReadableStream(body) ? body : Readable.fromWeb(body as never);
130+
const limiter = new Transform({
131+
transform(chunk: unknown, _encoding, callback) {
132+
totalBytes += chunkByteLength(chunk);
133+
if (totalBytes > MAX_SIGNAL_CLI_ARCHIVE_BYTES) {
134+
callback(new Error("signal-cli archive exceeds 256 MiB limit"));
104135
return;
105136
}
106-
const redirectUrl = new URL(location, url).href;
107-
resolve(downloadToFile(redirectUrl, dest, maxRedirects - 1));
108-
return;
109-
}
110-
if (!res.statusCode || res.statusCode >= 400) {
111-
reject(new Error(`HTTP ${res.statusCode ?? "?"} downloading file`));
112-
return;
113-
}
114-
const out = createWriteStream(dest);
115-
pipeline(res, out).then(resolve).catch(reject);
137+
callback(null, chunk);
138+
},
116139
});
117-
req.on("error", reject);
118-
req.end();
119-
});
140+
141+
const out = createWriteStream(dest);
142+
await pipeline(readable, limiter, out);
143+
} finally {
144+
await release();
145+
}
120146
}
121147

122148
async function findSignalCliBinary(root: string): Promise<string | null> {

‎scripts/test-built-plugin-singleton.mjs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@ assert.equal(typeof getPluginCommandSpecs, "function", "getPluginCommandSpecs mi
2121
assert.equal(typeof matchPluginCommand, "function", "matchPluginCommand missing");
2222

2323
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-build-smoke-"));
24+
const pluginId = "build-smoke-plugin";
25+
const distPluginDir = path.join(repoRoot, "dist", "extensions", pluginId);
26+
const runtimePluginDir = path.join(repoRoot, "dist-runtime", "extensions", pluginId);
2427

2528
function cleanup() {
2629
clearPluginCommands();
30+
fs.rmSync(distPluginDir, { recursive: true, force: true });
31+
fs.rmSync(runtimePluginDir, { recursive: true, force: true });
2732
fs.rmSync(tempRoot, { recursive: true, force: true });
2833
}
2934

@@ -37,10 +42,7 @@ process.on("SIGTERM", () => {
3742
process.exit(143);
3843
});
3944

40-
const pluginId = "build-smoke-plugin";
41-
const distPluginDir = path.join(tempRoot, "dist", "extensions", pluginId);
4245
fs.mkdirSync(distPluginDir, { recursive: true });
43-
fs.writeFileSync(path.join(tempRoot, "package.json"), '{ "type": "module" }\n', "utf8");
4446
fs.writeFileSync(
4547
path.join(distPluginDir, "package.json"),
4648
JSON.stringify(
@@ -98,12 +100,12 @@ fs.writeFileSync(
98100
"utf8",
99101
);
100102

101-
stageBundledPluginRuntime({ repoRoot: tempRoot });
103+
stageBundledPluginRuntime({ repoRoot });
102104

103-
const runtimeEntryPath = path.join(tempRoot, "dist-runtime", "extensions", pluginId, "index.js");
105+
const runtimeEntryPath = path.join(runtimePluginDir, "index.js");
104106
assert.ok(fs.existsSync(runtimeEntryPath), "runtime overlay entry missing");
105107
assert.equal(
106-
fs.existsSync(path.join(tempRoot, "dist-runtime", "plugins", "commands.js")),
108+
fs.existsSync(path.join(repoRoot, "dist-runtime", "plugins", "commands.js")),
107109
false,
108110
"dist-runtime must not stage a duplicate commands module",
109111
);
@@ -115,7 +117,7 @@ const registry = loadOpenClawPlugins({
115117
workspaceDir: tempRoot,
116118
env: {
117119
...process.env,
118-
OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(tempRoot, "dist-runtime", "extensions"),
120+
OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(repoRoot, "dist-runtime", "extensions"),
119121
OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: "1",
120122
},
121123
config: {

‎src/infra/net/fetch-guard.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export type GuardedFetchOptions = {
6767
allowCrossOriginUnsafeRedirectReplay?: boolean;
6868
timeoutMs?: number;
6969
signal?: AbortSignal;
70+
requireHttps?: boolean;
7071
policy?: SsrFPolicy;
7172
lookupFn?: LookupFn;
7273
dispatcherPolicy?: PinnedDispatcherPolicy;
@@ -346,6 +347,10 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
346347
await release();
347348
throw new Error("Invalid URL: must be http or https");
348349
}
350+
if (params.requireHttps === true && parsedUrl.protocol !== "https:") {
351+
await release();
352+
throw new Error("URL must use https");
353+
}
349354

350355
let dispatcher: Dispatcher | null = null;
351356
try {

‎src/plugins/bundled-dir.test.ts‎

Lines changed: 140 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ function expectResolvedBundledDirFromRoot(params: {
108108
expectResolvedBundledDir({
109109
cwd: params.cwd ?? params.repoRoot,
110110
expectedDir: path.join(params.repoRoot, params.expectedRelativeDir),
111-
...(params.argv1 ? { argv1: params.argv1 } : {}),
111+
argv1: params.argv1 ?? path.join(params.repoRoot, "openclaw.mjs"),
112112
...(params.bundledDirOverride ? { bundledDirOverride: params.bundledDirOverride } : {}),
113113
...(params.vitest !== undefined ? { vitest: params.vitest } : {}),
114114
...(params.execArgv ? { execArgv: params.execArgv } : {}),
@@ -202,15 +202,15 @@ describe("resolveBundledPluginsDir", () => {
202202
},
203203
],
204204
[
205-
"prefers source extensions under vitest to avoid stale staged plugins",
205+
"does not prefer source extensions from VITEST alone",
206206
{
207207
prefix: "openclaw-bundled-dir-vitest-",
208208
hasExtensions: true,
209209
hasDistRuntimeExtensions: true,
210210
hasDistExtensions: true,
211211
},
212212
{
213-
expectedRelativeDir: "extensions",
213+
expectedRelativeDir: path.join("dist-runtime", "extensions"),
214214
vitest: "true",
215215
},
216216
],
@@ -248,6 +248,8 @@ describe("resolveBundledPluginsDir", () => {
248248
seedBundledPluginTree(repoRoot, path.join("dist-runtime", "extensions"));
249249
} else if (expectation.expectedRelativeDir === path.join("dist", "extensions")) {
250250
seedBundledPluginTree(repoRoot, path.join("dist", "extensions"));
251+
} else if (expectation.expectedRelativeDir === "extensions") {
252+
seedBundledPluginTree(repoRoot, "extensions");
251253
}
252254
expectResolvedBundledDirFromRoot({
253255
repoRoot,
@@ -270,6 +272,7 @@ describe("resolveBundledPluginsDir", () => {
270272
fs.mkdirSync(path.join(repoRoot, "dist-runtime", "extensions", "discord"), {
271273
recursive: true,
272274
});
275+
seedBundledPluginTree(repoRoot, "extensions");
273276

274277
expectResolvedBundledDirFromRoot({
275278
repoRoot,
@@ -296,6 +299,140 @@ describe("resolveBundledPluginsDir", () => {
296299
expect(fs.readdirSync(bundledDir ?? "")).toEqual([]);
297300
});
298301

302+
it("ignores an existing override under an argv1-derived fake package root", () => {
303+
const installedRoot = createOpenClawRoot({
304+
prefix: "openclaw-bundled-dir-argv-override-reject-",
305+
hasDistExtensions: true,
306+
});
307+
seedBundledPluginTree(installedRoot, path.join("dist", "extensions"));
308+
309+
vi.spyOn(process, "cwd").mockReturnValue(installedRoot);
310+
process.argv[1] = path.join(installedRoot, "openclaw.mjs");
311+
process.execArgv.length = 0;
312+
delete process.env.VITEST;
313+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(installedRoot, "dist", "extensions");
314+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
315+
316+
const bundledDir = resolveBundledPluginsDir();
317+
318+
expect(bundledDir).toBeDefined();
319+
expect(fs.realpathSync(bundledDir!)).not.toBe(
320+
fs.realpathSync(path.join(installedRoot, "dist", "extensions")),
321+
);
322+
});
323+
324+
it("does not let VITEST relax existing override trust checks", () => {
325+
const overrideRoot = makeRepoRoot("openclaw-bundled-dir-vitest-override-reject-");
326+
seedBundledPluginTree(overrideRoot, "extensions", "memory-core");
327+
328+
vi.spyOn(process, "cwd").mockReturnValue(overrideRoot);
329+
process.argv[1] = "/usr/bin/env";
330+
process.execArgv.length = 0;
331+
process.env.VITEST = "true";
332+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(overrideRoot, "extensions");
333+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
334+
335+
const bundledDir = resolveBundledPluginsDir();
336+
337+
expect(bundledDir).toBeDefined();
338+
expect(fs.realpathSync(bundledDir!)).not.toBe(
339+
fs.realpathSync(path.join(overrideRoot, "extensions")),
340+
);
341+
});
342+
343+
it("does not let VITEST add cwd to bundled plugin resolution candidates", () => {
344+
const cwdRepoRoot = createOpenClawRoot({
345+
prefix: "openclaw-bundled-dir-vitest-cwd-",
346+
hasExtensions: true,
347+
hasSrc: true,
348+
hasGitCheckout: true,
349+
});
350+
seedBundledPluginTree(cwdRepoRoot, "extensions", "memory-core");
351+
352+
vi.spyOn(process, "cwd").mockReturnValue(cwdRepoRoot);
353+
process.argv[1] = "/usr/bin/env";
354+
process.execArgv.length = 0;
355+
process.env.VITEST = "true";
356+
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
357+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
358+
359+
const bundledDir = resolveBundledPluginsDir();
360+
361+
expect(bundledDir).toBeDefined();
362+
expect(fs.realpathSync(bundledDir!)).not.toBe(
363+
fs.realpathSync(path.join(cwdRepoRoot, "extensions")),
364+
);
365+
});
366+
367+
it("falls back from a missing override instead of returning an untrusted future path", () => {
368+
vi.spyOn(process, "cwd").mockReturnValue(makeRepoRoot("openclaw-bundled-dir-missing-cwd-"));
369+
process.argv[1] = "/usr/bin/env";
370+
process.execArgv.length = 0;
371+
delete process.env.VITEST;
372+
const missingOverride = path.join(
373+
makeRepoRoot("openclaw-bundled-dir-missing-override-"),
374+
"extensions",
375+
);
376+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = missingOverride;
377+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
378+
379+
const bundledDir = resolveBundledPluginsDir();
380+
381+
expect(bundledDir).toBeDefined();
382+
expect(path.resolve(bundledDir!)).not.toBe(path.resolve(missingOverride));
383+
});
384+
385+
it("falls back to argv root when an existing rejected override is unrelated", () => {
386+
const installedRoot = createOpenClawRoot({
387+
prefix: "openclaw-bundled-dir-rejected-override-argv-",
388+
hasDistExtensions: true,
389+
});
390+
seedBundledPluginTree(installedRoot, path.join("dist", "extensions"));
391+
const overrideRoot = makeRepoRoot("openclaw-bundled-dir-rejected-override-");
392+
seedBundledPluginTree(overrideRoot, "extensions", "memory-core");
393+
394+
vi.spyOn(process, "cwd").mockReturnValue(makeRepoRoot("openclaw-bundled-dir-rejected-cwd-"));
395+
process.argv[1] = path.join(installedRoot, "openclaw.mjs");
396+
process.execArgv.length = 0;
397+
delete process.env.VITEST;
398+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(overrideRoot, "extensions");
399+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
400+
401+
const bundledDir = resolveBundledPluginsDir();
402+
403+
expect(fs.realpathSync(bundledDir ?? "")).toBe(
404+
fs.realpathSync(path.join(installedRoot, "dist", "extensions")),
405+
);
406+
});
407+
408+
it("does not resolve bundled plugins from cwd when argv1 is not a package root", () => {
409+
const cwdRepoRoot = createOpenClawRoot({
410+
prefix: "openclaw-bundled-dir-untrusted-cwd-",
411+
hasExtensions: true,
412+
hasSrc: true,
413+
hasGitCheckout: true,
414+
});
415+
fs.mkdirSync(path.join(cwdRepoRoot, "extensions", "memory-core"), { recursive: true });
416+
fs.writeFileSync(
417+
path.join(cwdRepoRoot, "extensions", "memory-core", "runtime-api.js"),
418+
"export const marker = 'untrusted-cwd';\n",
419+
"utf8",
420+
);
421+
vi.spyOn(process, "cwd").mockReturnValue(cwdRepoRoot);
422+
process.argv[1] = "/usr/bin/env";
423+
process.execArgv.length = 0;
424+
delete process.env.VITEST;
425+
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
426+
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
427+
428+
const bundledDir = resolveBundledPluginsDir();
429+
430+
expect(bundledDir).toBeDefined();
431+
expect(fs.realpathSync(bundledDir!)).not.toBe(
432+
fs.realpathSync(path.join(cwdRepoRoot, "extensions")),
433+
);
434+
});
435+
299436
it.each([
300437
{
301438
name: "prefers the running CLI package root over an unrelated cwd checkout",

0 commit comments

Comments
 (0)