fix: harden node-host shell payload mutability checks

This commit is contained in:
Ayaan Zaidi
2026-04-16 20:34:17 +05:30
parent 29919bb6e4
commit 75c551e89e
2 changed files with 40 additions and 7 deletions

View File

@@ -865,6 +865,28 @@ describe("hardenApprovedExecutionPaths", () => {
}
});
it("keeps fail-closed behavior for owner-controlled read-only absolute binaries", () => {
if (process.platform === "win32") {
return;
}
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-owned-readonly-binding-"));
const binaryPath = path.join(tmp, "tool");
try {
fs.copyFileSync(resolveNativeBinaryFixturePath(), binaryPath);
fs.chmodSync(binaryPath, 0o555);
fs.chmodSync(tmp, 0o555);
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "-lc", binaryPath],
rawCommand: binaryPath,
cwd: tmp,
});
expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL);
} finally {
fs.chmodSync(tmp, 0o755);
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it("keeps fail-closed behavior for symlinked binaries with writable targets", () => {
if (process.platform === "win32") {
return;

View File

@@ -223,12 +223,23 @@ function pathComponentsFromRootSync(targetPath: string): string[] {
}
}
function isWritableByCurrentProcessSync(candidate: string): boolean {
function isOwnedByCurrentProcessSync(candidate: string): boolean {
if (process.platform === "win32" || typeof process.getuid !== "function") {
return false;
}
try {
return fs.statSync(candidate).uid === process.getuid();
} catch {
return false;
}
}
function isMutableByCurrentProcessSync(candidate: string): boolean {
try {
fs.accessSync(candidate, fs.constants.W_OK);
return true;
} catch {
return false;
return isOwnedByCurrentProcessSync(candidate);
}
}
@@ -239,7 +250,7 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
continue;
}
const parentDir = path.dirname(component);
if (isWritableByCurrentProcessSync(parentDir)) {
if (isMutableByCurrentProcessSync(parentDir)) {
return true;
}
} catch {
@@ -251,8 +262,8 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
function pathLooksMutableForShellPayloadSync(targetPath: string): boolean {
if (
isWritableByCurrentProcessSync(targetPath) ||
isWritableByCurrentProcessSync(path.dirname(targetPath)) ||
isMutableByCurrentProcessSync(targetPath) ||
isMutableByCurrentProcessSync(path.dirname(targetPath)) ||
hasMutableSymlinkPathComponentSync(targetPath)
) {
return true;
@@ -264,8 +275,8 @@ function pathLooksMutableForShellPayloadSync(targetPath: string): boolean {
return true;
}
return (
isWritableByCurrentProcessSync(realPath) ||
isWritableByCurrentProcessSync(path.dirname(realPath)) ||
isMutableByCurrentProcessSync(realPath) ||
isMutableByCurrentProcessSync(path.dirname(realPath)) ||
hasMutableSymlinkPathComponentSync(realPath)
);
}