fix(gateway): block agents.files symlink escapes

2026-03-07 22:44:16 +00:00 · 2026-02-26 00:30:43 +01:00
parent 45d59971e6
commit 125f4071bc
3 changed files with 421 additions and 21 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai

 - Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
 - Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
+- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
 - Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
 - 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.
--- a/src/gateway/server-methods/agents-mutate.test.ts
+++ b/src/gateway/server-methods/agents-mutate.test.ts
@@ -26,7 +26,10 @@ const mocks = vi.hoisted(() => ({
  fsMkdir: vi.fn(async () => undefined),
  fsAppendFile: vi.fn(async () => {}),
  fsReadFile: vi.fn(async () => ""),
-  fsStat: vi.fn(async () => null),
+  fsStat: vi.fn(async (..._args: unknown[]) => null as import("node:fs").Stats | null),
+  fsLstat: vi.fn(async (..._args: unknown[]) => null as import("node:fs").Stats | null),
+  fsRealpath: vi.fn(async (p: string) => p),
+  fsOpen: vi.fn(async () => ({}) as unknown),
 }));

 vi.mock("../../config/config.js", () => ({
@@ -85,6 +88,9 @@ vi.mock("node:fs/promises", async () => {
    appendFile: mocks.fsAppendFile,
    readFile: mocks.fsReadFile,
    stat: mocks.fsStat,
+    lstat: mocks.fsLstat,
+    realpath: mocks.fsRealpath,
+    open: mocks.fsOpen,
  };
  return { ...patched, default: patched };
 });
@@ -125,6 +131,33 @@ function createErrnoError(code: string) {
  return err;
 }

+function makeFileStat(params?: {
+  size?: number;
+  mtimeMs?: number;
+  dev?: number;
+  ino?: number;
+}): import("node:fs").Stats {
+  return {
+    isFile: () => true,
+    isSymbolicLink: () => false,
+    size: params?.size ?? 10,
+    mtimeMs: params?.mtimeMs ?? 1234,
+    dev: params?.dev ?? 1,
+    ino: params?.ino ?? 1,
+  } as unknown as import("node:fs").Stats;
+}
+
+function makeSymlinkStat(params?: { dev?: number; ino?: number }): import("node:fs").Stats {
+  return {
+    isFile: () => false,
+    isSymbolicLink: () => true,
+    size: 0,
+    mtimeMs: 0,
+    dev: params?.dev ?? 1,
+    ino: params?.ino ?? 2,
+  } as unknown as import("node:fs").Stats;
+}
+
 function mockWorkspaceStateRead(params: {
  onboardingCompletedAt?: string;
  errorCode?: string;
@@ -172,6 +205,19 @@ beforeEach(() => {
  mocks.fsStat.mockImplementation(async () => {
    throw createEnoentError();
  });
+  mocks.fsLstat.mockImplementation(async () => {
+    throw createEnoentError();
+  });
+  mocks.fsRealpath.mockImplementation(async (p: string) => p);
+  mocks.fsOpen.mockImplementation(
+    async () =>
+      ({
+        stat: async () => makeFileStat(),
+        readFile: async () => Buffer.from(""),
+        writeFile: async () => {},
+        close: async () => {},
+      }) as unknown,
+  );
 });

 /* ------------------------------------------------------------------ */
@@ -459,3 +505,147 @@ describe("agents.files.list", () => {
    expect(names).toContain("BOOTSTRAP.md");
  });
 });
+
+describe("agents.files.get/set symlink safety", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.loadConfigReturn = {};
+    mocks.fsMkdir.mockResolvedValue(undefined);
+  });
+
+  it("rejects agents.files.get when allowlisted file symlink escapes workspace", async () => {
+    const workspace = "/workspace/test-agent";
+    const candidate = `${workspace}/AGENTS.md`;
+    mocks.fsRealpath.mockImplementation(async (p: string) => {
+      if (p === workspace) {
+        return workspace;
+      }
+      if (p === candidate) {
+        return "/outside/secret.txt";
+      }
+      return p;
+    });
+    mocks.fsLstat.mockImplementation(async (...args: unknown[]) => {
+      const p = typeof args[0] === "string" ? args[0] : "";
+      if (p === candidate) {
+        return makeSymlinkStat();
+      }
+      throw createEnoentError();
+    });
+
+    const { respond, promise } = makeCall("agents.files.get", {
+      agentId: "main",
+      name: "AGENTS.md",
+    });
+    await promise;
+
+    expect(respond).toHaveBeenCalledWith(
+      false,
+      undefined,
+      expect.objectContaining({ message: expect.stringContaining("unsafe workspace file") }),
+    );
+  });
+
+  it("rejects agents.files.set when allowlisted file symlink escapes workspace", async () => {
+    const workspace = "/workspace/test-agent";
+    const candidate = `${workspace}/AGENTS.md`;
+    mocks.fsRealpath.mockImplementation(async (p: string) => {
+      if (p === workspace) {
+        return workspace;
+      }
+      if (p === candidate) {
+        return "/outside/secret.txt";
+      }
+      return p;
+    });
+    mocks.fsLstat.mockImplementation(async (...args: unknown[]) => {
+      const p = typeof args[0] === "string" ? args[0] : "";
+      if (p === candidate) {
+        return makeSymlinkStat();
+      }
+      throw createEnoentError();
+    });
+
+    const { respond, promise } = makeCall("agents.files.set", {
+      agentId: "main",
+      name: "AGENTS.md",
+      content: "x",
+    });
+    await promise;
+
+    expect(respond).toHaveBeenCalledWith(
+      false,
+      undefined,
+      expect.objectContaining({ message: expect.stringContaining("unsafe workspace file") }),
+    );
+    expect(mocks.fsOpen).not.toHaveBeenCalled();
+  });
+
+  it("allows in-workspace symlink targets for get/set", async () => {
+    const workspace = "/workspace/test-agent";
+    const candidate = `${workspace}/AGENTS.md`;
+    const target = `${workspace}/policies/AGENTS.md`;
+    const targetStat = makeFileStat({ size: 7, mtimeMs: 1700, dev: 9, ino: 42 });
+
+    mocks.fsRealpath.mockImplementation(async (p: string) => {
+      if (p === workspace) {
+        return workspace;
+      }
+      if (p === candidate) {
+        return target;
+      }
+      return p;
+    });
+    mocks.fsLstat.mockImplementation(async (...args: unknown[]) => {
+      const p = typeof args[0] === "string" ? args[0] : "";
+      if (p === candidate) {
+        return makeSymlinkStat({ dev: 9, ino: 41 });
+      }
+      if (p === target) {
+        return targetStat;
+      }
+      throw createEnoentError();
+    });
+    mocks.fsStat.mockImplementation(async (...args: unknown[]) => {
+      const p = typeof args[0] === "string" ? args[0] : "";
+      if (p === target) {
+        return targetStat;
+      }
+      throw createEnoentError();
+    });
+    mocks.fsOpen.mockImplementation(
+      async () =>
+        ({
+          stat: async () => targetStat,
+          readFile: async () => Buffer.from("inside\n"),
+          writeFile: async () => {},
+          close: async () => {},
+        }) as unknown,
+    );
+
+    const getCall = makeCall("agents.files.get", { agentId: "main", name: "AGENTS.md" });
+    await getCall.promise;
+    expect(getCall.respond).toHaveBeenCalledWith(
+      true,
+      expect.objectContaining({
+        file: expect.objectContaining({ missing: false, content: "inside\n" }),
+      }),
+      undefined,
+    );
+
+    const setCall = makeCall("agents.files.set", {
+      agentId: "main",
+      name: "AGENTS.md",
+      content: "updated\n",
+    });
+    await setCall.promise;
+    expect(setCall.respond).toHaveBeenCalledWith(
+      true,
+      expect.objectContaining({
+        ok: true,
+        file: expect.objectContaining({ missing: false, content: "updated\n" }),
+      }),
+      undefined,
+    );
+  });
+});
--- a/src/gateway/server-methods/agents.ts
+++ b/src/gateway/server-methods/agents.ts
@@ -1,3 +1,4 @@
+import { constants as fsConstants } from "node:fs";
 import fs from "node:fs/promises";
 import path from "node:path";
 import {
@@ -27,6 +28,9 @@ import {
 } from "../../commands/agents.config.js";
 import { loadConfig, writeConfigFile } from "../../config/config.js";
 import { resolveSessionTranscriptsDirForAgent } from "../../config/sessions/paths.js";
+import { sameFileIdentity } from "../../infra/file-identity.js";
+import { SafeOpenError, readLocalFileSafely } from "../../infra/fs-safe.js";
+import { isNotFoundPathError, isPathInside } from "../../infra/path-guards.js";
 import { DEFAULT_AGENT_ID, normalizeAgentId } from "../../routing/session-key.js";
 import { resolveUserPath } from "../../utils.js";
 import {
@@ -97,10 +101,113 @@ type FileMeta = {
  updatedAtMs: number;
 };

-async function statFile(filePath: string): Promise<FileMeta | null> {
+type ResolvedAgentWorkspaceFilePath =
+  | {
+      kind: "ready";
+      requestPath: string;
+      ioPath: string;
+      workspaceReal: string;
+    }
+  | {
+      kind: "missing";
+      requestPath: string;
+      ioPath: string;
+      workspaceReal: string;
+    }
+  | {
+      kind: "invalid";
+      requestPath: string;
+      reason: string;
+    };
+
+const SUPPORTS_NOFOLLOW = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants;
+const OPEN_WRITE_FLAGS =
+  fsConstants.O_WRONLY |
+  fsConstants.O_CREAT |
+  fsConstants.O_TRUNC |
+  (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0);
+
+async function resolveWorkspaceRealPath(workspaceDir: string): Promise<string> {
  try {
-    const stat = await fs.stat(filePath);
-    if (!stat.isFile()) {
+    return await fs.realpath(workspaceDir);
+  } catch {
+    return path.resolve(workspaceDir);
+  }
+}
+
+async function resolveAgentWorkspaceFilePath(params: {
+  workspaceDir: string;
+  name: string;
+  allowMissing: boolean;
+}): Promise<ResolvedAgentWorkspaceFilePath> {
+  const requestPath = path.join(params.workspaceDir, params.name);
+  const workspaceReal = await resolveWorkspaceRealPath(params.workspaceDir);
+  const candidatePath = path.resolve(workspaceReal, params.name);
+  if (!isPathInside(workspaceReal, candidatePath)) {
+    return { kind: "invalid", requestPath, reason: "path escapes workspace root" };
+  }
+
+  let candidateLstat: Awaited<ReturnType<typeof fs.lstat>>;
+  try {
+    candidateLstat = await fs.lstat(candidatePath);
+  } catch (err) {
+    if (isNotFoundPathError(err)) {
+      if (params.allowMissing) {
+        return { kind: "missing", requestPath, ioPath: candidatePath, workspaceReal };
+      }
+      return { kind: "invalid", requestPath, reason: "file not found" };
+    }
+    throw err;
+  }
+
+  if (candidateLstat.isSymbolicLink()) {
+    let targetReal: string;
+    try {
+      targetReal = await fs.realpath(candidatePath);
+    } catch (err) {
+      if (isNotFoundPathError(err)) {
+        if (params.allowMissing) {
+          return { kind: "missing", requestPath, ioPath: candidatePath, workspaceReal };
+        }
+        return { kind: "invalid", requestPath, reason: "symlink target not found" };
+      }
+      throw err;
+    }
+    if (!isPathInside(workspaceReal, targetReal)) {
+      return { kind: "invalid", requestPath, reason: "symlink target escapes workspace root" };
+    }
+    try {
+      const targetStat = await fs.stat(targetReal);
+      if (!targetStat.isFile()) {
+        return { kind: "invalid", requestPath, reason: "symlink target is not a file" };
+      }
+    } catch (err) {
+      if (isNotFoundPathError(err) && params.allowMissing) {
+        return { kind: "missing", requestPath, ioPath: targetReal, workspaceReal };
+      }
+      throw err;
+    }
+    return { kind: "ready", requestPath, ioPath: targetReal, workspaceReal };
+  }
+
+  if (!candidateLstat.isFile()) {
+    return { kind: "invalid", requestPath, reason: "path is not a regular file" };
+  }
+
+  const candidateReal = await fs.realpath(candidatePath).catch(() => candidatePath);
+  if (!isPathInside(workspaceReal, candidateReal)) {
+    return { kind: "invalid", requestPath, reason: "resolved file escapes workspace root" };
+  }
+  return { kind: "ready", requestPath, ioPath: candidateReal, workspaceReal };
+}
+
+async function statFileSafely(filePath: string): Promise<FileMeta | null> {
+  try {
+    const [stat, lstat] = await Promise.all([fs.stat(filePath), fs.lstat(filePath)]);
+    if (lstat.isSymbolicLink() || !stat.isFile()) {
+      return null;
+    }
+    if (!sameFileIdentity(stat, lstat)) {
      return null;
    }
    return {
@@ -112,6 +219,22 @@ async function statFile(filePath: string): Promise<FileMeta | null> {
  }
 }

+async function writeFileSafely(filePath: string, content: string): Promise<void> {
+  const handle = await fs.open(filePath, OPEN_WRITE_FLAGS, 0o600);
+  try {
+    const [stat, lstat] = await Promise.all([handle.stat(), fs.lstat(filePath)]);
+    if (lstat.isSymbolicLink() || !stat.isFile()) {
+      throw new Error("unsafe file path");
+    }
+    if (!sameFileIdentity(stat, lstat)) {
+      throw new Error("path changed during write");
+    }
+    await handle.writeFile(content, "utf-8");
+  } finally {
+    await handle.close().catch(() => {});
+  }
+}
+
 async function listAgentFiles(workspaceDir: string, options?: { hideBootstrap?: boolean }) {
  const files: Array<{
    name: string;
@@ -125,8 +248,18 @@ async function listAgentFiles(workspaceDir: string, options?: { hideBootstrap?:
    ? BOOTSTRAP_FILE_NAMES_POST_ONBOARDING
    : BOOTSTRAP_FILE_NAMES;
  for (const name of bootstrapFileNames) {
-    const filePath = path.join(workspaceDir, name);
-    const meta = await statFile(filePath);
+    const resolved = await resolveAgentWorkspaceFilePath({
+      workspaceDir,
+      name,
+      allowMissing: true,
+    });
+    const filePath = resolved.requestPath;
+    const meta =
+      resolved.kind === "ready"
+        ? await statFileSafely(resolved.ioPath)
+        : resolved.kind === "missing"
+          ? null
+          : null;
    if (meta) {
      files.push({
        name,
@@ -140,29 +273,43 @@ async function listAgentFiles(workspaceDir: string, options?: { hideBootstrap?:
    }
  }

-  const primaryMemoryPath = path.join(workspaceDir, DEFAULT_MEMORY_FILENAME);
-  const primaryMeta = await statFile(primaryMemoryPath);
+  const primaryResolved = await resolveAgentWorkspaceFilePath({
+    workspaceDir,
+    name: DEFAULT_MEMORY_FILENAME,
+    allowMissing: true,
+  });
+  const primaryMeta =
+    primaryResolved.kind === "ready" ? await statFileSafely(primaryResolved.ioPath) : null;
  if (primaryMeta) {
    files.push({
      name: DEFAULT_MEMORY_FILENAME,
-      path: primaryMemoryPath,
+      path: primaryResolved.requestPath,
      missing: false,
      size: primaryMeta.size,
      updatedAtMs: primaryMeta.updatedAtMs,
    });
  } else {
-    const altMemoryPath = path.join(workspaceDir, DEFAULT_MEMORY_ALT_FILENAME);
-    const altMeta = await statFile(altMemoryPath);
+    const altMemoryResolved = await resolveAgentWorkspaceFilePath({
+      workspaceDir,
+      name: DEFAULT_MEMORY_ALT_FILENAME,
+      allowMissing: true,
+    });
+    const altMeta =
+      altMemoryResolved.kind === "ready" ? await statFileSafely(altMemoryResolved.ioPath) : null;
    if (altMeta) {
      files.push({
        name: DEFAULT_MEMORY_ALT_FILENAME,
-        path: altMemoryPath,
+        path: altMemoryResolved.requestPath,
        missing: false,
        size: altMeta.size,
        updatedAtMs: altMeta.updatedAtMs,
      });
    } else {
-      files.push({ name: DEFAULT_MEMORY_FILENAME, path: primaryMemoryPath, missing: true });
+      files.push({
+        name: DEFAULT_MEMORY_FILENAME,
+        path: primaryResolved.requestPath,
+        missing: true,
+      });
    }
  }

@@ -453,8 +600,23 @@ export const agentsHandlers: GatewayRequestHandlers = {
    }
    const { agentId, workspaceDir, name } = resolved;
    const filePath = path.join(workspaceDir, name);
-    const meta = await statFile(filePath);
-    if (!meta) {
+    const resolvedPath = await resolveAgentWorkspaceFilePath({
+      workspaceDir,
+      name,
+      allowMissing: true,
+    });
+    if (resolvedPath.kind === "invalid") {
+      respond(
+        false,
+        undefined,
+        errorShape(
+          ErrorCodes.INVALID_REQUEST,
+          `unsafe workspace file "${name}" (${resolvedPath.reason})`,
+        ),
+      );
+      return;
+    }
+    if (resolvedPath.kind === "missing") {
      respond(
        true,
        {
@@ -466,7 +628,29 @@ export const agentsHandlers: GatewayRequestHandlers = {
      );
      return;
    }
-    const content = await fs.readFile(filePath, "utf-8");
+    let safeRead: Awaited<ReturnType<typeof readLocalFileSafely>>;
+    try {
+      safeRead = await readLocalFileSafely({ filePath: resolvedPath.ioPath });
+    } catch (err) {
+      if (err instanceof SafeOpenError && err.code === "not-found") {
+        respond(
+          true,
+          {
+            agentId,
+            workspace: workspaceDir,
+            file: { name, path: filePath, missing: true },
+          },
+          undefined,
+        );
+        return;
+      }
+      respond(
+        false,
+        undefined,
+        errorShape(ErrorCodes.INVALID_REQUEST, `unsafe workspace file "${name}"`),
+      );
+      return;
+    }
    respond(
      true,
      {
@@ -476,9 +660,9 @@ export const agentsHandlers: GatewayRequestHandlers = {
          name,
          path: filePath,
          missing: false,
-          size: meta.size,
-          updatedAtMs: meta.updatedAtMs,
-          content,
+          size: safeRead.stat.size,
+          updatedAtMs: Math.floor(safeRead.stat.mtimeMs),
+          content: safeRead.buffer.toString("utf-8"),
        },
      },
      undefined,
@@ -505,9 +689,34 @@ export const agentsHandlers: GatewayRequestHandlers = {
    const { agentId, workspaceDir, name } = resolved;
    await fs.mkdir(workspaceDir, { recursive: true });
    const filePath = path.join(workspaceDir, name);
+    const resolvedPath = await resolveAgentWorkspaceFilePath({
+      workspaceDir,
+      name,
+      allowMissing: true,
+    });
+    if (resolvedPath.kind === "invalid") {
+      respond(
+        false,
+        undefined,
+        errorShape(
+          ErrorCodes.INVALID_REQUEST,
+          `unsafe workspace file "${name}" (${resolvedPath.reason})`,
+        ),
+      );
+      return;
+    }
    const content = String(params.content ?? "");
-    await fs.writeFile(filePath, content, "utf-8");
-    const meta = await statFile(filePath);
+    try {
+      await writeFileSafely(resolvedPath.ioPath, content);
+    } catch {
+      respond(
+        false,
+        undefined,
+        errorShape(ErrorCodes.INVALID_REQUEST, `unsafe workspace file "${name}"`),
+      );
+      return;
+    }
+    const meta = await statFileSafely(resolvedPath.ioPath);
    respond(
      true,
      {