fix(browser): block upload symlink escapes (#21972)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 4381ef9a4d
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Reviewed-by: @mbelinky

CHANGELOG.md

@@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai
 - iOS/Security: force `https://` for non-loopback manual gateway hosts during iOS onboarding to block insecure remote transport URLs. (#21969) Thanks @mbelinky.
 - Shared/Security: reject insecure deep links that use `ws://` non-loopback gateway URLs to prevent plaintext remote websocket configuration. (#21970) Thanks @mbelinky.
 - macOS/Security: reject non-loopback `ws://` remote gateway URLs in macOS remote config to block insecure plaintext websocket endpoints. (#21971) Thanks @mbelinky.
+- Browser/Security: block upload path symlink escapes so browser upload sources cannot traverse outside the allowed workspace via symlinked paths. (#21972) Thanks @mbelinky.
 - CLI/Onboarding: fix Anthropic-compatible custom provider verification by normalizing base URLs to avoid duplicate `/v1` paths during setup checks. (#21336) Thanks @17jmumford.
 - Security/Dependencies: bump transitive `hono` usage to `4.11.10` to incorporate timing-safe authentication comparison hardening for `basicAuth`/`bearerAuth` (`GHSA-gq3j-xvxp-8hrf`). Thanks @vincentkoc.
 

src/agents/tools/browser-tool.ts

@@ -21,7 +21,7 @@ import {
 } from "../../browser/client.js";
 import { resolveBrowserConfig } from "../../browser/config.js";
 import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js";
-import { DEFAULT_UPLOAD_DIR, resolvePathsWithinRoot } from "../../browser/paths.js";
+import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "../../browser/paths.js";
 import { applyBrowserProxyPaths, persistBrowserProxyFiles } from "../../browser/proxy-files.js";
 import { loadConfig } from "../../config/config.js";
 import { wrapExternalContent } from "../../security/external-content.js";
@@ -700,7 +700,7 @@ export function createBrowserTool(opts?: {
           if (paths.length === 0) {
             throw new Error("paths required");
           }
-          const uploadPathsResult = resolvePathsWithinRoot({
+          const uploadPathsResult = await resolveExistingPathsWithinRoot({
             rootDir: DEFAULT_UPLOAD_DIR,
             requestedPaths: paths,
             scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,

src/browser/paths.test.ts

@@ -0,0 +1,105 @@
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import { resolveExistingPathsWithinRoot } from "./paths.js";
+
+async function createFixtureRoot(): Promise<{ baseDir: string; uploadsDir: string }> {
+  const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-browser-paths-"));
+  const uploadsDir = path.join(baseDir, "uploads");
+  await fs.mkdir(uploadsDir, { recursive: true });
+  return { baseDir, uploadsDir };
+}
+
+describe("resolveExistingPathsWithinRoot", () => {
+  const cleanupDirs = new Set<string>();
+
+  afterEach(async () => {
+    await Promise.all(
+      Array.from(cleanupDirs).map(async (dir) => {
+        await fs.rm(dir, { recursive: true, force: true });
+      }),
+    );
+    cleanupDirs.clear();
+  });
+
+  it("accepts existing files under the upload root", async () => {
+    const { baseDir, uploadsDir } = await createFixtureRoot();
+    cleanupDirs.add(baseDir);
+
+    const nestedDir = path.join(uploadsDir, "nested");
+    await fs.mkdir(nestedDir, { recursive: true });
+    const filePath = path.join(nestedDir, "ok.txt");
+    await fs.writeFile(filePath, "ok", "utf8");
+
+    const result = await resolveExistingPathsWithinRoot({
+      rootDir: uploadsDir,
+      requestedPaths: [filePath],
+      scopeLabel: "uploads directory",
+    });
+
+    expect(result.ok).toBe(true);
+    if (result.ok) {
+      expect(result.paths).toEqual([await fs.realpath(filePath)]);
+    }
+  });
+
+  it("rejects traversal outside the upload root", async () => {
+    const { baseDir, uploadsDir } = await createFixtureRoot();
+    cleanupDirs.add(baseDir);
+
+    const outsidePath = path.join(baseDir, "outside.txt");
+    await fs.writeFile(outsidePath, "nope", "utf8");
+
+    const result = await resolveExistingPathsWithinRoot({
+      rootDir: uploadsDir,
+      requestedPaths: ["../outside.txt"],
+      scopeLabel: "uploads directory",
+    });
+
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.error).toContain("must stay within uploads directory");
+    }
+  });
+
+  it("keeps lexical in-root paths when files do not exist yet", async () => {
+    const { baseDir, uploadsDir } = await createFixtureRoot();
+    cleanupDirs.add(baseDir);
+
+    const result = await resolveExistingPathsWithinRoot({
+      rootDir: uploadsDir,
+      requestedPaths: ["missing.txt"],
+      scopeLabel: "uploads directory",
+    });
+
+    expect(result.ok).toBe(true);
+    if (result.ok) {
+      expect(result.paths).toEqual([path.join(uploadsDir, "missing.txt")]);
+    }
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "rejects symlink escapes outside upload root",
+    async () => {
+      const { baseDir, uploadsDir } = await createFixtureRoot();
+      cleanupDirs.add(baseDir);
+
+      const outsidePath = path.join(baseDir, "secret.txt");
+      await fs.writeFile(outsidePath, "secret", "utf8");
+      const symlinkPath = path.join(uploadsDir, "leak.txt");
+      await fs.symlink(outsidePath, symlinkPath);
+
+      const result = await resolveExistingPathsWithinRoot({
+        rootDir: uploadsDir,
+        requestedPaths: ["leak.txt"],
+        scopeLabel: "uploads directory",
+      });
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error).toContain("regular non-symlink file");
+      }
+    },
+  );
+});

src/browser/paths.ts

@@ -1,4 +1,5 @@
 import path from "node:path";
+import { SafeOpenError, openFileWithinRoot } from "../infra/fs-safe.js";
 import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
 
 export const DEFAULT_BROWSER_TMP_DIR = resolvePreferredOpenClawTmpDir();
@@ -47,3 +48,45 @@ export function resolvePathsWithinRoot(params: {
   }
   return { ok: true, paths: resolvedPaths };
 }
+
+export async function resolveExistingPathsWithinRoot(params: {
+  rootDir: string;
+  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,
+      scopeLabel: params.scopeLabel,
+    });
+    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<ReturnType<typeof openFileWithinRoot>> | undefined;
+    try {
+      opened = await openFileWithinRoot({
+        rootDir,
+        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);
+        continue;
+      }
+      return {
+        ok: false,
+        error: `Invalid path: must stay within ${params.scopeLabel} and be a regular non-symlink file`,
+      };
+    } finally {
+      await opened?.handle.close().catch(() => {});
+    }
+  }
+  return { ok: true, paths: resolvedPaths };
+}

src/browser/routes/agent.act.ts

@@ -16,7 +16,7 @@ import {
   DEFAULT_DOWNLOAD_DIR,
   DEFAULT_UPLOAD_DIR,
   resolvePathWithinRoot,
-  resolvePathsWithinRoot,
+  resolveExistingPathsWithinRoot,
 } from "./path-output.js";
 import type { BrowserResponse, BrowserRouteRegistrar } from "./types.js";
 import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js";
@@ -382,7 +382,7 @@ export function registerBrowserAgentActRoutes(
       targetId,
       feature: "file chooser hook",
       run: async ({ cdpUrl, tab, pw }) => {
-        const uploadPathsResult = resolvePathsWithinRoot({
+        const uploadPathsResult = await resolveExistingPathsWithinRoot({
           rootDir: DEFAULT_UPLOAD_DIR,
           requestedPaths: paths,
           scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,

src/cli/browser-cli-actions-input/register.files-downloads.ts

@@ -1,13 +1,13 @@
 import type { Command } from "commander";
-import { DEFAULT_UPLOAD_DIR, resolvePathsWithinRoot } from "../../browser/paths.js";
+import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "../../browser/paths.js";
 import { danger } from "../../globals.js";
 import { defaultRuntime } from "../../runtime.js";
 import { shortenHomePath } from "../../utils.js";
 import { callBrowserRequest, type BrowserParentOpts } from "../browser-cli-shared.js";
 import { resolveBrowserActionContext } from "./shared.js";
 
-function normalizeUploadPaths(paths: string[]): string[] {
-  const result = resolvePathsWithinRoot({
+async function normalizeUploadPaths(paths: string[]): Promise<string[]> {
+  const result = await resolveExistingPathsWithinRoot({
     rootDir: DEFAULT_UPLOAD_DIR,
     requestedPaths: paths,
     scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,
@@ -81,7 +81,7 @@ export function registerBrowserFilesAndDownloadsCommands(
     .action(async (paths: string[], opts, cmd) => {
       const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts);
       try {
-        const normalizedPaths = normalizeUploadPaths(paths);
+        const normalizedPaths = await normalizeUploadPaths(paths);
         const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts);
         const result = await callBrowserRequest<{ download: { path: string } }>(
           parent,