ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-24 19:35:28 -06:00
Code Issues Packages Projects Releases Wiki Activity

security fix(agents): close symlink-then-.. workspace boundary bypass in assertSandboxPath (#113405)

Browse Source 
* fix(agents): close symlink-then-.. workspace boundary bypass in assertSandboxPath

resolveSandboxPath builds on path.resolve, which collapses ".." lexically
before any symlink is resolved. When a "<symlink>/.." sequence appears, the
lexical collapse lands on a harmless in-root path while the OS resolves the
same raw string (symlink first, then "..") to a location outside the
workspace, so assertSandboxPath approved inputs whose real resolution escaped
the boundary.

Add assertRawParentWithinRoot: it resolves the raw (non-collapsed) parent
chain via the OS realpath (fs.realpath.native; the JS realpath and
path.resolve both pre-collapse "..") and asserts it stays inside the
workspace root. It runs after assertNoPathAliasEscape so that guard's more
specific messages still win for cases it already catches, adding coverage only
for the residual gap it never sees.

Not currently reachable through shipped tools (read/write/edit route I/O
through fs-safe Root, which opens its own collapsed path; other callers use
the returned .resolved) -- this hardens the validator so the boundary no
longer depends on caller discipline. Regression test included.

* fix(agents): simplify raw sandbox path guard

* fix(agents): preserve symlinked sandbox roots

* fix(agents): validate raw final path aliases

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Fede Kamelhar
2026-07-27 03:15:59 -04:00
committed by GitHub
parent 3e4c57dbeb
commit cc027149e5
2 changed files with 187 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/agents/sandbox-paths.test.ts
+113
View File
@@ -7,6 +7,7 @@ import { describe, expect, it, vi } from "vitest";
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
import { withEnvAsync } from "../test-utils/env.js";
import {
assertSandboxPath,
resolveAllowedManagedMediaPath,
resolveSandboxedMediaSource,
resolveSandboxPath,
@@ -116,6 +117,118 @@ describe("resolveSandboxPath", () => {
});
});
describe("assertSandboxPath", () => {
it.runIf(process.platform !== "win32")(
"rejects symlink-then-dot-dot traversal for existing and new files",
async () => {
const parent = await fs.realpath(
await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-symlink-dotdot-")),
);
const root = path.join(parent, "workspace");
const outside = path.join(parent, "outside");
try {
await fs.mkdir(path.join(root, "sub"), { recursive: true });
await fs.mkdir(outside);
await fs.symlink("..", path.join(root, "sub", "up"));
await fs.writeFile(path.join(outside, "secret.txt"), "outside", "utf8");
const escapedRead = `${root}/sub/up/../outside/secret.txt`;
await expect(fs.readFile(escapedRead, "utf8")).resolves.toBe("outside");
await expect(assertSandboxPath({ filePath: escapedRead, cwd: root, root })).rejects.toThrow(
/escapes sandbox root/i,
);
await expect(
assertSandboxPath({
filePath: `${root}/sub/up/../outside/new.txt`,
cwd: root,
root,
}),
).rejects.toThrow(/escapes sandbox root/i);
await expect(
assertSandboxPath({ filePath: `${root}/sub/up/../..`, cwd: root, root }),
).rejects.toThrow(/escapes sandbox root/i);
await fs.mkdir(path.join(root, "a"));
await fs.mkdir(path.join(root, "b"));
await fs.symlink("../b", path.join(root, "a", "up"));
await fs.symlink(path.join(outside, "secret.txt"), path.join(root, "escape"));
const escapedFinalSymlink = `${root}/a/up/../escape`;
await expect(fs.readFile(escapedFinalSymlink, "utf8")).resolves.toBe("outside");
await expect(
assertSandboxPath({ filePath: escapedFinalSymlink, cwd: root, root }),
).rejects.toThrow(/symlink escapes sandbox root/i);
await fs.symlink(outside, path.join(root, "outside-link"));
await expect(
assertSandboxPath({
filePath: `${root}/outside-link/`,
cwd: root,
root,
allowFinalSymlinkForUnlink: true,
}),
).rejects.toThrow(/escapes sandbox root/i);
await fs.mkdir(path.join(root, "real"));
await fs.symlink(path.join(root, "real"), path.join(root, "in-root-link"));
await expect(
assertSandboxPath({
filePath: `${root}/in-root-link/../real/new.txt`,
cwd: root,
root,
}),
).resolves.toBeTruthy();
await expect(assertSandboxPath({ filePath: root, cwd: root, root })).resolves.toBeTruthy();
} finally {
await fs.rm(parent, { recursive: true, force: true });
}
},
);
it.runIf(process.platform === "win32")(
"pins Win32 junction-then-dot-dot to lexical traversal semantics",
async () => {
const parent = await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-junction-dotdot-"));
const root = path.join(parent, "workspace");
const outside = path.join(parent, "outside");
try {
await fs.mkdir(path.join(root, "sub"), { recursive: true });
await fs.mkdir(outside);
await fs.symlink(root, path.join(root, "sub", "up"), "junction");
await fs.writeFile(path.join(outside, "secret.txt"), "outside", "utf8");
const attemptedEscape = `${root}\\sub\\up\\..\\outside\\secret.txt`;
await expect(fs.readFile(attemptedEscape, "utf8")).rejects.toMatchObject({
code: "ENOENT",
});
} finally {
await fs.rm(parent, { recursive: true, force: true });
}
},
);
it("accepts not-yet-created and symlinked roots", async () => {
const parent = await fs.realpath(
await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-missing-root-")),
);
try {
const root = path.join(parent, "workspace");
await expect(
assertSandboxPath({ filePath: "nested/new.txt", cwd: root, root }),
).resolves.toMatchObject({ relative: path.join("nested", "new.txt") });
const realRoot = path.join(parent, "real-workspace");
const linkedRoot = path.join(parent, "linked-workspace");
await fs.mkdir(realRoot);
await fs.symlink(realRoot, linkedRoot);
await expect(
assertSandboxPath({ filePath: linkedRoot, cwd: linkedRoot, root: linkedRoot }),
).resolves.toMatchObject({ relative: "" });
} finally {
await fs.rm(parent, { recursive: true, force: true });
}
});
});
describe("resolveSandboxedMediaSource", () => {
const openClawTmpDir = resolvePreferredOpenClawTmpDir();
src/agents/sandbox-paths.ts
+74 -1
View File
@@ -3,9 +3,11 @@
*
* Handles host paths, file URLs, temporary media paths, and workspace root assertions.
*/
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { URL } from "node:url";
import { promisify } from "node:util";
import { isPassThroughRemoteMediaSource } from "@openclaw/media-core/media-source-url";
import { isWindowsDrivePath } from "../infra/archive-path.js";
import {
@@ -14,7 +16,7 @@ import {
safeFileURLToPath,
} from "../infra/local-file-access.js";
import { assertNoPathAliasEscape, type PathAliasPolicy } from "../infra/path-alias-guards.js";
import { isPathInside } from "../infra/path-guards.js";
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
import { resolveConfigDir, shortenHomePath } from "../utils.js";
@@ -87,6 +89,66 @@ export function resolveSandboxPath(params: { filePath: string; cwd: string; root
return { resolved, relative };
}
const realpathNative = promisify(fs.realpath.native);
async function resolveRawPathViaExistingAncestor(rawPath: string): Promise<string> {
let cursor = rawPath;
const missingSuffix: string[] = [];
while (true) {
try {
return path.resolve(await realpathNative(cursor), ...missingSuffix);
} catch (error) {
if (!isNotFoundPathError(error)) {
throw error;
}
const parent = path.dirname(cursor);
if (parent === cursor) {
throw error;
}
missingSuffix.unshift(path.basename(cursor));
cursor = parent;
}
}
}
async function assertRawParentWithinRoot(params: {
filePath: string;
cwd: string;
root: string;
}): Promise<string> {
// Win32 resolves reparse-point/.. paths lexically, so it has no equivalent escape.
// Avoid adding another realpath to this hot path on Windows, where it is expensive.
if (process.platform === "win32") {
return resolveSandboxInputPath(params.filePath, params.cwd);
}
const expanded = expandPath(params.filePath);
if (isWindowsDrivePath(expanded)) {
return path.win32.normalize(expanded);
}
// Do not use path.resolve here: it would erase the symlink-sensitive `..` before
// native realpath can traverse the raw parent chain. The final component stays
// unresolved so assertNoPathAliasEscape retains final-link policy ownership.
const rawAbsolute = path.isAbsolute(expanded) ? expanded : `${params.cwd}${path.sep}${expanded}`;
const hasTrailingSeparator = rawAbsolute.endsWith(path.sep);
const rawParent = hasTrailingSeparator ? rawAbsolute : path.dirname(rawAbsolute);
const finalSegment = hasTrailingSeparator ? "." : path.basename(rawAbsolute);
const rootResolved = path.resolve(params.root);
const [rootCanonical, parentCanonical] = await Promise.all([
resolveRawPathViaExistingAncestor(rootResolved),
resolveRawPathViaExistingAncestor(rawParent),
]);
const targetCanonical =
path.resolve(rawAbsolute) === rootResolved
? await resolveRawPathViaExistingAncestor(rawAbsolute)
: path.resolve(parentCanonical, finalSegment);
if (targetCanonical !== rootCanonical && !isPathInside(rootCanonical, targetCanonical)) {
throw new Error(
`Path escapes sandbox root (${shortenHomePath(rootCanonical)}): ${params.filePath}`,
);
}
return targetCanonical;
}
export async function assertSandboxPath(params: {
filePath: string;
cwd: string;
@@ -105,6 +167,17 @@ export async function assertSandboxPath(params: {
boundaryLabel: "sandbox root",
policy,
});
// The alias guard owns its specific symlink/hardlink errors; this closes the raw
// symlink-then-`..` gap that lexical normalization hides from that guard.
const rawTarget = await assertRawParentWithinRoot(params);
if (path.resolve(rawTarget) !== path.resolve(resolved.resolved)) {
await assertNoPathAliasEscape({
absolutePath: rawTarget,
rootPath: params.root,
boundaryLabel: "sandbox root",
policy,
});
}
return resolved;
}