Agents: fix Windows drive path join for read/sandbox tools (#54039) (#66193)

* Agents: fix Windows drive path join for read/sandbox tools (#54039)

* fix(agents): harden Windows file URL path mapping

* fix(agents): reject encoded file URL separators

* Update CHANGELOG.md

---------

Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

CHANGELOG.md

@@ -40,6 +40,7 @@ Docs: https://docs.openclaw.ai
 - Voice-call/media-stream: resolve the source IP from trusted forwarding headers for per-IP pending-connection limits when `webhookSecurity.trustForwardingHeaders` and `trustedProxyIPs` are configured, and reserve `maxConnections` capacity for in-flight WebSocket upgrades so concurrent handshakes can no longer momentarily exceed the operator-set cap. (#66027) Thanks @eleqtrizit.
 - Feishu/allowlist: canonicalize allowlist entries by explicit `user`/`chat` kind, strip repeated `feishu:`/`lark:` provider prefixes, and stop folding opaque Feishu IDs to lowercase, so allowlist matching no longer crosses user/chat namespaces or widens to case-insensitive ID matches the operator did not intend. (#66021) Thanks @eleqtrizit.
 - TTS/reply media: persist OpenClaw temp voice outputs into managed outbound media and allow them through reply-media normalization, so voice-note replies stop silently dropping. (#63511) Thanks @jetd1.
+- Agents/tools: treat Windows drive-letter paths (`C:\\...`) as absolute when resolving sandbox and read-tool paths so workspace root is not prepended under POSIX path rules. (#54039) Thanks @ly85206559 and @vincentkoc.
 - Agents/OpenAI: recover embedded GPT-style runs when reasoning-only or empty turns need bounded continuation, with replay-safe retry gating and incomplete-turn fallback when no visible answer arrives. (#66167) thanks @jalehman
 - Outbound/relay-status: suppress internal relay-status placeholder payloads (`No channel reply.`, `Replied in-thread.`, `Replied in #...`, wiki-update status variants ending in `No channel reply.`) before channel delivery so internal housekeeping text does not leak to users.
 - Slack/doctor: add a dedicated doctor-contract sidecar so config warmup paths such as `openclaw cron` no longer fall back to Slack's broader contract surface, which could trigger Slack-related config-read crashes on affected setups. (#63192) Thanks @shhtheonlyperson.

src/agents/pi-tools.read.ts

@@ -1,7 +1,9 @@
 import fs from "node:fs/promises";
 import path from "node:path";
+import { URL } from "node:url";
 import type { AgentToolResult } from "@mariozechner/pi-agent-core";
 import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent";
+import { isWindowsDrivePath } from "../infra/archive-path.js";
 import {
   appendFileWithinRoot,
   SafeOpenError,
@@ -9,7 +11,7 @@ import {
   readFileWithinRoot,
   writeFileWithinRoot,
 } from "../infra/fs-safe.js";
-import { trySafeFileURLToPath } from "../infra/local-file-access.js";
+import { hasEncodedFileUrlSeparator, trySafeFileURLToPath } from "../infra/local-file-access.js";
 import { detectMime } from "../media/mime.js";
 import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js";
 import type { ImageSanitizationLimits } from "./image-sanitization.js";
@@ -373,10 +375,41 @@ function mapContainerPathToWorkspaceRoot(params: {
   let candidate = params.filePath.startsWith("@") ? params.filePath.slice(1) : params.filePath;
   if (/^file:\/\//i.test(candidate)) {
     const localFilePath = trySafeFileURLToPath(candidate);
-    if (!localFilePath) {
-      return params.filePath;
+    if (localFilePath) {
+      candidate = localFilePath;
+    } else {
+      // Windows rejects posix-style file:///workspace/... in fileURLToPath; map via URL pathname
+      // when it clearly refers to the container workdir (same idea as sandbox-paths).
+      let parsed: URL;
+      try {
+        parsed = new URL(candidate);
+      } catch {
+        return params.filePath;
+      }
+      if (parsed.protocol !== "file:") {
+        return params.filePath;
+      }
+      const host = parsed.hostname.trim().toLowerCase();
+      if (host && host !== "localhost") {
+        return params.filePath;
+      }
+      if (hasEncodedFileUrlSeparator(parsed.pathname)) {
+        return params.filePath;
+      }
+      let normalizedPathname: string;
+      try {
+        normalizedPathname = decodeURIComponent(parsed.pathname).replace(/\\/g, "/");
+      } catch {
+        return params.filePath;
+      }
+      if (
+        normalizedPathname !== normalizedWorkdir &&
+        !normalizedPathname.startsWith(`${normalizedWorkdir}/`)
+      ) {
+        return params.filePath;
+      }
+      candidate = normalizedPathname;
     }
-    candidate = localFilePath;
   }
 
   const normalizedCandidate = candidate.replace(/\\/g, "/");
@@ -401,9 +434,13 @@ export function resolveToolPathAgainstWorkspaceRoot(params: {
 }): string {
   const mapped = mapContainerPathToWorkspaceRoot(params);
   const candidate = mapped.startsWith("@") ? mapped.slice(1) : mapped;
-  return path.isAbsolute(candidate)
-    ? path.resolve(candidate)
-    : path.resolve(params.root, candidate || ".");
+  if (isWindowsDrivePath(candidate)) {
+    return path.win32.normalize(candidate);
+  }
+  if (path.isAbsolute(candidate)) {
+    return path.resolve(candidate);
+  }
+  return path.resolve(params.root, candidate || ".");
 }
 
 type MemoryFlushAppendOnlyWriteOptions = {

src/agents/pi-tools.read.workspace-root-guard.test.ts

@@ -96,6 +96,38 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => {
     });
   });
 
+  it("does not remap malformed file:// container workspace paths", async () => {
+    const { tool } = createToolHarness();
+    const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
+      containerWorkdir: "/workspace",
+    });
+
+    await wrapped.execute("tc-malformed-file-url", { path: "file:///workspace/%E0%A4%A" });
+
+    expect(mocks.assertSandboxPath).toHaveBeenCalledWith({
+      filePath: "file:///workspace/%E0%A4%A",
+      cwd: root,
+      root,
+    });
+  });
+
+  it("does not remap file:// container workspace paths with encoded separators", async () => {
+    const { tool } = createToolHarness();
+    const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
+      containerWorkdir: "/workspace",
+    });
+
+    await wrapped.execute("tc-encoded-separator-file-url", {
+      path: "file:///workspace/%2FREADME.md",
+    });
+
+    expect(mocks.assertSandboxPath).toHaveBeenCalledWith({
+      filePath: "file:///workspace/%2FREADME.md",
+      cwd: root,
+      root,
+    });
+  });
+
   it("maps @-prefixed container workspace paths to host workspace root", async () => {
     const { tool } = createToolHarness();
     const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {

src/agents/sandbox-paths.test.ts

@@ -167,11 +167,26 @@ describe("resolveSandboxedMediaSource", () => {
       media: "file://attacker/share/photo.png",
       expected: /remote hosts are not allowed/i,
     },
+    {
+      name: "file:// container URLs with remote hosts",
+      media: "file://attacker/workspace/photo.png",
+      expected: /remote hosts are not allowed/i,
+    },
     {
       name: "invalid file:// URLs",
       media: "file://not a valid url\x00",
       expected: /Invalid file:\/\/ URL/,
     },
+    {
+      name: "file:// URLs with malformed container pathname encoding",
+      media: "file:///workspace/%E0%A4%A",
+      expected: /Invalid file:\/\/ URL/,
+    },
+    {
+      name: "file:// URLs with encoded separators in the pathname",
+      media: "file:///workspace/%2FREADME.md",
+      expected: /cannot encode path separators/i,
+    },
   ])("rejects $name", async ({ media, expected }) => {
     await withSandboxRoot(async (sandboxDir) => {
       await expectSandboxRejection(media, sandboxDir, expected);

src/agents/sandbox-paths.ts

@@ -1,7 +1,12 @@
 import os from "node:os";
 import path from "node:path";
 import { URL } from "node:url";
-import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../infra/local-file-access.js";
+import { isWindowsDrivePath } from "../infra/archive-path.js";
+import {
+  assertNoWindowsNetworkPath,
+  hasEncodedFileUrlSeparator,
+  safeFileURLToPath,
+} from "../infra/local-file-access.js";
 import { assertNoPathAliasEscape, type PathAliasPolicy } from "../infra/path-alias-guards.js";
 import { isPathInside } from "../infra/path-guards.js";
 import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
@@ -30,8 +35,17 @@ function expandPath(filePath: string): string {
   return normalized;
 }
 
+/** True when the path is absolute for the current platform or a Windows drive path (e.g. C:\\...), even if path.isAbsolute is false under POSIX rules. */
+function hostPathLooksAbsolute(expanded: string): boolean {
+  return path.isAbsolute(expanded) || isWindowsDrivePath(expanded);
+}
+
 function resolveToCwd(filePath: string, cwd: string): string {
   const expanded = expandPath(filePath);
+  // Drive-letter paths first: on Unix path.isAbsolute is false for C:/...; on Windows we still normalize.
+  if (isWindowsDrivePath(expanded)) {
+    return path.win32.normalize(expanded);
+  }
   if (path.isAbsolute(expanded)) {
     return expanded;
   }
@@ -52,7 +66,7 @@ export function resolveSandboxPath(params: { filePath: string; cwd: string; root
   if (!relative || relative === "") {
     return { resolved, relative: "" };
   }
-  if (relative.startsWith("..") || path.isAbsolute(relative)) {
+  if (relative.startsWith("..") || path.isAbsolute(relative) || isWindowsDrivePath(relative)) {
     throw new Error(`Path escapes sandbox root (${shortPath(rootResolved)}): ${params.filePath}`);
   }
   return { resolved, relative };
@@ -151,9 +165,21 @@ function mapContainerWorkspaceFileUrl(params: {
   if (parsed.protocol !== "file:") {
     return undefined;
   }
+  const host = parsed.hostname.trim().toLowerCase();
+  if (host && host !== "localhost") {
+    return undefined;
+  }
+  if (hasEncodedFileUrlSeparator(parsed.pathname)) {
+    return undefined;
+  }
   // Sandbox paths are Linux-style (/workspace/*). Parse the URL path directly so
   // Windows hosts can still accept file:///workspace/... media references.
-  const normalizedPathname = decodeURIComponent(parsed.pathname).replace(/\\/g, "/");
+  let normalizedPathname: string;
+  try {
+    normalizedPathname = decodeURIComponent(parsed.pathname).replace(/\\/g, "/");
+  } catch {
+    return undefined;
+  }
   if (
     normalizedPathname !== SANDBOX_CONTAINER_WORKDIR &&
     !normalizedPathname.startsWith(`${SANDBOX_CONTAINER_WORKDIR}/`)
@@ -189,7 +215,7 @@ async function resolveAllowedTmpMediaPath(params: {
   candidate: string;
   sandboxRoot: string;
 }): Promise<string | undefined> {
-  const candidateIsAbsolute = path.isAbsolute(expandPath(params.candidate));
+  const candidateIsAbsolute = hostPathLooksAbsolute(expandPath(params.candidate));
   if (!candidateIsAbsolute) {
     return undefined;
   }

src/agents/sandbox-paths.windows-drive-resolve.test.ts

@@ -0,0 +1,33 @@
+import path from "node:path";
+import { describe, expect, it } from "vitest";
+import { resolveSandboxInputPath } from "./sandbox-paths.js";
+import { resolveToolPathAgainstWorkspaceRoot } from "./pi-tools.read.js";
+
+describe("resolveSandboxInputPath (Windows drive paths under POSIX rules)", () => {
+  it("does not join workspace cwd when path looks like a Windows drive path", () => {
+    const cwd = path.resolve("/workspace/project");
+    const resolved = resolveSandboxInputPath("C:/Users/test/file.txt", cwd);
+    expect(resolved).toBe(path.win32.normalize("C:/Users/test/file.txt"));
+    expect(resolved).not.toContain("workspace");
+  });
+
+  it("treats backslash Windows drive paths as absolute vs cwd", () => {
+    const cwd = path.resolve("/app/sandbox");
+    const resolved = resolveSandboxInputPath("D:\\data\\out.log", cwd);
+    expect(resolved).toBe(path.win32.normalize("D:\\data\\out.log"));
+    expect(resolved).not.toContain("sandbox");
+  });
+});
+
+describe("resolveToolPathAgainstWorkspaceRoot (Windows drive paths)", () => {
+  const root = path.resolve("/host/workspace");
+
+  it("does not prefix workspace root for drive-letter paths", () => {
+    const resolved = resolveToolPathAgainstWorkspaceRoot({
+      filePath: "C:/temp/agent-output.txt",
+      root,
+    });
+    expect(resolved).toBe(path.win32.normalize("C:/temp/agent-output.txt"));
+    expect(resolved).not.toContain("host");
+  });
+});

src/infra/local-file-access.ts

@@ -2,11 +2,17 @@ import path from "node:path";
 import { fileURLToPath, URL } from "node:url";
 import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
 
+const ENCODED_FILE_URL_SEPARATOR_RE = /%(?:2f|5c)/i;
+
 function isLocalFileUrlHost(hostname: string): boolean {
   const normalized = normalizeLowercaseStringOrEmpty(hostname);
   return normalized === "" || normalized === "localhost";
 }
 
+export function hasEncodedFileUrlSeparator(pathname: string): boolean {
+  return ENCODED_FILE_URL_SEPARATOR_RE.test(pathname);
+}
+
 export function isWindowsNetworkPath(filePath: string): boolean {
   if (process.platform !== "win32") {
     return false;
@@ -34,6 +40,9 @@ export function safeFileURLToPath(fileUrl: string): string {
   if (!isLocalFileUrlHost(parsed.hostname)) {
     throw new Error(`file:// URLs with remote hosts are not allowed: ${fileUrl}`);
   }
+  if (hasEncodedFileUrlSeparator(parsed.pathname)) {
+    throw new Error(`file:// URLs cannot encode path separators: ${fileUrl}`);
+  }
   const filePath = fileURLToPath(parsed);
   assertNoWindowsNetworkPath(filePath, "Local file URL");
   return filePath;