From d02fbc6116ed9cbd501ad6a1e4d08f3fc71c1dd8 Mon Sep 17 00:00:00 2001 From: 6607changchun <84566142+6607changchun@users.noreply.github.com> Date: Tue, 5 May 2026 12:39:56 +0800 Subject: [PATCH] fix(sandbox): support Windows drive-letter bind sources Accept drive-absolute Windows sandbox Docker bind sources in config and runtime validation while keeping blocked-path and allowed-root comparisons case-insensitive for Windows drive paths. Also remove a stale WhatsApp setup import that blocked extension lint after the rebase. Co-authored-by: 6607changchun <84566142+6607changchun@users.noreply.github.com> Co-authored-by: Brad Groux <3053586+BradGroux@users.noreply.github.com> --- CHANGELOG.md | 1 + extensions/whatsapp/src/setup-finalize.ts | 1 - src/agents/sandbox/host-paths.test.ts | 39 +++++++++++++++++++ src/agents/sandbox/host-paths.ts | 37 ++++++++++++++++-- .../sandbox/validate-sandbox-security.test.ts | 32 ++++++++++----- .../sandbox/validate-sandbox-security.ts | 18 ++++++--- src/config/config.sandbox-docker.test.ts | 36 +++++++++++++++++ src/config/zod-schema.agent-runtime.ts | 11 ++++-- .../audit-sandbox-docker-config.test.ts | 17 ++++++++ 9 files changed, 167 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc90ba75631..e337ee64c11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ Docs: https://docs.openclaw.ai - Plugins/ClawHub: annotate 429 errors from ClawHub with the reset window from `RateLimit-Reset`/`Retry-After` and append a `Sign in for higher rate limits.` hint when the request was unauthenticated, so users can see when downloads will recover and how to lift the cap. Thanks @romneyda. - Plugins/runtime state: add `registerIfAbsent` for atomic keyed-store dedupe claims that return whether a plugin successfully claimed a key without overwriting an existing live value. Thanks @amknight. - Plugin SDK: add plugin-owned `SessionEntry` slot projection and scoped trusted-policy session extension reads. (#75609; replaces part of #73384/#74483) Thanks @100yenadmin. +- Sandbox/Windows: accept drive-absolute Docker bind sources while keeping sandbox blocked-path and allowed-root policy comparisons Windows-case-insensitive. (#42174) Thanks @6607changchun. ### Fixes diff --git a/extensions/whatsapp/src/setup-finalize.ts b/extensions/whatsapp/src/setup-finalize.ts index daa30302084..08207a30d06 100644 --- a/extensions/whatsapp/src/setup-finalize.ts +++ b/extensions/whatsapp/src/setup-finalize.ts @@ -1,7 +1,6 @@ import path from "node:path"; import { DEFAULT_ACCOUNT_ID, - normalizeE164, pathExists, splitSetupEntries, type DmPolicy, diff --git a/src/agents/sandbox/host-paths.test.ts b/src/agents/sandbox/host-paths.test.ts index 30933a5e03e..8ceb1d75d4e 100644 --- a/src/agents/sandbox/host-paths.test.ts +++ b/src/agents/sandbox/host-paths.test.ts @@ -3,6 +3,8 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { + getSandboxHostPathPolicyKey, + isSandboxHostPathAbsolute, normalizeSandboxHostPath, resolveSandboxHostPathViaExistingAncestor, } from "./host-paths.js"; @@ -11,6 +13,33 @@ describe("normalizeSandboxHostPath", () => { it("normalizes dot segments and strips trailing slash", () => { expect(normalizeSandboxHostPath("/tmp/a/../b//")).toBe("/tmp/b"); }); + + it("normalizes Windows drive-letter paths without losing the drive root", () => { + expect(normalizeSandboxHostPath("c:\\Users\\Kai\\..\\Project\\")).toBe("C:/Users/Project"); + expect(normalizeSandboxHostPath("d:/")).toBe("D:/"); + }); +}); + +describe("isSandboxHostPathAbsolute", () => { + it("accepts POSIX and drive-absolute Windows paths", () => { + expect(isSandboxHostPathAbsolute("/tmp/project")).toBe(true); + expect(isSandboxHostPathAbsolute("C:/Users/kai/project")).toBe(true); + expect(isSandboxHostPathAbsolute("C:\\Users\\kai\\project")).toBe(true); + }); + + it("rejects relative paths, named volumes, and drive-relative Windows paths", () => { + expect(isSandboxHostPathAbsolute("relative/path")).toBe(false); + expect(isSandboxHostPathAbsolute("my-volume")).toBe(false); + expect(isSandboxHostPathAbsolute("C:relative\\path")).toBe(false); + }); +}); + +describe("getSandboxHostPathPolicyKey", () => { + it("compares Windows drive-letter paths case-insensitively", () => { + expect(getSandboxHostPathPolicyKey("c:\\Users\\Kai\\.SSH\\config")).toBe( + "c:/users/kai/.ssh/config", + ); + }); }); describe("resolveSandboxHostPathViaExistingAncestor", () => { @@ -18,6 +47,16 @@ describe("resolveSandboxHostPathViaExistingAncestor", () => { expect(resolveSandboxHostPathViaExistingAncestor("relative/path")).toBe("relative/path"); }); + it("normalizes Windows paths without resolving them through POSIX cwd on non-Windows hosts", () => { + if (process.platform === "win32") { + return; + } + + expect(resolveSandboxHostPathViaExistingAncestor("C:/Users/kai/project")).toBe( + "C:/Users/kai/project", + ); + }); + it("resolves symlink parents when the final leaf does not exist", () => { if (process.platform === "win32") { return; diff --git a/src/agents/sandbox/host-paths.ts b/src/agents/sandbox/host-paths.ts index f07f44d2ff4..a5c012b4d67 100644 --- a/src/agents/sandbox/host-paths.ts +++ b/src/agents/sandbox/host-paths.ts @@ -19,16 +19,42 @@ function stripWindowsNamespacePrefix(input: string): string { return input; } +export function isWindowsDriveAbsolutePath(raw: string): boolean { + return /^[A-Za-z]:[\\/]/.test(stripWindowsNamespacePrefix(raw.trim())); +} + +export function isSandboxHostPathAbsolute(raw: string): boolean { + const trimmed = stripWindowsNamespacePrefix(raw.trim()); + return trimmed.startsWith("/") || isWindowsDriveAbsolutePath(trimmed); +} + /** - * Normalize a POSIX host path: resolve `.`, `..`, collapse `//`, strip trailing `/`. + * Normalize a host path: resolve `.`, `..`, collapse `//`, strip trailing `/`. + * Windows drive-letter paths preserve the drive root and uppercase the drive letter. */ export function normalizeSandboxHostPath(raw: string): string { const trimmed = stripWindowsNamespacePrefix(raw.trim()); if (!trimmed) { return "/"; } - const normalized = posix.normalize(trimmed.replaceAll("\\", "/")); - return normalized.replace(/\/+$/, "") || "/"; + let normalTrimmed = trimmed.replaceAll("\\", "/"); + if (isWindowsDriveAbsolutePath(normalTrimmed)) { + normalTrimmed = normalTrimmed.charAt(0).toUpperCase() + normalTrimmed.slice(1); + } + const normalized = posix.normalize(normalTrimmed); + const withoutTrailingSlash = normalized.replace(/\/+$/, "") || "/"; + if (/^[A-Z]:$/.test(withoutTrailingSlash)) { + return `${withoutTrailingSlash}/`; + } + return withoutTrailingSlash; +} + +export function getSandboxHostPathPolicyKey(raw: string): string { + const normalized = normalizeSandboxHostPath(raw); + if (isWindowsDriveAbsolutePath(normalized)) { + return normalized.toLowerCase(); + } + return normalized; } /** @@ -36,8 +62,11 @@ export function normalizeSandboxHostPath(raw: string): string { * even when the final source leaf does not exist yet. */ export function resolveSandboxHostPathViaExistingAncestor(sourcePath: string): string { - if (!sourcePath.startsWith("/")) { + if (!isSandboxHostPathAbsolute(sourcePath)) { return sourcePath; } + if (isWindowsDriveAbsolutePath(sourcePath) && process.platform !== "win32") { + return normalizeSandboxHostPath(sourcePath); + } return normalizeSandboxHostPath(resolvePathViaExistingAncestorSync(sourcePath)); } diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index 63a3146394a..c67f0a8f14e 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -174,6 +174,25 @@ describe("validateBindMounts", () => { expect(() => validateBindMounts(["/home/tester/.netrc:/mnt/netrc:ro"])).toThrow(/blocked path/); }); + it("allows drive-absolute Windows bind sources", () => { + expect(() => validateBindMounts(["D:/data/openclaw/src:/src:ro"])).not.toThrow(); + expect(() => validateBindMounts(["D:\\data\\openclaw\\output:/output:rw"])).not.toThrow(); + }); + + it("compares Windows allowed roots case-insensitively", () => { + expect(() => + validateBindMounts(["d:/DATA/OpenClaw/src:/src:ro"], { + allowedSourceRoots: ["D:/data/openclaw"], + }), + ).not.toThrow(); + + expect(() => + validateBindMounts(["D:/other/project:/src:ro"], { + allowedSourceRoots: ["d:/data/openclaw"], + }), + ).toThrow(/outside allowed roots/); + }); + it("blocks credential binds through canonical home aliases", () => { if (process.platform === "win32") { return; @@ -193,14 +212,7 @@ describe("validateBindMounts", () => { it("blocks symlink escapes into blocked directories", () => { if (process.platform === "win32") { - // Symlinks to non-existent targets like /etc require - // SeCreateSymbolicLinkPrivilege on Windows. The Windows branch of this - // test does not need a real symlink — it only asserts that Windows source - // paths are rejected as non-POSIX. - const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-")); - const fakePath = join(dir, "etc-link", "passwd"); - const run = () => validateBindMounts([`${fakePath}:/mnt/passwd:ro`]); - expect(run).toThrow(/non-absolute source path/); + // Symlink setup for blocked POSIX targets like /etc is POSIX-only. return; } @@ -213,7 +225,7 @@ describe("validateBindMounts", () => { it("blocks symlink-parent escapes with non-existent leaf outside allowed roots", () => { if (process.platform === "win32") { - // Windows source paths (e.g. C:\\...) are intentionally rejected as non-POSIX. + // Windows symlink semantics differ; POSIX symlink escape coverage runs on POSIX hosts. return; } const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-")); @@ -233,7 +245,7 @@ describe("validateBindMounts", () => { it("blocks symlink-parent escapes into blocked paths when leaf does not exist", () => { if (process.platform === "win32") { - // Windows source paths (e.g. C:\\...) are intentionally rejected as non-POSIX. + // Symlink setup for blocked POSIX targets like /var/run is POSIX-only. return; } const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-")); diff --git a/src/agents/sandbox/validate-sandbox-security.ts b/src/agents/sandbox/validate-sandbox-security.ts index 35098d42d2c..2d4de737dec 100644 --- a/src/agents/sandbox/validate-sandbox-security.ts +++ b/src/agents/sandbox/validate-sandbox-security.ts @@ -12,6 +12,8 @@ import { normalizeOptionalLowercaseString } from "../../shared/string-coerce.js" import { splitSandboxBindSpec } from "./bind-spec.js"; import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js"; import { + getSandboxHostPathPolicyKey, + isSandboxHostPathAbsolute, normalizeSandboxHostPath, resolveSandboxHostPathViaExistingAncestor, } from "./host-paths.js"; @@ -101,6 +103,7 @@ function parseBindTargetPath(bind: string): string { /** * Normalize a POSIX path: resolve `.`, `..`, collapse `//`, strip trailing `/`. + * If it starts with the drive letter, convert it to the upper case. */ function normalizeHostPath(raw: string): string { return normalizeSandboxHostPath(raw); @@ -115,10 +118,9 @@ function normalizeHostPath(raw: string): string { */ export function getBlockedBindReason(bind: string): BlockedBindReason | null { const sourceRaw = parseBindSourcePath(bind); - if (!sourceRaw.startsWith("/")) { + if (!isSandboxHostPathAbsolute(sourceRaw)) { return { kind: "non_absolute", sourcePath: sourceRaw }; } - const normalized = normalizeHostPath(sourceRaw); const blockedHostPaths = getBlockedHostPaths(); const directReason = getBlockedReasonForSourcePath(normalized, blockedHostPaths); @@ -141,8 +143,10 @@ function getBlockedReasonForSourcePath( if (sourceNormalized === "/") { return { kind: "covers", blockedPath: "/" }; } + const sourceKey = getSandboxHostPathPolicyKey(sourceNormalized); for (const blocked of blockedHostPaths) { - if (sourceNormalized === blocked || sourceNormalized.startsWith(blocked + "/")) { + const blockedKey = getSandboxHostPathPolicyKey(blocked); + if (sourceKey === blockedKey || sourceKey.startsWith(`${blockedKey}/`)) { return { kind: "targets", blockedPath: blocked }; } } @@ -193,7 +197,7 @@ function normalizeAllowedRoots(roots: string[] | undefined): string[] { } const normalized = roots .map((entry) => entry.trim()) - .filter((entry) => entry.startsWith("/")) + .filter(isSandboxHostPathAbsolute) .map(normalizeHostPath); const expanded = new Set(); for (const root of normalized) { @@ -210,7 +214,9 @@ function isPathInsidePosix(root: string, target: string): boolean { if (root === "/") { return true; } - return target === root || target.startsWith(`${root}/`); + const rootKey = getSandboxHostPathPolicyKey(root); + const targetKey = getSandboxHostPathPolicyKey(target); + return targetKey === rootKey || targetKey.startsWith(`${rootKey}/`); } function getOutsideAllowedRootsReason( @@ -274,7 +280,7 @@ function formatBindBlockedError(params: { bind: string; reason: BlockedBindReaso if (params.reason.kind === "non_absolute") { return new Error( `Sandbox security: bind mount "${params.bind}" uses a non-absolute source path ` + - `"${params.reason.sourcePath}". Only absolute POSIX paths are supported for sandbox binds.`, + `"${params.reason.sourcePath}". Only absolute POSIX or Windows drive-letter paths are supported for sandbox binds.`, ); } if (params.reason.kind === "outside_allowed_roots") { diff --git a/src/config/config.sandbox-docker.test.ts b/src/config/config.sandbox-docker.test.ts index db8a6eac6ba..6fb98f3df50 100644 --- a/src/config/config.sandbox-docker.test.ts +++ b/src/config/config.sandbox-docker.test.ts @@ -62,6 +62,42 @@ describe("sandbox docker config", () => { } }); + it("accepts Windows drive-letter binds in sandbox.docker config", () => { + const res = validateConfigObject({ + agents: { + defaults: { + sandbox: { + docker: { + binds: ["D:/data/openclaw/src:/src:ro", "D:\\data\\openclaw\\output:/output:rw"], + }, + }, + }, + }, + }); + expect(res.ok).toBe(true); + if (res.ok) { + expect(res.config.agents?.defaults?.sandbox?.docker?.binds).toEqual([ + "D:/data/openclaw/src:/src:ro", + "D:\\data\\openclaw\\output:/output:rw", + ]); + } + }); + + it("rejects drive-relative Windows binds in sandbox.docker config", () => { + const res = validateConfigObject({ + agents: { + defaults: { + sandbox: { + docker: { + binds: ["D:relative\\path:/src:ro"], + }, + }, + }, + }, + }); + expect(res.ok).toBe(false); + }); + it("accepts non-empty Docker GPU passthrough config", () => { const res = validateConfigObject({ agents: { diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 16527d064d5..daa2f0116c2 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -1,4 +1,6 @@ import { z } from "zod"; +import { splitSandboxBindSpec } from "../agents/sandbox/bind-spec.js"; +import { isSandboxHostPathAbsolute } from "../agents/sandbox/host-paths.js"; import { getBlockedNetworkModeReason } from "../agents/sandbox/network-mode.js"; import { parseDurationMs } from "../cli/parse-duration.js"; import { @@ -158,15 +160,16 @@ const SandboxDockerSchema = z }); continue; } - const firstColon = bind.indexOf(":"); - const source = (firstColon <= 0 ? bind : bind.slice(0, firstColon)).trim(); - if (!source.startsWith("/")) { + + const parsed = splitSandboxBindSpec(bind); + const source = (parsed ? parsed.host : bind).trim(); + if (!isSandboxHostPathAbsolute(source)) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["binds", i], message: `Sandbox security: bind mount "${bind}" uses a non-absolute source path "${source}". ` + - "Only absolute POSIX paths are supported for sandbox binds.", + "Only absolute POSIX or Windows drive-letter paths are supported for sandbox binds.", }); } } diff --git a/src/security/audit-sandbox-docker-config.test.ts b/src/security/audit-sandbox-docker-config.test.ts index 5eb631bd8f8..96654130072 100644 --- a/src/security/audit-sandbox-docker-config.test.ts +++ b/src/security/audit-sandbox-docker-config.test.ts @@ -126,6 +126,23 @@ describe("security audit sandbox docker config", () => { }, ], }, + { + name: "Windows drive-letter bind is absolute", + cfg: { + agents: { + defaults: { + sandbox: { + mode: "all", + docker: { + binds: ["D:/data/openclaw/src:/src:ro"], + }, + }, + }, + }, + } as OpenClawConfig, + expectedFindings: [], + expectedAbsent: ["sandbox.bind_mount_non_absolute"], + }, { name: "container namespace join network mode", cfg: {