fix: retry protected file reads with sudo (#1349)

src/backend/hosts/file-manager/content-routes.ts

@@ -3,6 +3,7 @@ import Busboy from "busboy";
 import type { AuthenticatedRequest } from "../../../types/index.js";
 import { fileLogger } from "../../utils/logger.js";
 import {
+  execBuffer,
   execChannel,
   execWithSudo,
   execWithSudoBuffer,
@@ -262,7 +263,7 @@ export function registerFileContentRoutes(
    *       500:
    *         description: Failed to read file.
    */
-  app.get("/ssh/file_manager/ssh/readFile", (req, res) => {
+  app.get("/ssh/file_manager/ssh/readFile", async (req, res) => {
     const sessionId = req.query.sessionId as string;
     const sshConn = sshSessions[sessionId];
     const filePath = decodeURIComponent(req.query.path as string);
@@ -295,166 +296,120 @@ export function registerFileContentRoutes(
     const MAX_READ_SIZE = 500 * 1024 * 1024;
     const escapedPath = filePath.replace(/'/g, "'\"'\"'");
 
-    execChannel(
+    const isPermissionDenied = (message: string) =>
-      sshConn,
+      message.toLowerCase().includes("permission denied");
-      `stat -c%s '${escapedPath}' 2>/dev/null || wc -c < '${escapedPath}'`,
+    const isFileNotFound = (message: string) => {
-      (sizeErr, sizeStream) => {
+      const lower = message.toLowerCase();
-        if (sizeErr) {
+      return (
-          fileLogger.error("SSH file size check error:", sizeErr);
+        lower.includes("no such file or directory") ||
-          return res.status(500).json({ error: sizeErr.message });
+        lower.includes("cannot access") ||
-        }
+        lower.includes("not found") ||
+        lower.includes("resource not found")
+      );
+    };
 
-        let sizeData = "";
+    try {
-        let sizeErrorData = "";
+      let sizeResult = await execBuffer(
+        sshConn,
+        `stat -c%s '${escapedPath}' 2>/dev/null || wc -c < '${escapedPath}'`,
+      );
+      let sizeError = sizeResult.stderr || sizeResult.stdout.toString("utf8");
 
-        sizeStream.on("data", (chunk: Buffer) => {
+      if (
-          sizeData += chunk.toString();
+        sizeResult.code !== 0 &&
+        isPermissionDenied(sizeError) &&
+        sshConn.sudoPassword
+      ) {
+        sizeResult = await execWithSudoBuffer(
+          sshConn,
+          `stat -c%s '${escapedPath}'`,
+          sshConn.sudoPassword,
+        );
+        sizeError = sizeResult.stderr || sizeResult.stdout.toString("utf8");
+      }
+
+      if (sizeResult.code !== 0) {
+        const missing = isFileNotFound(sizeError);
+        const permissionDenied = isPermissionDenied(sizeError);
+        fileLogger.error(`File size check failed: ${sizeError}`);
+        return res.status(missing ? 404 : permissionDenied ? 403 : 500).json({
+          error: `Cannot check file size: ${sizeError}`,
+          fileNotFound: missing,
+          needsSudo: permissionDenied,
         });
+      }
 
-        sizeStream.stderr.on("data", (chunk: Buffer) => {
+      const fileSize = parseInt(sizeResult.stdout.toString("utf8").trim(), 10);
-          sizeErrorData += chunk.toString();
+      if (isNaN(fileSize)) {
+        fileLogger.error("Invalid file size response:", sizeResult.stdout);
+        return res.status(500).json({ error: "Cannot determine file size" });
+      }
+
+      if (fileSize > MAX_READ_SIZE) {
+        fileLogger.warn("File too large for reading", {
+          operation: "file_read",
+          sessionId,
+          filePath,
+          fileSize,
+          maxSize: MAX_READ_SIZE,
         });
-
+        return res.status(400).json({
-        sizeStream.on("close", (sizeCode) => {
+          error: `File too large to open in editor. Maximum size is ${MAX_READ_SIZE / 1024 / 1024}MB, file is ${(fileSize / 1024 / 1024).toFixed(2)}MB. Use download instead.`,
-          if (sizeCode !== 0) {
+          fileSize,
-            const errorLower = sizeErrorData.toLowerCase();
+          maxSize: MAX_READ_SIZE,
-            const isFileNotFound =
+          tooLarge: true,
-              errorLower.includes("no such file or directory") ||
-              errorLower.includes("cannot access") ||
-              errorLower.includes("not found") ||
-              errorLower.includes("resource not found");
-
-            fileLogger.error(`File size check failed: ${sizeErrorData}`);
-            return res.status(isFileNotFound ? 404 : 500).json({
-              error: `Cannot check file size: ${sizeErrorData}`,
-              fileNotFound: isFileNotFound,
-            });
-          }
-
-          const fileSize = parseInt(sizeData.trim(), 10);
-
-          if (isNaN(fileSize)) {
-            fileLogger.error("Invalid file size response:", sizeData);
-            return res
-              .status(500)
-              .json({ error: "Cannot determine file size" });
-          }
-
-          if (fileSize > MAX_READ_SIZE) {
-            fileLogger.warn("File too large for reading", {
-              operation: "file_read",
-              sessionId,
-              filePath,
-              fileSize,
-              maxSize: MAX_READ_SIZE,
-            });
-            return res.status(400).json({
-              error: `File too large to open in editor. Maximum size is ${MAX_READ_SIZE / 1024 / 1024}MB, file is ${(fileSize / 1024 / 1024).toFixed(2)}MB. Use download instead.`,
-              fileSize,
-              maxSize: MAX_READ_SIZE,
-              tooLarge: true,
-            });
-          }
-
-          execChannel(sshConn, `cat '${escapedPath}'`, (err, stream) => {
-            if (err) {
-              fileLogger.error("SSH readFile error:", err);
-              return res.status(500).json({ error: err.message });
-            }
-
-            let binaryData = Buffer.alloc(0);
-            let errorData = "";
-
-            stream.on("data", (chunk: Buffer) => {
-              binaryData = Buffer.concat([binaryData, chunk]);
-            });
-
-            stream.stderr.on("data", (chunk: Buffer) => {
-              errorData += chunk.toString();
-            });
-
-            stream.on("close", (code) => {
-              if (code !== 0) {
-                const isPermissionDenied = errorData
-                  .toLowerCase()
-                  .includes("permission denied");
-
-                if (isPermissionDenied && sshConn.sudoPassword) {
-                  execWithSudoBuffer(
-                    sshConn,
-                    `cat '${escapedPath}'`,
-                    sshConn.sudoPassword,
-                  )
-                    .then((result) => {
-                      if (result.code !== 0) {
-                        return res.status(403).json({
-                          error: `Permission denied: ${result.stderr || result.stdout.toString("utf8")}`,
-                          needsSudo: true,
-                        });
-                      }
-
-                      const sudoData = result.stdout;
-                      const isBinary = detectBinary(sudoData);
-                      res.json({
-                        content: isBinary
-                          ? sudoData.toString("base64")
-                          : sudoData.toString("utf8"),
-                        isBinary,
-                        size: sudoData.length,
-                      });
-                    })
-                    .catch(() => {
-                      res
-                        .status(403)
-                        .json({ error: "Permission denied", needsSudo: true });
-                    });
-                  return;
-                }
-
-                fileLogger.error(
-                  `SSH readFile command failed with code ${code}: ${errorData.replace(/\n/g, " ").trim()}`,
-                );
-
-                const isFileNotFound =
-                  errorData.includes("No such file or directory") ||
-                  errorData.includes("cannot access") ||
-                  errorData.includes("not found");
-
-                return res.status(isFileNotFound ? 404 : 500).json({
-                  error: `Command failed: ${errorData}`,
-                  fileNotFound: isFileNotFound,
-                });
-              }
-
-              const isBinary = detectBinary(binaryData);
-              fileLogger.success("File read successfully", {
-                operation: "file_read_success",
-                sessionId,
-                userId,
-                path: filePath,
-                bytes: binaryData.length,
-              });
-
-              if (isBinary) {
-                const base64Content = binaryData.toString("base64");
-                res.json({
-                  content: base64Content,
-                  path: filePath,
-                  encoding: "base64",
-                });
-              } else {
-                const textContent = binaryData.toString("utf8");
-                res.json({
-                  content: textContent,
-                  path: filePath,
-                  encoding: "utf8",
-                });
-              }
-            });
-          });
         });
-      },
+      }
-    );
+
+      let contentResult = await execBuffer(sshConn, `cat '${escapedPath}'`);
+      let contentError =
+        contentResult.stderr || contentResult.stdout.toString("utf8");
+
+      if (
+        contentResult.code !== 0 &&
+        isPermissionDenied(contentError) &&
+        sshConn.sudoPassword
+      ) {
+        contentResult = await execWithSudoBuffer(
+          sshConn,
+          `cat '${escapedPath}'`,
+          sshConn.sudoPassword,
+        );
+        contentError =
+          contentResult.stderr || contentResult.stdout.toString("utf8");
+      }
+
+      if (contentResult.code !== 0) {
+        const missing = isFileNotFound(contentError);
+        const permissionDenied = isPermissionDenied(contentError);
+        fileLogger.error(
+          `SSH readFile command failed with code ${contentResult.code}: ${contentError.replace(/\n/g, " ").trim()}`,
+        );
+        return res.status(missing ? 404 : permissionDenied ? 403 : 500).json({
+          error: `Command failed: ${contentError}`,
+          fileNotFound: missing,
+          needsSudo: permissionDenied,
+        });
+      }
+
+      const isBinary = detectBinary(contentResult.stdout);
+      fileLogger.success("File read successfully", {
+        operation: "file_read_success",
+        sessionId,
+        userId,
+        path: filePath,
+        bytes: contentResult.stdout.length,
+      });
+      return res.json({
+        content: contentResult.stdout.toString(isBinary ? "base64" : "utf8"),
+        path: filePath,
+        encoding: isBinary ? "base64" : "utf8",
+      });
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      fileLogger.error("SSH readFile error:", error);
+      return res.status(500).json({ error: message });
+    }
   });
 
   /**

src/backend/hosts/file-manager/session.ts

@@ -120,6 +120,40 @@ export function execWithSudoBuffer(
   });
 }
 
+export function execBuffer(
+  session: SSHSession,
+  command: string,
+): Promise<{ stdout: Buffer; stderr: string; code: number }> {
+  return new Promise((resolve) => {
+    execChannel(session, command, (err, stream) => {
+      if (err) {
+        resolve({ stdout: Buffer.alloc(0), stderr: err.message, code: 1 });
+        return;
+      }
+
+      const stdoutChunks: Buffer[] = [];
+      let stderr = "";
+      let settled = false;
+
+      const finish = (code: number) => {
+        if (settled) return;
+        settled = true;
+        resolve({ stdout: Buffer.concat(stdoutChunks), stderr, code });
+      };
+
+      stream.on("data", (chunk: Buffer) => stdoutChunks.push(chunk));
+      stream.stderr.on("data", (chunk: Buffer) => {
+        stderr += chunk.toString();
+      });
+      stream.on("close", (code: number) => finish(code || 0));
+      stream.on("error", (streamErr: Error) => {
+        stderr = stderr || streamErr.message;
+        finish(1);
+      });
+    });
+  });
+}
+
 export function getSessionSftp(
   session: SSHSession,
 ): Promise<import("ssh2").SFTPWrapper> {

src/backend/tests/hosts/file-manager/content-routes.test.ts

@@ -0,0 +1,154 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { Express, Request, Response } from "express";
+import type { SSHSession } from "../../../hosts/file-manager/session.js";
+
+const commandMocks = vi.hoisted(() => ({
+  execBuffer: vi.fn(),
+  execWithSudoBuffer: vi.fn(),
+}));
+
+vi.mock("../../../hosts/file-manager/session.js", async (importOriginal) => ({
+  ...(await importOriginal<
+    typeof import("../../../hosts/file-manager/session.js")
+  >()),
+  execBuffer: commandMocks.execBuffer,
+  execWithSudoBuffer: commandMocks.execWithSudoBuffer,
+}));
+
+import { registerFileContentRoutes } from "../../../hosts/file-manager/content-routes.js";
+
+type RouteHandler = (request: Request, response: Response) => Promise<unknown>;
+
+function setupReadRoute() {
+  const routes = new Map<string, RouteHandler>();
+  const app = {
+    get: (path: string, handler: RouteHandler) => routes.set(path, handler),
+    post: vi.fn(),
+  } as unknown as Express;
+  const session = {
+    isConnected: true,
+    lastActive: 0,
+    sudoPassword: "sudo-secret",
+  } as SSHSession;
+
+  registerFileContentRoutes(app, {
+    sshSessions: { session: session },
+    verifySessionOwnership: () => true,
+  });
+
+  const handler = routes.get("/ssh/file_manager/ssh/readFile");
+  if (!handler) throw new Error("readFile route was not registered");
+
+  const request = {
+    query: { sessionId: "session", path: "/root/secret.txt" },
+    userId: "user",
+  } as unknown as Request;
+  const response = {
+    status: vi.fn(),
+    json: vi.fn(),
+  } as unknown as Response;
+  vi.mocked(response.status).mockReturnValue(response);
+  vi.mocked(response.json).mockReturnValue(response);
+
+  return { handler, request, response, session };
+}
+
+describe("file manager readFile", () => {
+  beforeEach(() => {
+    commandMocks.execBuffer.mockReset();
+    commandMocks.execWithSudoBuffer.mockReset();
+  });
+
+  it("falls back to sudo for both the size check and content read", async () => {
+    const { handler, request, response, session } = setupReadRoute();
+    commandMocks.execBuffer
+      .mockResolvedValueOnce({
+        stdout: Buffer.alloc(0),
+        stderr: "Permission denied",
+        code: 1,
+      })
+      .mockResolvedValueOnce({
+        stdout: Buffer.alloc(0),
+        stderr: "Permission denied",
+        code: 1,
+      });
+    commandMocks.execWithSudoBuffer
+      .mockResolvedValueOnce({
+        stdout: Buffer.from("6\n"),
+        stderr: "",
+        code: 0,
+      })
+      .mockResolvedValueOnce({
+        stdout: Buffer.from("secret"),
+        stderr: "",
+        code: 0,
+      });
+
+    await handler(request, response);
+
+    expect(commandMocks.execWithSudoBuffer).toHaveBeenNthCalledWith(
+      1,
+      session,
+      "stat -c%s '/root/secret.txt'",
+      "sudo-secret",
+    );
+    expect(commandMocks.execWithSudoBuffer).toHaveBeenNthCalledWith(
+      2,
+      session,
+      "cat '/root/secret.txt'",
+      "sudo-secret",
+    );
+    expect(response.json).toHaveBeenCalledWith({
+      content: "secret",
+      path: "/root/secret.txt",
+      encoding: "utf8",
+    });
+  });
+
+  it("keeps readable files on the unprivileged path", async () => {
+    const { handler, request, response } = setupReadRoute();
+    commandMocks.execBuffer
+      .mockResolvedValueOnce({
+        stdout: Buffer.from("5\n"),
+        stderr: "",
+        code: 0,
+      })
+      .mockResolvedValueOnce({
+        stdout: Buffer.from("hello"),
+        stderr: "",
+        code: 0,
+      });
+
+    await handler(request, response);
+
+    expect(commandMocks.execWithSudoBuffer).not.toHaveBeenCalled();
+    expect(response.json).toHaveBeenCalledWith({
+      content: "hello",
+      path: "/root/secret.txt",
+      encoding: "utf8",
+    });
+  });
+
+  it("rejects oversized sudo-only files before reading their content", async () => {
+    const { handler, request, response } = setupReadRoute();
+    commandMocks.execBuffer.mockResolvedValueOnce({
+      stdout: Buffer.alloc(0),
+      stderr: "Permission denied",
+      code: 1,
+    });
+    commandMocks.execWithSudoBuffer.mockResolvedValueOnce({
+      stdout: Buffer.from(String(501 * 1024 * 1024)),
+      stderr: "",
+      code: 0,
+    });
+
+    await handler(request, response);
+
+    expect(response.status).toHaveBeenCalledWith(400);
+    expect(response.json).toHaveBeenCalledWith(
+      expect.objectContaining({ tooLarge: true }),
+    );
+    expect(commandMocks.execBuffer).toHaveBeenCalledTimes(1);
+    expect(commandMocks.execWithSudoBuffer).toHaveBeenCalledTimes(1);
+  });
+});