fix: land node-host approval binding for native binaries (#66731) (thanks @tmimmanuel)

* fix(node-host): allow absolute-path native binaries through approval binder

* test(node-host): cover binary binder edge cases

* test(node-host): use stable native binary fixture

* fix(ci): restore fail-closed race handling

* refactor(node-host): distill approval binding regressions

* fix(node-host): fail closed on unknown shell payload headers

* fix: land node-host approval binding for native binaries (#66731) (thanks @tmimmanuel)

* fix: keep relative shell binary payloads fail-closed (#66731) (thanks @tmimmanuel)

* fix: keep shell binary bypass on stable paths only (#66731) (thanks @tmimmanuel)

* fix: fail closed on symlinked shell binary targets (#66731) (thanks @tmimmanuel)

---------

Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
tmimmanuel
2026-04-16 17:00:09 +02:00
committed by GitHub
parent 69d25f5f16
commit 29919bb6e4
3 changed files with 290 additions and 2 deletions

View File

@@ -1,7 +1,7 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import { formatExecCommand } from "../infra/system-run-command.js";
import {
buildSystemRunApprovalPlan,
@@ -122,6 +122,43 @@ function withFakeRuntimeBins<T>(params: {
}
}
function resolveNativeBinaryFixturePath(): string {
for (const candidate of ["/bin/ls", "/usr/bin/ls", "/bin/echo", "/usr/bin/printf"]) {
try {
if (fs.statSync(candidate).isFile()) {
return candidate;
}
} catch {
continue;
}
}
throw new Error("expected a native binary fixture path");
}
function expectShellPayloadApprovalDenied(params: {
tmpPrefix: string;
fileName: string;
body: string;
}) {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), params.tmpPrefix));
try {
const scriptPath = path.join(tmp, params.fileName);
fs.writeFileSync(scriptPath, params.body);
fs.chmodSync(scriptPath, 0o755);
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", scriptPath],
rawCommand: scriptPath,
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
}
function expectMutableFileOperandApprovalPlan(fixture: ScriptOperandFixture, cwd: string) {
const prepared = buildSystemRunApprovalPlan({
command: fixture.command,
@@ -769,6 +806,162 @@ describe("hardenApprovedExecutionPaths", () => {
);
});
it("allows shell payloads that invoke absolute-path native binaries", () => {
if (process.platform === "win32") {
return;
}
const binaryPath = resolveNativeBinaryFixturePath();
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", binaryPath],
rawCommand: binaryPath,
cwd: process.cwd(),
});
expect(prepared.ok).toBe(true);
if (!prepared.ok) {
throw new Error("unreachable");
}
expect(prepared.plan.mutableFileOperand).toBeUndefined();
});
it("keeps fail-closed behavior for relative native-binary shell payloads", () => {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-relative-binary-binding-"));
try {
const binaryPath = resolveNativeBinaryFixturePath();
const relativeBinaryPath = path.join(tmp, "tool");
fs.copyFileSync(binaryPath, relativeBinaryPath);
fs.chmodSync(relativeBinaryPath, 0o755);
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", "./tool"],
rawCommand: "./tool",
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it("keeps fail-closed behavior for writable absolute native-binary shell payloads", () => {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-absolute-binary-binding-"));
try {
const binaryPath = resolveNativeBinaryFixturePath();
const copiedBinaryPath = path.join(tmp, "tool");
fs.copyFileSync(binaryPath, copiedBinaryPath);
fs.chmodSync(copiedBinaryPath, 0o755);
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", copiedBinaryPath],
rawCommand: copiedBinaryPath,
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it("keeps fail-closed behavior for symlinked binaries with writable targets", () => {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-symlink-binary-binding-"));
const stableDir = path.join(tmp, "stable");
const mutableDir = path.join(tmp, "mutable");
try {
const binaryPath = resolveNativeBinaryFixturePath();
fs.mkdirSync(stableDir);
fs.mkdirSync(mutableDir);
const targetBinaryPath = path.join(mutableDir, "tool");
const symlinkPath = path.join(stableDir, "tool");
fs.copyFileSync(binaryPath, targetBinaryPath);
fs.chmodSync(targetBinaryPath, 0o755);
fs.symlinkSync(targetBinaryPath, symlinkPath);
fs.chmodSync(stableDir, 0o555);
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", symlinkPath],
rawCommand: symlinkPath,
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
fs.chmodSync(stableDir, 0o755);
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it("keeps fail-closed behavior for shell payloads that invoke mutable script files", () => {
expectShellPayloadApprovalDenied({
tmpPrefix: "openclaw-shell-script-binding-",
fileName: "run.sh",
body: "#!/bin/sh\necho SAFE\n",
});
});
it("keeps fail-closed behavior for empty shell payload files", () => {
expectShellPayloadApprovalDenied({
tmpPrefix: "openclaw-shell-empty-binding-",
fileName: "empty",
body: "",
});
});
it("does not treat weak MZ text headers as native binaries", () => {
expectShellPayloadApprovalDenied({
tmpPrefix: "openclaw-shell-mz-text-binding-",
fileName: "mz-script",
body: "MZ not really a PE file\n",
});
});
it("keeps fail-closed behavior for unknown NUL-bearing headers", () => {
expectShellPayloadApprovalDenied({
tmpPrefix: "openclaw-shell-nul-header-binding-",
fileName: "nul-script",
body: "SAFE\u0000maybe-binary\n",
});
});
it("keeps fail-closed behavior when the shell payload probe stops seeing a file", () => {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-race-binding-"));
try {
const scriptPath = path.join(tmp, "run.sh");
fs.writeFileSync(scriptPath, "#!/bin/sh\necho SAFE\n");
fs.chmodSync(scriptPath, 0o755);
const realStatSync = fs.statSync;
let targetStatCalls = 0;
const statSyncSpy = vi.spyOn(fs, "statSync").mockImplementation((pathLike, options) => {
const targetPath = typeof pathLike === "string" ? pathLike : pathLike.toString();
if (targetPath === scriptPath) {
targetStatCalls += 1;
if (targetStatCalls === 2) {
return realStatSync(tmp, options);
}
}
return realStatSync(pathLike, options);
});
try {
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", scriptPath],
rawCommand: scriptPath,
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
statSyncSpy.mockRestore();
}
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it.each(unsafeRuntimeInvocationCases)("$name", (testCase) => {
withFakeRuntimeBin({
binName: testCase.binName,

View File

@@ -249,6 +249,27 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
return false;
}
function pathLooksMutableForShellPayloadSync(targetPath: string): boolean {
if (
isWritableByCurrentProcessSync(targetPath) ||
isWritableByCurrentProcessSync(path.dirname(targetPath)) ||
hasMutableSymlinkPathComponentSync(targetPath)
) {
return true;
}
let realPath: string;
try {
realPath = fs.realpathSync(targetPath);
} catch {
return true;
}
return (
isWritableByCurrentProcessSync(realPath) ||
isWritableByCurrentProcessSync(path.dirname(realPath)) ||
hasMutableSymlinkPathComponentSync(realPath)
);
}
function shouldPinExecutableForApproval(params: {
shellCommand: string | null;
wrapperChain: string[] | undefined;
@@ -285,6 +306,69 @@ function resolvesToExistingFileSync(rawOperand: string, cwd: string | undefined)
}
}
function isKnownBinaryExecutableHeader(buffer: Buffer): boolean {
if (buffer.length >= 4 && buffer.subarray(0, 4).equals(Buffer.from([0x7f, 0x45, 0x4c, 0x46]))) {
return true;
}
if (
buffer.length >= 4 &&
(buffer.subarray(0, 4).equals(Buffer.from([0xfe, 0xed, 0xfa, 0xce])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xce, 0xfa, 0xed, 0xfe])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xfe, 0xed, 0xfa, 0xcf])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xcf, 0xfa, 0xed, 0xfe])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xca, 0xfe, 0xba, 0xbe])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xbe, 0xba, 0xfe, 0xca])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xca, 0xfe, 0xba, 0xbf])) ||
buffer.subarray(0, 4).equals(Buffer.from([0xbf, 0xba, 0xfe, 0xca])))
) {
return true;
}
if (buffer.length < 0x40 || !buffer.subarray(0, 2).equals(Buffer.from([0x4d, 0x5a]))) {
return false;
}
const peOffset = buffer.readUInt32LE(0x3c);
return (
peOffset >= 0 &&
peOffset <= buffer.length - 4 &&
buffer.subarray(peOffset, peOffset + 4).equals(Buffer.from([0x50, 0x45, 0x00, 0x00]))
);
}
function isLikelyScriptLikePathSync(targetPath: string): boolean {
let stat: fs.Stats;
try {
stat = fs.statSync(targetPath);
} catch {
return true;
}
if (!stat.isFile()) {
return true;
}
let header: Buffer;
try {
const fd = fs.openSync(targetPath, "r");
try {
header = Buffer.alloc(1024);
const bytesRead = fs.readSync(fd, header, 0, header.length, 0);
header = header.subarray(0, bytesRead);
} finally {
fs.closeSync(fd);
}
} catch {
return true;
}
if (header.length === 0) {
return true;
}
if (header.subarray(0, 2).equals(Buffer.from("#!"))) {
return true;
}
if (isKnownBinaryExecutableHeader(header)) {
return false;
}
return true;
}
function unwrapArgvForMutableOperand(argv: string[]): {
argv: string[];
baseIndex: number;
@@ -832,7 +916,17 @@ function shellPayloadNeedsStableBinding(shellCommand: string, cwd: string | unde
return true;
}
const firstToken = readTrimmedArgToken(argv, 0);
return resolvesToExistingFileSync(firstToken, cwd);
if (!resolvesToExistingFileSync(firstToken, cwd)) {
return false;
}
if (!path.isAbsolute(firstToken)) {
return true;
}
const resolvedPath = path.resolve(cwd ?? process.cwd(), firstToken);
if (pathLooksMutableForShellPayloadSync(resolvedPath)) {
return true;
}
return isLikelyScriptLikePathSync(resolvedPath);
}
function requiresStableInterpreterApprovalBindingWithShellCommand(params: {