mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(security): block workspace hardlink alias escapes
This commit is contained in:
@@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
|
||||
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
||||
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
||||
- Security/Workspace FS: reject hardlinked workspace file aliases in `tools.fs.workspaceOnly` and `tools.exec.applyPatch.workspaceOnly` boundary checks (including sandbox mount-root guards) to prevent out-of-workspace read/write via in-workspace hardlink paths. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
|
||||
- Security/Browser temp paths: harden trace/download output-path handling against symlink-root and symlink-parent escapes with realpath-based write-path checks plus secure fallback tmp-dir validation that fails closed on unsafe fallback links. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
|
||||
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
|
||||
- Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
|
||||
|
||||
@@ -159,6 +159,42 @@ describe("applyPatch", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects hardlink alias escapes by default", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
await withTempDir(async (dir) => {
|
||||
const outside = path.join(
|
||||
path.dirname(dir),
|
||||
`outside-hardlink-${process.pid}-${Date.now()}.txt`,
|
||||
);
|
||||
const linkPath = path.join(dir, "hardlink.txt");
|
||||
await fs.writeFile(outside, "initial\n", "utf8");
|
||||
try {
|
||||
try {
|
||||
await fs.link(outside, linkPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
const patch = `*** Begin Patch
|
||||
*** Update File: hardlink.txt
|
||||
@@
|
||||
-initial
|
||||
+pwned
|
||||
*** End Patch`;
|
||||
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/hardlink|sandbox/i);
|
||||
const outsideContents = await fs.readFile(outside, "utf8");
|
||||
expect(outsideContents).toBe("initial\n");
|
||||
} finally {
|
||||
await fs.rm(linkPath, { force: true });
|
||||
await fs.rm(outside, { force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("allows symlinks that resolve within cwd by default", async () => {
|
||||
await withTempDir(async (dir) => {
|
||||
const target = path.join(dir, "target.txt");
|
||||
|
||||
@@ -266,6 +266,7 @@ async function resolvePatchPath(
|
||||
cwd: options.cwd,
|
||||
root: options.cwd,
|
||||
allowFinalSymlink: purpose === "unlink",
|
||||
allowFinalHardlink: purpose === "unlink",
|
||||
});
|
||||
}
|
||||
return {
|
||||
@@ -282,6 +283,7 @@ async function resolvePatchPath(
|
||||
cwd: options.cwd,
|
||||
root: options.cwd,
|
||||
allowFinalSymlink: purpose === "unlink",
|
||||
allowFinalHardlink: purpose === "unlink",
|
||||
})
|
||||
).resolved
|
||||
: resolvePathFromCwd(filePath, options.cwd);
|
||||
|
||||
@@ -151,6 +151,46 @@ describe("workspace path resolution", () => {
|
||||
).rejects.toThrow(/Path escapes sandbox root/i);
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects hardlinked file aliases when workspaceOnly is enabled", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
await withTempDir("openclaw-ws-", async (workspaceDir) => {
|
||||
const cfg: OpenClawConfig = { tools: { fs: { workspaceOnly: true } } };
|
||||
const tools = createOpenClawCodingTools({ workspaceDir, config: cfg });
|
||||
const { readTool, writeTool } = expectReadWriteEditTools(tools);
|
||||
const outsidePath = path.join(
|
||||
path.dirname(workspaceDir),
|
||||
`outside-hardlink-${process.pid}-${Date.now()}.txt`,
|
||||
);
|
||||
const hardlinkPath = path.join(workspaceDir, "linked.txt");
|
||||
await fs.writeFile(outsidePath, "top-secret", "utf8");
|
||||
try {
|
||||
try {
|
||||
await fs.link(outsidePath, hardlinkPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
await expect(readTool.execute("ws-read-hardlink", { path: "linked.txt" })).rejects.toThrow(
|
||||
/hardlink|sandbox/i,
|
||||
);
|
||||
await expect(
|
||||
writeTool.execute("ws-write-hardlink", {
|
||||
path: "linked.txt",
|
||||
content: "pwned",
|
||||
}),
|
||||
).rejects.toThrow(/hardlink|sandbox/i);
|
||||
expect(await fs.readFile(outsidePath, "utf8")).toBe("top-secret");
|
||||
} finally {
|
||||
await fs.rm(hardlinkPath, { force: true });
|
||||
await fs.rm(outsidePath, { force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("sandboxed workspace paths", () => {
|
||||
|
||||
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath, URL } from "node:url";
|
||||
import { assertNoHardlinkedFinalPath } from "../infra/hardlink-guards.js";
|
||||
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
|
||||
@@ -62,11 +63,18 @@ export async function assertSandboxPath(params: {
|
||||
cwd: string;
|
||||
root: string;
|
||||
allowFinalSymlink?: boolean;
|
||||
allowFinalHardlink?: boolean;
|
||||
}) {
|
||||
const resolved = resolveSandboxPath(params);
|
||||
await assertNoSymlinkEscape(resolved.relative, path.resolve(params.root), {
|
||||
allowFinalSymlink: params.allowFinalSymlink,
|
||||
});
|
||||
await assertNoHardlinkedFinalPath({
|
||||
filePath: resolved.resolved,
|
||||
root: path.resolve(params.root),
|
||||
boundaryLabel: "sandbox root",
|
||||
allowFinalHardlink: params.allowFinalHardlink,
|
||||
});
|
||||
return resolved;
|
||||
}
|
||||
|
||||
@@ -195,27 +203,11 @@ async function assertNoTmpAliasEscape(params: {
|
||||
tmpRoot: string;
|
||||
}): Promise<void> {
|
||||
await assertNoSymlinkEscape(path.relative(params.tmpRoot, params.filePath), params.tmpRoot);
|
||||
await assertNoHardlinkedFinalPath(params.filePath, params.tmpRoot);
|
||||
}
|
||||
|
||||
async function assertNoHardlinkedFinalPath(filePath: string, tmpRoot: string): Promise<void> {
|
||||
let stat: Awaited<ReturnType<typeof fs.stat>>;
|
||||
try {
|
||||
stat = await fs.stat(filePath);
|
||||
} catch (err) {
|
||||
if (isNotFoundPathError(err)) {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
return;
|
||||
}
|
||||
if (stat.nlink > 1) {
|
||||
throw new Error(
|
||||
`Hardlinked tmp media path is not allowed under tmp root (${shortPath(tmpRoot)}): ${shortPath(filePath)}`,
|
||||
);
|
||||
}
|
||||
await assertNoHardlinkedFinalPath({
|
||||
filePath: params.filePath,
|
||||
root: params.tmpRoot,
|
||||
boundaryLabel: "tmp root",
|
||||
});
|
||||
}
|
||||
|
||||
async function assertNoSymlinkEscape(
|
||||
|
||||
@@ -195,6 +195,42 @@ describe("sandbox fs bridge shell compatibility", () => {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("rejects pre-existing host hardlink escapes before docker exec", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-hardlink-"));
|
||||
const workspaceDir = path.join(stateDir, "workspace");
|
||||
const outsideDir = path.join(stateDir, "outside");
|
||||
const outsideFile = path.join(outsideDir, "secret.txt");
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
await fs.mkdir(outsideDir, { recursive: true });
|
||||
await fs.writeFile(outsideFile, "classified");
|
||||
const hardlinkPath = path.join(workspaceDir, "link.txt");
|
||||
try {
|
||||
try {
|
||||
await fs.link(outsideFile, hardlinkPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const bridge = createSandboxFsBridge({
|
||||
sandbox: createSandbox({
|
||||
workspaceDir,
|
||||
agentWorkspaceDir: workspaceDir,
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(bridge.readFile({ filePath: "link.txt" })).rejects.toThrow(/hardlink|sandbox/i);
|
||||
expect(mockedExecDockerRaw).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects container-canonicalized paths outside allowed mounts", async () => {
|
||||
mockedExecDockerRaw.mockImplementation(async (args) => {
|
||||
const script = getDockerScript(args);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { assertNoHardlinkedFinalPath } from "../../infra/hardlink-guards.js";
|
||||
import { isNotFoundPathError, isPathInside } from "../../infra/path-guards.js";
|
||||
import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
|
||||
import {
|
||||
@@ -21,6 +22,7 @@ type RunCommandOptions = {
|
||||
type PathSafetyOptions = {
|
||||
action: string;
|
||||
allowFinalSymlink?: boolean;
|
||||
allowFinalHardlink?: boolean;
|
||||
requireWritable?: boolean;
|
||||
};
|
||||
|
||||
@@ -151,6 +153,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
action: "remove files",
|
||||
requireWritable: true,
|
||||
allowFinalSymlink: true,
|
||||
allowFinalHardlink: true,
|
||||
});
|
||||
const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
|
||||
Boolean,
|
||||
@@ -176,6 +179,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
action: "rename files",
|
||||
requireWritable: true,
|
||||
allowFinalSymlink: true,
|
||||
allowFinalHardlink: true,
|
||||
});
|
||||
await this.assertPathSafety(to, {
|
||||
action: "rename files",
|
||||
@@ -257,6 +261,12 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
rootPath: lexicalMount.hostRoot,
|
||||
allowFinalSymlink: options.allowFinalSymlink === true,
|
||||
});
|
||||
await assertNoHardlinkedFinalPath({
|
||||
filePath: target.hostPath,
|
||||
root: lexicalMount.hostRoot,
|
||||
boundaryLabel: "sandbox mount root",
|
||||
allowFinalHardlink: options.allowFinalHardlink === true,
|
||||
});
|
||||
|
||||
const canonicalContainerPath = await this.resolveCanonicalContainerPath({
|
||||
containerPath: target.containerPath,
|
||||
|
||||
38
src/infra/hardlink-guards.ts
Normal file
38
src/infra/hardlink-guards.ts
Normal file
@@ -0,0 +1,38 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import { isNotFoundPathError } from "./path-guards.js";
|
||||
|
||||
export async function assertNoHardlinkedFinalPath(params: {
|
||||
filePath: string;
|
||||
root: string;
|
||||
boundaryLabel: string;
|
||||
allowFinalHardlink?: boolean;
|
||||
}): Promise<void> {
|
||||
if (params.allowFinalHardlink) {
|
||||
return;
|
||||
}
|
||||
let stat: Awaited<ReturnType<typeof fs.stat>>;
|
||||
try {
|
||||
stat = await fs.stat(params.filePath);
|
||||
} catch (err) {
|
||||
if (isNotFoundPathError(err)) {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
return;
|
||||
}
|
||||
if (stat.nlink > 1) {
|
||||
throw new Error(
|
||||
`Hardlinked path is not allowed under ${params.boundaryLabel} (${shortPath(params.root)}): ${shortPath(params.filePath)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
function shortPath(value: string) {
|
||||
if (value.startsWith(os.homedir())) {
|
||||
return `~${value.slice(os.homedir().length)}`;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
Reference in New Issue
Block a user