mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 03:54:32 +00:00
fix(sandbox): anchor fs-bridge destructive ops
This commit is contained in:
@@ -732,6 +732,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Slack/download-file scoping: thread/channel-aware `download-file` actions now propagate optional scope context and reject downloads when Slack metadata definitively shows the file is outside the requested channel/thread, while preserving legacy behavior when share metadata is unavailable.
|
- Slack/download-file scoping: thread/channel-aware `download-file` actions now propagate optional scope context and reject downloads when Slack metadata definitively shows the file is outside the requested channel/thread, while preserving legacy behavior when share metadata is unavailable.
|
||||||
- Security/Sandbox media reads: eliminate sandbox media TOCTOU symlink-retarget escapes by enforcing root-scoped boundary-safe reads at attachment/image load time and consolidating shared safe-read helpers across sandbox media callsites. This ships in the next npm release. Thanks @tdjackey for reporting.
|
- Security/Sandbox media reads: eliminate sandbox media TOCTOU symlink-retarget escapes by enforcing root-scoped boundary-safe reads at attachment/image load time and consolidating shared safe-read helpers across sandbox media callsites. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||||
- Security/Sandbox media staging: block destination symlink escapes in `stageSandboxMedia` by replacing direct destination copies with root-scoped safe writes for both local and SCP-staged attachments, preventing out-of-workspace file overwrite through `media/inbound` alias traversal. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.
|
- Security/Sandbox media staging: block destination symlink escapes in `stageSandboxMedia` by replacing direct destination copies with root-scoped safe writes for both local and SCP-staged attachments, preventing out-of-workspace file overwrite through `media/inbound` alias traversal. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.
|
||||||
|
- Security/Sandbox fs bridge: harden sandbox `remove` and `rename` operations by anchoring destructive actions to verified canonical parent directories plus basenames instead of passing mutable full path strings to `rm` and `mv`, reducing parent-directory symlink-rebind TOCTOU exposure in sandbox file operations. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||||
- Node host/service auth env: include `OPENCLAW_GATEWAY_TOKEN` in `openclaw node install` service environments (with `CLAWDBOT_GATEWAY_TOKEN` compatibility fallback) so installed node services keep remote gateway token auth across restart/reboot. Fixes #31041. Thanks @OneStepAt4time for reporting, @byungsker, @liuxiaopai-ai, and @vincentkoc.
|
- Node host/service auth env: include `OPENCLAW_GATEWAY_TOKEN` in `openclaw node install` service environments (with `CLAWDBOT_GATEWAY_TOKEN` compatibility fallback) so installed node services keep remote gateway token auth across restart/reboot. Fixes #31041. Thanks @OneStepAt4time for reporting, @byungsker, @liuxiaopai-ai, and @vincentkoc.
|
||||||
- Security/Subagents sandbox inheritance: block sandboxed sessions from spawning cross-agent subagents that would run unsandboxed, preventing runtime sandbox downgrade via `sessions_spawn agentId`. Thanks @tdjackey for reporting.
|
- Security/Subagents sandbox inheritance: block sandboxed sessions from spawning cross-agent subagents that would run unsandboxed, preventing runtime sandbox downgrade via `sessions_spawn agentId`. Thanks @tdjackey for reporting.
|
||||||
- Security/Workspace safe writes: harden `writeFileWithinRoot` against symlink-retarget TOCTOU races by opening existing files without truncation, creating missing files with exclusive create, deferring truncation until post-open identity+boundary validation, and removing out-of-root create artifacts on blocked races; added regression tests for truncate/create race paths. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.
|
- Security/Workspace safe writes: harden `writeFileWithinRoot` against symlink-retarget TOCTOU races by opening existing files without truncation, creating missing files with exclusive create, deferring truncation until post-open identity+boundary validation, and removing out-of-root create artifacts on blocked races; added regression tests for truncate/create race paths. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.
|
||||||
|
|||||||
@@ -46,6 +46,12 @@ function findCallByScriptFragment(fragment: string) {
|
|||||||
return mockedExecDockerRaw.mock.calls.find(([args]) => getDockerScript(args).includes(fragment));
|
return mockedExecDockerRaw.mock.calls.find(([args]) => getDockerScript(args).includes(fragment));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function findCallsByScriptFragment(fragment: string) {
|
||||||
|
return mockedExecDockerRaw.mock.calls.filter(([args]) =>
|
||||||
|
getDockerScript(args).includes(fragment),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
function dockerExecResult(stdout: string) {
|
function dockerExecResult(stdout: string) {
|
||||||
return {
|
return {
|
||||||
stdout: Buffer.from(stdout),
|
stdout: Buffer.from(stdout),
|
||||||
@@ -244,6 +250,40 @@ describe("sandbox fs bridge shell compatibility", () => {
|
|||||||
expect(scripts.some((script) => script.includes('mv -f -- "$1" "$2"'))).toBe(true);
|
expect(scripts.some((script) => script.includes('mv -f -- "$1" "$2"'))).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("anchors remove operations on canonical parent + basename", async () => {
|
||||||
|
const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
|
||||||
|
|
||||||
|
await bridge.remove({ filePath: "nested/file.txt" });
|
||||||
|
|
||||||
|
const removeCall = findCallByScriptFragment('rm -f -- "$2"');
|
||||||
|
expect(removeCall).toBeDefined();
|
||||||
|
const args = removeCall?.[0] ?? [];
|
||||||
|
expect(getDockerArg(args, 1)).toBe("/workspace/nested");
|
||||||
|
expect(getDockerArg(args, 2)).toBe("file.txt");
|
||||||
|
expect(args).not.toContain("/workspace/nested/file.txt");
|
||||||
|
|
||||||
|
const canonicalCalls = findCallsByScriptFragment('readlink -f -- "$cursor"');
|
||||||
|
expect(
|
||||||
|
canonicalCalls.some(([callArgs]) => getDockerArg(callArgs, 1) === "/workspace/nested"),
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("anchors rename operations on canonical parents + basenames", async () => {
|
||||||
|
const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
|
||||||
|
|
||||||
|
await bridge.rename({ from: "from.txt", to: "nested/to.txt" });
|
||||||
|
|
||||||
|
const renameCall = findCallByScriptFragment('mv -- "$3" "$2/$4"');
|
||||||
|
expect(renameCall).toBeDefined();
|
||||||
|
const args = renameCall?.[0] ?? [];
|
||||||
|
expect(getDockerArg(args, 1)).toBe("/workspace");
|
||||||
|
expect(getDockerArg(args, 2)).toBe("/workspace/nested");
|
||||||
|
expect(getDockerArg(args, 3)).toBe("from.txt");
|
||||||
|
expect(getDockerArg(args, 4)).toBe("to.txt");
|
||||||
|
expect(args).not.toContain("/workspace/from.txt");
|
||||||
|
expect(args).not.toContain("/workspace/nested/to.txt");
|
||||||
|
});
|
||||||
|
|
||||||
it("re-validates target before final rename and cleans temp file on failure", async () => {
|
it("re-validates target before final rename and cleans temp file on failure", async () => {
|
||||||
mockedOpenBoundaryFile
|
mockedOpenBoundaryFile
|
||||||
.mockImplementationOnce(async () => ({ ok: false, reason: "path" }))
|
.mockImplementationOnce(async () => ({ ok: false, reason: "path" }))
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import fs from "node:fs";
|
import fs from "node:fs";
|
||||||
|
import path from "node:path";
|
||||||
import { openBoundaryFile } from "../../infra/boundary-file-read.js";
|
import { openBoundaryFile } from "../../infra/boundary-file-read.js";
|
||||||
import { PATH_ALIAS_POLICIES, type PathAliasPolicy } from "../../infra/path-alias-guards.js";
|
import { PATH_ALIAS_POLICIES, type PathAliasPolicy } from "../../infra/path-alias-guards.js";
|
||||||
import type { SafeOpenSyncAllowedType } from "../../infra/safe-open-sync.js";
|
import type { SafeOpenSyncAllowedType } from "../../infra/safe-open-sync.js";
|
||||||
@@ -31,6 +32,11 @@ type PathSafetyCheck = {
|
|||||||
options: PathSafetyOptions;
|
options: PathSafetyOptions;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type AnchoredSandboxEntry = {
|
||||||
|
canonicalParentPath: string;
|
||||||
|
basename: string;
|
||||||
|
};
|
||||||
|
|
||||||
export type SandboxResolvedPath = {
|
export type SandboxResolvedPath = {
|
||||||
hostPath: string;
|
hostPath: string;
|
||||||
relativePath: string;
|
relativePath: string;
|
||||||
@@ -175,6 +181,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
const target = this.resolveResolvedPath(params);
|
const target = this.resolveResolvedPath(params);
|
||||||
this.ensureWriteAccess(target, "remove files");
|
this.ensureWriteAccess(target, "remove files");
|
||||||
|
const anchoredTarget = await this.resolveAnchoredSandboxEntry(target);
|
||||||
const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
|
const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
|
||||||
Boolean,
|
Boolean,
|
||||||
);
|
);
|
||||||
@@ -191,8 +198,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
},
|
},
|
||||||
],
|
],
|
||||||
recheckBeforeCommand: true,
|
recheckBeforeCommand: true,
|
||||||
script: `set -eu; ${rmCommand} -- "$1"`,
|
script: `set -eu\ncd -- "$1"\n${rmCommand} -- "$2"`,
|
||||||
args: [target.containerPath],
|
args: [anchoredTarget.canonicalParentPath, anchoredTarget.basename],
|
||||||
signal: params.signal,
|
signal: params.signal,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -207,6 +214,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd });
|
const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd });
|
||||||
this.ensureWriteAccess(from, "rename files");
|
this.ensureWriteAccess(from, "rename files");
|
||||||
this.ensureWriteAccess(to, "rename files");
|
this.ensureWriteAccess(to, "rename files");
|
||||||
|
const anchoredFrom = await this.resolveAnchoredSandboxEntry(from);
|
||||||
|
const anchoredTo = await this.resolveAnchoredSandboxEntry(to);
|
||||||
await this.runCheckedCommand({
|
await this.runCheckedCommand({
|
||||||
checks: [
|
checks: [
|
||||||
{
|
{
|
||||||
@@ -226,9 +235,13 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
},
|
},
|
||||||
],
|
],
|
||||||
recheckBeforeCommand: true,
|
recheckBeforeCommand: true,
|
||||||
script:
|
script: ["set -eu", 'mkdir -p -- "$2"', 'cd -- "$1"', 'mv -- "$3" "$2/$4"'].join("\n"),
|
||||||
'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"',
|
args: [
|
||||||
args: [from.containerPath, to.containerPath],
|
anchoredFrom.canonicalParentPath,
|
||||||
|
anchoredTo.canonicalParentPath,
|
||||||
|
anchoredFrom.basename,
|
||||||
|
anchoredTo.basename,
|
||||||
|
],
|
||||||
signal: params.signal,
|
signal: params.signal,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -416,6 +429,24 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
|||||||
return normalizeContainerPath(canonical);
|
return normalizeContainerPath(canonical);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async resolveAnchoredSandboxEntry(
|
||||||
|
target: SandboxResolvedFsPath,
|
||||||
|
): 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,
|
||||||
|
});
|
||||||
|
return {
|
||||||
|
canonicalParentPath,
|
||||||
|
basename,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
private async writeFileToTempPath(params: {
|
private async writeFileToTempPath(params: {
|
||||||
targetContainerPath: string;
|
targetContainerPath: string;
|
||||||
mkdir: boolean;
|
mkdir: boolean;
|
||||||
|
|||||||
Reference in New Issue
Block a user