mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 14:10:51 +00:00
fix(security): block npm_execpath injection from workspace .env [AI-assisted] (#73262)
* fix: address issue * fix: finalize issue changes * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
7a23c18830
commit
ccb3af556f
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(security): block npm_execpath injection from workspace .env [AI-assisted]. (#73262) Thanks @pgondhi987.
|
||||
- Tools/web_fetch: decode response bodies from raw bytes using declared HTTP, XML, or HTML meta charsets before extraction, so Shift_JIS and other legacy-charset pages no longer return mojibake. Fixes #72916. Thanks @amknight.
|
||||
- Channels/Discord: bound message read/search REST calls, route those actions through Gateway execution, and fall back to `CommandTargetSessionKey` for inbound hook session keys so Discord reads do not hang and hooks still fire when `SessionKey` is empty. Fixes #73431. (#73521) Thanks @amknight.
|
||||
- Plugins/media: auto-enable provider plugins referenced by `agents.defaults.imageGenerationModel`, `videoGenerationModel`, and `musicGenerationModel` primary/fallback refs, so configured Google and MiniMax media providers do not stay disabled behind a restrictive plugin allowlist. Thanks @vincentkoc.
|
||||
|
||||
@@ -366,6 +366,20 @@ describe("loadDotEnv", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["npm_execpath", "NPM_EXECPATH"])("blocks %s from workspace .env", async (key) => {
|
||||
await withIsolatedEnvAndCwd(async () => {
|
||||
await withDotEnvFixture(async ({ cwdDir }) => {
|
||||
await writeEnvFile(path.join(cwdDir, ".env"), `${key}=./evil/npm-cli.js\n`);
|
||||
|
||||
delete process.env[key];
|
||||
|
||||
loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true });
|
||||
|
||||
expect(process.env[key]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("still allows trusted global .env to set non-workspace runtime vars", async () => {
|
||||
await withIsolatedEnvAndCwd(async () => {
|
||||
await withDotEnvFixture(async ({ cwdDir, stateDir }) => {
|
||||
|
||||
@@ -27,6 +27,7 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
|
||||
"MINIMAX_API_HOST",
|
||||
"NODE_TLS_REJECT_UNAUTHORIZED",
|
||||
"NO_PROXY",
|
||||
"NPM_EXECPATH",
|
||||
"OPENAI_API_KEY",
|
||||
"OPENAI_API_KEYS",
|
||||
"OPENCLAW_AGENT_DIR",
|
||||
|
||||
@@ -105,18 +105,24 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe("resolveBundledRuntimeDepsNpmRunner", () => {
|
||||
it("uses npm_execpath through node on Windows when available", () => {
|
||||
it("ignores npm_execpath and uses the Node-adjacent npm CLI on Windows", () => {
|
||||
const execPath = "C:\\Program Files\\nodejs\\node.exe";
|
||||
const npmCliPath = path.win32.resolve(
|
||||
path.win32.dirname(execPath),
|
||||
"node_modules/npm/bin/npm-cli.js",
|
||||
);
|
||||
const runner = resolveBundledRuntimeDepsNpmRunner({
|
||||
env: { npm_execpath: "C:\\node\\node_modules\\npm\\bin\\npm-cli.js" },
|
||||
execPath: "C:\\Program Files\\nodejs\\node.exe",
|
||||
existsSync: (candidate) => candidate === "C:\\node\\node_modules\\npm\\bin\\npm-cli.js",
|
||||
env: { npm_execpath: "C:\\repo\\evil\\npm-cli.js" },
|
||||
execPath,
|
||||
existsSync: (candidate) =>
|
||||
candidate === "C:\\repo\\evil\\npm-cli.js" || candidate === npmCliPath,
|
||||
npmArgs: ["install", "acpx@0.5.3"],
|
||||
platform: "win32",
|
||||
});
|
||||
|
||||
expect(runner).toEqual({
|
||||
command: "C:\\Program Files\\nodejs\\node.exe",
|
||||
args: ["C:\\node\\node_modules\\npm\\bin\\npm-cli.js", "install", "acpx@0.5.3"],
|
||||
command: execPath,
|
||||
args: [npmCliPath, "install", "acpx@0.5.3"],
|
||||
});
|
||||
});
|
||||
|
||||
@@ -139,6 +145,8 @@ describe("resolveBundledRuntimeDepsNpmRunner", () => {
|
||||
npm_config_global: "true",
|
||||
npm_config_location: "global",
|
||||
npm_config_prefix: "/opt/homebrew",
|
||||
npm_execpath: "/repo/evil/npm-cli.js",
|
||||
NPM_EXECPATH: "/repo/evil-uppercase/npm-cli.js",
|
||||
},
|
||||
{ cacheDir: "/opt/openclaw/runtime-cache" },
|
||||
),
|
||||
@@ -175,15 +183,16 @@ describe("resolveBundledRuntimeDepsNpmRunner", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores pnpm npm_execpath and falls back to npm", () => {
|
||||
it("ignores npm_execpath and falls back to Node-adjacent npm", () => {
|
||||
const execPath = "/opt/node/bin/node";
|
||||
const npmCliPath = "/opt/node/lib/node_modules/npm/bin/npm-cli.js";
|
||||
const runner = resolveBundledRuntimeDepsNpmRunner({
|
||||
env: {
|
||||
npm_execpath: "/home/runner/setup-pnpm/node_modules/.bin/pnpm.cjs",
|
||||
npm_execpath: "/home/runner/repo/evil/npm-cli.js",
|
||||
},
|
||||
execPath,
|
||||
existsSync: (candidate) => candidate === npmCliPath,
|
||||
existsSync: (candidate) =>
|
||||
candidate === "/home/runner/repo/evil/npm-cli.js" || candidate === npmCliPath,
|
||||
npmArgs: ["install", "acpx@0.5.3"],
|
||||
platform: "linux",
|
||||
});
|
||||
@@ -206,24 +215,18 @@ describe("resolveBundledRuntimeDepsNpmRunner", () => {
|
||||
).toThrow("Unable to resolve a safe npm executable on Windows");
|
||||
});
|
||||
|
||||
it("prefixes PATH with the active Node directory on POSIX", () => {
|
||||
const runner = resolveBundledRuntimeDepsNpmRunner({
|
||||
env: {
|
||||
PATH: "/usr/bin:/bin",
|
||||
},
|
||||
execPath: "/opt/node/bin/node",
|
||||
existsSync: () => false,
|
||||
npmArgs: ["install", "acpx@0.5.3"],
|
||||
platform: "linux",
|
||||
});
|
||||
|
||||
expect(runner).toEqual({
|
||||
command: "npm",
|
||||
args: ["install", "acpx@0.5.3"],
|
||||
env: {
|
||||
PATH: `/opt/node/bin${path.delimiter}/usr/bin:/bin`,
|
||||
},
|
||||
});
|
||||
it("refuses POSIX npm shim fallback when npm-cli.js is unavailable", () => {
|
||||
expect(() =>
|
||||
resolveBundledRuntimeDepsNpmRunner({
|
||||
env: {
|
||||
PATH: "/repo/evil/bin:/usr/bin:/bin",
|
||||
},
|
||||
execPath: "/opt/node/bin/node",
|
||||
existsSync: (candidate) => candidate === "/opt/node/bin/npm",
|
||||
npmArgs: ["install"],
|
||||
platform: "linux",
|
||||
}),
|
||||
).toThrow("Unable to resolve a safe npm executable");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -287,11 +290,16 @@ describe("installBundledRuntimeDeps", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the npm cmd shim on Windows", () => {
|
||||
it("ignores npm_execpath during Windows installs", () => {
|
||||
const installRoot = makeTempDir();
|
||||
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
const safeNpmCliPath = path.win32.resolve(
|
||||
path.win32.dirname(process.execPath),
|
||||
"node_modules/npm/bin/npm-cli.js",
|
||||
);
|
||||
const attackerNpmCliPath = "C:\\repo\\evil\\npm-cli.js";
|
||||
vi.spyOn(fs, "existsSync").mockImplementation(
|
||||
(candidate) => candidate === "C:\\node\\node_modules\\npm\\bin\\npm-cli.js",
|
||||
(candidate) => candidate === attackerNpmCliPath || candidate === safeNpmCliPath,
|
||||
);
|
||||
spawnSyncMock.mockImplementation((_command, _args, options) => {
|
||||
writeInstalledPackage(String(options?.cwd ?? ""), "acpx", "0.5.3");
|
||||
@@ -311,13 +319,13 @@ describe("installBundledRuntimeDeps", () => {
|
||||
env: {
|
||||
npm_config_prefix: "C:\\prefix",
|
||||
PATH: "C:\\node",
|
||||
npm_execpath: "C:\\node\\node_modules\\npm\\bin\\npm-cli.js",
|
||||
npm_execpath: attackerNpmCliPath,
|
||||
},
|
||||
});
|
||||
|
||||
expect(spawnSyncMock).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
["C:\\node\\node_modules\\npm\\bin\\npm-cli.js", "install", "--ignore-scripts", "acpx@0.5.3"],
|
||||
[safeNpmCliPath, "install", "--ignore-scripts", "acpx@0.5.3"],
|
||||
expect.objectContaining({
|
||||
cwd: installRoot,
|
||||
windowsHide: true,
|
||||
@@ -338,6 +346,15 @@ describe("installBundledRuntimeDeps", () => {
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(spawnSyncMock).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.any(Array),
|
||||
expect.objectContaining({
|
||||
env: expect.not.objectContaining({
|
||||
npm_execpath: expect.any(String),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("hides async npm child windows for startup repair installs", async () => {
|
||||
|
||||
@@ -75,6 +75,7 @@ const MIRRORED_PACKAGE_RUNTIME_DEP_PLUGIN_ID = "openclaw-core";
|
||||
const BUNDLED_RUNTIME_MIRROR_PLUGIN_REGION_RE = /(?:^|\n)\/\/#region extensions\/[^/\s]+(?:\/|$)/u;
|
||||
const BUNDLED_RUNTIME_MIRROR_IMPORT_SPECIFIER_RE =
|
||||
/(?:^|[;\n])\s*(?:import|export)\s+(?:[^'"()]+?\s+from\s+)?["']([^"']+)["']|\bimport\(\s*["']([^"']+)["']\s*\)|\brequire\(\s*["']([^"']+)["']\s*\)/g;
|
||||
const NPM_EXECPATH_ENV_KEY = "npm_execpath";
|
||||
|
||||
const registeredBundledRuntimeDepNodePaths = new Set<string>();
|
||||
|
||||
@@ -1267,10 +1268,16 @@ export function createBundledRuntimeDepsInstallEnv(
|
||||
env: NodeJS.ProcessEnv,
|
||||
options: { cacheDir?: string } = {},
|
||||
): NodeJS.ProcessEnv {
|
||||
return {
|
||||
const nextEnv: NodeJS.ProcessEnv = {
|
||||
...createNpmProjectInstallEnv(env, options),
|
||||
npm_config_legacy_peer_deps: "true",
|
||||
};
|
||||
for (const key of Object.keys(nextEnv)) {
|
||||
if (key.toLowerCase() === NPM_EXECPATH_ENV_KEY) {
|
||||
delete nextEnv[key];
|
||||
}
|
||||
}
|
||||
return nextEnv;
|
||||
}
|
||||
|
||||
export function createBundledRuntimeDepsInstallArgs(missingSpecs: readonly string[]): string[] {
|
||||
@@ -1280,18 +1287,6 @@ export function createBundledRuntimeDepsInstallArgs(missingSpecs: readonly strin
|
||||
return ["install", "--ignore-scripts", ...missingSpecs];
|
||||
}
|
||||
|
||||
function resolvePathEnvKey(env: NodeJS.ProcessEnv, platform: NodeJS.Platform): string {
|
||||
if (platform !== "win32") {
|
||||
return "PATH";
|
||||
}
|
||||
return Object.keys(env).find((key) => key.toLowerCase() === "path") ?? "Path";
|
||||
}
|
||||
|
||||
function isNpmCliPath(candidate: string): boolean {
|
||||
const normalized = candidate.replaceAll("\\", "/").toLowerCase();
|
||||
return normalized.endsWith("/npm-cli.js") || normalized.endsWith("/npm/bin/npm-cli.js");
|
||||
}
|
||||
|
||||
export function resolveBundledRuntimeDepsNpmRunner(params: {
|
||||
npmArgs: string[];
|
||||
env?: NodeJS.ProcessEnv;
|
||||
@@ -1299,22 +1294,16 @@ export function resolveBundledRuntimeDepsNpmRunner(params: {
|
||||
existsSync?: typeof fs.existsSync;
|
||||
platform?: NodeJS.Platform;
|
||||
}): BundledRuntimeDepsNpmRunner {
|
||||
const env = params.env ?? process.env;
|
||||
const execPath = params.execPath ?? process.execPath;
|
||||
const existsSync = params.existsSync ?? fs.existsSync;
|
||||
const platform = params.platform ?? process.platform;
|
||||
const pathImpl = platform === "win32" ? path.win32 : path.posix;
|
||||
const nodeDir = pathImpl.dirname(execPath);
|
||||
const rawNpmExecPath = normalizeOptionalLowercaseString(env.npm_execpath)
|
||||
? env.npm_execpath
|
||||
: undefined;
|
||||
const npmExecPath = rawNpmExecPath && isNpmCliPath(rawNpmExecPath) ? rawNpmExecPath : undefined;
|
||||
|
||||
const npmCliCandidates = [
|
||||
npmExecPath,
|
||||
pathImpl.resolve(nodeDir, "../lib/node_modules/npm/bin/npm-cli.js"),
|
||||
pathImpl.resolve(nodeDir, "node_modules/npm/bin/npm-cli.js"),
|
||||
].filter((candidate): candidate is string => Boolean(candidate));
|
||||
];
|
||||
const npmCliPath = npmCliCandidates.find(
|
||||
(candidate) => pathImpl.isAbsolute(candidate) && existsSync(candidate),
|
||||
);
|
||||
@@ -1336,19 +1325,7 @@ export function resolveBundledRuntimeDepsNpmRunner(params: {
|
||||
throw new Error("Unable to resolve a safe npm executable on Windows");
|
||||
}
|
||||
|
||||
const pathKey = resolvePathEnvKey(env, platform);
|
||||
const currentPath = env[pathKey];
|
||||
return {
|
||||
command: "npm",
|
||||
args: params.npmArgs,
|
||||
env: {
|
||||
...env,
|
||||
[pathKey]:
|
||||
typeof currentPath === "string" && currentPath.length > 0
|
||||
? `${nodeDir}${path.delimiter}${currentPath}`
|
||||
: nodeDir,
|
||||
},
|
||||
};
|
||||
throw new Error("Unable to resolve a safe npm executable");
|
||||
}
|
||||
type BundledPluginRuntimeDepsManifest = {
|
||||
channels: string[];
|
||||
|
||||
Reference in New Issue
Block a user