From b5787e4abba0dcc6baf09051099f6773c1679ec1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 14:37:13 +0000 Subject: [PATCH] fix(sandbox): harden bind validation for symlink missing-leaf paths --- CHANGELOG.md | 1 + .../sandbox/validate-sandbox-security.test.ts | 40 +++++++++++- .../sandbox/validate-sandbox-security.ts | 63 ++++++++++++------- 3 files changed, 81 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bd95ed2f5c..fb88e0dbb7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai - macOS/Menu bar: stop reusing the injector delegate for the "Usage cost (30 days)" submenu to prevent recursive submenu injection loops when opening cost history. (#25341) Thanks @yingchunbai. - Control UI/Chat images: harden image-open clicks against reverse tabnabbing by using opener isolation (`noopener,noreferrer` plus `window.opener = null`). (#18685) Thanks @Mariana-Codebase. - CLI/Doctor: correct stale recovery hints to use valid commands (`openclaw gateway status --deep` and `openclaw configure --section model`). (#24485) Thanks @chilu18. +- Security/Sandbox: canonicalize bind-mount source paths via existing-ancestor realpath so symlink-parent + non-existent-leaf paths cannot bypass allowed-source-roots or blocked-path checks. ## 2026.2.23 diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index fae66cc7924..22a5be14d5d 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, symlinkSync } from "node:fs"; +import { mkdirSync, mkdtempSync, symlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -117,6 +117,44 @@ describe("validateBindMounts", () => { expect(run).toThrow(/blocked path/); }); + 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. + return; + } + const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-")); + const workspace = join(dir, "workspace"); + const outside = join(dir, "outside"); + mkdirSync(workspace, { recursive: true }); + mkdirSync(outside, { recursive: true }); + const link = join(workspace, "alias-out"); + symlinkSync(outside, link); + const missingLeaf = join(link, "not-yet-created"); + expect(() => + validateBindMounts([`${missingLeaf}:/mnt/data:ro`], { + allowedSourceRoots: [workspace], + }), + ).toThrow(/outside allowed roots/); + }); + + 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. + return; + } + const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-")); + const workspace = join(dir, "workspace"); + mkdirSync(workspace, { recursive: true }); + const link = join(workspace, "run-link"); + symlinkSync("/var/run", link); + const missingLeaf = join(link, "openclaw-not-created"); + expect(() => + validateBindMounts([`${missingLeaf}:/mnt/run:ro`], { + allowedSourceRoots: [workspace], + }), + ).toThrow(/blocked path/); + }); + it("rejects non-absolute source paths (relative or named volumes)", () => { const cases = ["../etc/passwd:/mnt/passwd", "etc/passwd:/mnt/passwd", "myvol:/mnt"] as const; for (const source of cases) { diff --git a/src/agents/sandbox/validate-sandbox-security.ts b/src/agents/sandbox/validate-sandbox-security.ts index a14fd50d036..44fe9f7ba0d 100644 --- a/src/agents/sandbox/validate-sandbox-security.ts +++ b/src/agents/sandbox/validate-sandbox-security.ts @@ -119,18 +119,38 @@ export function getBlockedReasonForSourcePath(sourceNormalized: string): Blocked return null; } -function tryRealpathAbsolute(path: string): string { - if (!path.startsWith("/")) { - return path; +function resolvePathViaExistingAncestor(sourcePath: string): string { + if (!sourcePath.startsWith("/")) { + return sourcePath; } - if (!existsSync(path)) { - return path; + + const normalized = normalizeHostPath(sourcePath); + let current = normalized; + const missingSegments: string[] = []; + + // Resolve through the deepest existing ancestor so symlink parents are honored + // even when the final source leaf does not exist yet. + while (current !== "/" && !existsSync(current)) { + missingSegments.unshift(posix.basename(current)); + const parent = posix.dirname(current); + if (parent === current) { + break; + } + current = parent; } + + if (!existsSync(current)) { + return normalized; + } + try { - // Use native when available (keeps platform semantics); normalize for prefix checks. - return normalizeHostPath(realpathSync.native(path)); + const resolvedAncestor = normalizeHostPath(realpathSync.native(current)); + if (missingSegments.length === 0) { + return resolvedAncestor; + } + return normalizeHostPath(posix.join(resolvedAncestor, ...missingSegments)); } catch { - return path; + return normalized; } } @@ -145,7 +165,7 @@ function normalizeAllowedRoots(roots: string[] | undefined): string[] { const expanded = new Set(); for (const root of normalized) { expanded.add(root); - const real = tryRealpathAbsolute(root); + const real = resolvePathViaExistingAncestor(root); if (real !== root) { expanded.add(real); } @@ -227,7 +247,8 @@ function formatBindBlockedError(params: { bind: string; reason: BlockedBindReaso /** * Validate bind mounts — throws if any source path is dangerous. - * Includes a symlink/realpath pass when the source path exists. + * Includes a symlink/realpath pass via existing ancestors so non-existent leaf + * paths cannot bypass source-root and blocked-path checks. */ export function validateBindMounts( binds: string[] | undefined, @@ -268,18 +289,16 @@ export function validateBindMounts( } } - // Symlink escape hardening: resolve existing absolute paths and re-check. - const sourceReal = tryRealpathAbsolute(sourceNormalized); - if (sourceReal !== sourceNormalized) { - const reason = getBlockedReasonForSourcePath(sourceReal); - if (reason) { - throw formatBindBlockedError({ bind, reason }); - } - if (!options?.allowSourcesOutsideAllowedRoots) { - const allowedReason = getOutsideAllowedRootsReason(sourceReal, allowedRoots); - if (allowedReason) { - throw formatBindBlockedError({ bind, reason: allowedReason }); - } + // Symlink escape hardening: resolve through existing ancestors and re-check. + const sourceCanonical = resolvePathViaExistingAncestor(sourceNormalized); + const reason = getBlockedReasonForSourcePath(sourceCanonical); + if (reason) { + throw formatBindBlockedError({ bind, reason }); + } + if (!options?.allowSourcesOutsideAllowedRoots) { + const allowedReason = getOutsideAllowedRootsReason(sourceCanonical, allowedRoots); + if (allowedReason) { + throw formatBindBlockedError({ bind, reason: allowedReason }); } } }