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
This commit is contained in:
Pavan Kumar Gondhi
2026-04-28 21:35:32 +05:30
committed by GitHub
parent 230f7122dd
commit bdfb408ce6
7 changed files with 289 additions and 58 deletions

View File

@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- fix(plugins): restrict bundled plugin dir resolution to trusted package roots. (#73275) Thanks @pgondhi987.
- fix(security): prevent workspace PATH injection via service env and trash helpers. (#73264) Thanks @pgondhi987.
- 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.
- 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.

View File

@@ -1,12 +1,13 @@
import { createWriteStream } from "node:fs";
import fs from "node:fs/promises";
import { request } from "node:https";
import path from "node:path";
import { Readable, Transform } from "node:stream";
import { pipeline } from "node:stream/promises";
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
import { runPluginCommandWithTimeout } from "openclaw/plugin-sdk/run-command";
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
import { CONFIG_DIR, extractArchive, resolveBrewExecutable } from "openclaw/plugin-sdk/setup-tools";
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path";
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
@@ -25,6 +26,8 @@ type ReleaseResponse = {
assets?: ReleaseAsset[];
};
const MAX_SIGNAL_CLI_ARCHIVE_BYTES = 256 * 1024 * 1024;
export type SignalInstallResult = {
ok: boolean;
cliPath?: string;
@@ -46,6 +49,20 @@ export function looksLikeArchive(name: string): boolean {
return name.endsWith(".tar.gz") || name.endsWith(".tgz") || name.endsWith(".zip");
}
function isNodeReadableStream(value: unknown): value is Readable {
return Boolean(value && typeof (value as { pipe?: unknown }).pipe === "function");
}
function chunkByteLength(chunk: unknown): number {
if (typeof chunk === "string") {
return Buffer.byteLength(chunk);
}
if (chunk instanceof Uint8Array) {
return chunk.byteLength;
}
return Buffer.byteLength(String(chunk));
}
/**
* Pick a native release asset from the official GitHub releases.
*
@@ -95,28 +112,37 @@ export function pickAsset(
}
async function downloadToFile(url: string, dest: string, maxRedirects = 5): Promise<void> {
await new Promise<void>((resolve, reject) => {
const req = request(url, (res) => {
if (res.statusCode && res.statusCode >= 300 && res.statusCode < 400) {
const location = res.headers.location;
if (!location || maxRedirects <= 0) {
reject(new Error("Redirect loop or missing Location header"));
const { response, release } = await fetchWithSsrFGuard({
url,
maxRedirects,
requireHttps: true,
capture: false,
auditContext: "signal-cli-install-archive",
});
try {
if (!response.ok || !response.body) {
throw new Error(`HTTP ${response.status || "?"} downloading file`);
}
let totalBytes = 0;
const body = response.body;
const readable = isNodeReadableStream(body) ? body : Readable.fromWeb(body as never);
const limiter = new Transform({
transform(chunk: unknown, _encoding, callback) {
totalBytes += chunkByteLength(chunk);
if (totalBytes > MAX_SIGNAL_CLI_ARCHIVE_BYTES) {
callback(new Error("signal-cli archive exceeds 256 MiB limit"));
return;
}
const redirectUrl = new URL(location, url).href;
resolve(downloadToFile(redirectUrl, dest, maxRedirects - 1));
return;
}
if (!res.statusCode || res.statusCode >= 400) {
reject(new Error(`HTTP ${res.statusCode ?? "?"} downloading file`));
return;
}
const out = createWriteStream(dest);
pipeline(res, out).then(resolve).catch(reject);
callback(null, chunk);
},
});
req.on("error", reject);
req.end();
});
const out = createWriteStream(dest);
await pipeline(readable, limiter, out);
} finally {
await release();
}
}
async function findSignalCliBinary(root: string): Promise<string | null> {

View File

@@ -21,9 +21,14 @@ assert.equal(typeof getPluginCommandSpecs, "function", "getPluginCommandSpecs mi
assert.equal(typeof matchPluginCommand, "function", "matchPluginCommand missing");
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-build-smoke-"));
const pluginId = "build-smoke-plugin";
const distPluginDir = path.join(repoRoot, "dist", "extensions", pluginId);
const runtimePluginDir = path.join(repoRoot, "dist-runtime", "extensions", pluginId);
function cleanup() {
clearPluginCommands();
fs.rmSync(distPluginDir, { recursive: true, force: true });
fs.rmSync(runtimePluginDir, { recursive: true, force: true });
fs.rmSync(tempRoot, { recursive: true, force: true });
}
@@ -37,10 +42,7 @@ process.on("SIGTERM", () => {
process.exit(143);
});
const pluginId = "build-smoke-plugin";
const distPluginDir = path.join(tempRoot, "dist", "extensions", pluginId);
fs.mkdirSync(distPluginDir, { recursive: true });
fs.writeFileSync(path.join(tempRoot, "package.json"), '{ "type": "module" }\n', "utf8");
fs.writeFileSync(
path.join(distPluginDir, "package.json"),
JSON.stringify(
@@ -98,12 +100,12 @@ fs.writeFileSync(
"utf8",
);
stageBundledPluginRuntime({ repoRoot: tempRoot });
stageBundledPluginRuntime({ repoRoot });
const runtimeEntryPath = path.join(tempRoot, "dist-runtime", "extensions", pluginId, "index.js");
const runtimeEntryPath = path.join(runtimePluginDir, "index.js");
assert.ok(fs.existsSync(runtimeEntryPath), "runtime overlay entry missing");
assert.equal(
fs.existsSync(path.join(tempRoot, "dist-runtime", "plugins", "commands.js")),
fs.existsSync(path.join(repoRoot, "dist-runtime", "plugins", "commands.js")),
false,
"dist-runtime must not stage a duplicate commands module",
);
@@ -115,7 +117,7 @@ const registry = loadOpenClawPlugins({
workspaceDir: tempRoot,
env: {
...process.env,
OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(tempRoot, "dist-runtime", "extensions"),
OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(repoRoot, "dist-runtime", "extensions"),
OPENCLAW_DISABLE_PLUGIN_DISCOVERY_CACHE: "1",
},
config: {

View File

@@ -67,6 +67,7 @@ export type GuardedFetchOptions = {
allowCrossOriginUnsafeRedirectReplay?: boolean;
timeoutMs?: number;
signal?: AbortSignal;
requireHttps?: boolean;
policy?: SsrFPolicy;
lookupFn?: LookupFn;
dispatcherPolicy?: PinnedDispatcherPolicy;
@@ -346,6 +347,10 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
await release();
throw new Error("Invalid URL: must be http or https");
}
if (params.requireHttps === true && parsedUrl.protocol !== "https:") {
await release();
throw new Error("URL must use https");
}
let dispatcher: Dispatcher | null = null;
try {

View File

@@ -108,7 +108,7 @@ function expectResolvedBundledDirFromRoot(params: {
expectResolvedBundledDir({
cwd: params.cwd ?? params.repoRoot,
expectedDir: path.join(params.repoRoot, params.expectedRelativeDir),
...(params.argv1 ? { argv1: params.argv1 } : {}),
argv1: params.argv1 ?? path.join(params.repoRoot, "openclaw.mjs"),
...(params.bundledDirOverride ? { bundledDirOverride: params.bundledDirOverride } : {}),
...(params.vitest !== undefined ? { vitest: params.vitest } : {}),
...(params.execArgv ? { execArgv: params.execArgv } : {}),
@@ -202,7 +202,7 @@ describe("resolveBundledPluginsDir", () => {
},
],
[
"prefers source extensions under vitest to avoid stale staged plugins",
"does not prefer source extensions from VITEST alone",
{
prefix: "openclaw-bundled-dir-vitest-",
hasExtensions: true,
@@ -210,7 +210,7 @@ describe("resolveBundledPluginsDir", () => {
hasDistExtensions: true,
},
{
expectedRelativeDir: "extensions",
expectedRelativeDir: path.join("dist-runtime", "extensions"),
vitest: "true",
},
],
@@ -248,6 +248,8 @@ describe("resolveBundledPluginsDir", () => {
seedBundledPluginTree(repoRoot, path.join("dist-runtime", "extensions"));
} else if (expectation.expectedRelativeDir === path.join("dist", "extensions")) {
seedBundledPluginTree(repoRoot, path.join("dist", "extensions"));
} else if (expectation.expectedRelativeDir === "extensions") {
seedBundledPluginTree(repoRoot, "extensions");
}
expectResolvedBundledDirFromRoot({
repoRoot,
@@ -270,6 +272,7 @@ describe("resolveBundledPluginsDir", () => {
fs.mkdirSync(path.join(repoRoot, "dist-runtime", "extensions", "discord"), {
recursive: true,
});
seedBundledPluginTree(repoRoot, "extensions");
expectResolvedBundledDirFromRoot({
repoRoot,
@@ -296,6 +299,140 @@ describe("resolveBundledPluginsDir", () => {
expect(fs.readdirSync(bundledDir ?? "")).toEqual([]);
});
it("ignores an existing override under an argv1-derived fake package root", () => {
const installedRoot = createOpenClawRoot({
prefix: "openclaw-bundled-dir-argv-override-reject-",
hasDistExtensions: true,
});
seedBundledPluginTree(installedRoot, path.join("dist", "extensions"));
vi.spyOn(process, "cwd").mockReturnValue(installedRoot);
process.argv[1] = path.join(installedRoot, "openclaw.mjs");
process.execArgv.length = 0;
delete process.env.VITEST;
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(installedRoot, "dist", "extensions");
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(bundledDir).toBeDefined();
expect(fs.realpathSync(bundledDir!)).not.toBe(
fs.realpathSync(path.join(installedRoot, "dist", "extensions")),
);
});
it("does not let VITEST relax existing override trust checks", () => {
const overrideRoot = makeRepoRoot("openclaw-bundled-dir-vitest-override-reject-");
seedBundledPluginTree(overrideRoot, "extensions", "memory-core");
vi.spyOn(process, "cwd").mockReturnValue(overrideRoot);
process.argv[1] = "/usr/bin/env";
process.execArgv.length = 0;
process.env.VITEST = "true";
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(overrideRoot, "extensions");
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(bundledDir).toBeDefined();
expect(fs.realpathSync(bundledDir!)).not.toBe(
fs.realpathSync(path.join(overrideRoot, "extensions")),
);
});
it("does not let VITEST add cwd to bundled plugin resolution candidates", () => {
const cwdRepoRoot = createOpenClawRoot({
prefix: "openclaw-bundled-dir-vitest-cwd-",
hasExtensions: true,
hasSrc: true,
hasGitCheckout: true,
});
seedBundledPluginTree(cwdRepoRoot, "extensions", "memory-core");
vi.spyOn(process, "cwd").mockReturnValue(cwdRepoRoot);
process.argv[1] = "/usr/bin/env";
process.execArgv.length = 0;
process.env.VITEST = "true";
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(bundledDir).toBeDefined();
expect(fs.realpathSync(bundledDir!)).not.toBe(
fs.realpathSync(path.join(cwdRepoRoot, "extensions")),
);
});
it("falls back from a missing override instead of returning an untrusted future path", () => {
vi.spyOn(process, "cwd").mockReturnValue(makeRepoRoot("openclaw-bundled-dir-missing-cwd-"));
process.argv[1] = "/usr/bin/env";
process.execArgv.length = 0;
delete process.env.VITEST;
const missingOverride = path.join(
makeRepoRoot("openclaw-bundled-dir-missing-override-"),
"extensions",
);
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = missingOverride;
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(bundledDir).toBeDefined();
expect(path.resolve(bundledDir!)).not.toBe(path.resolve(missingOverride));
});
it("falls back to argv root when an existing rejected override is unrelated", () => {
const installedRoot = createOpenClawRoot({
prefix: "openclaw-bundled-dir-rejected-override-argv-",
hasDistExtensions: true,
});
seedBundledPluginTree(installedRoot, path.join("dist", "extensions"));
const overrideRoot = makeRepoRoot("openclaw-bundled-dir-rejected-override-");
seedBundledPluginTree(overrideRoot, "extensions", "memory-core");
vi.spyOn(process, "cwd").mockReturnValue(makeRepoRoot("openclaw-bundled-dir-rejected-cwd-"));
process.argv[1] = path.join(installedRoot, "openclaw.mjs");
process.execArgv.length = 0;
delete process.env.VITEST;
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(overrideRoot, "extensions");
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(fs.realpathSync(bundledDir ?? "")).toBe(
fs.realpathSync(path.join(installedRoot, "dist", "extensions")),
);
});
it("does not resolve bundled plugins from cwd when argv1 is not a package root", () => {
const cwdRepoRoot = createOpenClawRoot({
prefix: "openclaw-bundled-dir-untrusted-cwd-",
hasExtensions: true,
hasSrc: true,
hasGitCheckout: true,
});
fs.mkdirSync(path.join(cwdRepoRoot, "extensions", "memory-core"), { recursive: true });
fs.writeFileSync(
path.join(cwdRepoRoot, "extensions", "memory-core", "runtime-api.js"),
"export const marker = 'untrusted-cwd';\n",
"utf8",
);
vi.spyOn(process, "cwd").mockReturnValue(cwdRepoRoot);
process.argv[1] = "/usr/bin/env";
process.execArgv.length = 0;
delete process.env.VITEST;
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
delete process.env.OPENCLAW_DISABLE_BUNDLED_PLUGINS;
const bundledDir = resolveBundledPluginsDir();
expect(bundledDir).toBeDefined();
expect(fs.realpathSync(bundledDir!)).not.toBe(
fs.realpathSync(path.join(cwdRepoRoot, "extensions")),
);
});
it.each([
{
name: "prefers the running CLI package root over an unrelated cwd checkout",

View File

@@ -46,6 +46,65 @@ function hasUsableBundledPluginTree(pluginsDir: string): boolean {
}
}
function safeRealpathSync(targetPath: string): string | null {
try {
return fs.realpathSync.native(targetPath);
} catch {
return null;
}
}
function pathContains(parentDir: string, childPath: string): boolean {
const relative = path.relative(parentDir, childPath);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
function trustedBundledPluginRootsForPackageRoot(packageRoot: string): string[] {
const roots = [
path.join(packageRoot, "dist", "extensions"),
path.join(packageRoot, "dist-runtime", "extensions"),
];
if (isSourceCheckoutRoot(packageRoot)) {
roots.push(path.join(packageRoot, "extensions"));
}
return roots;
}
function resolveTrustedExistingOverride(resolvedOverride: string): string | null {
const realOverride = safeRealpathSync(resolvedOverride);
if (!realOverride) {
return null;
}
const modulePackageRoot = resolveOpenClawPackageRootSync({ moduleUrl: import.meta.url });
const packageRoots = modulePackageRoot ? [modulePackageRoot] : [];
const trustedRoots = packageRoots
.flatMap((packageRoot) => trustedBundledPluginRootsForPackageRoot(packageRoot))
.map((trustedRoot) => safeRealpathSync(trustedRoot))
.filter((entry): entry is string => Boolean(entry));
if (!trustedRoots.some((trustedRoot) => pathContains(trustedRoot, realOverride))) {
return null;
}
if (!hasUsableBundledPluginTree(realOverride)) {
return null;
}
return realOverride;
}
function overrideResolvesUnderPackageBundledRoot(params: {
resolvedOverride: string;
packageRoot: string;
}): boolean {
const realOverride = safeRealpathSync(params.resolvedOverride);
if (!realOverride) {
return false;
}
return trustedBundledPluginRootsForPackageRoot(params.packageRoot)
.map((trustedRoot) => safeRealpathSync(trustedRoot))
.filter((entry): entry is string => Boolean(entry))
.some((trustedRoot) => pathContains(trustedRoot, realOverride));
}
function runningSourceTypeScriptProcess(): boolean {
const argv1 = process.argv[1]?.toLowerCase();
if (
@@ -83,7 +142,8 @@ function resolveBundledDirFromPackageRoot(
const sourceExtensionsDir = path.join(packageRoot, "extensions");
const builtExtensionsDir = path.join(packageRoot, "dist", "extensions");
const sourceCheckout = isSourceCheckoutRoot(packageRoot);
if (preferSourceCheckout && fs.existsSync(sourceExtensionsDir)) {
const hasUsableSourceTree = sourceCheckout && hasUsableBundledPluginTree(sourceExtensionsDir);
if (preferSourceCheckout && hasUsableSourceTree) {
return sourceExtensionsDir;
}
// Local source checkouts stage a runtime-complete bundled plugin tree under
@@ -102,7 +162,7 @@ function resolveBundledDirFromPackageRoot(
if (hasUsableBuiltTree) {
return builtExtensionsDir;
}
if (sourceCheckout && fs.existsSync(sourceExtensionsDir)) {
if (hasUsableSourceTree) {
return sourceExtensionsDir;
}
return undefined;
@@ -114,37 +174,33 @@ export function resolveBundledPluginsDir(env: NodeJS.ProcessEnv = process.env):
}
const override = env.OPENCLAW_BUNDLED_PLUGINS_DIR?.trim();
let rejectedExistingOverride: string | null = null;
if (override) {
const resolvedOverride = resolveUserPath(override, env);
if (fs.existsSync(resolvedOverride)) {
return resolvedOverride;
}
// Installed CLIs can inherit stale bundled-dir overrides from older shells
// or debug sessions. Prefer the package that owns argv[1] over a broken
// override so bundled providers keep working in packaged installs.
try {
const argvPackageRoot = resolveOpenClawPackageRootSync({ argv1: process.argv[1] });
if (argvPackageRoot && !isSourceCheckoutRoot(argvPackageRoot)) {
const argvFallback = resolveBundledDirFromPackageRoot(argvPackageRoot, false);
if (argvFallback) {
return argvFallback;
}
const trustedOverride = resolveTrustedExistingOverride(resolvedOverride);
if (trustedOverride) {
return trustedOverride;
}
} catch {
// ignore
rejectedExistingOverride = resolvedOverride;
}
return resolvedOverride;
}
const preferSourceCheckout = Boolean(env.VITEST) || runningSourceTypeScriptProcess();
const preferSourceCheckout = runningSourceTypeScriptProcess();
try {
const argvRoot = resolveOpenClawPackageRootSync({ argv1: process.argv[1] });
const cwdRoot = resolveOpenClawPackageRootSync({ cwd: process.cwd() });
const rejectedOverrideUsesArgvRoot = Boolean(
argvRoot &&
rejectedExistingOverride &&
overrideResolvesUnderPackageBundledRoot({
resolvedOverride: rejectedExistingOverride,
packageRoot: argvRoot,
}),
);
const safeArgvRoot = rejectedOverrideUsesArgvRoot ? null : argvRoot;
const moduleRoot = resolveOpenClawPackageRootSync({ moduleUrl: import.meta.url });
const packageRoots = (
preferSourceCheckout ? [cwdRoot, argvRoot, moduleRoot] : [argvRoot, cwdRoot, moduleRoot]
).filter(
const packageRoots = [safeArgvRoot, moduleRoot].filter(
(entry, index, all): entry is string => Boolean(entry) && all.indexOf(entry) === index,
);
for (const packageRoot of packageRoots) {

View File

@@ -1,6 +1,7 @@
import path from "node:path";
import { describe, expect, it } from "vitest";
import { withPathResolutionEnv } from "../test-utils/env.js";
import { resolveBundledPluginsDir } from "./bundled-dir.js";
import { formatPluginSourceForTable, resolvePluginSourceRoots } from "./source-display.js";
const PLUGIN_SOURCE_ROOTS = {
@@ -71,17 +72,20 @@ describe("formatPluginSourceForTable", () => {
createFormattedSourceExpectation("global", "global", "demo-global", "index.js"),
])("shortens $origin sources under the $sourceKey root", expectFormattedSourceCase);
it("resolves source roots from an explicit env override", () => {
it("ignores untrusted explicit env override for the stock source root", () => {
const homeDir = path.resolve(path.sep, "tmp", "openclaw-home");
const rawEnv = {
OPENCLAW_BUNDLED_PLUGINS_DIR: "~/bundled",
OPENCLAW_STATE_DIR: "~/state",
} as NodeJS.ProcessEnv;
const stock = withPathResolutionEnv(homeDir, rawEnv, (env) => resolveBundledPluginsDir(env));
expect(stock).toBeDefined();
expectResolvedSourceRoots({
homeDir,
env: {
OPENCLAW_BUNDLED_PLUGINS_DIR: "~/bundled",
OPENCLAW_STATE_DIR: "~/state",
} as NodeJS.ProcessEnv,
env: rawEnv,
workspaceDir: "~/ws",
expected: {
stock: path.join(homeDir, "bundled"),
stock: stock!,
global: path.join(homeDir, "state", "extensions"),
workspace: path.join(homeDir, "ws", ".openclaw", "extensions"),
},