fix(browser): revalidate upload paths at use time

CHANGELOG.md

@@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai
 - Security/Nextcloud Talk: stop treating DM pairing-store entries as group allowlist senders, so group authorization remains bounded to configured group allowlists. (#26116) Thanks @bmendonca3.
 - Security/IRC: keep pairing-store approvals DM-only and out of IRC group allowlist authorization, with policy regression tests for allowlist resolution. (#26112) Thanks @bmendonca3.
 - Security/Microsoft Teams: isolate group allowlist and command authorization from DM pairing-store entries to prevent cross-context authorization bleed. (#26111) Thanks @bmendonca3.
+- Security/Browser uploads: revalidate upload paths at use-time in Playwright file-chooser and direct-input flows so missing/rebound paths are rejected before `setFiles`, with regression coverage for strict missing-path handling.
 - Security/LINE: cap unsigned webhook body reads before auth/signature handling to bound unauthenticated body processing. (#26095) Thanks @bmendonca3.
 - Agents/Model fallback: keep explicit text + image fallback chains reachable even when `agents.defaults.models` allowlists are present, prefer explicit run `agentId` over session-key parsing for followup fallback override resolution (with session-key fallback), treat agent-level fallback overrides as configured in embedded runner preflight, and classify `model_cooldown` / `cooling down` errors as `rate_limit` so failover continues. (#11972, #24137, #17231)
 - Followups/Routing: when explicit origin routing fails, allow same-channel fallback dispatch (while still blocking cross-channel fallback) so followup replies do not get dropped on transient origin-adapter failures. (#26109) Thanks @Sid-Qin.

src/browser/paths.test.ts

@@ -6,6 +6,7 @@ import {
   resolveExistingPathsWithinRoot,
   resolvePathsWithinRoot,
   resolvePathWithinRoot,
+  resolveStrictExistingPathsWithinRoot,
 } from "./paths.js";
 
 async function createFixtureRoot(): Promise<{ baseDir: string; uploadsDir: string }> {
@@ -194,6 +195,29 @@ describe("resolveExistingPathsWithinRoot", () => {
   );
 });
 
+describe("resolveStrictExistingPathsWithinRoot", () => {
+  function expectInvalidResult(
+    result: Awaited<ReturnType<typeof resolveStrictExistingPathsWithinRoot>>,
+    expectedSnippet: string,
+  ) {
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.error).toContain(expectedSnippet);
+    }
+  }
+
+  it("rejects missing files instead of returning lexical fallbacks", async () => {
+    await withFixtureRoot(async ({ uploadsDir }) => {
+      const result = await resolveStrictExistingPathsWithinRoot({
+        rootDir: uploadsDir,
+        requestedPaths: ["missing.txt"],
+        scopeLabel: "uploads directory",
+      });
+      expectInvalidResult(result, "regular non-symlink file");
+    });
+  });
+});
+
 describe("resolvePathWithinRoot", () => {
   it("uses default file name when requested path is blank", () => {
     const result = resolvePathWithinRoot({

src/browser/paths.ts

@@ -54,6 +54,29 @@ export async function resolveExistingPathsWithinRoot(params: {
   rootDir: string;
   requestedPaths: string[];
   scopeLabel: string;
+}): Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> {
+  return await resolveCheckedPathsWithinRoot({
+    ...params,
+    allowMissingFallback: true,
+  });
+}
+
+export async function resolveStrictExistingPathsWithinRoot(params: {
+  rootDir: string;
+  requestedPaths: string[];
+  scopeLabel: string;
+}): Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> {
+  return await resolveCheckedPathsWithinRoot({
+    ...params,
+    allowMissingFallback: false,
+  });
+}
+
+async function resolveCheckedPathsWithinRoot(params: {
+  rootDir: string;
+  requestedPaths: string[];
+  scopeLabel: string;
+  allowMissingFallback: boolean;
 }): Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> {
   const rootDir = path.resolve(params.rootDir);
   let rootRealPath: string | undefined;
@@ -119,7 +142,7 @@ export async function resolveExistingPathsWithinRoot(params: {
       });
       resolvedPaths.push(opened.realPath);
     } catch (err) {
-      if (err instanceof SafeOpenError && err.code === "not-found") {
+      if (params.allowMissingFallback && err instanceof SafeOpenError && err.code === "not-found") {
         // Preserve historical behavior for paths that do not exist yet.
         resolvedPaths.push(pathResult.fallbackPath);
         continue;

src/browser/pw-tools-core.downloads.ts

@@ -3,6 +3,7 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import type { Page } from "playwright-core";
 import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
+import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js";
 import {
   ensurePageState,
   getPageForTargetId,
@@ -166,7 +167,20 @@ export async function armFileUploadViaPlaywright(opts: {
         }
         return;
       }
-      await fileChooser.setFiles(opts.paths);
+      const uploadPathsResult = await resolveStrictExistingPathsWithinRoot({
+        rootDir: DEFAULT_UPLOAD_DIR,
+        requestedPaths: opts.paths,
+        scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,
+      });
+      if (!uploadPathsResult.ok) {
+        try {
+          await page.keyboard.press("Escape");
+        } catch {
+          // Best-effort.
+        }
+        return;
+      }
+      await fileChooser.setFiles(uploadPathsResult.paths);
       try {
         const input =
           typeof fileChooser.element === "function"

src/browser/pw-tools-core.interactions.set-input-files.test.ts

@@ -0,0 +1,111 @@
+import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
+
+let page: Record<string, unknown> | null = null;
+let locator: Record<string, unknown> | null = null;
+
+const getPageForTargetId = vi.fn(async () => {
+  if (!page) {
+    throw new Error("test: page not set");
+  }
+  return page;
+});
+const ensurePageState = vi.fn(() => ({}));
+const restoreRoleRefsForTarget = vi.fn(() => {});
+const refLocator = vi.fn(() => {
+  if (!locator) {
+    throw new Error("test: locator not set");
+  }
+  return locator;
+});
+const forceDisconnectPlaywrightForTarget = vi.fn(async () => {});
+
+const resolveStrictExistingPathsWithinRoot =
+  vi.fn<typeof import("./paths.js").resolveStrictExistingPathsWithinRoot>();
+
+vi.mock("./pw-session.js", () => {
+  return {
+    ensurePageState,
+    forceDisconnectPlaywrightForTarget,
+    getPageForTargetId,
+    refLocator,
+    restoreRoleRefsForTarget,
+  };
+});
+
+vi.mock("./paths.js", () => {
+  return {
+    DEFAULT_UPLOAD_DIR: "/tmp/openclaw/uploads",
+    resolveStrictExistingPathsWithinRoot,
+  };
+});
+
+let setInputFilesViaPlaywright: typeof import("./pw-tools-core.interactions.js").setInputFilesViaPlaywright;
+
+describe("setInputFilesViaPlaywright", () => {
+  beforeAll(async () => {
+    ({ setInputFilesViaPlaywright } = await import("./pw-tools-core.interactions.js"));
+  });
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    page = null;
+    locator = null;
+    resolveStrictExistingPathsWithinRoot.mockResolvedValue({
+      ok: true,
+      paths: ["/private/tmp/openclaw/uploads/ok.txt"],
+    });
+  });
+
+  it("revalidates upload paths and uses resolved canonical paths for inputRef", async () => {
+    const setInputFiles = vi.fn(async () => {});
+    locator = {
+      setInputFiles,
+      elementHandle: vi.fn(async () => null),
+    };
+    page = {
+      locator: vi.fn(() => ({ first: () => locator })),
+    };
+
+    await setInputFilesViaPlaywright({
+      cdpUrl: "http://127.0.0.1:18792",
+      targetId: "T1",
+      inputRef: "e7",
+      paths: ["/tmp/openclaw/uploads/ok.txt"],
+    });
+
+    expect(resolveStrictExistingPathsWithinRoot).toHaveBeenCalledWith({
+      rootDir: "/tmp/openclaw/uploads",
+      requestedPaths: ["/tmp/openclaw/uploads/ok.txt"],
+      scopeLabel: "uploads directory (/tmp/openclaw/uploads)",
+    });
+    expect(refLocator).toHaveBeenCalledWith(page, "e7");
+    expect(setInputFiles).toHaveBeenCalledWith(["/private/tmp/openclaw/uploads/ok.txt"]);
+  });
+
+  it("throws and skips setInputFiles when use-time validation fails", async () => {
+    resolveStrictExistingPathsWithinRoot.mockResolvedValueOnce({
+      ok: false,
+      error: "Invalid path: must stay within uploads directory",
+    });
+
+    const setInputFiles = vi.fn(async () => {});
+    locator = {
+      setInputFiles,
+      elementHandle: vi.fn(async () => null),
+    };
+    page = {
+      locator: vi.fn(() => ({ first: () => locator })),
+    };
+
+    await expect(
+      setInputFilesViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        targetId: "T1",
+        element: "input[type=file]",
+        paths: ["/tmp/openclaw/uploads/missing.txt"],
+      }),
+    ).rejects.toThrow("Invalid path: must stay within uploads directory");
+
+    expect(setInputFiles).not.toHaveBeenCalled();
+  });
+});

src/browser/pw-tools-core.interactions.ts

@@ -1,4 +1,5 @@
 import type { BrowserFormField } from "./client-actions-core.js";
+import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js";
 import {
   ensurePageState,
   forceDisconnectPlaywrightForTarget,
@@ -626,9 +627,18 @@ export async function setInputFilesViaPlaywright(opts: {
   }
 
   const locator = inputRef ? refLocator(page, inputRef) : page.locator(element).first();
+  const uploadPathsResult = await resolveStrictExistingPathsWithinRoot({
+    rootDir: DEFAULT_UPLOAD_DIR,
+    requestedPaths: opts.paths,
+    scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,
+  });
+  if (!uploadPathsResult.ok) {
+    throw new Error(uploadPathsResult.error);
+  }
+  const resolvedPaths = uploadPathsResult.paths;
 
   try {
-    await locator.setInputFiles(opts.paths);
+    await locator.setInputFiles(resolvedPaths);
   } catch (err) {
     throw toAIFriendlyError(err, inputRef || element);
   }

src/browser/pw-tools-core.last-file-chooser-arm-wins.test.ts

@@ -1,4 +1,8 @@
+import crypto from "node:crypto";
+import fs from "node:fs/promises";
+import path from "node:path";
 import { describe, expect, it, vi } from "vitest";
+import { DEFAULT_UPLOAD_DIR } from "./paths.js";
 import {
   installPwToolsCoreTestHooks,
   setPwToolsCoreCurrentPage,
@@ -9,6 +13,15 @@ const mod = await import("./pw-tools-core.js");
 
 describe("pw-tools-core", () => {
   it("last file-chooser arm wins", async () => {
+    const firstPath = path.join(DEFAULT_UPLOAD_DIR, `vitest-arm-1-${crypto.randomUUID()}.txt`);
+    const secondPath = path.join(DEFAULT_UPLOAD_DIR, `vitest-arm-2-${crypto.randomUUID()}.txt`);
+    await fs.mkdir(DEFAULT_UPLOAD_DIR, { recursive: true });
+    await Promise.all([
+      fs.writeFile(firstPath, "1", "utf8"),
+      fs.writeFile(secondPath, "2", "utf8"),
+    ]);
+    const secondCanonicalPath = await fs.realpath(secondPath);
+
     let resolve1: ((value: unknown) => void) | null = null;
     let resolve2: ((value: unknown) => void) | null = null;
 
@@ -35,24 +48,30 @@ describe("pw-tools-core", () => {
       keyboard: { press: vi.fn(async () => {}) },
     });
 
-    await mod.armFileUploadViaPlaywright({
-      cdpUrl: "http://127.0.0.1:18792",
-      paths: ["/tmp/1"],
-    });
-    await mod.armFileUploadViaPlaywright({
-      cdpUrl: "http://127.0.0.1:18792",
-      paths: ["/tmp/2"],
-    });
+    try {
+      await mod.armFileUploadViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        paths: [firstPath],
+      });
+      await mod.armFileUploadViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        paths: [secondPath],
+      });
 
-    if (!resolve1 || !resolve2) {
-      throw new Error("file chooser handlers were not registered");
+      if (!resolve1 || !resolve2) {
+        throw new Error("file chooser handlers were not registered");
+      }
+      (resolve1 as (value: unknown) => void)(fc1);
+      (resolve2 as (value: unknown) => void)(fc2);
+      await Promise.resolve();
+
+      expect(fc1.setFiles).not.toHaveBeenCalled();
+      await vi.waitFor(() => {
+        expect(fc2.setFiles).toHaveBeenCalledWith([secondCanonicalPath]);
+      });
+    } finally {
+      await Promise.all([fs.rm(firstPath, { force: true }), fs.rm(secondPath, { force: true })]);
     }
-    (resolve1 as (value: unknown) => void)(fc1);
-    (resolve2 as (value: unknown) => void)(fc2);
-    await Promise.resolve();
-
-    expect(fc1.setFiles).not.toHaveBeenCalled();
-    expect(fc2.setFiles).toHaveBeenCalledWith(["/tmp/2"]);
   });
   it("arms the next dialog and accepts/dismisses (default timeout)", async () => {
     const accept = vi.fn(async () => {});

src/browser/pw-tools-core.screenshots-element-selector.test.ts

@@ -1,4 +1,8 @@
+import crypto from "node:crypto";
+import fs from "node:fs/promises";
+import path from "node:path";
 import { describe, expect, it, vi } from "vitest";
+import { DEFAULT_UPLOAD_DIR } from "./paths.js";
 import {
   getPwToolsCoreSessionMocks,
   installPwToolsCoreTestHooks,
@@ -81,6 +85,10 @@ describe("pw-tools-core", () => {
     ).rejects.toThrow(/fullPage is not supported/i);
   });
   it("arms the next file chooser and sets files (default timeout)", async () => {
+    const uploadPath = path.join(DEFAULT_UPLOAD_DIR, `vitest-upload-${crypto.randomUUID()}.txt`);
+    await fs.mkdir(path.dirname(uploadPath), { recursive: true });
+    await fs.writeFile(uploadPath, "fixture", "utf8");
+    const canonicalUploadPath = await fs.realpath(uploadPath);
     const fileChooser = { setFiles: vi.fn(async () => {}) };
     const waitForEvent = vi.fn(async (_event: string, _opts: unknown) => fileChooser);
     setPwToolsCoreCurrentPage({
@@ -88,19 +96,47 @@ describe("pw-tools-core", () => {
       keyboard: { press: vi.fn(async () => {}) },
     });
 
+    try {
+      await mod.armFileUploadViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        targetId: "T1",
+        paths: [uploadPath],
+      });
+
+      // waitForEvent is awaited immediately; handler continues async.
+      await Promise.resolve();
+
+      expect(waitForEvent).toHaveBeenCalledWith("filechooser", {
+        timeout: 120_000,
+      });
+      await vi.waitFor(() => {
+        expect(fileChooser.setFiles).toHaveBeenCalledWith([canonicalUploadPath]);
+      });
+    } finally {
+      await fs.rm(uploadPath, { force: true });
+    }
+  });
+  it("revalidates file-chooser paths at use-time and cancels missing files", async () => {
+    const missingPath = path.join(DEFAULT_UPLOAD_DIR, `vitest-missing-${crypto.randomUUID()}.txt`);
+    const fileChooser = { setFiles: vi.fn(async () => {}) };
+    const press = vi.fn(async () => {});
+    const waitForEvent = vi.fn(async () => fileChooser);
+    setPwToolsCoreCurrentPage({
+      waitForEvent,
+      keyboard: { press },
+    });
+
     await mod.armFileUploadViaPlaywright({
       cdpUrl: "http://127.0.0.1:18792",
       targetId: "T1",
-      paths: ["/tmp/a.txt"],
+      paths: [missingPath],
     });
-
-    // waitForEvent is awaited immediately; handler continues async.
     await Promise.resolve();
 
-    expect(waitForEvent).toHaveBeenCalledWith("filechooser", {
-      timeout: 120_000,
+    await vi.waitFor(() => {
+      expect(press).toHaveBeenCalledWith("Escape");
     });
-    expect(fileChooser.setFiles).toHaveBeenCalledWith(["/tmp/a.txt"]);
+    expect(fileChooser.setFiles).not.toHaveBeenCalled();
   });
   it("arms the next file chooser and escapes if no paths provided", async () => {
     const fileChooser = { setFiles: vi.fn(async () => {}) };