mirror of https://github.com/openclaw/openclaw.git
fix(node-host): harden ambiguous approval operand binding (#44247)
* fix(node-host): harden approval operand binding * test(node-host): cover approval parser hardening * docs(changelog): note approval hardening GHSA cluster * Update CHANGELOG.md * fix(node-host): remove dead approval parser entries * test(node-host): cover bunx approval wrapper * fix(node-host): unwrap pnpm shim exec forms * test(node-host): cover pnpm shim wrappers
This commit is contained in:
parent
136adb4c02
commit
33ba3ce951
|
|
@ -44,6 +44,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Security/Feishu reactions: preserve looked-up group chat typing and fail closed on ambiguous reaction context so group authorization and mention gating cannot be bypassed through synthetic `p2p` reactions. (`GHSA-m69h-jm2f-2pv8`)(#44088) Thanks @zpbrent and @vincentkoc.
|
||||
- Security/LINE webhook: require signatures for empty-event POST probes too so unsigned requests no longer confirm webhook reachability with a `200` response. (`GHSA-mhxh-9pjm-w7q5`)(#44090) Thanks @TerminalsandCoffee and @vincentkoc.
|
||||
- Security/Zalo webhook: rate limit invalid secret guesses before auth so weak webhook secrets cannot be brute-forced through unauthenticated churned requests without pre-auth `429` responses. (`GHSA-5m9r-p9g7-679c`)(#44173) Thanks @zpbrent and @vincentkoc.
|
||||
- Security/exec approvals: fail closed for ambiguous inline loader and shell-payload script execution, bind the real script after POSIX shell value-taking flags, and unwrap `pnpm`/`npm exec`/`npx` script runners before approval binding. (`GHSA-57jw-9722-6rf2`)(`GHSA-jvqh-rfmh-jh27`)(`GHSA-x7pp-23xv-mmr4`)(`GHSA-jc5j-vg4r-j5jx`)(#44247) Thanks @tdjackey and @vincentkoc.
|
||||
- Doctor/gateway service audit: canonicalize service entrypoint paths before comparing them so symlink-vs-realpath installs no longer trigger false "entrypoint does not match the current install" repair prompts. (#43882) Thanks @ngutman.
|
||||
- Doctor/gateway service audit: earlier groundwork for this fix landed in the superseded #28338 branch. Thanks @realriphub.
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { formatExecCommand } from "../infra/system-run-command.js";
|
|||
import {
|
||||
buildSystemRunApprovalPlan,
|
||||
hardenApprovedExecutionPaths,
|
||||
resolveMutableFileOperandSnapshotSync,
|
||||
} from "./invoke-system-run-plan.js";
|
||||
|
||||
type PathTokenSetup = {
|
||||
|
|
@ -94,6 +95,36 @@ function withFakeRuntimeBin<T>(params: { binName: string; run: () => T }): T {
|
|||
}
|
||||
}
|
||||
|
||||
function withFakeRuntimeBins<T>(params: { binNames: string[]; run: () => T }): T {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-runtime-bins-"));
|
||||
const binDir = path.join(tmp, "bin");
|
||||
fs.mkdirSync(binDir, { recursive: true });
|
||||
for (const binName of params.binNames) {
|
||||
const runtimePath =
|
||||
process.platform === "win32"
|
||||
? path.join(binDir, `${binName}.cmd`)
|
||||
: path.join(binDir, binName);
|
||||
const runtimeBody =
|
||||
process.platform === "win32" ? "@echo off\r\nexit /b 0\r\n" : "#!/bin/sh\nexit 0\n";
|
||||
fs.writeFileSync(runtimePath, runtimeBody, { mode: 0o755 });
|
||||
if (process.platform !== "win32") {
|
||||
fs.chmodSync(runtimePath, 0o755);
|
||||
}
|
||||
}
|
||||
const oldPath = process.env.PATH;
|
||||
process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`;
|
||||
try {
|
||||
return params.run();
|
||||
} finally {
|
||||
if (oldPath === undefined) {
|
||||
delete process.env.PATH;
|
||||
} else {
|
||||
process.env.PATH = oldPath;
|
||||
}
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("hardenApprovedExecutionPaths", () => {
|
||||
const cases: HardeningCase[] = [
|
||||
{
|
||||
|
|
@ -318,16 +349,67 @@ describe("hardenApprovedExecutionPaths", () => {
|
|||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
{
|
||||
name: "pnpm exec tsx file",
|
||||
argv: ["pnpm", "exec", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 3,
|
||||
},
|
||||
{
|
||||
name: "pnpm js shim exec tsx file",
|
||||
argv: ["./pnpm.js", "exec", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 3,
|
||||
},
|
||||
{
|
||||
name: "pnpm exec double-dash tsx file",
|
||||
argv: ["pnpm", "exec", "--", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 4,
|
||||
},
|
||||
{
|
||||
name: "npx tsx file",
|
||||
argv: ["npx", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
{
|
||||
name: "bunx tsx file",
|
||||
argv: ["bunx", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 2,
|
||||
},
|
||||
{
|
||||
name: "npm exec tsx file",
|
||||
argv: ["npm", "exec", "--", "tsx", "./run.ts"],
|
||||
scriptName: "run.ts",
|
||||
initialBody: 'console.log("SAFE");\n',
|
||||
expectedArgvIndex: 4,
|
||||
},
|
||||
];
|
||||
|
||||
for (const runtimeCase of mutableOperandCases) {
|
||||
it(`captures mutable ${runtimeCase.name} operands in approval plans`, () => {
|
||||
withFakeRuntimeBin({
|
||||
binName: runtimeCase.binName!,
|
||||
const binNames = runtimeCase.binName
|
||||
? [runtimeCase.binName]
|
||||
: ["bunx", "pnpm", "npm", "npx", "tsx"];
|
||||
withFakeRuntimeBins({
|
||||
binNames,
|
||||
run: () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-plan-"));
|
||||
const fixture = createScriptOperandFixture(tmp, runtimeCase);
|
||||
fs.writeFileSync(fixture.scriptPath, fixture.initialBody);
|
||||
const executablePath = fixture.command[0];
|
||||
if (executablePath?.endsWith("pnpm.js")) {
|
||||
const shimPath = path.join(tmp, "pnpm.js");
|
||||
fs.writeFileSync(shimPath, "#!/usr/bin/env node\nconsole.log('shim')\n");
|
||||
fs.chmodSync(shimPath, 0o755);
|
||||
}
|
||||
try {
|
||||
const prepared = buildSystemRunApprovalPlan({
|
||||
command: fixture.command,
|
||||
|
|
@ -441,4 +523,75 @@ describe("hardenApprovedExecutionPaths", () => {
|
|||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects node inline import operands that cannot be bound to one stable file", () => {
|
||||
withFakeRuntimeBin({
|
||||
binName: "node",
|
||||
run: () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-node-import-inline-"));
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmp, "main.mjs"), 'console.log("SAFE")\n');
|
||||
fs.writeFileSync(path.join(tmp, "preload.mjs"), 'console.log("SAFE")\n');
|
||||
const prepared = buildSystemRunApprovalPlan({
|
||||
command: ["node", "--import=./preload.mjs", "./main.mjs"],
|
||||
cwd: tmp,
|
||||
});
|
||||
expect(prepared).toEqual({
|
||||
ok: false,
|
||||
message:
|
||||
"SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects shell payloads that hide mutable interpreter scripts", () => {
|
||||
withFakeRuntimeBin({
|
||||
binName: "node",
|
||||
run: () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-inline-shell-node-"));
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmp, "run.js"), 'console.log("SAFE")\n');
|
||||
const prepared = buildSystemRunApprovalPlan({
|
||||
command: ["sh", "-lc", "node ./run.js"],
|
||||
cwd: tmp,
|
||||
});
|
||||
expect(prepared).toEqual({
|
||||
ok: false,
|
||||
message:
|
||||
"SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("captures the real shell script operand after value-taking shell flags", () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-option-value-"));
|
||||
try {
|
||||
const scriptPath = path.join(tmp, "run.sh");
|
||||
fs.writeFileSync(scriptPath, "#!/bin/sh\necho SAFE\n");
|
||||
fs.writeFileSync(path.join(tmp, "errexit"), "decoy\n");
|
||||
const snapshot = resolveMutableFileOperandSnapshotSync({
|
||||
argv: ["/bin/bash", "-o", "errexit", "./run.sh"],
|
||||
cwd: tmp,
|
||||
shellCommand: null,
|
||||
});
|
||||
expect(snapshot).toEqual({
|
||||
ok: true,
|
||||
snapshot: {
|
||||
argvIndex: 3,
|
||||
path: fs.realpathSync(scriptPath),
|
||||
sha256: expect.any(String),
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import {
|
|||
resolveInlineCommandMatch,
|
||||
} from "../infra/shell-inline-command.js";
|
||||
import { formatExecCommand, resolveSystemRunCommandRequest } from "../infra/system-run-command.js";
|
||||
import { splitShellArgs } from "../utils/shell-argv.js";
|
||||
|
||||
export type ApprovedCwdSnapshot = {
|
||||
cwd: string;
|
||||
|
|
@ -125,6 +126,47 @@ const DENO_RUN_OPTIONS_WITH_VALUE = new Set([
|
|||
"-L",
|
||||
]);
|
||||
|
||||
const NODE_OPTIONS_WITH_FILE_VALUE = new Set([
|
||||
"-r",
|
||||
"--experimental-loader",
|
||||
"--import",
|
||||
"--loader",
|
||||
"--require",
|
||||
]);
|
||||
|
||||
const POSIX_SHELL_OPTIONS_WITH_VALUE = new Set([
|
||||
"--init-file",
|
||||
"--rcfile",
|
||||
"--startup-script",
|
||||
"-o",
|
||||
]);
|
||||
|
||||
const NPM_EXEC_OPTIONS_WITH_VALUE = new Set([
|
||||
"--cache",
|
||||
"--package",
|
||||
"--prefix",
|
||||
"--script-shell",
|
||||
"--userconfig",
|
||||
"--workspace",
|
||||
"-p",
|
||||
"-w",
|
||||
]);
|
||||
|
||||
const NPM_EXEC_FLAG_OPTIONS = new Set([
|
||||
"--no",
|
||||
"--quiet",
|
||||
"--ws",
|
||||
"--workspaces",
|
||||
"--yes",
|
||||
"-q",
|
||||
"-y",
|
||||
]);
|
||||
|
||||
type FileOperandCollection = {
|
||||
hits: number[];
|
||||
sawOptionValueFile: boolean;
|
||||
};
|
||||
|
||||
function normalizeString(value: unknown): string | null {
|
||||
if (typeof value !== "string") {
|
||||
return null;
|
||||
|
|
@ -225,10 +267,129 @@ function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseInde
|
|||
current = shellMultiplexerUnwrap.argv;
|
||||
continue;
|
||||
}
|
||||
const packageManagerUnwrap = unwrapKnownPackageManagerExecInvocation(current);
|
||||
if (packageManagerUnwrap) {
|
||||
baseIndex += current.length - packageManagerUnwrap.length;
|
||||
current = packageManagerUnwrap;
|
||||
continue;
|
||||
}
|
||||
return { argv: current, baseIndex };
|
||||
}
|
||||
}
|
||||
|
||||
function unwrapKnownPackageManagerExecInvocation(argv: string[]): string[] | null {
|
||||
const executable = normalizePackageManagerExecToken(argv[0] ?? "");
|
||||
switch (executable) {
|
||||
case "npm":
|
||||
return unwrapNpmExecInvocation(argv);
|
||||
case "npx":
|
||||
case "bunx":
|
||||
return unwrapDirectPackageExecInvocation(argv);
|
||||
case "pnpm":
|
||||
return unwrapPnpmExecInvocation(argv);
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function normalizePackageManagerExecToken(token: string): string {
|
||||
const normalized = normalizeExecutableToken(token);
|
||||
if (!normalized) {
|
||||
return normalized;
|
||||
}
|
||||
return normalized.replace(/\.(?:c|m)?js$/i, "");
|
||||
}
|
||||
|
||||
function unwrapPnpmExecInvocation(argv: string[]): string[] | null {
|
||||
let idx = 1;
|
||||
while (idx < argv.length) {
|
||||
const token = argv[idx]?.trim() ?? "";
|
||||
if (!token) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (token === "--") {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (!token.startsWith("-")) {
|
||||
if (token !== "exec" || idx + 1 >= argv.length) {
|
||||
return null;
|
||||
}
|
||||
const tail = argv.slice(idx + 1);
|
||||
return tail[0] === "--" ? (tail.length > 1 ? tail.slice(1) : null) : tail;
|
||||
}
|
||||
if ((token === "-C" || token === "--dir" || token === "--filter") && !token.includes("=")) {
|
||||
idx += 2;
|
||||
continue;
|
||||
}
|
||||
idx += 1;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function unwrapDirectPackageExecInvocation(argv: string[]): string[] | null {
|
||||
let idx = 1;
|
||||
while (idx < argv.length) {
|
||||
const token = argv[idx]?.trim() ?? "";
|
||||
if (!token) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (!token.startsWith("-")) {
|
||||
return argv.slice(idx);
|
||||
}
|
||||
const [flag] = token.toLowerCase().split("=", 2);
|
||||
if (flag === "-c" || flag === "--call") {
|
||||
return null;
|
||||
}
|
||||
if (NPM_EXEC_OPTIONS_WITH_VALUE.has(flag)) {
|
||||
idx += token.includes("=") ? 1 : 2;
|
||||
continue;
|
||||
}
|
||||
if (NPM_EXEC_FLAG_OPTIONS.has(flag)) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function unwrapNpmExecInvocation(argv: string[]): string[] | null {
|
||||
let idx = 1;
|
||||
while (idx < argv.length) {
|
||||
const token = argv[idx]?.trim() ?? "";
|
||||
if (!token) {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if (!token.startsWith("-")) {
|
||||
if (token !== "exec") {
|
||||
return null;
|
||||
}
|
||||
idx += 1;
|
||||
break;
|
||||
}
|
||||
if (
|
||||
(token === "-C" || token === "--prefix" || token === "--userconfig") &&
|
||||
!token.includes("=")
|
||||
) {
|
||||
idx += 2;
|
||||
continue;
|
||||
}
|
||||
idx += 1;
|
||||
}
|
||||
if (idx >= argv.length) {
|
||||
return null;
|
||||
}
|
||||
const tail = argv.slice(idx);
|
||||
if (tail[0] === "--") {
|
||||
return tail.length > 1 ? tail.slice(1) : null;
|
||||
}
|
||||
return unwrapDirectPackageExecInvocation(["npx", ...tail]);
|
||||
}
|
||||
|
||||
function resolvePosixShellScriptOperandIndex(argv: string[]): number | null {
|
||||
if (
|
||||
resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, {
|
||||
|
|
@ -254,6 +415,13 @@ function resolvePosixShellScriptOperandIndex(argv: string[]): number | null {
|
|||
return null;
|
||||
}
|
||||
if (!afterDoubleDash && token.startsWith("-")) {
|
||||
const [flag] = token.toLowerCase().split("=", 2);
|
||||
if (POSIX_SHELL_OPTIONS_WITH_VALUE.has(flag)) {
|
||||
if (!token.includes("=")) {
|
||||
i += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
return i;
|
||||
|
|
@ -330,7 +498,8 @@ function collectExistingFileOperandIndexes(params: {
|
|||
argv: string[];
|
||||
startIndex: number;
|
||||
cwd: string | undefined;
|
||||
}): number[] {
|
||||
optionsWithFileValue?: ReadonlySet<string>;
|
||||
}): FileOperandCollection {
|
||||
let afterDoubleDash = false;
|
||||
const hits: number[] = [];
|
||||
for (let i = params.startIndex; i < params.argv.length; i += 1) {
|
||||
|
|
@ -349,28 +518,45 @@ function collectExistingFileOperandIndexes(params: {
|
|||
continue;
|
||||
}
|
||||
if (token === "-") {
|
||||
return [];
|
||||
return { hits: [], sawOptionValueFile: false };
|
||||
}
|
||||
if (token.startsWith("-")) {
|
||||
const [flag, inlineValue] = token.split("=", 2);
|
||||
if (params.optionsWithFileValue?.has(flag.toLowerCase())) {
|
||||
if (inlineValue && resolvesToExistingFileSync(inlineValue, params.cwd)) {
|
||||
hits.push(i);
|
||||
return { hits, sawOptionValueFile: true };
|
||||
}
|
||||
const nextToken = params.argv[i + 1]?.trim() ?? "";
|
||||
if (!inlineValue && nextToken && resolvesToExistingFileSync(nextToken, params.cwd)) {
|
||||
hits.push(i + 1);
|
||||
return { hits, sawOptionValueFile: true };
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (resolvesToExistingFileSync(token, params.cwd)) {
|
||||
hits.push(i);
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
return { hits, sawOptionValueFile: false };
|
||||
}
|
||||
|
||||
function resolveGenericInterpreterScriptOperandIndex(params: {
|
||||
argv: string[];
|
||||
cwd: string | undefined;
|
||||
optionsWithFileValue?: ReadonlySet<string>;
|
||||
}): number | null {
|
||||
const hits = collectExistingFileOperandIndexes({
|
||||
const collection = collectExistingFileOperandIndexes({
|
||||
argv: params.argv,
|
||||
startIndex: 1,
|
||||
cwd: params.cwd,
|
||||
optionsWithFileValue: params.optionsWithFileValue,
|
||||
});
|
||||
return hits.length === 1 ? hits[0] : null;
|
||||
if (collection.sawOptionValueFile) {
|
||||
return null;
|
||||
}
|
||||
return collection.hits.length === 1 ? collection.hits[0] : null;
|
||||
}
|
||||
|
||||
function resolveBunScriptOperandIndex(params: {
|
||||
|
|
@ -462,16 +648,39 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined)
|
|||
const genericIndex = resolveGenericInterpreterScriptOperandIndex({
|
||||
argv: unwrapped.argv,
|
||||
cwd,
|
||||
optionsWithFileValue:
|
||||
executable === "node" || executable === "nodejs" ? NODE_OPTIONS_WITH_FILE_VALUE : undefined,
|
||||
});
|
||||
return genericIndex === null ? null : unwrapped.baseIndex + genericIndex;
|
||||
}
|
||||
|
||||
function shellPayloadNeedsStableBinding(shellCommand: string, cwd: string | undefined): boolean {
|
||||
const argv = splitShellArgs(shellCommand);
|
||||
if (!argv || argv.length === 0) {
|
||||
return false;
|
||||
}
|
||||
const snapshot = resolveMutableFileOperandSnapshotSync({
|
||||
argv,
|
||||
cwd,
|
||||
shellCommand: null,
|
||||
});
|
||||
if (!snapshot.ok) {
|
||||
return true;
|
||||
}
|
||||
if (snapshot.snapshot) {
|
||||
return true;
|
||||
}
|
||||
const firstToken = argv[0]?.trim() ?? "";
|
||||
return resolvesToExistingFileSync(firstToken, cwd);
|
||||
}
|
||||
|
||||
function requiresStableInterpreterApprovalBindingWithShellCommand(params: {
|
||||
argv: string[];
|
||||
shellCommand: string | null;
|
||||
cwd: string | undefined;
|
||||
}): boolean {
|
||||
if (params.shellCommand !== null) {
|
||||
return false;
|
||||
return shellPayloadNeedsStableBinding(params.shellCommand, params.cwd);
|
||||
}
|
||||
const unwrapped = unwrapArgvForMutableOperand(params.argv);
|
||||
const executable = normalizeExecutableToken(unwrapped.argv[0] ?? "");
|
||||
|
|
@ -495,6 +704,7 @@ export function resolveMutableFileOperandSnapshotSync(params: {
|
|||
requiresStableInterpreterApprovalBindingWithShellCommand({
|
||||
argv: params.argv,
|
||||
shellCommand: params.shellCommand,
|
||||
cwd: params.cwd,
|
||||
})
|
||||
) {
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in New Issue