mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-18 22:37:26 +00:00
fix: tighten sandbox mkdirp boundary checks (#30610) (thanks @glitch418x)
This commit is contained in:
@@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Sandbox/mkdirp boundary checks: allow directory-safe boundary validation for existing in-boundary subdirectories, preventing false `cannot create directories` failures in sandbox write mode. (#30610) Thanks @glitch418x.
|
||||
- Android/Voice screen TTS: stream assistant speech via ElevenLabs WebSocket in Talk Mode, stop cleanly on speaker mute/barge-in, and ignore stale out-of-order stream events. (#29521) Thanks @gregmousseau.
|
||||
- Web UI/Cron: include configured agent model defaults/fallbacks in cron model suggestions so scheduled-job model autocomplete reflects configured models. (#29709)
|
||||
- Cron/Delivery: disable the agent messaging tool when `delivery.mode` is `"none"` so cron output is not sent to Telegram or other channels. (#21808)
|
||||
|
||||
@@ -198,6 +198,30 @@ describe("sandbox fs bridge shell compatibility", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects mkdirp when target exists as a file", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-mkdirp-file-"));
|
||||
try {
|
||||
const workspaceDir = path.join(stateDir, "workspace");
|
||||
const filePath = path.join(workspaceDir, "memory", "kemik");
|
||||
await fs.mkdir(path.dirname(filePath), { recursive: true });
|
||||
await fs.writeFile(filePath, "not a directory");
|
||||
|
||||
const bridge = createSandboxFsBridge({
|
||||
sandbox: createSandbox({
|
||||
workspaceDir,
|
||||
agentWorkspaceDir: workspaceDir,
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(bridge.mkdirp({ filePath: "memory/kemik" })).rejects.toThrow(
|
||||
/cannot create directories/i,
|
||||
);
|
||||
expect(mockedExecDockerRaw).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects pre-existing host symlink escapes before docker exec", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-"));
|
||||
const workspaceDir = path.join(stateDir, "workspace");
|
||||
|
||||
@@ -24,7 +24,7 @@ type PathSafetyOptions = {
|
||||
aliasPolicy?: PathAliasPolicy;
|
||||
requireWritable?: boolean;
|
||||
allowMissingTarget?: boolean;
|
||||
allowedTypes?: readonly SafeOpenSyncAllowedType[];
|
||||
allowedType?: SafeOpenSyncAllowedType;
|
||||
};
|
||||
|
||||
export type SandboxResolvedPath = {
|
||||
@@ -137,7 +137,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
await this.assertPathSafety(target, {
|
||||
action: "create directories",
|
||||
requireWritable: true,
|
||||
allowedTypes: ["directory"],
|
||||
allowedType: "directory",
|
||||
});
|
||||
await this.runCommand('set -eu; mkdir -p -- "$1"', {
|
||||
args: [target.containerPath],
|
||||
@@ -264,7 +264,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
rootPath: lexicalMount.hostRoot,
|
||||
boundaryLabel: "sandbox mount root",
|
||||
aliasPolicy: options.aliasPolicy,
|
||||
allowedTypes: options.allowedTypes,
|
||||
allowedType: options.allowedType,
|
||||
});
|
||||
if (!guarded.ok) {
|
||||
if (guarded.reason !== "path" || options.allowMissingTarget === false) {
|
||||
|
||||
@@ -32,7 +32,7 @@ export type OpenBoundaryFileSyncParams = {
|
||||
rootRealPath?: string;
|
||||
maxBytes?: number;
|
||||
rejectHardlinks?: boolean;
|
||||
allowedTypes?: readonly SafeOpenSyncAllowedType[];
|
||||
allowedType?: SafeOpenSyncAllowedType;
|
||||
skipLexicalRootCheck?: boolean;
|
||||
ioFs?: BoundaryReadFs;
|
||||
};
|
||||
@@ -79,7 +79,7 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda
|
||||
resolvedPath,
|
||||
rejectHardlinks: params.rejectHardlinks ?? true,
|
||||
maxBytes: params.maxBytes,
|
||||
allowedTypes: params.allowedTypes,
|
||||
allowedType: params.allowedType,
|
||||
ioFs,
|
||||
});
|
||||
if (!opened.ok) {
|
||||
|
||||
@@ -28,14 +28,14 @@ describe("openVerifiedFileSync", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts directories when allowedTypes includes directory", async () => {
|
||||
it("accepts directories when allowedType is directory", async () => {
|
||||
await withTempDir("openclaw-safe-open-", async (root) => {
|
||||
const targetDir = path.join(root, "nested");
|
||||
await fsp.mkdir(targetDir, { recursive: true });
|
||||
|
||||
const opened = openVerifiedFileSync({
|
||||
filePath: targetDir,
|
||||
allowedTypes: ["directory"],
|
||||
allowedType: "directory",
|
||||
rejectHardlinks: true,
|
||||
});
|
||||
expect(opened.ok).toBe(true);
|
||||
|
||||
@@ -30,11 +30,11 @@ export function openVerifiedFileSync(params: {
|
||||
rejectPathSymlink?: boolean;
|
||||
rejectHardlinks?: boolean;
|
||||
maxBytes?: number;
|
||||
allowedTypes?: readonly SafeOpenSyncAllowedType[];
|
||||
allowedType?: SafeOpenSyncAllowedType;
|
||||
ioFs?: SafeOpenSyncFs;
|
||||
}): SafeOpenSyncResult {
|
||||
const ioFs = params.ioFs ?? fs;
|
||||
const allowedTypes = params.allowedTypes ?? ["file"];
|
||||
const allowedType = params.allowedType ?? "file";
|
||||
const openReadFlags =
|
||||
ioFs.constants.O_RDONLY |
|
||||
(typeof ioFs.constants.O_NOFOLLOW === "number" ? ioFs.constants.O_NOFOLLOW : 0);
|
||||
@@ -49,7 +49,7 @@ export function openVerifiedFileSync(params: {
|
||||
|
||||
const realPath = params.resolvedPath ?? ioFs.realpathSync(params.filePath);
|
||||
const preOpenStat = ioFs.lstatSync(realPath);
|
||||
if (!isAllowedType(preOpenStat, allowedTypes)) {
|
||||
if (!isAllowedType(preOpenStat, allowedType)) {
|
||||
return { ok: false, reason: "validation" };
|
||||
}
|
||||
if (params.rejectHardlinks && preOpenStat.isFile() && preOpenStat.nlink > 1) {
|
||||
@@ -65,7 +65,7 @@ export function openVerifiedFileSync(params: {
|
||||
|
||||
fd = ioFs.openSync(realPath, openReadFlags);
|
||||
const openedStat = ioFs.fstatSync(fd);
|
||||
if (!isAllowedType(openedStat, allowedTypes)) {
|
||||
if (!isAllowedType(openedStat, allowedType)) {
|
||||
return { ok: false, reason: "validation" };
|
||||
}
|
||||
if (params.rejectHardlinks && openedStat.isFile() && openedStat.nlink > 1) {
|
||||
@@ -93,11 +93,9 @@ export function openVerifiedFileSync(params: {
|
||||
}
|
||||
}
|
||||
|
||||
function isAllowedType(stat: fs.Stats, allowedTypes: readonly SafeOpenSyncAllowedType[]): boolean {
|
||||
return allowedTypes.some((allowedType) => {
|
||||
if (allowedType === "file") {
|
||||
return stat.isFile();
|
||||
}
|
||||
function isAllowedType(stat: fs.Stats, allowedType: SafeOpenSyncAllowedType): boolean {
|
||||
if (allowedType === "directory") {
|
||||
return stat.isDirectory();
|
||||
});
|
||||
}
|
||||
return stat.isFile();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user