refactor(gateway): share safe avatar file open checks

src/gateway/control-ui.ts

@@ -4,6 +4,7 @@ import path from "node:path";
 import type { OpenClawConfig } from "../config/config.js";
 import { resolveControlUiRootSync } from "../infra/control-ui-assets.js";
 import { isWithinDir } from "../infra/path-safety.js";
+import { openVerifiedFileSync } from "../infra/safe-open-sync.js";
 import { AVATAR_MAX_BYTES } from "../shared/avatar-policy.js";
 import { DEFAULT_ASSISTANT_IDENTITY, resolveAssistantIdentity } from "./assistant-identity.js";
 import {
@@ -220,84 +221,40 @@ function isExpectedSafePathError(error: unknown): boolean {
   return code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP";
 }
 
-function areSameFileIdentity(preOpen: fs.Stats, opened: fs.Stats): boolean {
-  return preOpen.dev === opened.dev && preOpen.ino === opened.ino;
-}
-
 function resolveSafeAvatarFile(filePath: string): { path: string; fd: number } | null {
-  let fd: number | null = null;
-  try {
-    const candidateStat = fs.lstatSync(filePath);
-    if (candidateStat.isSymbolicLink()) {
-      return null;
-    }
-    const fileReal = fs.realpathSync(filePath);
-    const preOpenStat = fs.lstatSync(fileReal);
-    if (!preOpenStat.isFile() || preOpenStat.size > AVATAR_MAX_BYTES) {
-      return null;
-    }
-    const openFlags =
-      fs.constants.O_RDONLY |
-      (typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0);
-    fd = fs.openSync(fileReal, openFlags);
-    const openedStat = fs.fstatSync(fd);
-    if (
-      !openedStat.isFile() ||
-      openedStat.size > AVATAR_MAX_BYTES ||
-      !areSameFileIdentity(preOpenStat, openedStat)
-    ) {
-      return null;
-    }
-    const safeFile = { path: fileReal, fd };
-    fd = null;
-    return safeFile;
-  } catch {
+  const opened = openVerifiedFileSync({
+    filePath,
+    rejectPathSymlink: true,
+    maxBytes: AVATAR_MAX_BYTES,
+  });
+  if (!opened.ok) {
     return null;
-  } finally {
-    if (fd !== null) {
-      fs.closeSync(fd);
-    }
   }
+  return { path: opened.path, fd: opened.fd };
 }
 
 function resolveSafeControlUiFile(
   rootReal: string,
   filePath: string,
 ): { path: string; fd: number } | null {
-  let fd: number | null = null;
   try {
     const fileReal = fs.realpathSync(filePath);
     if (!isContainedPath(rootReal, fileReal)) {
       return null;
     }
-
-    const preOpenStat = fs.lstatSync(fileReal);
-    if (!preOpenStat.isFile()) {
+    const opened = openVerifiedFileSync({ filePath: fileReal, resolvedPath: fileReal });
+    if (!opened.ok) {
+      if (opened.reason === "io") {
+        throw opened.error;
+      }
       return null;
     }
-
-    const openFlags =
-      fs.constants.O_RDONLY |
-      (typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0);
-    fd = fs.openSync(fileReal, openFlags);
-    const openedStat = fs.fstatSync(fd);
-    // Compare inode identity so swaps between validation and open are rejected.
-    if (!openedStat.isFile() || !areSameFileIdentity(preOpenStat, openedStat)) {
-      return null;
-    }
-
-    const resolved = { path: fileReal, fd };
-    fd = null;
-    return resolved;
+    return { path: opened.path, fd: opened.fd };
   } catch (error) {
     if (isExpectedSafePathError(error)) {
       return null;
     }
     throw error;
-  } finally {
-    if (fd !== null) {
-      fs.closeSync(fd);
-    }
   }
 }
 

src/gateway/session-utils.ts

@@ -21,6 +21,7 @@ import {
   type SessionEntry,
   type SessionScope,
 } from "../config/sessions.js";
+import { openVerifiedFileSync } from "../infra/safe-open-sync.js";
 import {
   normalizeAgentId,
   normalizeMainKey,
@@ -75,10 +76,6 @@ function tryResolveExistingPath(value: string): string | null {
   }
 }
 
-function areSameFileIdentity(preOpen: fs.Stats, opened: fs.Stats): boolean {
-  return preOpen.dev === opened.dev && preOpen.ino === opened.ino;
-}
-
 function resolveIdentityAvatarUrl(
   cfg: OpenClawConfig,
   agentId: string,
@@ -103,37 +100,28 @@ function resolveIdentityAvatarUrl(
   if (!isPathWithinRoot(workspaceRoot, resolvedCandidate)) {
     return undefined;
   }
-  let fd: number | null = null;
   try {
     const resolvedReal = fs.realpathSync(resolvedCandidate);
     if (!isPathWithinRoot(workspaceRoot, resolvedReal)) {
       return undefined;
     }
-    const preOpenStat = fs.lstatSync(resolvedReal);
-    if (!preOpenStat.isFile() || preOpenStat.size > AVATAR_MAX_BYTES) {
+    const opened = openVerifiedFileSync({
+      filePath: resolvedReal,
+      resolvedPath: resolvedReal,
+      maxBytes: AVATAR_MAX_BYTES,
+    });
+    if (!opened.ok) {
       return undefined;
     }
-    const openFlags =
-      fs.constants.O_RDONLY |
-      (typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0);
-    fd = fs.openSync(resolvedReal, openFlags);
-    const openedStat = fs.fstatSync(fd);
-    if (
-      !openedStat.isFile() ||
-      openedStat.size > AVATAR_MAX_BYTES ||
-      !areSameFileIdentity(preOpenStat, openedStat)
-    ) {
-      return undefined;
+    try {
+      const buffer = fs.readFileSync(opened.fd);
+      const mime = resolveAvatarMime(resolvedCandidate);
+      return `data:${mime};base64,${buffer.toString("base64")}`;
+    } finally {
+      fs.closeSync(opened.fd);
     }
-    const buffer = fs.readFileSync(fd);
-    const mime = resolveAvatarMime(resolvedCandidate);
-    return `data:${mime};base64,${buffer.toString("base64")}`;
   } catch {
     return undefined;
-  } finally {
-    if (fd !== null) {
-      fs.closeSync(fd);
-    }
   }
 }
 

src/infra/safe-open-sync.ts

@@ -0,0 +1,72 @@
+import fs from "node:fs";
+
+export type SafeOpenSyncFailureReason = "path" | "validation" | "io";
+
+export type SafeOpenSyncResult =
+  | { ok: true; path: string; fd: number; stat: fs.Stats }
+  | { ok: false; reason: SafeOpenSyncFailureReason; error?: unknown };
+
+const OPEN_READ_FLAGS =
+  fs.constants.O_RDONLY |
+  (typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0);
+
+function isExpectedPathError(error: unknown): boolean {
+  const code =
+    typeof error === "object" && error !== null && "code" in error ? String(error.code) : "";
+  return code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP";
+}
+
+export function sameFileIdentity(left: fs.Stats, right: fs.Stats): boolean {
+  return left.dev === right.dev && left.ino === right.ino;
+}
+
+export function openVerifiedFileSync(params: {
+  filePath: string;
+  resolvedPath?: string;
+  rejectPathSymlink?: boolean;
+  maxBytes?: number;
+}): SafeOpenSyncResult {
+  let fd: number | null = null;
+  try {
+    if (params.rejectPathSymlink) {
+      const candidateStat = fs.lstatSync(params.filePath);
+      if (candidateStat.isSymbolicLink()) {
+        return { ok: false, reason: "validation" };
+      }
+    }
+
+    const realPath = params.resolvedPath ?? fs.realpathSync(params.filePath);
+    const preOpenStat = fs.lstatSync(realPath);
+    if (!preOpenStat.isFile()) {
+      return { ok: false, reason: "validation" };
+    }
+    if (params.maxBytes !== undefined && preOpenStat.size > params.maxBytes) {
+      return { ok: false, reason: "validation" };
+    }
+
+    fd = fs.openSync(realPath, OPEN_READ_FLAGS);
+    const openedStat = fs.fstatSync(fd);
+    if (!openedStat.isFile()) {
+      return { ok: false, reason: "validation" };
+    }
+    if (params.maxBytes !== undefined && openedStat.size > params.maxBytes) {
+      return { ok: false, reason: "validation" };
+    }
+    if (!sameFileIdentity(preOpenStat, openedStat)) {
+      return { ok: false, reason: "validation" };
+    }
+
+    const opened = { ok: true as const, path: realPath, fd, stat: openedStat };
+    fd = null;
+    return opened;
+  } catch (error) {
+    if (isExpectedPathError(error)) {
+      return { ok: false, reason: "path", error };
+    }
+    return { ok: false, reason: "io", error };
+  } finally {
+    if (fd !== null) {
+      fs.closeSync(fd);
+    }
+  }
+}