From b35f2638b5dc8b51d984e084772dc315d74018a3 Mon Sep 17 00:00:00 2001 From: Andreas Asprou Date: Mon, 29 Dec 2025 18:15:48 +0200 Subject: [PATCH 01/14] feat(desktop): add Workbench/Review mode with FileViewer panes Introduces a workspace-level view mode toggle allowing users to switch between: - **Workbench mode**: Mosaic panes layout with terminals + file viewers for in-flow work - **Review mode**: Dedicated Changes page for focused code review Key changes: - Add ViewModeToggle component in workspace header (prominent segmented control) - Add FileViewerPane with Raw/Rendered/Diff modes, lock/unlock, and split support - Add GroupStrip for group switching above Mosaic content - Unify sidebar to use full ChangesView in both modes (with onFileOpen callback) - Add workspace-view-mode store with per-workspace persistence - Add readWorkingFile tRPC procedure for safe file reads (size/binary checks) - Wire file clicks to open/reuse FileViewer panes (MRU unlocked policy) - Cmd+T in Review mode switches to Workbench first, then creates terminal --- .../lib/trpc/routers/changes/file-contents.ts | 133 ++++- .../TabsContent/GroupStrip/GroupStrip.tsx | 151 +++++ .../TabsContent/GroupStrip/index.ts | 1 + .../TabView/FileViewerPane/FileViewerPane.tsx | 537 ++++++++++++++++++ .../TabView/FileViewerPane/index.ts | 1 + .../ContentView/TabsContent/TabView/index.tsx | 25 + .../ContentView/TabsContent/index.tsx | 10 +- .../WorkspaceView/ContentView/index.tsx | 17 +- .../Sidebar/ChangesView/ChangesView.tsx | 15 +- .../WorkspaceView/Sidebar/index.tsx | 49 +- .../WorkspaceActionBar/WorkspaceActionBar.tsx | 8 +- .../ViewModeToggle/ViewModeToggle.tsx | 55 ++ .../components/ViewModeToggle/index.ts | 1 + .../main/components/WorkspaceView/index.tsx | 18 +- apps/desktop/src/renderer/stores/index.ts | 1 + .../desktop/src/renderer/stores/tabs/store.ts | 109 +++- .../desktop/src/renderer/stores/tabs/types.ts | 15 + .../desktop/src/renderer/stores/tabs/utils.ts | 61 ++ .../renderer/stores/workspace-view-mode.ts | 56 ++ apps/desktop/src/shared/tabs-types.ts | 35 +- 20 files changed, 1267 insertions(+), 31 deletions(-) create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/index.ts create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/index.ts create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx create mode 100644 apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/index.ts create mode 100644 apps/desktop/src/renderer/stores/workspace-view-mode.ts diff --git a/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts b/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts index 8af04dd68e4..29f035a058f 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts @@ -1,11 +1,78 @@ -import { readFile, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { lstat, readFile, realpath, writeFile } from "node:fs/promises"; +import { isAbsolute, join, normalize, relative } from "node:path"; import type { FileContents } from "shared/changes-types"; import simpleGit from "simple-git"; import { z } from "zod"; import { publicProcedure, router } from "../.."; 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"; + }; + +/** + * Validates that a file path is within the worktree and doesn't escape via symlinks + */ +async function validatePathInWorktree( + worktreePath: string, + filePath: string, +): Promise<{ valid: boolean; resolvedPath?: string; reason?: string }> { + // Reject absolute paths + if (isAbsolute(filePath)) { + return { valid: false, reason: "outside-worktree" }; + } + + // Normalize and check for traversal + const normalizedPath = normalize(filePath); + if (normalizedPath.startsWith("..") || normalizedPath.includes("/../")) { + return { valid: false, reason: "outside-worktree" }; + } + + const fullPath = join(worktreePath, normalizedPath); + + // Resolve symlinks and verify the real path is still within worktree + try { + const realWorktreePath = await realpath(worktreePath); + const realFilePath = await realpath(fullPath); + const relativePath = relative(realWorktreePath, realFilePath); + + // If relative path starts with "..", the file is outside worktree + if (relativePath.startsWith("..") || isAbsolute(relativePath)) { + return { valid: false, reason: "outside-worktree" }; + } + + return { valid: true, resolvedPath: realFilePath }; + } catch { + // File doesn't exist + return { valid: false, reason: "not-found" }; + } +} + +/** + * 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 @@ -54,6 +121,68 @@ export const createFileContentsRouter = () => { await writeFile(fullPath, input.content, "utf-8"); return { success: true }; }), + + /** + * Read a working tree file safely with size cap and binary detection. + * Used for File Viewer raw/rendered modes. + * Follows DL-005 (path validation) and DL-008 (size/binary policy). + */ + readWorkingFile: publicProcedure + .input( + z.object({ + worktreePath: z.string(), + filePath: z.string(), + }), + ) + .query(async ({ input }): Promise => { + // Validate path is within worktree + const validation = await validatePathInWorktree( + input.worktreePath, + input.filePath, + ); + + if (!validation.valid || !validation.resolvedPath) { + return { + ok: false, + reason: (validation.reason ?? "not-found") as + | "not-found" + | "outside-worktree", + }; + } + + const resolvedPath = validation.resolvedPath; + + // Check file size + try { + const stats = await lstat(resolvedPath); + if (stats.size > MAX_FILE_SIZE) { + return { ok: false, reason: "too-large" }; + } + } catch { + return { ok: false, reason: "not-found" }; + } + + // Read file content + let buffer: Buffer; + try { + buffer = await readFile(resolvedPath); + } catch { + return { ok: false, reason: "not-found" }; + } + + // Check for binary content + if (isBinaryContent(buffer)) { + return { ok: false, reason: "binary" }; + } + + // Return content as string + return { + ok: true, + content: buffer.toString("utf-8"), + truncated: false, + byteLength: buffer.length, + }; + }), }); }; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx new file mode 100644 index 00000000000..e4aa4e47352 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx @@ -0,0 +1,151 @@ +import { Button } from "@superset/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; +import { useMemo } from "react"; +import { HiMiniPlus, HiMiniXMark } from "react-icons/hi2"; +import { trpc } from "renderer/lib/trpc"; +import { useTabsStore } from "renderer/stores/tabs/store"; +import type { Tab } from "renderer/stores/tabs/types"; +import { getTabDisplayName } from "renderer/stores/tabs/utils"; + +interface GroupItemProps { + tab: Tab; + isActive: boolean; + needsAttention: boolean; + onSelect: () => void; + onClose: () => void; +} + +function GroupItem({ + tab, + isActive, + needsAttention, + onSelect, + onClose, +}: GroupItemProps) { + const displayName = getTabDisplayName(tab); + + return ( +
+ + + + + + {displayName} + + + +
+ ); +} + +export function GroupStrip() { + const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); + const activeWorkspaceId = activeWorkspace?.id; + + const allTabs = useTabsStore((s) => s.tabs); + const panes = useTabsStore((s) => s.panes); + const activeTabIds = useTabsStore((s) => s.activeTabIds); + const addTab = useTabsStore((s) => s.addTab); + const removeTab = useTabsStore((s) => s.removeTab); + const setActiveTab = useTabsStore((s) => s.setActiveTab); + + const tabs = useMemo( + () => + activeWorkspaceId + ? allTabs.filter((tab) => tab.workspaceId === activeWorkspaceId) + : [], + [activeWorkspaceId, allTabs], + ); + + const activeTabId = activeWorkspaceId + ? activeTabIds[activeWorkspaceId] + : null; + + // Check which tabs have panes that need attention + const tabsWithAttention = useMemo(() => { + const result = new Set(); + for (const pane of Object.values(panes)) { + if (pane.needsAttention) { + result.add(pane.tabId); + } + } + return result; + }, [panes]); + + const handleAddGroup = () => { + if (activeWorkspaceId) { + addTab(activeWorkspaceId); + } + }; + + const handleSelectGroup = (tabId: string) => { + if (activeWorkspaceId) { + setActiveTab(activeWorkspaceId, tabId); + } + }; + + const handleCloseGroup = (tabId: string) => { + removeTab(tabId); + }; + + if (tabs.length === 0) { + return null; + } + + return ( +
+
+ {tabs.map((tab) => ( + handleSelectGroup(tab.id)} + onClose={() => handleCloseGroup(tab.id)} + /> + ))} +
+ + + + + + New Group + + +
+ ); +} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/index.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/index.ts new file mode 100644 index 00000000000..e905a6c8bad --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/index.ts @@ -0,0 +1 @@ +export { GroupStrip } from "./GroupStrip"; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx new file mode 100644 index 00000000000..a9dac12d7a5 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx @@ -0,0 +1,537 @@ +import Editor, { type OnMount } from "@monaco-editor/react"; +import { Badge } from "@superset/ui/badge"; +import { ToggleGroup, ToggleGroupItem } from "@superset/ui/toggle-group"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; +import type * as Monaco from "monaco-editor"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { + HiMiniLockClosed, + HiMiniLockOpen, + HiMiniPencil, + HiMiniXMark, +} from "react-icons/hi2"; +import { LuLoader } from "react-icons/lu"; +import { TbLayoutColumns, TbLayoutRows } from "react-icons/tb"; +import type { MosaicBranch } from "react-mosaic-component"; +import { MosaicWindow } from "react-mosaic-component"; +import { MarkdownRenderer } from "renderer/components/MarkdownRenderer"; +import { + monaco, + SUPERSET_THEME, + useMonacoReady, +} from "renderer/contexts/MonacoProvider"; +import { trpc } from "renderer/lib/trpc"; +import { useTabsStore } from "renderer/stores/tabs/store"; +import type { Pane } from "renderer/stores/tabs/types"; +import type { FileViewerMode } from "shared/tabs-types"; +import { DiffViewer } from "../../../ChangesContent/components/DiffViewer"; + +type SplitOrientation = "vertical" | "horizontal"; + +/** Client-side language detection for Monaco editor */ +function detectLanguage(filePath: string): string { + const ext = filePath.split(".").pop()?.toLowerCase() ?? ""; + const languageMap: Record = { + ts: "typescript", + tsx: "typescript", + js: "javascript", + jsx: "javascript", + mjs: "javascript", + cjs: "javascript", + json: "json", + md: "markdown", + mdx: "markdown", + css: "css", + scss: "scss", + less: "less", + html: "html", + xml: "xml", + yaml: "yaml", + yml: "yaml", + py: "python", + rs: "rust", + go: "go", + java: "java", + kt: "kotlin", + swift: "swift", + c: "c", + cpp: "cpp", + h: "c", + hpp: "cpp", + sh: "shell", + bash: "shell", + zsh: "shell", + sql: "sql", + graphql: "graphql", + gql: "graphql", + }; + return languageMap[ext] ?? "plaintext"; +} + +interface FileViewerPaneProps { + paneId: string; + path: MosaicBranch[]; + pane: Pane; + isActive: boolean; + tabId: string; + worktreePath: string; + splitPaneAuto: ( + tabId: string, + sourcePaneId: string, + dimensions: { width: number; height: number }, + path?: MosaicBranch[], + ) => void; + removePane: (paneId: string) => void; + setFocusedPane: (tabId: string, paneId: string) => void; +} + +export function FileViewerPane({ + paneId, + path, + pane, + isActive, + tabId, + worktreePath, + splitPaneAuto, + removePane, + setFocusedPane, +}: FileViewerPaneProps) { + const containerRef = useRef(null); + const [splitOrientation, setSplitOrientation] = + useState("vertical"); + const isMonacoReady = useMonacoReady(); + const editorRef = useRef(null); + const [isDirty, setIsDirty] = useState(false); + const originalContentRef = useRef(""); + const utils = trpc.useUtils(); + + // Track container dimensions for auto-split orientation + useEffect(() => { + const container = containerRef.current; + if (!container) return; + + const updateOrientation = () => { + const { width, height } = container.getBoundingClientRect(); + setSplitOrientation(width >= height ? "vertical" : "horizontal"); + }; + + updateOrientation(); + + const resizeObserver = new ResizeObserver(updateOrientation); + resizeObserver.observe(container); + + return () => { + resizeObserver.disconnect(); + }; + }, []); + + const fileViewer = pane.fileViewer; + + // Extract values with defaults for hooks (hooks must be called unconditionally) + const filePath = fileViewer?.filePath ?? ""; + const viewMode = fileViewer?.viewMode ?? "raw"; + const isLocked = fileViewer?.isLocked ?? false; + const diffCategory = fileViewer?.diffCategory; + const commitHash = fileViewer?.commitHash; + const oldPath = fileViewer?.oldPath; + + // Save mutation + const saveFileMutation = trpc.changes.saveFile.useMutation({ + onSuccess: () => { + setIsDirty(false); + // Update original content to current content after save + if (editorRef.current) { + originalContentRef.current = editorRef.current.getValue(); + } + // Invalidate queries to refresh data + utils.changes.readWorkingFile.invalidate(); + utils.changes.getFileContents.invalidate(); + utils.changes.getStatus.invalidate(); + }, + }); + + // Save handler for raw mode editor + const handleSaveRaw = useCallback(() => { + if (!editorRef.current || !filePath) return; + saveFileMutation.mutate({ + worktreePath, + filePath, + content: editorRef.current.getValue(), + }); + }, [worktreePath, filePath, saveFileMutation]); + + // Save handler for diff mode + const handleSaveDiff = useCallback( + (content: string) => { + if (!filePath) return; + saveFileMutation.mutate({ + worktreePath, + filePath, + content, + }); + }, + [worktreePath, filePath, saveFileMutation], + ); + + // Editor mount handler - set up Cmd+S keybinding + const handleEditorMount: OnMount = useCallback( + (editor) => { + editorRef.current = editor; + // Store original content for dirty tracking + originalContentRef.current = editor.getValue(); + + // Register save action with Cmd+S / Ctrl+S + editor.addAction({ + id: "save-file", + label: "Save File", + keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS], + run: () => { + handleSaveRaw(); + }, + }); + }, + [handleSaveRaw], + ); + + // Track content changes for dirty state + const handleEditorChange = useCallback((value: string | undefined) => { + if (value !== undefined) { + setIsDirty(value !== originalContentRef.current); + } + }, []); + + // Reset dirty state when file changes + // biome-ignore lint/correctness/useExhaustiveDependencies: Reset on file change only + useEffect(() => { + setIsDirty(false); + originalContentRef.current = ""; + }, [filePath]); + + // Fetch raw file content - always call hook, use enabled to control fetching + const { data: rawFileData, isLoading: isLoadingRaw } = + trpc.changes.readWorkingFile.useQuery( + { worktreePath, filePath }, + { enabled: !!fileViewer && viewMode !== "diff" && !!filePath }, + ); + + // Fetch diff content - always call hook, use enabled to control fetching + const { data: diffData, isLoading: isLoadingDiff } = + trpc.changes.getFileContents.useQuery( + { + worktreePath, + filePath, + oldPath, + category: diffCategory ?? "unstaged", + commitHash, + }, + { + enabled: + !!fileViewer && viewMode === "diff" && !!diffCategory && !!filePath, + }, + ); + + // Early return AFTER hooks + if (!fileViewer) { + return ( + path={path} title=""> +
+ No file viewer state +
+ + ); + } + + const handleFocus = () => { + setFocusedPane(tabId, paneId); + }; + + const handleClosePane = (e: React.MouseEvent) => { + e.stopPropagation(); + removePane(paneId); + }; + + const handleSplitPane = (e: React.MouseEvent) => { + e.stopPropagation(); + const container = containerRef.current; + if (!container) return; + + const { width, height } = container.getBoundingClientRect(); + splitPaneAuto(tabId, paneId, { width, height }, path); + }; + + const handleToggleLock = () => { + // Update the pane's lock state in the store + const panes = useTabsStore.getState().panes; + const currentPane = panes[paneId]; + if (currentPane?.fileViewer) { + useTabsStore.setState({ + panes: { + ...panes, + [paneId]: { + ...currentPane, + fileViewer: { + ...currentPane.fileViewer, + isLocked: !currentPane.fileViewer.isLocked, + }, + }, + }, + }); + } + }; + + const handleViewModeChange = (value: string) => { + if (!value) return; + const newMode = value as FileViewerMode; + + // Update the pane's view mode in the store + const panes = useTabsStore.getState().panes; + const currentPane = panes[paneId]; + if (currentPane?.fileViewer) { + useTabsStore.setState({ + panes: { + ...panes, + [paneId]: { + ...currentPane, + fileViewer: { + ...currentPane.fileViewer, + viewMode: newMode, + }, + }, + }, + }); + } + }; + + const fileName = filePath.split("/").pop() || filePath; + + // Render content based on view mode + const renderContent = () => { + if (viewMode === "diff") { + if (isLoadingDiff) { + return ( +
+ Loading diff... +
+ ); + } + if (!diffData) { + return ( +
+ No diff available +
+ ); + } + return ( + + ); + } + + if (isLoadingRaw) { + return ( +
+ Loading... +
+ ); + } + + if (!rawFileData?.ok) { + const errorMessage = + rawFileData?.reason === "too-large" + ? "File is too large to preview" + : rawFileData?.reason === "binary" + ? "Binary file preview not supported" + : rawFileData?.reason === "outside-worktree" + ? "File is outside worktree" + : "File not found"; + return ( +
+ {errorMessage} +
+ ); + } + + if (viewMode === "rendered") { + return ( +
+ +
+ ); + } + + // Raw mode - editable Monaco editor + if (!isMonacoReady) { + return ( +
+ + Loading editor... +
+ ); + } + + return ( + + + Loading editor... + + } + options={{ + minimap: { enabled: false }, + scrollBeyondLastLine: false, + wordWrap: "on", + fontSize: 13, + lineHeight: 20, + fontFamily: + "ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace", + padding: { top: 8, bottom: 8 }, + scrollbar: { + verticalScrollbarSize: 8, + horizontalScrollbarSize: 8, + }, + }} + /> + ); + }; + + // Determine which view modes are available + const isMarkdown = filePath.endsWith(".md") || filePath.endsWith(".markdown"); + const hasDiff = !!diffCategory; + + const splitIcon = + splitOrientation === "vertical" ? ( + + ) : ( + + ); + + // Show editable badge for raw and diff modes (not rendered) + const showEditableBadge = viewMode !== "rendered"; + const isSaving = saveFileMutation.isPending; + + return ( + + path={path} + title="" + renderToolbar={() => ( +
+
+ + {isDirty && ●} + {fileName} + + {showEditableBadge && ( + + + {isSaving ? "Saving..." : "⌘S"} + + )} +
+
+ + {isMarkdown && ( + + Rendered + + )} + + Raw + + {hasDiff && ( + + Diff + + )} + + + + + + + Split pane + + + + + + + + {isLocked + ? "Unlock (allow file replacement)" + : "Lock (prevent file replacement)"} + + + + + + + + Close + + +
+
+ )} + className={isActive ? "mosaic-window-focused" : ""} + > + {/* biome-ignore lint/a11y/useKeyWithClickEvents lint/a11y/noStaticElementInteractions: Focus handler */} +
+ {renderContent()} +
+ + ); +} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/index.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/index.ts new file mode 100644 index 00000000000..96c33fa0b12 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/index.ts @@ -0,0 +1 @@ +export { FileViewerPane } from "./FileViewerPane"; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx index b6df3608e13..3502a82dce2 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx @@ -8,6 +8,7 @@ import { type MosaicNode, } from "react-mosaic-component"; import { dragDropManager } from "renderer/lib/dnd"; +import { trpc } from "renderer/lib/trpc"; import { useTabsStore } from "renderer/stores/tabs/store"; import type { Pane, Tab } from "renderer/stores/tabs/types"; import { @@ -15,6 +16,7 @@ import { extractPaneIdsFromLayout, getPaneIdsForTab, } from "renderer/stores/tabs/utils"; +import { FileViewerPane } from "./FileViewerPane"; import { TabPane } from "./TabPane"; interface TabViewProps { @@ -35,6 +37,10 @@ export function TabView({ tab, panes }: TabViewProps) { const movePaneToNewTab = useTabsStore((s) => s.movePaneToNewTab); const allTabs = useTabsStore((s) => s.tabs); + // Get worktree path for file viewer panes + const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); + const worktreePath = activeWorkspace?.worktreePath ?? ""; + // Get tabs in the same workspace for move targets const workspaceTabs = allTabs.filter( (t) => t.workspaceId === tab.workspaceId, @@ -90,6 +96,24 @@ export function TabView({ tab, panes }: TabViewProps) { ); } + // Route file-viewer panes to FileViewerPane component + if (pane.type === "file-viewer") { + return ( + + ); + } + + // Default: terminal panes return ( ; } - return ; + return ( +
+ +
+ +
+
+ ); } diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx index 1a2e20bd0fb..835e914fc05 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx @@ -1,11 +1,22 @@ -import { SidebarMode, useSidebarStore } from "renderer/stores"; +import { trpc } from "renderer/lib/trpc"; +import { useWorkspaceViewModeStore } from "renderer/stores/workspace-view-mode"; import { ChangesContent } from "./ChangesContent"; import { TabsContent } from "./TabsContent"; export function ContentView() { - const { currentMode } = useSidebarStore(); + const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); + const workspaceId = activeWorkspace?.id; - if (currentMode === SidebarMode.Changes) { + // Subscribe to the actual data, not just the getter function + const viewModeByWorkspaceId = useWorkspaceViewModeStore( + (s) => s.viewModeByWorkspaceId, + ); + + const viewMode = workspaceId + ? (viewModeByWorkspaceId[workspaceId] ?? "workbench") + : "workbench"; + + if (viewMode === "review") { return (
diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx index ee0ec6d2e9c..b269533cc7f 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx @@ -6,13 +6,22 @@ import { HiMiniMinus, HiMiniPlus } from "react-icons/hi2"; import { trpc } from "renderer/lib/trpc"; import { useChangesStore } from "renderer/stores/changes"; import type { ChangeCategory, ChangedFile } from "shared/changes-types"; +import { PortsList } from "../TabsView/PortsList"; import { CategorySection } from "./components/CategorySection"; import { ChangesHeader } from "./components/ChangesHeader"; import { CommitInput } from "./components/CommitInput"; import { CommitItem } from "./components/CommitItem"; import { FileList } from "./components/FileList"; -export function ChangesView() { +interface ChangesViewProps { + onFileOpen?: ( + file: ChangedFile, + category: ChangeCategory, + commitHash?: string, + ) => void; +} + +export function ChangesView({ onFileOpen }: ChangesViewProps) { const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); const worktreePath = activeWorkspace?.worktreePath; @@ -128,11 +137,13 @@ export function ChangesView() { const handleFileSelect = (file: ChangedFile, category: ChangeCategory) => { if (!worktreePath) return; selectFile(worktreePath, file, category, null); + onFileOpen?.(file, category); }; const handleCommitFileSelect = (file: ChangedFile, commitHash: string) => { if (!worktreePath) return; selectFile(worktreePath, file, "committed", commitHash); + onFileOpen?.(file, "committed", commitHash); }; const handleCommitToggle = (hash: string) => { @@ -349,6 +360,8 @@ export function ChangesView() {
)} + +
); } diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx index 7511587610a..b93ef1f3849 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx @@ -1,29 +1,40 @@ -import { useSidebarStore } from "renderer/stores"; -import { SidebarMode } from "renderer/stores/sidebar-state"; +import { trpc } from "renderer/lib/trpc"; +import { useTabsStore } from "renderer/stores/tabs/store"; +import { useWorkspaceViewModeStore } from "renderer/stores/workspace-view-mode"; +import type { ChangeCategory, ChangedFile } from "shared/changes-types"; import { ChangesView } from "./ChangesView"; -import { ModeCarousel } from "./ModeCarousel"; -import { TabsView } from "./TabsView"; export function Sidebar() { - const { currentMode, setMode } = useSidebarStore(); + const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); + const workspaceId = activeWorkspace?.id; - const modes: SidebarMode[] = [SidebarMode.Tabs, SidebarMode.Changes]; + // Subscribe to the actual data, not just the getter function + const viewModeByWorkspaceId = useWorkspaceViewModeStore( + (s) => s.viewModeByWorkspaceId, + ); + + const viewMode = workspaceId + ? (viewModeByWorkspaceId[workspaceId] ?? "workbench") + : "workbench"; + + const addFileViewerPane = useTabsStore((s) => s.addFileViewerPane); + + // In Workbench mode, open files in FileViewerPane + const handleFileOpen = + viewMode === "workbench" && workspaceId + ? (file: ChangedFile, category: ChangeCategory, commitHash?: string) => { + addFileViewerPane(workspaceId, { + filePath: file.path, + diffCategory: category, + commitHash, + oldPath: file.oldPath, + }); + } + : undefined; return ( ); } diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/WorkspaceActionBar.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/WorkspaceActionBar.tsx index f641fde5af1..526bca55e10 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/WorkspaceActionBar.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/WorkspaceActionBar.tsx @@ -1,3 +1,4 @@ +import { ViewModeToggle } from "./components/ViewModeToggle"; import { WorkspaceActionBarLeft } from "./components/WorkspaceActionBarLeft"; import { WorkspaceActionBarRight } from "./components/WorkspaceActionBarRight"; @@ -9,11 +10,14 @@ export function WorkspaceActionBar({ worktreePath }: WorkspaceActionBarProps) { if (!worktreePath) return null; return ( -
+
-
+
+ +
+
diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx new file mode 100644 index 00000000000..bce7ecd1289 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx @@ -0,0 +1,55 @@ +import { cn } from "@superset/ui/utils"; +import { trpc } from "renderer/lib/trpc"; +import { + useWorkspaceViewModeStore, + type WorkspaceViewMode, +} from "renderer/stores/workspace-view-mode"; + +export function ViewModeToggle() { + const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); + const workspaceId = activeWorkspace?.id; + + const viewModeByWorkspaceId = useWorkspaceViewModeStore( + (s) => s.viewModeByWorkspaceId, + ); + const setWorkspaceViewMode = useWorkspaceViewModeStore( + (s) => s.setWorkspaceViewMode, + ); + + if (!workspaceId) return null; + + const currentMode = viewModeByWorkspaceId[workspaceId] ?? "workbench"; + + const handleModeChange = (mode: WorkspaceViewMode) => { + setWorkspaceViewMode(workspaceId, mode); + }; + + return ( +
+ + +
+ ); +} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/index.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/index.ts new file mode 100644 index 00000000000..5e69ac17ef8 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/index.ts @@ -0,0 +1 @@ +export { ViewModeToggle } from "./ViewModeToggle"; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx index 661543c8580..a1d61bf3e53 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx @@ -3,6 +3,7 @@ import { trpc } from "renderer/lib/trpc"; import { useAppHotkey } from "renderer/stores/hotkeys"; import { useTabsStore } from "renderer/stores/tabs/store"; import { getNextPaneId, getPreviousPaneId } from "renderer/stores/tabs/utils"; +import { useWorkspaceViewModeStore } from "renderer/stores/workspace-view-mode"; import { ContentView } from "./ContentView"; import { ResizableSidebar } from "./ResizableSidebar"; import { WorkspaceActionBar } from "./WorkspaceActionBar"; @@ -39,16 +40,31 @@ export function WorkspaceView() { // Get focused pane ID for the active tab const focusedPaneId = activeTabId ? focusedPaneIds[activeTabId] : null; + // View mode for terminal creation - subscribe to actual data for reactivity + const viewModeByWorkspaceId = useWorkspaceViewModeStore( + (s) => s.viewModeByWorkspaceId, + ); + const setWorkspaceViewMode = useWorkspaceViewModeStore( + (s) => s.setWorkspaceViewMode, + ); + const viewMode = activeWorkspaceId + ? (viewModeByWorkspaceId[activeWorkspaceId] ?? "workbench") + : "workbench"; + // Tab management shortcuts useAppHotkey( "NEW_TERMINAL", () => { if (activeWorkspaceId) { + // If in Review mode, switch to Workbench first + if (viewMode === "review") { + setWorkspaceViewMode(activeWorkspaceId, "workbench"); + } addTab(activeWorkspaceId); } }, undefined, - [activeWorkspaceId, addTab], + [activeWorkspaceId, addTab, viewMode, setWorkspaceViewMode], ); useAppHotkey( diff --git a/apps/desktop/src/renderer/stores/index.ts b/apps/desktop/src/renderer/stores/index.ts index b289f0bfb38..886d0523e00 100644 --- a/apps/desktop/src/renderer/stores/index.ts +++ b/apps/desktop/src/renderer/stores/index.ts @@ -6,3 +6,4 @@ export * from "./ringtone"; export * from "./sidebar-state"; export * from "./tabs"; export * from "./theme"; +export * from "./workspace-view-mode"; diff --git a/apps/desktop/src/renderer/stores/tabs/store.ts b/apps/desktop/src/renderer/stores/tabs/store.ts index d28d8316add..b139bf235fd 100644 --- a/apps/desktop/src/renderer/stores/tabs/store.ts +++ b/apps/desktop/src/renderer/stores/tabs/store.ts @@ -4,9 +4,10 @@ import { create } from "zustand"; import { devtools, persist } from "zustand/middleware"; import { trpcTabsStorage } from "../../lib/trpc-storage"; import { movePaneToNewTab, movePaneToTab } from "./actions/move-pane"; -import type { TabsState, TabsStore } from "./types"; +import type { AddFileViewerPaneOptions, TabsState, TabsStore } from "./types"; import { type CreatePaneOptions, + createFileViewerPane, createPane, createTabWithPane, extractPaneIdsFromLayout, @@ -340,6 +341,112 @@ export const useTabsStore = create()( return newPane.id; }, + addFileViewerPane: ( + workspaceId: string, + options: AddFileViewerPaneOptions, + ) => { + const state = get(); + const activeTabId = state.activeTabIds[workspaceId]; + const activeTab = state.tabs.find((t) => t.id === activeTabId); + + // If no active tab, create a new one (this shouldn't normally happen) + if (!activeTab) { + const { tabId, paneId } = get().addTab(workspaceId); + // Update the pane to be a file-viewer + const pane = state.panes[paneId]; + if (pane) { + const fileViewerPane = createFileViewerPane(tabId, options); + set((s) => ({ + panes: { + ...s.panes, + [paneId]: { + ...fileViewerPane, + id: paneId, // Keep the original ID + }, + }, + })); + } + return paneId; + } + + // Look for an existing unlocked file-viewer pane in the active tab + const tabPaneIds = extractPaneIdsFromLayout(activeTab.layout); + const fileViewerPanes = tabPaneIds + .map((id) => state.panes[id]) + .filter( + (p) => + p?.type === "file-viewer" && + p.fileViewer && + !p.fileViewer.isLocked, + ); + + // If we found an unlocked file-viewer pane, reuse it + if (fileViewerPanes.length > 0) { + const paneToReuse = fileViewerPanes[0]; + const fileName = + options.filePath.split("/").pop() || options.filePath; + + // Determine default view mode + let viewMode: "raw" | "rendered" | "diff" = "raw"; + if (options.diffCategory) { + viewMode = "diff"; + } else if ( + options.filePath.endsWith(".md") || + options.filePath.endsWith(".markdown") + ) { + viewMode = "rendered"; + } + + set({ + panes: { + ...state.panes, + [paneToReuse.id]: { + ...paneToReuse, + name: fileName, + fileViewer: { + filePath: options.filePath, + viewMode, + isLocked: false, + diffLayout: "inline", + diffCategory: options.diffCategory, + commitHash: options.commitHash, + oldPath: options.oldPath, + }, + }, + }, + focusedPaneIds: { + ...state.focusedPaneIds, + [activeTab.id]: paneToReuse.id, + }, + }); + + return paneToReuse.id; + } + + // No reusable pane found, create a new one + const newPane = createFileViewerPane(activeTab.id, options); + + const newLayout: MosaicNode = { + direction: "row", + first: activeTab.layout, + second: newPane.id, + splitPercentage: 50, + }; + + set({ + tabs: state.tabs.map((t) => + t.id === activeTab.id ? { ...t, layout: newLayout } : t, + ), + panes: { ...state.panes, [newPane.id]: newPane }, + focusedPaneIds: { + ...state.focusedPaneIds, + [activeTab.id]: newPane.id, + }, + }); + + return newPane.id; + }, + removePane: (paneId) => { const state = get(); const pane = state.panes[paneId]; diff --git a/apps/desktop/src/renderer/stores/tabs/types.ts b/apps/desktop/src/renderer/stores/tabs/types.ts index bcb0f70af82..03fc45d921b 100644 --- a/apps/desktop/src/renderer/stores/tabs/types.ts +++ b/apps/desktop/src/renderer/stores/tabs/types.ts @@ -1,4 +1,5 @@ import type { MosaicBranch, MosaicNode } from "react-mosaic-component"; +import type { ChangeCategory } from "shared/changes-types"; import type { BaseTab, BaseTabsState, Pane, PaneType } from "shared/tabs-types"; // Re-export shared types @@ -28,6 +29,16 @@ export interface AddTabOptions { initialCwd?: string; } +/** + * Options for opening a file in a file-viewer pane + */ +export interface AddFileViewerPaneOptions { + filePath: string; + diffCategory?: ChangeCategory; + commitHash?: string; + oldPath?: string; +} + /** * Actions available on the tabs store */ @@ -51,6 +62,10 @@ export interface TabsStore extends TabsState { // Pane operations addPane: (tabId: string, options?: AddTabOptions) => string; + addFileViewerPane: ( + workspaceId: string, + options: AddFileViewerPaneOptions, + ) => string; removePane: (paneId: string) => void; setFocusedPane: (tabId: string, paneId: string) => void; markPaneAsUsed: (paneId: string) => void; diff --git a/apps/desktop/src/renderer/stores/tabs/utils.ts b/apps/desktop/src/renderer/stores/tabs/utils.ts index a1e7bef16cf..207ac8295a7 100644 --- a/apps/desktop/src/renderer/stores/tabs/utils.ts +++ b/apps/desktop/src/renderer/stores/tabs/utils.ts @@ -1,4 +1,10 @@ import type { MosaicBranch, MosaicNode } from "react-mosaic-component"; +import type { ChangeCategory } from "shared/changes-types"; +import type { + DiffLayout, + FileViewerMode, + FileViewerState, +} from "shared/tabs-types"; import type { Pane, PaneType, Tab } from "./types"; /** @@ -82,6 +88,61 @@ export const createPane = ( }; }; +/** + * Options for creating a file-viewer pane + */ +export interface CreateFileViewerPaneOptions { + filePath: string; + viewMode?: FileViewerMode; + isLocked?: boolean; + diffLayout?: DiffLayout; + diffCategory?: ChangeCategory; + commitHash?: string; + oldPath?: string; +} + +/** + * Creates a new file-viewer pane with the given properties + */ +export const createFileViewerPane = ( + tabId: string, + options: CreateFileViewerPaneOptions, +): Pane => { + const id = generateId("pane"); + + // Determine default view mode based on file and category + let defaultViewMode: FileViewerMode = "raw"; + if (options.diffCategory) { + defaultViewMode = "diff"; + } else if ( + options.filePath.endsWith(".md") || + options.filePath.endsWith(".markdown") + ) { + defaultViewMode = "rendered"; + } + + const fileViewer: FileViewerState = { + filePath: options.filePath, + viewMode: options.viewMode ?? defaultViewMode, + isLocked: options.isLocked ?? false, + diffLayout: options.diffLayout ?? "inline", + diffCategory: options.diffCategory, + commitHash: options.commitHash, + oldPath: options.oldPath, + }; + + // Use filename for display name + const fileName = options.filePath.split("/").pop() || options.filePath; + + return { + id, + tabId, + type: "file-viewer", + name: fileName, + fileViewer, + }; +}; + /** * Generates a static tab name based on existing tabs * (e.g., "Terminal 1", "Terminal 2", finding the next available number) diff --git a/apps/desktop/src/renderer/stores/workspace-view-mode.ts b/apps/desktop/src/renderer/stores/workspace-view-mode.ts new file mode 100644 index 00000000000..b9bee9b2665 --- /dev/null +++ b/apps/desktop/src/renderer/stores/workspace-view-mode.ts @@ -0,0 +1,56 @@ +import { create } from "zustand"; +import { devtools, persist } from "zustand/middleware"; + +/** + * Workspace view modes: + * - "workbench": Groups + Mosaic panes layout for in-flow work + * - "review": Dedicated Changes page for focused code review + */ +export type WorkspaceViewMode = "workbench" | "review"; + +interface WorkspaceViewModeState { + /** + * Per-workspace view mode. Defaults to "workbench" when not set. + */ + viewModeByWorkspaceId: Record; + + /** + * Get the view mode for a workspace, defaulting to "workbench" + */ + getWorkspaceViewMode: (workspaceId: string) => WorkspaceViewMode; + + /** + * Set the view mode for a workspace + */ + setWorkspaceViewMode: (workspaceId: string, mode: WorkspaceViewMode) => void; +} + +export const useWorkspaceViewModeStore = create()( + devtools( + persist( + (set, get) => ({ + viewModeByWorkspaceId: {}, + + getWorkspaceViewMode: (workspaceId: string) => { + return get().viewModeByWorkspaceId[workspaceId] ?? "workbench"; + }, + + setWorkspaceViewMode: ( + workspaceId: string, + mode: WorkspaceViewMode, + ) => { + set((state) => ({ + viewModeByWorkspaceId: { + ...state.viewModeByWorkspaceId, + [workspaceId]: mode, + }, + })); + }, + }), + { + name: "workspace-view-mode-store", + }, + ), + { name: "WorkspaceViewModeStore" }, + ), +); diff --git a/apps/desktop/src/shared/tabs-types.ts b/apps/desktop/src/shared/tabs-types.ts index 8ae323601eb..d38ce4c4284 100644 --- a/apps/desktop/src/shared/tabs-types.ts +++ b/apps/desktop/src/shared/tabs-types.ts @@ -3,10 +3,42 @@ * Renderer extends these with MosaicNode layout specifics. */ +import type { ChangeCategory } from "./changes-types"; + /** * Pane types that can be displayed within a tab */ -export type PaneType = "terminal" | "webview"; +export type PaneType = "terminal" | "webview" | "file-viewer"; + +/** + * File viewer display modes + */ +export type FileViewerMode = "rendered" | "raw" | "diff"; + +/** + * Diff layout options for file viewer + */ +export type DiffLayout = "inline" | "side-by-side"; + +/** + * File viewer pane-specific properties + */ +export interface FileViewerState { + /** Worktree-relative file path */ + filePath: string; + /** Display mode: rendered (markdown), raw (source), or diff */ + viewMode: FileViewerMode; + /** If true, this pane won't be reused for new file clicks */ + isLocked: boolean; + /** Diff display layout */ + diffLayout: DiffLayout; + /** Category for diff source (against-main, committed, staged, unstaged) */ + diffCategory?: ChangeCategory; + /** Commit hash for committed category diffs */ + commitHash?: string; + /** Original path for renamed files */ + oldPath?: string; +} /** * Base Pane interface - shared between main and renderer @@ -23,6 +55,7 @@ export interface Pane { url?: string; // For webview panes cwd?: string | null; // Current working directory cwdConfirmed?: boolean; // True if cwd confirmed via OSC-7, false if seeded + fileViewer?: FileViewerState; // For file-viewer panes } /** From 0d91d554d99fac1dc31bebdd1d91cfb3270676b5 Mon Sep 17 00:00:00 2001 From: Andreas Asprou Date: Mon, 29 Dec 2025 18:32:50 +0200 Subject: [PATCH 02/14] fix(desktop): add worktreePath guards and fix stale state in tabs store - Add !!worktreePath checks to FileViewerPane query enabled conditions - Add !!worktreePath checks to save handler guards (handleSaveRaw, handleSaveDiff) - Fix stale state reference after addTab() in addFileViewerPane action Addresses CodeRabbit review feedback. --- .../TabView/FileViewerPane/FileViewerPane.tsx | 15 ++++++++---- .../desktop/src/renderer/stores/tabs/store.ts | 23 ++++++++----------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx index a9dac12d7a5..a607ed7b184 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx @@ -152,7 +152,7 @@ export function FileViewerPane({ // Save handler for raw mode editor const handleSaveRaw = useCallback(() => { - if (!editorRef.current || !filePath) return; + if (!editorRef.current || !filePath || !worktreePath) return; saveFileMutation.mutate({ worktreePath, filePath, @@ -163,7 +163,7 @@ export function FileViewerPane({ // Save handler for diff mode const handleSaveDiff = useCallback( (content: string) => { - if (!filePath) return; + if (!filePath || !worktreePath) return; saveFileMutation.mutate({ worktreePath, filePath, @@ -211,7 +211,10 @@ export function FileViewerPane({ const { data: rawFileData, isLoading: isLoadingRaw } = trpc.changes.readWorkingFile.useQuery( { worktreePath, filePath }, - { enabled: !!fileViewer && viewMode !== "diff" && !!filePath }, + { + enabled: + !!fileViewer && viewMode !== "diff" && !!filePath && !!worktreePath, + }, ); // Fetch diff content - always call hook, use enabled to control fetching @@ -226,7 +229,11 @@ export function FileViewerPane({ }, { enabled: - !!fileViewer && viewMode === "diff" && !!diffCategory && !!filePath, + !!fileViewer && + viewMode === "diff" && + !!diffCategory && + !!filePath && + !!worktreePath, }, ); diff --git a/apps/desktop/src/renderer/stores/tabs/store.ts b/apps/desktop/src/renderer/stores/tabs/store.ts index b139bf235fd..ee1943fe7a7 100644 --- a/apps/desktop/src/renderer/stores/tabs/store.ts +++ b/apps/desktop/src/renderer/stores/tabs/store.ts @@ -352,20 +352,17 @@ export const useTabsStore = create()( // If no active tab, create a new one (this shouldn't normally happen) if (!activeTab) { const { tabId, paneId } = get().addTab(workspaceId); - // Update the pane to be a file-viewer - const pane = state.panes[paneId]; - if (pane) { - const fileViewerPane = createFileViewerPane(tabId, options); - set((s) => ({ - panes: { - ...s.panes, - [paneId]: { - ...fileViewerPane, - id: paneId, // Keep the original ID - }, + // Update the pane to be a file-viewer (must use set() to get fresh state after addTab) + const fileViewerPane = createFileViewerPane(tabId, options); + set((s) => ({ + panes: { + ...s.panes, + [paneId]: { + ...fileViewerPane, + id: paneId, // Keep the original ID }, - })); - } + }, + })); return paneId; } From 530a423d3e07e601937f519782abee5eabd78e78 Mon Sep 17 00:00:00 2001 From: Andreas Asprou Date: Mon, 29 Dec 2025 18:50:33 +0200 Subject: [PATCH 03/14] fix(desktop): address CodeRabbit review - path traversal security fix and UX improvements - Add validatePathForWrite() to prevent path traversal attacks in saveFile - Add aria-pressed attribute to ViewModeToggle buttons for accessibility - Increase close button touch target in GroupStrip for better UX - Add .mdx file support for rendered view mode --- .../lib/trpc/routers/changes/file-contents.ts | 62 ++++++++++++++++++- .../TabsContent/GroupStrip/GroupStrip.tsx | 4 +- .../ViewModeToggle/ViewModeToggle.tsx | 2 + .../desktop/src/renderer/stores/tabs/store.ts | 3 +- .../desktop/src/renderer/stores/tabs/utils.ts | 3 +- 5 files changed, 67 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts b/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts index 29f035a058f..262a03797f1 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/file-contents.ts @@ -23,7 +23,8 @@ type ReadWorkingFileResult = }; /** - * Validates that a file path is within the worktree and doesn't escape via symlinks + * Validates that a file path is within the worktree and doesn't escape via symlinks. + * Requires the file to exist (uses realpath). */ async function validatePathInWorktree( worktreePath: string, @@ -60,6 +61,48 @@ async function validatePathInWorktree( } } +/** + * Validates that a file path is safe for writing within the worktree. + * Does not require the file to exist (validates path structure and parent directory). + */ +async function validatePathForWrite( + worktreePath: string, + filePath: string, +): Promise<{ valid: boolean; resolvedPath?: string; reason?: string }> { + // Reject absolute paths + if (isAbsolute(filePath)) { + return { valid: false, reason: "outside-worktree" }; + } + + // Normalize and check for traversal + const normalizedPath = normalize(filePath); + if (normalizedPath.startsWith("..") || normalizedPath.includes("/../")) { + return { valid: false, reason: "outside-worktree" }; + } + + const fullPath = join(worktreePath, normalizedPath); + + // Resolve the worktree path and verify our target path is within it + try { + const realWorktreePath = await realpath(worktreePath); + + // For writes, we can't realpath the file (it may not exist), but we can check + // the normalized path structure is within the worktree + const candidatePath = join(realWorktreePath, normalizedPath); + const relativePath = relative(realWorktreePath, candidatePath); + + // If relative path starts with "..", the file is outside worktree + if (relativePath.startsWith("..") || isAbsolute(relativePath)) { + return { valid: false, reason: "outside-worktree" }; + } + + return { valid: true, resolvedPath: candidatePath }; + } catch { + // Worktree path doesn't exist or isn't accessible + return { valid: false, reason: "not-found" }; + } +} + /** * Detects if a buffer contains binary content by checking for NUL bytes */ @@ -117,8 +160,21 @@ export const createFileContentsRouter = () => { }), ) .mutation(async ({ input }): Promise<{ success: boolean }> => { - const fullPath = join(input.worktreePath, input.filePath); - await writeFile(fullPath, input.content, "utf-8"); + // Validate path is within worktree (prevents path traversal attacks) + const validation = await validatePathForWrite( + input.worktreePath, + input.filePath, + ); + + if (!validation.valid || !validation.resolvedPath) { + throw new Error( + validation.reason === "outside-worktree" + ? "Cannot write to files outside worktree" + : "File path validation failed", + ); + } + + await writeFile(validation.resolvedPath, input.content, "utf-8"); return { success: true }; }), diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx index e4aa4e47352..162114e9687 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx @@ -55,9 +55,9 @@ function GroupItem({ e.stopPropagation(); onClose(); }} - className="absolute -right-1 -top-1 p-0.5 rounded-full bg-muted opacity-0 group-hover:opacity-100 transition-opacity hover:bg-destructive hover:text-destructive-foreground" + className="absolute -right-1.5 -top-1.5 p-1 rounded-full bg-muted opacity-0 group-hover:opacity-100 transition-opacity hover:bg-destructive hover:text-destructive-foreground" > - +
); diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx index bce7ecd1289..00b93eaf6dc 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/ViewModeToggle/ViewModeToggle.tsx @@ -29,6 +29,7 @@ export function ViewModeToggle() {
); diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx index 20a261a4d85..4c67aba85b0 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx @@ -5,6 +5,7 @@ import "@xterm/xterm/css/xterm.css"; import debounce from "lodash/debounce"; import { useCallback, useEffect, useRef, useState } from "react"; import { trpc } from "renderer/lib/trpc"; +import { trpcClient } from "renderer/lib/trpc-client"; import { useAppHotkey } from "renderer/stores/hotkeys"; import { useTabsStore } from "renderer/stores/tabs/store"; import { useTerminalCallbacksStore } from "renderer/stores/tabs/terminal-callbacks"; @@ -47,6 +48,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { const setTabAutoTitle = useTabsStore((s) => s.setTabAutoTitle); const updatePaneCwd = useTabsStore((s) => s.updatePaneCwd); const focusedPaneIds = useTabsStore((s) => s.focusedPaneIds); + const addFileViewerPane = useTabsStore((s) => s.addFileViewerPane); const terminalTheme = useTerminalTheme(); // Ref for initial theme to avoid recreating terminal on theme change @@ -68,6 +70,41 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { const { data: workspaceCwd } = trpc.terminal.getWorkspaceCwd.useQuery(workspaceId); + // Query terminal link behavior setting + const { data: terminalLinkBehavior } = + trpc.settings.getTerminalLinkBehavior.useQuery(); + + // Handler for file link clicks - uses current setting value + const handleFileLinkClick = useCallback( + (path: string, line?: number, column?: number) => { + const behavior = terminalLinkBehavior ?? "external-editor"; + + if (behavior === "file-viewer") { + addFileViewerPane(workspaceId, { filePath: path }); + } else { + trpcClient.external.openFileInEditor + .mutate({ + path, + line, + column, + cwd: workspaceCwd ?? undefined, + }) + .catch((error) => { + console.error( + "[Terminal] Failed to open file in editor:", + path, + error, + ); + }); + } + }, + [terminalLinkBehavior, workspaceId, workspaceCwd, addFileViewerPane], + ); + + // Ref to avoid terminal recreation when callback changes + const handleFileLinkClickRef = useRef(handleFileLinkClick); + handleFileLinkClickRef.current = handleFileLinkClick; + // Seed cwd from initialCwd or workspace path (shell spawns there) // OSC-7 will override if/when the shell reports directory changes useEffect(() => { @@ -197,11 +234,12 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => { xterm, fitAddon, cleanup: cleanupQuerySuppression, - } = createTerminalInstance( - container, - workspaceCwd, - initialThemeRef.current, - ); + } = createTerminalInstance(container, { + cwd: workspaceCwd, + initialTheme: initialThemeRef.current, + onFileLinkClick: (path, line, column) => + handleFileLinkClickRef.current(path, line, column), + }); xtermRef.current = xterm; fitAddonRef.current = fitAddon; isExitedRef.current = false; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts index 6058503b816..89473cdf9d6 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts @@ -92,19 +92,26 @@ function loadRenderer(xterm: XTerm): { dispose: () => void } { }; } +export interface CreateTerminalOptions { + cwd?: string; + initialTheme?: ITheme | null; + onFileLinkClick?: (path: string, line?: number, column?: number) => void; +} + export function createTerminalInstance( container: HTMLDivElement, - cwd?: string, - initialTheme?: ITheme | null, + options: CreateTerminalOptions = {}, ): { xterm: XTerm; fitAddon: FitAddon; cleanup: () => void; } { + const { cwd, initialTheme, onFileLinkClick } = options; + // Use provided theme, or fall back to localStorage-based default to prevent flash const theme = initialTheme ?? getDefaultTerminalTheme(); - const options = { ...TERMINAL_OPTIONS, theme }; - const xterm = new XTerm(options); + const terminalOptions = { ...TERMINAL_OPTIONS, theme }; + const xterm = new XTerm(terminalOptions); const fitAddon = new FitAddon(); const clipboardAddon = new ClipboardAddon(); @@ -142,20 +149,25 @@ export function createTerminalInstance( const filePathLinkProvider = new FilePathLinkProvider( xterm, (_event, path, line, column) => { - trpcClient.external.openFileInEditor - .mutate({ - path, - line, - column, - cwd, - }) - .catch((error) => { - console.error( - "[Terminal] Failed to open file in editor:", + if (onFileLinkClick) { + onFileLinkClick(path, line, column); + } else { + // Fallback to default behavior (external editor) + trpcClient.external.openFileInEditor + .mutate({ path, - error, - ); - }); + line, + column, + cwd, + }) + .catch((error) => { + console.error( + "[Terminal] Failed to open file in editor:", + path, + error, + ); + }); + } }, ); xterm.registerLinkProvider(filePathLinkProvider); diff --git a/apps/desktop/src/shared/constants.ts b/apps/desktop/src/shared/constants.ts index 6bc788cead4..1ec904ae1fc 100644 --- a/apps/desktop/src/shared/constants.ts +++ b/apps/desktop/src/shared/constants.ts @@ -46,3 +46,4 @@ export const NOTIFICATION_EVENTS = { // Default user preference values export const DEFAULT_CONFIRM_ON_QUIT = true; +export const DEFAULT_TERMINAL_LINK_BEHAVIOR = "external-editor" as const; diff --git a/packages/local-db/drizzle/0004_add_terminal_link_behavior_setting.sql b/packages/local-db/drizzle/0004_add_terminal_link_behavior_setting.sql new file mode 100644 index 00000000000..ad70f21f3fe --- /dev/null +++ b/packages/local-db/drizzle/0004_add_terminal_link_behavior_setting.sql @@ -0,0 +1 @@ +ALTER TABLE `settings` ADD `terminal_link_behavior` text; \ No newline at end of file diff --git a/packages/local-db/drizzle/meta/0004_snapshot.json b/packages/local-db/drizzle/meta/0004_snapshot.json new file mode 100644 index 00000000000..991b5469eb5 --- /dev/null +++ b/packages/local-db/drizzle/meta/0004_snapshot.json @@ -0,0 +1,977 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "bb9f9f85-bcbb-4003-b20f-4c172a1c6fc8", + "prevId": "d5a52ac9-bc1e-4529-89bf-5748d4df5006", + "tables": { + "organization_members": { + "name": "organization_members", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "organization_id": { + "name": "organization_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "role": { + "name": "role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "organization_members_organization_id_idx": { + "name": "organization_members_organization_id_idx", + "columns": [ + "organization_id" + ], + "isUnique": false + }, + "organization_members_user_id_idx": { + "name": "organization_members_user_id_idx", + "columns": [ + "user_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "organization_members_organization_id_organizations_id_fk": { + "name": "organization_members_organization_id_organizations_id_fk", + "tableFrom": "organization_members", + "tableTo": "organizations", + "columnsFrom": [ + "organization_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "organization_members_user_id_users_id_fk": { + "name": "organization_members_user_id_users_id_fk", + "tableFrom": "organization_members", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "organizations": { + "name": "organizations", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "clerk_org_id": { + "name": "clerk_org_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "github_org": { + "name": "github_org", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "organizations_clerk_org_id_unique": { + "name": "organizations_clerk_org_id_unique", + "columns": [ + "clerk_org_id" + ], + "isUnique": true + }, + "organizations_slug_unique": { + "name": "organizations_slug_unique", + "columns": [ + "slug" + ], + "isUnique": true + }, + "organizations_slug_idx": { + "name": "organizations_slug_idx", + "columns": [ + "slug" + ], + "isUnique": false + }, + "organizations_clerk_org_id_idx": { + "name": "organizations_clerk_org_id_idx", + "columns": [ + "clerk_org_id" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "projects": { + "name": "projects", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "main_repo_path": { + "name": "main_repo_path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "color": { + "name": "color", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "tab_order": { + "name": "tab_order", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_opened_at": { + "name": "last_opened_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "config_toast_dismissed": { + "name": "config_toast_dismissed", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "default_branch": { + "name": "default_branch", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "projects_main_repo_path_idx": { + "name": "projects_main_repo_path_idx", + "columns": [ + "main_repo_path" + ], + "isUnique": false + }, + "projects_last_opened_at_idx": { + "name": "projects_last_opened_at_idx", + "columns": [ + "last_opened_at" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "settings": { + "name": "settings", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "last_active_workspace_id": { + "name": "last_active_workspace_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_used_app": { + "name": "last_used_app", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "terminal_presets": { + "name": "terminal_presets", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "terminal_presets_initialized": { + "name": "terminal_presets_initialized", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "selected_ringtone_id": { + "name": "selected_ringtone_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "active_organization_id": { + "name": "active_organization_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "confirm_on_quit": { + "name": "confirm_on_quit", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "terminal_link_behavior": { + "name": "terminal_link_behavior", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "tasks": { + "name": "tasks", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status_color": { + "name": "status_color", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status_type": { + "name": "status_type", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status_position": { + "name": "status_position", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "priority": { + "name": "priority", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "organization_id": { + "name": "organization_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repository_id": { + "name": "repository_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "assignee_id": { + "name": "assignee_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "creator_id": { + "name": "creator_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "estimate": { + "name": "estimate", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "due_date": { + "name": "due_date", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "labels": { + "name": "labels", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "branch": { + "name": "branch", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "pr_url": { + "name": "pr_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "external_provider": { + "name": "external_provider", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "external_id": { + "name": "external_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "external_key": { + "name": "external_key", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "external_url": { + "name": "external_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_synced_at": { + "name": "last_synced_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "sync_error": { + "name": "sync_error", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "started_at": { + "name": "started_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "completed_at": { + "name": "completed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "tasks_slug_unique": { + "name": "tasks_slug_unique", + "columns": [ + "slug" + ], + "isUnique": true + }, + "tasks_slug_idx": { + "name": "tasks_slug_idx", + "columns": [ + "slug" + ], + "isUnique": false + }, + "tasks_organization_id_idx": { + "name": "tasks_organization_id_idx", + "columns": [ + "organization_id" + ], + "isUnique": false + }, + "tasks_assignee_id_idx": { + "name": "tasks_assignee_id_idx", + "columns": [ + "assignee_id" + ], + "isUnique": false + }, + "tasks_status_idx": { + "name": "tasks_status_idx", + "columns": [ + "status" + ], + "isUnique": false + }, + "tasks_created_at_idx": { + "name": "tasks_created_at_idx", + "columns": [ + "created_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "tasks_organization_id_organizations_id_fk": { + "name": "tasks_organization_id_organizations_id_fk", + "tableFrom": "tasks", + "tableTo": "organizations", + "columnsFrom": [ + "organization_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "tasks_assignee_id_users_id_fk": { + "name": "tasks_assignee_id_users_id_fk", + "tableFrom": "tasks", + "tableTo": "users", + "columnsFrom": [ + "assignee_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "set null", + "onUpdate": "no action" + }, + "tasks_creator_id_users_id_fk": { + "name": "tasks_creator_id_users_id_fk", + "tableFrom": "tasks", + "tableTo": "users", + "columnsFrom": [ + "creator_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "clerk_id": { + "name": "clerk_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "users_clerk_id_unique": { + "name": "users_clerk_id_unique", + "columns": [ + "clerk_id" + ], + "isUnique": true + }, + "users_email_unique": { + "name": "users_email_unique", + "columns": [ + "email" + ], + "isUnique": true + }, + "users_email_idx": { + "name": "users_email_idx", + "columns": [ + "email" + ], + "isUnique": false + }, + "users_clerk_id_idx": { + "name": "users_clerk_id_idx", + "columns": [ + "clerk_id" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "workspaces": { + "name": "workspaces", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "project_id": { + "name": "project_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "worktree_id": { + "name": "worktree_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "branch": { + "name": "branch", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "tab_order": { + "name": "tab_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_opened_at": { + "name": "last_opened_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "workspaces_project_id_idx": { + "name": "workspaces_project_id_idx", + "columns": [ + "project_id" + ], + "isUnique": false + }, + "workspaces_worktree_id_idx": { + "name": "workspaces_worktree_id_idx", + "columns": [ + "worktree_id" + ], + "isUnique": false + }, + "workspaces_last_opened_at_idx": { + "name": "workspaces_last_opened_at_idx", + "columns": [ + "last_opened_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "workspaces_project_id_projects_id_fk": { + "name": "workspaces_project_id_projects_id_fk", + "tableFrom": "workspaces", + "tableTo": "projects", + "columnsFrom": [ + "project_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "workspaces_worktree_id_worktrees_id_fk": { + "name": "workspaces_worktree_id_worktrees_id_fk", + "tableFrom": "workspaces", + "tableTo": "worktrees", + "columnsFrom": [ + "worktree_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "worktrees": { + "name": "worktrees", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "project_id": { + "name": "project_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "path": { + "name": "path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "branch": { + "name": "branch", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "base_branch": { + "name": "base_branch", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "git_status": { + "name": "git_status", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_status": { + "name": "github_status", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "worktrees_project_id_idx": { + "name": "worktrees_project_id_idx", + "columns": [ + "project_id" + ], + "isUnique": false + }, + "worktrees_branch_idx": { + "name": "worktrees_branch_idx", + "columns": [ + "branch" + ], + "isUnique": false + } + }, + "foreignKeys": { + "worktrees_project_id_projects_id_fk": { + "name": "worktrees_project_id_projects_id_fk", + "tableFrom": "worktrees", + "tableTo": "projects", + "columnsFrom": [ + "project_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/packages/local-db/drizzle/meta/_journal.json b/packages/local-db/drizzle/meta/_journal.json index 3117a6e2266..65dc59e762b 100644 --- a/packages/local-db/drizzle/meta/_journal.json +++ b/packages/local-db/drizzle/meta/_journal.json @@ -29,6 +29,13 @@ "when": 1766932805546, "tag": "0003_add_confirm_on_quit_setting", "breakpoints": true + }, + { + "idx": 4, + "version": "6", + "when": 1767166138761, + "tag": "0004_add_terminal_link_behavior_setting", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/local-db/src/schema/schema.ts b/packages/local-db/src/schema/schema.ts index 9b0639805f0..0f98d17ada9 100644 --- a/packages/local-db/src/schema/schema.ts +++ b/packages/local-db/src/schema/schema.ts @@ -5,6 +5,7 @@ import type { ExternalApp, GitHubStatus, GitStatus, + TerminalLinkBehavior, TerminalPreset, WorkspaceType, } from "./zod"; @@ -127,6 +128,9 @@ export const settings = sqliteTable("settings", { selectedRingtoneId: text("selected_ringtone_id"), activeOrganizationId: text("active_organization_id"), confirmOnQuit: integer("confirm_on_quit", { mode: "boolean" }), + terminalLinkBehavior: text( + "terminal_link_behavior", + ).$type(), }); export type InsertSettings = typeof settings.$inferInsert; diff --git a/packages/local-db/src/schema/zod.ts b/packages/local-db/src/schema/zod.ts index bb33e1d1596..ca8221e405d 100644 --- a/packages/local-db/src/schema/zod.ts +++ b/packages/local-db/src/schema/zod.ts @@ -96,3 +96,13 @@ export const EXTERNAL_APPS = [ ] as const; export type ExternalApp = (typeof EXTERNAL_APPS)[number]; + +/** + * Terminal link behavior options + */ +export const TERMINAL_LINK_BEHAVIORS = [ + "external-editor", + "file-viewer", +] as const; + +export type TerminalLinkBehavior = (typeof TERMINAL_LINK_BEHAVIORS)[number]; From 0bfdbe6ee5ef3415c847306933576b7c9ff9d7e3 Mon Sep 17 00:00:00 2001 From: Andreas Asprou Date: Wed, 31 Dec 2025 09:35:50 +0200 Subject: [PATCH 14/14] refactor(desktop): simplify security - remove symlink escape detection Remove async symlink escape detection from path validation since the threat model doesn't justify it: a compromised renderer already has terminal access for arbitrary command execution. Changes: - Replace resolveSecurePath (async) with resolvePathInWorktree (sync) - Remove assertNoSymlinkEscape and checkSymlinks option - Add validateRelativePath for simple path safety checks - Update secure-fs.ts to use new sync functions - Update threat model documentation in path-validation.ts --- .../trpc/routers/changes/security/index.ts | 17 +- .../changes/security/path-validation.ts | 219 ++++++------------ .../routers/changes/security/secure-fs.ts | 62 ++--- 3 files changed, 94 insertions(+), 204 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/security/index.ts b/apps/desktop/src/lib/trpc/routers/changes/security/index.ts index b2763809771..8fdb09c9e7a 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/security/index.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/security/index.ts @@ -1,13 +1,11 @@ /** * Security module for changes routers. * - * This module provides: - * - Path validation with symlink escape protection - * - Secure filesystem wrappers - * - Worktree registration checks + * Security model: + * - PRIMARY: Worktree must be registered in localDb + * - SECONDARY: Paths validated for traversal attempts * - * All filesystem operations in the changes routers should go through - * this module to ensure consistent security checks. + * See path-validation.ts header for full threat model. */ export { @@ -18,13 +16,16 @@ export { gitUnstageAll, gitUnstageFile, } from "./git-commands"; + export { assertRegisteredWorktree, assertValidGitPath, getRegisteredWorktree, PathValidationError, type PathValidationErrorCode, - type ResolveSecurePathOptions, - resolveSecurePath, + resolvePathInWorktree, + type ValidatePathOptions, + validateRelativePath, } from "./path-validation"; + export { secureFs } from "./secure-fs"; diff --git a/apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts b/apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts index 675f209f030..72292859b02 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts @@ -1,23 +1,40 @@ -import { realpath } from "node:fs/promises"; -import { - dirname, - isAbsolute, - normalize, - relative, - resolve, - sep, -} from "node:path"; +import { isAbsolute, normalize, resolve, sep } from "node:path"; import { worktrees } from "@superset/local-db"; import { eq } from "drizzle-orm"; import { localDb } from "main/lib/local-db"; +/** + * Security model for desktop app filesystem access: + * + * THREAT MODEL ASSUMPTION: + * A compromised renderer can already execute arbitrary commands via + * terminal panes. Therefore, filesystem-level symlink protections + * provide no meaningful security boundary—an attacker with renderer + * access can simply run `cat /etc/passwd` in a terminal. + * + * If your deployment exposes the renderer to untrusted content WITHOUT + * terminal access, this model does NOT apply and symlink escape checks + * should be re-enabled. + * + * PRIMARY BOUNDARY: assertRegisteredWorktree() + * - Only worktree paths registered in localDb are accessible via tRPC + * - Prevents direct filesystem access to unregistered paths + * + * SECONDARY: validateRelativePath() + * - Rejects absolute paths and ".." traversal segments + * - Defense in depth against path manipulation + * + * NOT IMPLEMENTED (intentional, see threat model above): + * - Symlink escape detection + * - Realpath resolution + */ + /** * Security error codes for path validation failures. */ export type PathValidationErrorCode = | "ABSOLUTE_PATH" | "PATH_TRAVERSAL" - | "SYMLINK_ESCAPE" | "UNREGISTERED_WORKTREE" | "INVALID_TARGET"; @@ -37,9 +54,7 @@ export class PathValidationError extends Error { /** * Validates that a worktree path is registered in localDb. - * - * This is THE critical security boundary - prevents arbitrary filesystem access. - * A compromised renderer cannot access files outside registered worktrees. + * This is THE critical security boundary. * * @throws PathValidationError if worktree is not registered */ @@ -59,8 +74,7 @@ export function assertRegisteredWorktree(worktreePath: string): void { } /** - * Gets the worktree record if it exists in localDb. - * Returns the record for additional operations (e.g., updating branch). + * Gets the worktree record if registered. Returns record for updates. * * @throws PathValidationError if worktree is not registered */ @@ -84,86 +98,9 @@ export function getRegisteredWorktree( } /** - * Checks if a relative path escapes its parent directory. - * - * Uses the correct segment-aware check: - * - `..` alone escapes - * - `../anything` escapes - * - `..foo` does NOT escape (legitimate directory name) - */ -function escapesParent(relativePath: string): boolean { - return ( - relativePath === ".." || - relativePath.startsWith(`..${sep}`) || - isAbsolute(relativePath) - ); -} - -/** - * Validates a file path doesn't escape the worktree via symlinks. - * - * Handles new files by walking up to find the first existing ancestor - * and validating that ancestor is within the worktree. - * - * @throws PathValidationError if symlink escape detected or validation fails + * Options for path validation. */ -async function assertNoSymlinkEscape( - worktreePath: string, - fullPath: string, -): Promise { - const realWorktree = await realpath(worktreePath); - - // Walk up to find first existing ancestor - let checkPath = fullPath; - const root = resolve("/"); - - while (checkPath !== root) { - try { - const realPath = await realpath(checkPath); - const rel = relative(realWorktree, realPath); - - if (escapesParent(rel)) { - throw new PathValidationError( - "Path escapes worktree via symlink", - "SYMLINK_ESCAPE", - ); - } - - // Found existing path and validated it - we're done - return; - } catch (e) { - if (e instanceof PathValidationError) { - throw e; - } - - if ((e as NodeJS.ErrnoException).code === "ENOENT") { - // Path doesn't exist, check parent - const parent = dirname(checkPath); - if (parent === checkPath) { - // Hit filesystem root without finding existing path - // This shouldn't happen for valid worktree paths - break; - } - checkPath = parent; - continue; - } - - // For any other error (permissions, etc.), fail closed - throw new PathValidationError( - `Cannot verify path security: ${(e as Error).message}`, - "SYMLINK_ESCAPE", - ); - } - } -} - -export interface ResolveSecurePathOptions { - /** - * Check for symlink escapes. Required for destructive operations. - * Default: true (fail closed) - */ - checkSymlinks?: boolean; - +export interface ValidatePathOptions { /** * Allow empty/root path (resolves to worktree itself). * Default: false (prevents accidental worktree deletion) @@ -172,31 +109,18 @@ export interface ResolveSecurePathOptions { } /** - * Validates and resolves a file path within a worktree. - * - * Security checks: - * 1. Rejects absolute paths - * 2. Rejects path traversal via `..` segments - * 3. Rejects symlink escapes (by default) - * 4. Rejects root path unless explicitly allowed + * Validates a relative file path for safety. + * Rejects absolute paths and path traversal attempts. * - * Uses `path.relative()` containment check - the industry standard pattern - * from VSCode, MCP servers, and security-focused libraries. - * - * @param worktreePath - The registered worktree base path - * @param filePath - The relative file path to validate - * @param options - Validation options - * @returns The resolved full path - * @throws PathValidationError on any validation failure + * @throws PathValidationError if path is invalid */ -export async function resolveSecurePath( - worktreePath: string, +export function validateRelativePath( filePath: string, - options: ResolveSecurePathOptions = {}, -): Promise { - const { checkSymlinks = true, allowRoot = false } = options; + options: ValidatePathOptions = {}, +): void { + const { allowRoot = false } = options; - // 1. Reject absolute paths immediately + // Reject absolute paths if (isAbsolute(filePath)) { throw new PathValidationError( "Absolute paths are not allowed", @@ -204,61 +128,50 @@ export async function resolveSecurePath( ); } - // 2. Normalize and resolve const normalized = normalize(filePath); - const fullPath = resolve(worktreePath, normalized); - - // 3. Containment check via relative path - const relativePath = relative(worktreePath, fullPath); + const segments = normalized.split(sep); - if (escapesParent(relativePath)) { + // Reject ".." as a path segment (allows "..foo" directories) + if (segments.includes("..")) { throw new PathValidationError( - "Path escapes worktree boundary", + "Path traversal not allowed", "PATH_TRAVERSAL", ); } - // 4. Check for root path (empty or ".") - if (!allowRoot && (relativePath === "" || relativePath === ".")) { + // Reject root path unless explicitly allowed + if (!allowRoot && (normalized === "" || normalized === ".")) { throw new PathValidationError( "Cannot target worktree root", "INVALID_TARGET", ); } - - // 5. Symlink escape check (default: enabled for safety) - if (checkSymlinks) { - await assertNoSymlinkEscape(worktreePath, fullPath); - } - - return fullPath; } /** - * Validates a path for use in git commands (pathspec). + * Validates and resolves a path within a worktree. Sync, simple. * - * Lighter validation than resolveSecurePath - just checks for - * obvious escapes. Git itself provides additional sandboxing. + * @param worktreePath - The worktree base path + * @param filePath - The relative file path to validate + * @param options - Validation options + * @returns The resolved full path + * @throws PathValidationError if path is invalid + */ +export function resolvePathInWorktree( + worktreePath: string, + filePath: string, + options: ValidatePathOptions = {}, +): string { + validateRelativePath(filePath, options); + // Use resolve to handle any worktreePath (relative or absolute) + return resolve(worktreePath, normalize(filePath)); +} + +/** + * Validates a path for git commands. Lighter check that allows root. * - * @param filePath - The file path to validate - * @throws PathValidationError if path is suspicious + * @throws PathValidationError if path is invalid */ export function assertValidGitPath(filePath: string): void { - if (isAbsolute(filePath)) { - throw new PathValidationError( - "Absolute paths are not allowed in git operations", - "ABSOLUTE_PATH", - ); - } - - const normalized = normalize(filePath); - const segments = normalized.split(sep); - - // Check for ".." as a segment (not substring - allows "..foo") - if (segments.includes("..")) { - throw new PathValidationError( - "Path traversal not allowed in git operations", - "PATH_TRAVERSAL", - ); - } + validateRelativePath(filePath, { allowRoot: true }); } diff --git a/apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts b/apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts index 1f4bb0e3d5e..2e461dd731a 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts @@ -1,27 +1,23 @@ import type { Stats } from "node:fs"; import { lstat, readFile, rm, stat, writeFile } from "node:fs/promises"; -import { assertRegisteredWorktree, resolveSecurePath } from "./path-validation"; +import { + assertRegisteredWorktree, + resolvePathInWorktree, +} from "./path-validation"; /** - * Secure filesystem operations that enforce validation. + * Secure filesystem operations with built-in validation. * - * Design principle: You cannot perform filesystem operations without - * going through validation. The validation is built into each operation. + * Each operation: + * 1. Validates worktree is registered (security boundary) + * 2. Validates path doesn't escape worktree (defense in depth) + * 3. Performs the filesystem operation * - * All operations: - * 1. Validate worktree is registered in database - * 2. Validate path doesn't escape worktree - * 3. Check for symlink escapes (configurable) - * - * Use Biome's restricted-imports rule to ban direct `node:fs` imports - * in router files - this module should be the only FS access point. + * See path-validation.ts for the full security model and threat assumptions. */ export const secureFs = { /** * Read a file within a worktree. - * - * Validates path and checks for symlink escapes to prevent - * reading files outside the worktree via symlinks. */ async readFile( worktreePath: string, @@ -29,33 +25,24 @@ export const secureFs = { encoding: BufferEncoding = "utf-8", ): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); return readFile(fullPath, encoding); }, /** * Read a file as a Buffer within a worktree. - * - * Validates path and checks for symlink escapes. */ async readFileBuffer( worktreePath: string, filePath: string, ): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); return readFile(fullPath); }, /** * Write content to a file within a worktree. - * - * Validates path and checks for symlink escapes to prevent - * writing files outside the worktree via symlinks. */ async writeFile( worktreePath: string, @@ -63,9 +50,7 @@ export const secureFs = { content: string, ): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); await writeFile(fullPath, content, "utf-8"); }, @@ -73,14 +58,13 @@ export const secureFs = { * Delete a file or directory within a worktree. * * DANGEROUS: Uses recursive + force deletion. - * Validates path and checks for symlink escapes. * Explicitly prevents deleting the worktree root. */ async delete(worktreePath: string, filePath: string): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - allowRoot: false, // Explicitly prevent deleting worktree root + // allowRoot: false prevents deleting the worktree itself + const fullPath = resolvePathInWorktree(worktreePath, filePath, { + allowRoot: false, }); await rm(fullPath, { recursive: true, force: true }); }, @@ -89,14 +73,10 @@ export const secureFs = { * Get file stats within a worktree. * * Uses `stat` (follows symlinks) to get the real file size. - * This is important for size checks - lstat would return - * the symlink size, not the target file size. */ async stat(worktreePath: string, filePath: string): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); return stat(fullPath); }, @@ -108,9 +88,7 @@ export const secureFs = { */ async lstat(worktreePath: string, filePath: string): Promise { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); return lstat(fullPath); }, @@ -122,9 +100,7 @@ export const secureFs = { async exists(worktreePath: string, filePath: string): Promise { try { assertRegisteredWorktree(worktreePath); - const fullPath = await resolveSecurePath(worktreePath, filePath, { - checkSymlinks: true, - }); + const fullPath = resolvePathInWorktree(worktreePath, filePath); await stat(fullPath); return true; } catch {