fix: harden file reads and timer cleanup (#1352)

* fix: harden file reads and timer cleanup

* fix: preserve literal file path escapes

electron/remote-sync.cjs

@@ -191,6 +191,7 @@ class RemoteSyncEngine {
     this.getMainWindow = getMainWindow;
     this.localJwt = null;
     this.timer = null;
+    this.startupTimer = null;
     this.syncing = false;
     this.status = {
       connected: false,
@@ -220,11 +221,15 @@ class RemoteSyncEngine {
     const config = getRemoteSyncConfig();
     this.status.connected = !!config?.serverUrl;
     if (this.timer) clearInterval(this.timer);
+    if (this.startupTimer) clearTimeout(this.startupTimer);
     this.timer = setInterval(() => this.syncNow(), SYNC_INTERVAL_MS);
     if (config?.serverUrl) {
       // Fire an initial sync shortly after startup rather than waiting a
       // full interval, but don't block app boot on it.
-      setTimeout(() => this.syncNow(), 5000);
+      this.startupTimer = setTimeout(() => {
+        this.startupTimer = null;
+        this.syncNow();
+      }, 5000);
     }
   }
 
@@ -233,6 +238,10 @@ class RemoteSyncEngine {
       clearInterval(this.timer);
       this.timer = null;
     }
+    if (this.startupTimer) {
+      clearTimeout(this.startupTimer);
+      this.startupTimer = null;
+    }
   }
 
   async syncNow() {

src/backend/database/routes/session-log-routes.ts

@@ -85,7 +85,7 @@ async function pruneOldLogs(): Promise<void> {
 
 // Run prune once at startup, then every 24 hours
 pruneOldLogs();
-setInterval(pruneOldLogs, 24 * 60 * 60 * 1000);
+setInterval(pruneOldLogs, 24 * 60 * 60 * 1000).unref();
 
 const authManager = AuthManager.getInstance();
 const authenticateJWT = authManager.createAuthMiddleware();

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

@@ -65,7 +65,7 @@ export function registerFileContentRoutes(
   app.get("/ssh/file_manager/ssh/identifySymlink", (req, res) => {
     const sessionId = req.query.sessionId as string;
     const sshConn = sshSessions[sessionId];
-    const linkPath = decodeURIComponent(req.query.path as string);
+    const linkPath = req.query.path as string;
     const userId = (req as AuthenticatedRequest).userId;
 
     if (!sessionId) {
@@ -166,7 +166,7 @@ export function registerFileContentRoutes(
   app.get("/ssh/file_manager/ssh/resolvePath", (req, res) => {
     const sessionId = req.query.sessionId as string;
     const sshConn = sshSessions[sessionId];
-    const rawPath = decodeURIComponent(req.query.path as string);
+    const rawPath = req.query.path as string;
     const userId = (req as AuthenticatedRequest).userId;
 
     if (!sessionId) {
@@ -266,7 +266,7 @@ export function registerFileContentRoutes(
   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);
+    const filePath = req.query.path as string;
     const userId = (req as AuthenticatedRequest).userId;
 
     if (!sessionId) {
@@ -361,7 +361,11 @@ export function registerFileContentRoutes(
         });
       }
 
-      let contentResult = await execBuffer(sshConn, `cat '${escapedPath}'`);
+      let contentResult = await execBuffer(
+        sshConn,
+        `cat '${escapedPath}'`,
+        MAX_READ_SIZE,
+      );
       let contentError =
         contentResult.stderr || contentResult.stdout.toString("utf8");
 
@@ -374,11 +378,20 @@ export function registerFileContentRoutes(
           sshConn,
           `cat '${escapedPath}'`,
           sshConn.sudoPassword,
+          MAX_READ_SIZE,
         );
         contentError =
           contentResult.stderr || contentResult.stdout.toString("utf8");
       }
 
+      if (contentResult.exceededLimit) {
+        return res.status(400).json({
+          error: `File grew beyond the ${MAX_READ_SIZE / 1024 / 1024}MB read limit`,
+          maxSize: MAX_READ_SIZE,
+          tooLarge: true,
+        });
+      }
+
       if (contentResult.code !== 0) {
         const missing = isFileNotFound(contentError);
         const permissionDenied = isPermissionDenied(contentError);

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

@@ -54,7 +54,7 @@ export function registerFileListingRoutes(
   app.get("/ssh/file_manager/ssh/listFiles", (req, res) => {
     const sessionId = req.query.sessionId as string;
     const sshConn = sshSessions[sessionId];
-    const sshPath = decodeURIComponent((req.query.path as string) || "/");
+    const sshPath = (req.query.path as string) || "/";
     const userId = (req as AuthenticatedRequest).userId;
 
     if (!sessionId) {

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

@@ -76,10 +76,15 @@ export function execWithSudoBuffer(
   session: SSHSession,
   command: string,
   sudoPassword: string,
-): Promise<{ stdout: Buffer; stderr: string; code: number }> {
+  maxStdoutBytes?: number,
+): Promise<{
+  stdout: Buffer;
+  stderr: string;
+  code: number;
+  exceededLimit?: boolean;
+}> {
   return new Promise((resolve) => {
-    const escapedPassword = sudoPassword.replace(/'/g, "'\"'\"'");
+    const sudoCommand = `sudo -S -p '' ${command} 2>&1`;
-    const sudoCommand = `echo '${escapedPassword}' | sudo -S ${command} 2>&1`;
 
     execChannel(session, sudoCommand, (err, stream) => {
       if (err) {
@@ -88,9 +93,33 @@ export function execWithSudoBuffer(
       }
 
       const stdoutChunks: Buffer[] = [];
+      let stdoutBytes = 0;
       let stderr = "";
+      let settled = false;
+
+      const finish = (
+        code: number,
+        extra: { exceededLimit?: boolean } = {},
+      ) => {
+        if (settled) return;
+        settled = true;
+        resolve({
+          stdout: Buffer.concat(stdoutChunks),
+          stderr,
+          code,
+          ...extra,
+        });
+      };
 
       stream.on("data", (chunk: Buffer) => {
+        if (settled) return;
+        stdoutBytes += chunk.length;
+        if (maxStdoutBytes !== undefined && stdoutBytes > maxStdoutBytes) {
+          stderr = `Command output exceeds ${maxStdoutBytes} bytes`;
+          finish(1, { exceededLimit: true });
+          stream.close();
+          return;
+        }
         stdoutChunks.push(chunk);
       });
 
@@ -99,6 +128,7 @@ export function execWithSudoBuffer(
       });
 
       stream.on("close", (code: number) => {
+        if (settled) return;
         let stdout = Buffer.concat(stdoutChunks);
         const sudoPromptMatch = stdout
           .toString("utf8", 0, Math.min(stdout.length, 256))
@@ -106,16 +136,16 @@ export function execWithSudoBuffer(
         if (sudoPromptMatch) {
           stdout = stdout.subarray(Buffer.byteLength(sudoPromptMatch[0]));
         }
+        settled = true;
         resolve({ stdout, stderr, code: code || 0 });
       });
 
       stream.on("error", (streamErr: Error) => {
-        resolve({
+        stderr = streamErr.message;
-          stdout: Buffer.concat(stdoutChunks),
+        finish(1);
-          stderr: streamErr.message,
-          code: 1,
-        });
       });
+
+      stream.write(`${sudoPassword}\n`);
     });
   });
 }
@@ -123,7 +153,13 @@ export function execWithSudoBuffer(
 export function execBuffer(
   session: SSHSession,
   command: string,
-): Promise<{ stdout: Buffer; stderr: string; code: number }> {
+  maxStdoutBytes?: number,
+): Promise<{
+  stdout: Buffer;
+  stderr: string;
+  code: number;
+  exceededLimit?: boolean;
+}> {
   return new Promise((resolve) => {
     execChannel(session, command, (err, stream) => {
       if (err) {
@@ -132,16 +168,35 @@ export function execBuffer(
       }
 
       const stdoutChunks: Buffer[] = [];
+      let stdoutBytes = 0;
       let stderr = "";
       let settled = false;
 
-      const finish = (code: number) => {
+      const finish = (
+        code: number,
+        extra: { exceededLimit?: boolean } = {},
+      ) => {
         if (settled) return;
         settled = true;
-        resolve({ stdout: Buffer.concat(stdoutChunks), stderr, code });
+        resolve({
+          stdout: Buffer.concat(stdoutChunks),
+          stderr,
+          code,
+          ...extra,
+        });
       };
 
-      stream.on("data", (chunk: Buffer) => stdoutChunks.push(chunk));
+      stream.on("data", (chunk: Buffer) => {
+        if (settled) return;
+        stdoutBytes += chunk.length;
+        if (maxStdoutBytes !== undefined && stdoutBytes > maxStdoutBytes) {
+          stderr = `Command output exceeds ${maxStdoutBytes} bytes`;
+          finish(1, { exceededLimit: true });
+          stream.close();
+          return;
+        }
+        stdoutChunks.push(chunk);
+      });
       stream.stderr.on("data", (chunk: Buffer) => {
         stderr += chunk.toString();
       });

src/backend/hosts/session-sharing/routes.ts

@@ -61,7 +61,7 @@ setInterval(
     }
   },
   5 * 60 * 1000,
-);
+).unref();
 
 /**
  * @openapi

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

@@ -97,12 +97,45 @@ describe("file manager readFile", () => {
       session,
       "cat '/root/secret.txt'",
       "sudo-secret",
+      500 * 1024 * 1024,
     );
     expect(response.json).toHaveBeenCalledWith({
       content: "secret",
       path: "/root/secret.txt",
       encoding: "utf8",
     });
+    expect(commandMocks.execBuffer).toHaveBeenNthCalledWith(
+      2,
+      session,
+      "cat '/root/secret.txt'",
+      500 * 1024 * 1024,
+    );
+  });
+
+  it("rejects a file that grows beyond the limit while being read", async () => {
+    const { handler, request, response } = setupReadRoute();
+    commandMocks.execBuffer
+      .mockResolvedValueOnce({
+        stdout: Buffer.from("5\n"),
+        stderr: "",
+        code: 0,
+      })
+      .mockResolvedValueOnce({
+        stdout: Buffer.alloc(0),
+        stderr: "Command output exceeds 524288000 bytes",
+        code: 1,
+        exceededLimit: true,
+      });
+
+    await handler(request, response);
+
+    expect(response.status).toHaveBeenCalledWith(400);
+    expect(response.json).toHaveBeenCalledWith(
+      expect.objectContaining({
+        maxSize: 500 * 1024 * 1024,
+        tooLarge: true,
+      }),
+    );
   });
 
   it("keeps readable files on the unprivileged path", async () => {
@@ -129,6 +162,34 @@ describe("file manager readFile", () => {
     });
   });
 
+  it("preserves percent escapes that are literal path characters", async () => {
+    const { handler, request, response, session } = setupReadRoute();
+    request.query.path = "/tmp/100%/literal%2Fname";
+    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.execBuffer).toHaveBeenNthCalledWith(
+      2,
+      session,
+      "cat '/tmp/100%/literal%2Fname'",
+      500 * 1024 * 1024,
+    );
+    expect(response.json).toHaveBeenCalledWith(
+      expect.objectContaining({ path: "/tmp/100%/literal%2Fname" }),
+    );
+  });
+
   it("rejects oversized sudo-only files before reading their content", async () => {
     const { handler, request, response } = setupReadRoute();
     commandMocks.execBuffer.mockResolvedValueOnce({

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

@@ -0,0 +1,68 @@
+import { EventEmitter } from "node:events";
+import { describe, expect, it, vi } from "vitest";
+import {
+  ChannelOpenSerializer,
+  execBuffer,
+  execWithSudoBuffer,
+  type SSHSession,
+} from "../../../hosts/file-manager/session.js";
+
+function setupSession() {
+  const stream = new EventEmitter() as EventEmitter & {
+    stderr: EventEmitter;
+    close: ReturnType<typeof vi.fn>;
+    write: ReturnType<typeof vi.fn>;
+  };
+  stream.stderr = new EventEmitter();
+  stream.close = vi.fn();
+  stream.write = vi.fn();
+
+  const exec = vi.fn(
+    (
+      _command: string,
+      callback: (error: undefined, channel: typeof stream) => void,
+    ) => callback(undefined, stream),
+  );
+  const session = {
+    client: { exec },
+    channelOpener: new ChannelOpenSerializer(),
+  } as unknown as SSHSession;
+
+  return { exec, session, stream };
+}
+
+describe("file-manager command helpers", () => {
+  it("writes the sudo password to stdin instead of the command line", async () => {
+    const { exec, session, stream } = setupSession();
+    const resultPromise = execWithSudoBuffer(
+      session,
+      "cat '/root/secret.txt'",
+      "private password",
+    );
+
+    await vi.waitFor(() => expect(stream.write).toHaveBeenCalled());
+    expect(exec).toHaveBeenCalledWith(
+      "sudo -S -p '' cat '/root/secret.txt' 2>&1",
+      expect.any(Function),
+    );
+    expect(exec.mock.calls[0][0]).not.toContain("private password");
+    expect(stream.write).toHaveBeenCalledWith("private password\n");
+
+    stream.emit("close", 0);
+    await expect(resultPromise).resolves.toMatchObject({ code: 0 });
+  });
+
+  it("closes the channel when stdout exceeds the configured limit", async () => {
+    const { session, stream } = setupSession();
+    const resultPromise = execBuffer(session, "cat file", 4);
+
+    await vi.waitFor(() => expect(stream.listenerCount("data")).toBe(1));
+    stream.emit("data", Buffer.from("12345"));
+
+    await expect(resultPromise).resolves.toMatchObject({
+      code: 1,
+      exceededLimit: true,
+    });
+    expect(stream.close).toHaveBeenCalledOnce();
+  });
+});