fix(sandbox): anchor fs-bridge writeFile commit to canonical parent path
Refs: GHSA-xvx8-77m6-gwg6
This commit is contained in:
parent
43a10677ed
commit
e95f2dcd6e
@ -24,6 +24,11 @@ export type PinnedSandboxEntry = {
|
|||||||
basename: string;
|
basename: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
export type AnchoredSandboxEntry = {
|
||||||
|
canonicalParentPath: string;
|
||||||
|
basename: string;
|
||||||
|
};
|
||||||
|
|
||||||
export type PinnedSandboxDirectoryEntry = {
|
export type PinnedSandboxDirectoryEntry = {
|
||||||
mountRootPath: string;
|
mountRootPath: string;
|
||||||
relativePath: string;
|
relativePath: string;
|
||||||
@ -154,6 +159,48 @@ export class SandboxFsPathGuard {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async resolveAnchoredSandboxEntry(
|
||||||
|
target: SandboxResolvedFsPath,
|
||||||
|
action: string,
|
||||||
|
): Promise<AnchoredSandboxEntry> {
|
||||||
|
const basename = path.posix.basename(target.containerPath);
|
||||||
|
if (!basename || basename === "." || basename === "/") {
|
||||||
|
throw new Error(`Invalid sandbox entry target: ${target.containerPath}`);
|
||||||
|
}
|
||||||
|
const parentPath = normalizeContainerPath(path.posix.dirname(target.containerPath));
|
||||||
|
const canonicalParentPath = await this.resolveCanonicalContainerPath({
|
||||||
|
containerPath: parentPath,
|
||||||
|
allowFinalSymlinkForUnlink: false,
|
||||||
|
});
|
||||||
|
this.resolveRequiredMount(canonicalParentPath, action);
|
||||||
|
return {
|
||||||
|
canonicalParentPath,
|
||||||
|
basename,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async resolveAnchoredPinnedEntry(
|
||||||
|
target: SandboxResolvedFsPath,
|
||||||
|
action: string,
|
||||||
|
): Promise<PinnedSandboxEntry> {
|
||||||
|
const anchoredTarget = await this.resolveAnchoredSandboxEntry(target, action);
|
||||||
|
const mount = this.resolveRequiredMount(anchoredTarget.canonicalParentPath, action);
|
||||||
|
const relativeParentPath = path.posix.relative(
|
||||||
|
mount.containerRoot,
|
||||||
|
anchoredTarget.canonicalParentPath,
|
||||||
|
);
|
||||||
|
if (relativeParentPath.startsWith("..") || path.posix.isAbsolute(relativeParentPath)) {
|
||||||
|
throw new Error(
|
||||||
|
`Sandbox path escapes allowed mounts; cannot ${action}: ${target.containerPath}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
mountRootPath: mount.containerRoot,
|
||||||
|
relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath,
|
||||||
|
basename: anchoredTarget.basename,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
resolvePinnedDirectoryEntry(
|
resolvePinnedDirectoryEntry(
|
||||||
target: SandboxResolvedFsPath,
|
target: SandboxResolvedFsPath,
|
||||||
action: string,
|
action: string,
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
import type { PathSafetyCheck } from "./fs-bridge-path-safety.js";
|
import type { AnchoredSandboxEntry, PathSafetyCheck } from "./fs-bridge-path-safety.js";
|
||||||
import type { SandboxResolvedFsPath } from "./fs-paths.js";
|
import type { SandboxResolvedFsPath } from "./fs-paths.js";
|
||||||
|
|
||||||
export type SandboxFsCommandPlan = {
|
export type SandboxFsCommandPlan = {
|
||||||
@ -10,11 +10,14 @@ export type SandboxFsCommandPlan = {
|
|||||||
allowFailure?: boolean;
|
allowFailure?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
export function buildStatPlan(target: SandboxResolvedFsPath): SandboxFsCommandPlan {
|
export function buildStatPlan(
|
||||||
|
target: SandboxResolvedFsPath,
|
||||||
|
anchoredTarget: AnchoredSandboxEntry,
|
||||||
|
): SandboxFsCommandPlan {
|
||||||
return {
|
return {
|
||||||
checks: [{ target, options: { action: "stat files" } }],
|
checks: [{ target, options: { action: "stat files" } }],
|
||||||
script: 'set -eu; stat -c "%F|%s|%Y" -- "$1"',
|
script: 'set -eu\ncd -- "$1"\nstat -c "%F|%s|%Y" -- "$2"',
|
||||||
args: [target.containerPath],
|
args: [anchoredTarget.canonicalParentPath, anchoredTarget.basename],
|
||||||
allowFailure: true,
|
allowFailure: true,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@ -4,7 +4,12 @@ import { describe, expect, it } from "vitest";
|
|||||||
import {
|
import {
|
||||||
createSandbox,
|
createSandbox,
|
||||||
createSandboxFsBridge,
|
createSandboxFsBridge,
|
||||||
|
dockerExecResult,
|
||||||
|
findCallsByScriptFragment,
|
||||||
|
findCallByDockerArg,
|
||||||
|
findCallByScriptFragment,
|
||||||
getDockerArg,
|
getDockerArg,
|
||||||
|
getDockerScript,
|
||||||
installFsBridgeTestHarness,
|
installFsBridgeTestHarness,
|
||||||
mockedExecDockerRaw,
|
mockedExecDockerRaw,
|
||||||
withTempDir,
|
withTempDir,
|
||||||
@ -66,6 +71,13 @@ describe("sandbox fs bridge anchored ops", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const pinnedCases = [
|
const pinnedCases = [
|
||||||
|
{
|
||||||
|
name: "write pins canonical parent + basename",
|
||||||
|
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
||||||
|
bridge.writeFile({ filePath: "nested/file.txt", data: "updated" }),
|
||||||
|
expectedArgs: ["write", "/workspace", "nested", "file.txt", "1"],
|
||||||
|
forbiddenArgs: ["/workspace/nested/file.txt"],
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "mkdirp pins mount root + relative path",
|
name: "mkdirp pins mount root + relative path",
|
||||||
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
||||||
@ -121,4 +133,74 @@ describe("sandbox fs bridge anchored ops", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.runIf(process.platform !== "win32")(
|
||||||
|
"write resolves symlink parents to canonical pinned paths",
|
||||||
|
async () => {
|
||||||
|
await withTempDir("openclaw-fs-bridge-contract-write-", async (stateDir) => {
|
||||||
|
const workspaceDir = path.join(stateDir, "workspace");
|
||||||
|
const realDir = path.join(workspaceDir, "real");
|
||||||
|
await fs.mkdir(realDir, { recursive: true });
|
||||||
|
await fs.symlink(realDir, path.join(workspaceDir, "alias"));
|
||||||
|
|
||||||
|
mockedExecDockerRaw.mockImplementation(async (args) => {
|
||||||
|
const script = getDockerScript(args);
|
||||||
|
if (script.includes('readlink -f -- "$cursor"')) {
|
||||||
|
const target = getDockerArg(args, 1);
|
||||||
|
return dockerExecResult(`${target.replace("/workspace/alias", "/workspace/real")}\n`);
|
||||||
|
}
|
||||||
|
if (script.includes('stat -c "%F|%s|%Y"')) {
|
||||||
|
return dockerExecResult("regular file|1|2");
|
||||||
|
}
|
||||||
|
return dockerExecResult("");
|
||||||
|
});
|
||||||
|
|
||||||
|
const bridge = createSandboxFsBridge({
|
||||||
|
sandbox: createSandbox({
|
||||||
|
workspaceDir,
|
||||||
|
agentWorkspaceDir: workspaceDir,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
|
||||||
|
await bridge.writeFile({ filePath: "alias/note.txt", data: "updated" });
|
||||||
|
|
||||||
|
const writeCall = findCallByDockerArg(1, "write");
|
||||||
|
expect(writeCall).toBeDefined();
|
||||||
|
const args = writeCall?.[0] ?? [];
|
||||||
|
expect(getDockerArg(args, 2)).toBe("/workspace");
|
||||||
|
expect(getDockerArg(args, 3)).toBe("real");
|
||||||
|
expect(getDockerArg(args, 4)).toBe("note.txt");
|
||||||
|
expect(args).not.toContain("alias");
|
||||||
|
|
||||||
|
const canonicalCalls = findCallsByScriptFragment('readlink -f -- "$cursor"');
|
||||||
|
expect(
|
||||||
|
canonicalCalls.some(([callArgs]) => getDockerArg(callArgs, 1) === "/workspace/alias"),
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
it("stat anchors parent + basename", async () => {
|
||||||
|
await withTempDir("openclaw-fs-bridge-contract-stat-", async (stateDir) => {
|
||||||
|
const workspaceDir = path.join(stateDir, "workspace");
|
||||||
|
await fs.mkdir(path.join(workspaceDir, "nested"), { recursive: true });
|
||||||
|
await fs.writeFile(path.join(workspaceDir, "nested", "file.txt"), "bye", "utf8");
|
||||||
|
|
||||||
|
const bridge = createSandboxFsBridge({
|
||||||
|
sandbox: createSandbox({
|
||||||
|
workspaceDir,
|
||||||
|
agentWorkspaceDir: workspaceDir,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
|
||||||
|
await bridge.stat({ filePath: "nested/file.txt" });
|
||||||
|
|
||||||
|
const statCall = findCallByScriptFragment('stat -c "%F|%s|%Y" -- "$2"');
|
||||||
|
expect(statCall).toBeDefined();
|
||||||
|
const args = statCall?.[0] ?? [];
|
||||||
|
expect(getDockerArg(args, 1)).toBe("/workspace/nested");
|
||||||
|
expect(getDockerArg(args, 2)).toBe("file.txt");
|
||||||
|
expect(args).not.toContain("/workspace/nested/file.txt");
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -118,7 +118,10 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
const buffer = Buffer.isBuffer(params.data)
|
const buffer = Buffer.isBuffer(params.data)
|
||||||
? params.data
|
? params.data
|
||||||
: Buffer.from(params.data, params.encoding ?? "utf8");
|
: Buffer.from(params.data, params.encoding ?? "utf8");
|
||||||
const pinnedWriteTarget = this.pathGuard.resolvePinnedEntry(target, "write files");
|
const pinnedWriteTarget = await this.pathGuard.resolveAnchoredPinnedEntry(
|
||||||
|
target,
|
||||||
|
"write files",
|
||||||
|
);
|
||||||
await this.runCheckedCommand({
|
await this.runCheckedCommand({
|
||||||
...buildPinnedWritePlan({
|
...buildPinnedWritePlan({
|
||||||
check: writeCheck,
|
check: writeCheck,
|
||||||
@ -218,7 +221,11 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
signal?: AbortSignal;
|
signal?: AbortSignal;
|
||||||
}): Promise<SandboxFsStat | null> {
|
}): Promise<SandboxFsStat | null> {
|
||||||
const target = this.resolveResolvedPath(params);
|
const target = this.resolveResolvedPath(params);
|
||||||
const result = await this.runPlannedCommand(buildStatPlan(target), params.signal);
|
const anchoredTarget = await this.pathGuard.resolveAnchoredSandboxEntry(target, "stat files");
|
||||||
|
const result = await this.runPlannedCommand(
|
||||||
|
buildStatPlan(target, anchoredTarget),
|
||||||
|
params.signal,
|
||||||
|
);
|
||||||
if (result.code !== 0) {
|
if (result.code !== 0) {
|
||||||
const stderr = result.stderr.toString("utf8");
|
const stderr = result.stderr.toString("utf8");
|
||||||
if (stderr.includes("No such file or directory")) {
|
if (stderr.includes("No such file or directory")) {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user