From 21c0c1781e3383eeef6dcbea5f42861a0a761f99 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Sun, 26 Apr 2026 19:45:44 +0000 Subject: [PATCH 1/2] fix(snapshot): allow /sandbox/.openclaw-data symlinks in safeTarExtract nemoclaw snapshot create / rebuild / backup-all fail on 0.0.22 because safeTarExtract's post-extraction symlink audit rejects absolute symlinks whose target (/sandbox/.openclaw-data/media, etc.) does not exist on the host. These are legitimate intra-sandbox symlinks created by Dockerfile.base for the .openclaw / .openclaw-data state split. Fix: when resolving an absolute symlink target, if the path starts with /sandbox/.openclaw-data/ or /sandbox/.hermes-data/, also check whether it falls within the extraction root when the /sandbox/ prefix is stripped and resolved relative to that root. This mirrors how the symlink resolves inside the sandbox container (where /sandbox/.openclaw-data/* exists). Absolute paths outside these known prefixes (e.g. /etc/passwd) are still rejected as before. The security boundary is preserved. Added two regression tests: - Allows /sandbox/.openclaw-data/media symlinks (the #2317 repro case) - Still blocks /etc/passwd and other non-/sandbox/.openclaw-data targets Fixes #2317 Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> --- src/lib/sandbox-state.ts | 29 ++++++++++-- test/security-sandbox-tar-traversal.test.ts | 52 +++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/lib/sandbox-state.ts b/src/lib/sandbox-state.ts index 69ed5a1ccbd..eaafc8e062c 100644 --- a/src/lib/sandbox-state.ts +++ b/src/lib/sandbox-state.ts @@ -279,11 +279,34 @@ function auditExtractedSymlinks(dirPath: string, allowedRoots: string[]): string const stat = lstatSync(fullPath); if (stat.isSymbolicLink()) { const linkTarget = readlinkSync(fullPath); - const resolvedTarget = path.resolve(path.dirname(fullPath), linkTarget); - const inAnyAllowedRoot = allowedRoots.some((root) => isWithinRoot(resolvedTarget, root)); + + // Resolve relative to the symlink's containing directory (standard). + const resolvedRelative = path.resolve(path.dirname(fullPath), linkTarget); + + // For absolute symlinks that point into the canonical sandbox data + // directory (/sandbox/.openclaw-data/** or /sandbox/.hermes-data/**), + // also check whether the target falls within the extraction root when + // the leading /sandbox/ prefix is mapped onto the archive root. This + // mirrors how the symlink resolves once the backup is restored inside + // the sandbox container (where /sandbox/.openclaw-data/* exists). + // + // Only /sandbox/ prefixed targets receive this treatment so that + // symlinks pointing to arbitrary absolute paths (e.g. /etc/passwd) + // are still rejected. Fixes #2317. + const SANDBOX_DATA_PREFIXES = ["/sandbox/.openclaw-data/", "/sandbox/.hermes-data/"]; + const resolvedInArchive = + path.isAbsolute(linkTarget) && + SANDBOX_DATA_PREFIXES.some((p) => linkTarget.startsWith(p)) + ? path.resolve(dirPath, linkTarget.replace(/^\//, "")) + : null; + + const inAnyAllowedRoot = + allowedRoots.some((root) => isWithinRoot(resolvedRelative, root)) || + (resolvedInArchive !== null && isWithinRoot(resolvedInArchive, dirPath)); + if (!inAnyAllowedRoot) { violations.push( - `symlink escape: ${fullPath} -> ${linkTarget} (resolves to ${resolvedTarget})`, + `symlink escape: ${fullPath} -> ${linkTarget} (resolves to ${resolvedRelative})`, ); } } else if (stat.isDirectory()) { diff --git a/test/security-sandbox-tar-traversal.test.ts b/test/security-sandbox-tar-traversal.test.ts index 5052d6efca1..bcec0dd170c 100644 --- a/test/security-sandbox-tar-traversal.test.ts +++ b/test/security-sandbox-tar-traversal.test.ts @@ -385,6 +385,58 @@ describe("Fix: safeTarExtract blocks malicious archives and extracts safe ones", fs.rmSync(workDir, { recursive: true, force: true }); } }); + + // Regression #2317: /sandbox/.openclaw-data/* symlinks are created by + // Dockerfile.base for the .openclaw / .openclaw-data split. When a backup + // is extracted on the host, these absolute targets don't exist on the host + // and were falsely rejected as escapes. The fix maps /sandbox/ paths onto + // the extraction root before checking, matching the sandbox-internal view. + it("regression #2317: allows known-safe /sandbox/.openclaw-data symlinks in backup archives", async () => { + const { safeTarExtract } = await loadSandboxState(); + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2317-")); + try { + const targetDir = path.join(workDir, "backup"); + fs.mkdirSync(targetDir, { recursive: true }); + + // Simulate the workspace/media symlink created by Dockerfile.base + const tar = buildTar([ + { + path: "workspace/media", + type: "2", + linkTarget: "/sandbox/.openclaw-data/media", + }, + ]); + + const result = safeTarExtract(tar, targetDir); + expect(result.success).toBe(true); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + it("regression #2317: still blocks absolute symlinks outside /sandbox/.openclaw-data", async () => { + const { safeTarExtract } = await loadSandboxState(); + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2317-block-")); + try { + const targetDir = path.join(workDir, "backup"); + fs.mkdirSync(targetDir, { recursive: true }); + + // /etc/passwd should still be rejected — not in /sandbox/.openclaw-data/ + const tar = buildTar([ + { + path: "evil-link", + type: "2", + linkTarget: "/etc/passwd", + }, + ]); + + const result = safeTarExtract(tar, targetDir); + expect(result.success).toBe(false); + expect(result.error).toContain("symlink"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); }); describe("Fix: rejectHardLinks blocks hard-link entries at validation time", () => { From 9f614c5effc451e765a83636eb63e4c9f0583439 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Sun, 26 Apr 2026 19:54:16 +0000 Subject: [PATCH 2/2] fix(snapshot): normalize symlink target before prefix check to block traversal bypass Copilot review on #2488 correctly identified that the previous fix could be bypassed: a target like /sandbox/.openclaw-data/../../etc/passwd starts with the allowed prefix but after normalization resolves to /etc/passwd. Fix: apply path.posix.normalize() to the symlink target before the prefix check, so traversal sequences are collapsed and cannot be used to sneak past the allowlist. Added regression test: /sandbox/.openclaw-data/../../etc/passwd is blocked. Per Copilot + CodeRabbit review on #2488. Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> --- src/lib/sandbox-state.ts | 11 +++++++--- test/security-sandbox-tar-traversal.test.ts | 24 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/lib/sandbox-state.ts b/src/lib/sandbox-state.ts index eaafc8e062c..9480ca6cebd 100644 --- a/src/lib/sandbox-state.ts +++ b/src/lib/sandbox-state.ts @@ -294,10 +294,15 @@ function auditExtractedSymlinks(dirPath: string, allowedRoots: string[]): string // symlinks pointing to arbitrary absolute paths (e.g. /etc/passwd) // are still rejected. Fixes #2317. const SANDBOX_DATA_PREFIXES = ["/sandbox/.openclaw-data/", "/sandbox/.hermes-data/"]; + // Normalize the target first to collapse any .. traversal segments + // (e.g. /sandbox/.openclaw-data/../../etc/passwd → /etc/passwd). + // Only then check the prefix — this prevents a traversal bypass + // where a crafted target starts with an allowed prefix but escapes it. + const normalizedTarget = path.posix.normalize(linkTarget); const resolvedInArchive = - path.isAbsolute(linkTarget) && - SANDBOX_DATA_PREFIXES.some((p) => linkTarget.startsWith(p)) - ? path.resolve(dirPath, linkTarget.replace(/^\//, "")) + path.isAbsolute(normalizedTarget) && + SANDBOX_DATA_PREFIXES.some((p) => normalizedTarget.startsWith(p)) + ? path.resolve(dirPath, normalizedTarget.replace(/^\//, "")) : null; const inAnyAllowedRoot = diff --git a/test/security-sandbox-tar-traversal.test.ts b/test/security-sandbox-tar-traversal.test.ts index bcec0dd170c..e3a99140df5 100644 --- a/test/security-sandbox-tar-traversal.test.ts +++ b/test/security-sandbox-tar-traversal.test.ts @@ -437,6 +437,30 @@ describe("Fix: safeTarExtract blocks malicious archives and extracts safe ones", fs.rmSync(workDir, { recursive: true, force: true }); } }); + + it("regression #2317: blocks path traversal within allowed prefix (/sandbox/.openclaw-data/../../etc/passwd)", async () => { + const { safeTarExtract } = await loadSandboxState(); + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2317-traversal-")); + try { + const targetDir = path.join(workDir, "backup"); + fs.mkdirSync(targetDir, { recursive: true }); + + // Crafted target starts with allowed prefix but traverses out of it + const tar = buildTar([ + { + path: "evil-traversal", + type: "2", + linkTarget: "/sandbox/.openclaw-data/../../etc/passwd", + }, + ]); + + const result = safeTarExtract(tar, targetDir); + expect(result.success).toBe(false); + expect(result.error).toContain("symlink"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); }); describe("Fix: rejectHardLinks blocks hard-link entries at validation time", () => {