diff --git a/CHANGELOG.md b/CHANGELOG.md index c29a34c9bd4..c34663e412c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai - Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), and centralize range checks into a single CIDR policy table to reduce classifier drift. - Security/Archive: block zip symlink escapes during archive extraction. - Security/Media sandbox: keep tmp media allowance for absolute tmp paths only and enforce symlink-escape checks before sandbox-validated reads, preventing tmp symlink exfiltration and relative `../` sandbox escapes when sandboxes live under tmp. (#17892) Thanks @dashed. +- Browser/Upload: accept canonical in-root upload paths when the configured uploads directory is a symlink alias (for example `/tmp` -> `/private/tmp` on macOS), so browser upload validation no longer rejects valid files during client->server revalidation. (#23300, #23222, #22848) Thanks @bgaither4, @parkerati, and @Nabsku. - Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported, and canonicalize resolved Discord allowlist names to IDs at runtime without rewriting config files. Thanks @tdjackey for reporting. - Security/Gateway: block node-role connections when device identity metadata is missing. - Security/Media: enforce inbound media byte limits during download/read across Discord, Telegram, Zalo, Microsoft Teams, and BlueBubbles to prevent oversized payload memory spikes before rejection. This ships in the next npm release. Thanks @tdjackey for reporting. diff --git a/src/browser/paths.test.ts b/src/browser/paths.test.ts index 1178753ff92..441ee05b869 100644 --- a/src/browser/paths.test.ts +++ b/src/browser/paths.test.ts @@ -138,6 +138,60 @@ describe("resolveExistingPathsWithinRoot", () => { }); }, ); + + it.runIf(process.platform !== "win32")( + "accepts canonical absolute paths when upload root is a symlink alias", + async () => { + await withFixtureRoot(async ({ baseDir }) => { + const canonicalUploadsDir = path.join(baseDir, "canonical", "uploads"); + const aliasedUploadsDir = path.join(baseDir, "uploads-link"); + await fs.mkdir(canonicalUploadsDir, { recursive: true }); + await fs.symlink(canonicalUploadsDir, aliasedUploadsDir); + + const filePath = path.join(canonicalUploadsDir, "ok.txt"); + await fs.writeFile(filePath, "ok", "utf8"); + const canonicalPath = await fs.realpath(filePath); + + const firstPass = await resolveWithinUploads({ + uploadsDir: aliasedUploadsDir, + requestedPaths: [path.join(aliasedUploadsDir, "ok.txt")], + }); + expect(firstPass.ok).toBe(true); + + const secondPass = await resolveWithinUploads({ + uploadsDir: aliasedUploadsDir, + requestedPaths: [canonicalPath], + }); + expect(secondPass.ok).toBe(true); + if (secondPass.ok) { + expect(secondPass.paths).toEqual([canonicalPath]); + } + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "rejects canonical absolute paths outside symlinked upload root", + async () => { + await withFixtureRoot(async ({ baseDir }) => { + const canonicalUploadsDir = path.join(baseDir, "canonical", "uploads"); + const aliasedUploadsDir = path.join(baseDir, "uploads-link"); + await fs.mkdir(canonicalUploadsDir, { recursive: true }); + await fs.symlink(canonicalUploadsDir, aliasedUploadsDir); + + const outsideDir = path.join(baseDir, "outside"); + await fs.mkdir(outsideDir, { recursive: true }); + const outsideFile = path.join(outsideDir, "secret.txt"); + await fs.writeFile(outsideFile, "secret", "utf8"); + + const result = await resolveWithinUploads({ + uploadsDir: aliasedUploadsDir, + requestedPaths: [await fs.realpath(outsideFile)], + }); + expectInvalidResult(result, "must stay within uploads directory"); + }); + }, + ); }); describe("resolvePathWithinRoot", () => { diff --git a/src/browser/paths.ts b/src/browser/paths.ts index 3af2bd149e1..0b458e44dec 100644 --- a/src/browser/paths.ts +++ b/src/browser/paths.ts @@ -1,3 +1,4 @@ +import fs from "node:fs/promises"; import path from "node:path"; import { SafeOpenError, openFileWithinRoot } from "../infra/fs-safe.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; @@ -54,30 +55,73 @@ export async function resolveExistingPathsWithinRoot(params: { requestedPaths: string[]; scopeLabel: string; }): Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> { - const resolvedPaths: string[] = []; - for (const raw of params.requestedPaths) { - const pathResult = resolvePathWithinRoot({ - rootDir: params.rootDir, - requestedPath: raw, + const rootDir = path.resolve(params.rootDir); + let rootRealPath: string | undefined; + try { + rootRealPath = await fs.realpath(rootDir); + } catch { + // Keep historical behavior for missing roots and rely on openFileWithinRoot for final checks. + rootRealPath = undefined; + } + + const isInRoot = (relativePath: string) => + Boolean(relativePath) && !relativePath.startsWith("..") && !path.isAbsolute(relativePath); + + const resolveExistingRelativePath = async ( + requestedPath: string, + ): Promise< + { ok: true; relativePath: string; fallbackPath: string } | { ok: false; error: string } + > => { + const raw = requestedPath.trim(); + const lexicalPathResult = resolvePathWithinRoot({ + rootDir, + requestedPath, scopeLabel: params.scopeLabel, }); + if (lexicalPathResult.ok) { + return { + ok: true, + relativePath: path.relative(rootDir, lexicalPathResult.path), + fallbackPath: lexicalPathResult.path, + }; + } + if (!rootRealPath || !raw || !path.isAbsolute(raw)) { + return lexicalPathResult; + } + try { + const resolvedExistingPath = await fs.realpath(raw); + const relativePath = path.relative(rootRealPath, resolvedExistingPath); + if (!isInRoot(relativePath)) { + return lexicalPathResult; + } + return { + ok: true, + relativePath, + fallbackPath: resolvedExistingPath, + }; + } catch { + return lexicalPathResult; + } + }; + + const resolvedPaths: string[] = []; + for (const raw of params.requestedPaths) { + const pathResult = await resolveExistingRelativePath(raw); if (!pathResult.ok) { return { ok: false, error: pathResult.error }; } - const rootDir = path.resolve(params.rootDir); - const relativePath = path.relative(rootDir, pathResult.path); let opened: Awaited> | undefined; try { opened = await openFileWithinRoot({ rootDir, - relativePath, + relativePath: pathResult.relativePath, }); resolvedPaths.push(opened.realPath); } catch (err) { if (err instanceof SafeOpenError && err.code === "not-found") { // Preserve historical behavior for paths that do not exist yet. - resolvedPaths.push(pathResult.path); + resolvedPaths.push(pathResult.fallbackPath); continue; } return {