feat(desktop): add Workbench/Review mode with FileViewer panes

apps/desktop/src/lib/trpc/routers/changes/branches.ts

@@ -4,6 +4,11 @@ import { localDb } from "main/lib/local-db";
 import simpleGit from "simple-git";
 import { z } from "zod";
 import { publicProcedure, router } from "../..";
+import {
+	assertRegisteredWorktree,
+	getRegisteredWorktree,
+	gitSwitchBranch,
+} from "./security";
 
 export const createBranchesRouter = () => {
 	return router({
@@ -18,6 +23,8 @@ export const createBranchesRouter = () => {
 					defaultBranch: string;
 					checkedOutBranches: Record<string, string>;
 				}> => {
+					assertRegisteredWorktree(input.worktreePath);
+
 					const git = simpleGit(input.worktreePath);
 
 					const branchSummary = await git.branch(["-a"]);
@@ -59,18 +66,11 @@ export const createBranchesRouter = () => {
 				}),
 			)
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
-				const git = simpleGit(input.worktreePath);
-
-				const worktree = localDb
-					.select()
-					.from(worktrees)
-					.where(eq(worktrees.path, input.worktreePath))
-					.get();
-				if (!worktree) {
-					throw new Error(`No worktree found at path "${input.worktreePath}"`);
-				}
+				// Get worktree record for updating branch info
+				const worktree = getRegisteredWorktree(input.worktreePath);
 
-				await git.checkout(input.branch);
+				// Use gitSwitchBranch which uses `git switch` (correct branch syntax)
+				await gitSwitchBranch(input.worktreePath, input.branch);
 
 				// Update the branch in the worktree record
 				const gitStatus = worktree.gitStatus

apps/desktop/src/lib/trpc/routers/changes/file-contents.ts

@@ -1,11 +1,43 @@
-import { readFile, writeFile } from "node:fs/promises";
-import { join } from "node:path";
 import type { FileContents } from "shared/changes-types";
 import simpleGit from "simple-git";
 import { z } from "zod";
 import { publicProcedure, router } from "../..";
+import {
+	assertRegisteredWorktree,
+	PathValidationError,
+	secureFs,
+} from "./security";
 import { detectLanguage } from "./utils/parse-status";
 
+/** Maximum file size for reading (2 MiB) */
+const MAX_FILE_SIZE = 2 * 1024 * 1024;
+
+/** Bytes to scan for binary detection */
+const BINARY_CHECK_SIZE = 8192;
+
+/**
+ * Result type for readWorkingFile procedure
+ */
+type ReadWorkingFileResult =
+	| { ok: true; content: string; truncated: boolean; byteLength: number }
+	| {
+			ok: false;
+			reason: "not-found" | "too-large" | "binary" | "outside-worktree";
+	  };
+
+/**
+ * Detects if a buffer contains binary content by checking for NUL bytes
+ */
+function isBinaryContent(buffer: Buffer): boolean {
+	const checkLength = Math.min(buffer.length, BINARY_CHECK_SIZE);
+	for (let i = 0; i < checkLength; i++) {
+		if (buffer[i] === 0) {
+			return true;
+		}
+	}
+	return false;
+}
+
 export const createFileContentsRouter = () => {
 	return router({
 		getFileContents: publicProcedure
@@ -20,6 +52,8 @@ export const createFileContentsRouter = () => {
 				}),
 			)
 			.query(async ({ input }): Promise<FileContents> => {
+				assertRegisteredWorktree(input.worktreePath);
+
 				const git = simpleGit(input.worktreePath);
 				const defaultBranch = input.defaultBranch || "main";
 				const originalPath = input.oldPath || input.filePath;
@@ -50,10 +84,59 @@ export const createFileContentsRouter = () => {
 				}),
 			)
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
-				const fullPath = join(input.worktreePath, input.filePath);
-				await writeFile(fullPath, input.content, "utf-8");
+				// secureFs.writeFile handles all validation including symlink checks
+				await secureFs.writeFile(
+					input.worktreePath,
+					input.filePath,
+					input.content,
+				);
 				return { success: true };
 			}),
+
+		/**
+		 * Read a working tree file safely with size cap and binary detection.
+		 * Used for File Viewer raw/rendered modes.
+		 */
+		readWorkingFile: publicProcedure
+			.input(
+				z.object({
+					worktreePath: z.string(),
+					filePath: z.string(),
+				}),
+			)
+			.query(async ({ input }): Promise<ReadWorkingFileResult> => {
+				try {
+					// Check file size first (uses stat which follows symlinks)
+					const stats = await secureFs.stat(input.worktreePath, input.filePath);
+					if (stats.size > MAX_FILE_SIZE) {
+						return { ok: false, reason: "too-large" };
+					}
+
+					// Read file content as buffer for binary detection
+					const buffer = await secureFs.readFileBuffer(
+						input.worktreePath,
+						input.filePath,
+					);
+
+					// Check for binary content
+					if (isBinaryContent(buffer)) {
+						return { ok: false, reason: "binary" };
+					}
+
+					return {
+						ok: true,
+						content: buffer.toString("utf-8"),
+						truncated: false,
+						byteLength: buffer.length,
+					};
+				} catch (error) {
+					if (error instanceof PathValidationError) {
+						return { ok: false, reason: "outside-worktree" };
+					}
+					// File not found or other read error
+					return { ok: false, reason: "not-found" };
+				}
+			}),
 	});
 };
 
@@ -91,26 +174,41 @@ async function getFileVersions(
 	}
 }
 
+/** Helper to safely get git show content with size limit and memory protection */
+async function safeGitShow(
+	git: ReturnType<typeof simpleGit>,
+	spec: string,
+): Promise<string> {
+	try {
+		// Preflight: check blob size before loading into memory
+		// This prevents memory spikes from large files in git history
+		try {
+			const sizeOutput = await git.raw(["cat-file", "-s", spec]);
+			const blobSize = Number.parseInt(sizeOutput.trim(), 10);
+			if (!Number.isNaN(blobSize) && blobSize > MAX_FILE_SIZE) {
+				return `[File content truncated - exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit]`;
+			}
+		} catch {
+			// cat-file failed (blob doesn't exist) - let git.show handle the error
+		}
+
+		const content = await git.show([spec]);
+		return content;
+	} catch {
+		return "";
+	}
+}
+
 async function getAgainstBaseVersions(
 	git: ReturnType<typeof simpleGit>,
 	filePath: string,
 	originalPath: string,
 	defaultBranch: string,
 ): Promise<FileVersions> {
-	let original = "";
-	let modified = "";
-
-	try {
-		original = await git.show([`origin/${defaultBranch}:${originalPath}`]);
-	} catch {
-		original = "";
-	}
-
-	try {
-		modified = await git.show([`HEAD:${filePath}`]);
-	} catch {
-		modified = "";
-	}
+	const [original, modified] = await Promise.all([
+		safeGitShow(git, `origin/${defaultBranch}:${originalPath}`),
+		safeGitShow(git, `HEAD:${filePath}`),
+	]);
 
 	return { original, modified };
 }
@@ -121,20 +219,10 @@ async function getCommittedVersions(
 	originalPath: string,
 	commitHash: string,
 ): Promise<FileVersions> {
-	let original = "";
-	let modified = "";
-
-	try {
-		original = await git.show([`${commitHash}^:${originalPath}`]);
-	} catch {
-		original = "";
-	}
-
-	try {
-		modified = await git.show([`${commitHash}:${filePath}`]);
-	} catch {
-		modified = "";
-	}
+	const [original, modified] = await Promise.all([
+		safeGitShow(git, `${commitHash}^:${originalPath}`),
+		safeGitShow(git, `${commitHash}:${filePath}`),
+	]);
 
 	return { original, modified };
 }
@@ -144,20 +232,10 @@ async function getStagedVersions(
 	filePath: string,
 	originalPath: string,
 ): Promise<FileVersions> {
-	let original = "";
-	let modified = "";
-
-	try {
-		original = await git.show([`HEAD:${originalPath}`]);
-	} catch {
-		original = "";
-	}
-
-	try {
-		modified = await git.show([`:0:${filePath}`]);
-	} catch {
-		modified = "";
-	}
+	const [original, modified] = await Promise.all([
+		safeGitShow(git, `HEAD:${originalPath}`),
+		safeGitShow(git, `:0:${filePath}`),
+	]);
 
 	return { original, modified };
 }
@@ -168,22 +246,23 @@ async function getUnstagedVersions(
 	filePath: string,
 	originalPath: string,
 ): Promise<FileVersions> {
-	let original = "";
-	let modified = "";
-
-	try {
-		original = await git.show([`:0:${originalPath}`]);
-	} catch {
-		try {
-			original = await git.show([`HEAD:${originalPath}`]);
-		} catch {
-			original = "";
-		}
+	// Try staged version first, fall back to HEAD
+	let original = await safeGitShow(git, `:0:${originalPath}`);
+	if (!original) {
+		original = await safeGitShow(git, `HEAD:${originalPath}`);
 	}
 
+	let modified = "";
 	try {
-		modified = await readFile(join(worktreePath, filePath), "utf-8");
+		// Check file size before reading (uses stat which follows symlinks)
+		const stats = await secureFs.stat(worktreePath, filePath);
+		if (stats.size <= MAX_FILE_SIZE) {
+			modified = await secureFs.readFile(worktreePath, filePath);
+		} else {
+			modified = `[File content truncated - exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit]`;
+		}
 	} catch {
+		// File doesn't exist or validation failed - that's ok for diff display
 		modified = "";
 	}
 

apps/desktop/src/lib/trpc/routers/changes/git-operations.ts

@@ -1,10 +1,9 @@
-import { writeFile } from "node:fs/promises";
-import { resolve } from "node:path";
 import { shell } from "electron";
 import simpleGit from "simple-git";
 import { z } from "zod";
 import { publicProcedure, router } from "../..";
 import { isUpstreamMissingError } from "./git-utils";
+import { assertRegisteredWorktree } from "./security";
 
 export { isUpstreamMissingError };
 
@@ -21,25 +20,8 @@ async function hasUpstreamBranch(
 
 export const createGitOperationsRouter = () => {
 	return router({
-		saveFile: publicProcedure
-			.input(
-				z.object({
-					worktreePath: z.string(),
-					filePath: z.string(),
-					content: z.string(),
-				}),
-			)
-			.mutation(async ({ input }): Promise<{ success: boolean }> => {
-				const resolvedWorktree = resolve(input.worktreePath);
-				const fullPath = resolve(resolvedWorktree, input.filePath);
-
-				if (!fullPath.startsWith(`${resolvedWorktree}/`)) {
-					throw new Error("Invalid file path: path traversal detected");
-				}
-
-				await writeFile(fullPath, input.content, "utf-8");
-				return { success: true };
-			}),
+		// NOTE: saveFile is defined in file-contents.ts with hardened path validation
+		// Do NOT add saveFile here - it would overwrite the secure version
 
 		commit: publicProcedure
 			.input(
@@ -50,6 +32,9 @@ export const createGitOperationsRouter = () => {
 			)
 			.mutation(
 				async ({ input }): Promise<{ success: boolean; hash: string }> => {
+					// SECURITY: Validate worktreePath exists in localDb
+					assertRegisteredWorktree(input.worktreePath);
+
 					const git = simpleGit(input.worktreePath);
 					const result = await git.commit(input.message);
 					return { success: true, hash: result.commit };
@@ -64,6 +49,9 @@ export const createGitOperationsRouter = () => {
 				}),
 			)
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
+				// SECURITY: Validate worktreePath exists in localDb
+				assertRegisteredWorktree(input.worktreePath);
+
 				const git = simpleGit(input.worktreePath);
 				const hasUpstream = await hasUpstreamBranch(git);
 
@@ -84,6 +72,9 @@ export const createGitOperationsRouter = () => {
 				}),
 			)
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
+				// SECURITY: Validate worktreePath exists in localDb
+				assertRegisteredWorktree(input.worktreePath);
+
 				const git = simpleGit(input.worktreePath);
 				try {
 					await git.pull(["--rebase"]);
@@ -107,6 +98,9 @@ export const createGitOperationsRouter = () => {
 				}),
 			)
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
+				// SECURITY: Validate worktreePath exists in localDb
+				assertRegisteredWorktree(input.worktreePath);
+
 				const git = simpleGit(input.worktreePath);
 				try {
 					await git.pull(["--rebase"]);
@@ -134,6 +128,9 @@ export const createGitOperationsRouter = () => {
 			)
 			.mutation(
 				async ({ input }): Promise<{ success: boolean; url: string }> => {
+					// SECURITY: Validate worktreePath exists in localDb
+					assertRegisteredWorktree(input.worktreePath);
+
 					const git = simpleGit(input.worktreePath);
 					const branch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim();
 					const hasUpstream = await hasUpstreamBranch(git);