Filesystem management interface with Monaco Editor integration and a dedicated data viewer with SQL query capabilities. #66
+11,174
−166
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary of Changes Critical Security Fixes (2) SQL Injection in sqlite-parser.ts Added validateTableName() function to verify table names exist in the database Added escapeTableName() for proper SQL escaping Added validateSqlQuery() to restrict queries to SELECT-only operations SQL Injection in duckdb-parser.ts Added validateSqlQuery() function to prevent dangerous operations Only SELECT queries are now allowed; INSERT, UPDATE, DELETE, DROP, etc. are blocked High Priority Fixes (10) Path Traversal Protection (files.ts) Added validatePathWithinDirectory() helper function Applied to listFolderContents procedure Error Boundaries (error-boundary.tsx - new file) Created reusable ErrorBoundary and ViewerErrorBoundary components Wrapped all viewer components (code, image, markdown, data) with error boundaries Clipboard Error Handling (data-viewer-sidebar.tsx) Fixed unhandled promise rejections in handleCopyCellValue Added try/catch with toast notifications for all clipboard operations Shared Utilities (file-utils.ts - new file) Created shared getFileName(), formatFileSize(), getFileExtension() functions Updated all viewer files to use shared utilities, removing duplicates Type Safety Improvements Fixed gridRef type from any to proper type Fixed onCellContextMenu event parameter type Fixed markdown component callback types Removed Unused Props Removed chatId prop from FileViewerSidebar and DataViewerSidebar Updated call sites in active-chat.tsx Race Condition Fix (data-viewer-sidebar.tsx) Added queryRequestIdRef to track query requests Only updates state if the response matches the latest request ID Files Modified src/main/lib/parsers/sqlite-parser.ts src/main/lib/parsers/duckdb-parser.ts src/main/lib/trpc/routers/files.ts src/renderer/components/ui/error-boundary.tsx (new) src/renderer/features/file-viewer/utils/file-utils.ts (new) src/renderer/features/file-viewer/components/file-viewer-sidebar.tsx src/renderer/features/file-viewer/components/markdown-viewer.tsx src/renderer/features/file-viewer/components/image-viewer.tsx src/renderer/features/data/components/data-viewer-sidebar.tsx src/renderer/features/agents/main/active-chat.tsx Deferred to Future PRs Server-side sorting (requires API changes) Medium priority items (XSS sanitization, a11y improvements)
Phase 1: Security Fixes (Completed) Path validation utility - Added validatePathWithinDirectory() function that prevents path traversal attacks by ensuring all file paths resolve within the project directory. Symlink detection - Added isSymlinkOutsideDirectory() function that detects symlinks pointing outside the project directory and blocks destructive operations on them. Applied security to all CRUD operations: deleteFile - Now validates path and checks for dangerous symlinks deleteFiles - Same protections renameFile - Validates both old and new paths moveFile - Validates source and destination createFile - Validates path createFolder - Validates path copyFiles - Validates paths and checks for symlinks importFiles - Validates target directory Trash support - deleteFile and deleteFiles now use shell.trashItem() by default (recoverable), with a permanent: true option for hard delete. Phase 2: Performance Improvements (Completed) Search debounce - Added 150ms debounce to file tree search to avoid filtering on every keystroke. Virtualizer overscan - Already set to 15 (was done previously).
Fix for this review by coderabbit coderabbitai bot 2 hours ago⚠️ Potential issue | 🟠 Major Path matching bug: startsWith can match partial directory names. The filePath.startsWith(projectPath) check on line 82 will incorrectly match paths where projectPath is a prefix of a different directory or filename. For example, if projectPath = "/home/project" and filePath = "/home/projectile.txt", the condition passes and produces "ile.txt" instead of the correct path. Ensure the path boundary is respected by checking for the trailing separator: 🐛 Proposed fix 🤖 Prompt for AI Agents In `@src/renderer/features/file-viewer/hooks/use-file-content.ts` around lines 76 - 86, The relative path computation in use-file-content.ts (inside the useMemo that computes relativePath) incorrectly uses filePath.startsWith(projectPath) which can match partial names; update the check to ensure a path boundary by confirming either filePath === projectPath or filePath startsWith(projectPath + "/") (or ensure projectPath ends with "/" before slicing) before slicing with projectPath.length + 1, so you only strip the projectPath when it is a full directory prefix.
Fix for below review by coderabbit coderabbitai bot 2 hours ago⚠️ Potential issue | 🟠 Major Missing dependencies in handleKeyDown will cause stale closure bugs. The callback uses several state setters and values that are not included in the dependency array. This will cause keyboard navigation to use stale state. Missing dependencies: virtualizer (used for scrollToIndex) expandedFolders (used for folder expand/collapse) setExpandedFolders (used for folder expand/collapse) setSelectedPath (used for selection updates) setSelectedPaths (used for selection updates) lastSelectedRef (ref, doesn't need to be in deps but worth noting) Suggested fix 📝 Committable suggestion 🤖 Prompt for AI Agents In `@src/renderer/features/agents/ui/file-tree/FileTreeSidebar.tsx` around lines 951 - 961, The handleKeyDown callback has missing dependencies causing stale closures; update its useCallback dependency array to include virtualizer, expandedFolders, setExpandedFolders, setSelectedPath, and setSelectedPaths (you can omit lastSelectedRef since refs are stable) and where state is updated inside handleKeyDown prefer functional updates (e.g., setExpandedFolders(prev => ...), setSelectedPath(prev => ...), setSelectedPaths(prev => ...)) to avoid stale values; locate the handleKeyDown definition in FileTreeSidebar.tsx and add those symbols to its dependency array so scrollToIndex and expand/collapse and selection logic use current state.
Fix for below⚠️ Potential issue | 🟡 Minor Unused import: dirname is imported but not used. The dirname function from the path utilities is imported but doesn't appear to be used anywhere in this file. -import { dirname } from "../../../../lib/utils/path"
Fix for In `@src/renderer/features/agents/ui/file-tree/FileTreeContextMenu.tsx` around lines 189 - 196, The ContextMenuShortcut display for ⌘N and ⇧⌘N is misleading because FileTreeSidebar's keyboard handler does not handle those keys; update the keyboard handling to invoke the same actions as the context menu or remove the shortcuts from the UI. Specifically, either add handling for Cmd+N and Shift+Cmd+N in the FileTreeSidebar keyboard handler to call the same callbacks that the context menu uses (onNewFile and onNewFolder), or remove the ContextMenuShortcut usages in FileTreeContextMenu around the ContextMenuItem entries tied to onNewFile and onNewFolder so the hints are not shown.
fix for⚠️ Potential issue | 🟠 Major 🧩 Analysis chain Nested symlinks are not validated during recursive copy. The copyFiles procedure validates the top-level sourcePath at line 1216, but when copyDir recursively processes directories, it does not check nested symlinks. Inside copyDir, entries from readdir with withFileTypes: true return symlinks as regular file entries (not directories). These symlinks are then passed to copyFile, which follows symlinks to copy their targets. If a nested symlink points outside the project directory, the initial validation is bypassed. Add symlink validation inside copyDir before calling copyFile, or check each nested path before following it. 🤖 Prompt for AI Agents In `@src/main/lib/trpc/routers/files.ts` around lines 64 - 79, The recursive copyDir function currently follows nested symlinks without validating them; update copyDir to detect symlinks (use Dirent.isSymbolicLink() or fs.lstat on srcPath), and before calling copyFile or recursing, resolve the symlink target (fs.realpath) and validate the resolved path is inside the allowed project root (compare path.resolve/realpath of target against the project root prefix) or reject it; ensure you call the same validation logic used for the top-level sourcePath (the one in copyFiles) for each nested symlink so copyFile never follows an out-of-project symlink.
Fix for coderabbitai bot 2 hours ago⚠️ Potential issue | 🟠 Major Path separator issue on Windows. The containment check uses a hardcoded forward slash ("/"), but on Windows, path.resolve() returns paths with backslashes. This could allow path traversal on Windows systems. Proposed fix using path.sep 🤖 Prompt for AI Agents In `@src/main/lib/trpc/routers/files.ts` around lines 27 - 37, The containment check in validatePathWithinDirectory uses a hardcoded "/" which fails on Windows; update the check to use path.sep (or normalized separators) so you compare consistent separators—compute a normalizedResolvedBase (e.g., ensure it ends with path.sep) and test resolvedPath.startsWith(normalizedResolvedBase) or equality to resolvedBase; adjust resolvedBase and resolvedPath handling to use path.normalize()/path.sep before the startsWith check and keep throwing the same error when the check fails.
Fix for coderabbitai bot 1 minute ago⚠️ Potential issue | 🟠 Major Stale closure in auto-expand timer can lose folder expansion state. The timer callback captures expandedFolders at creation time. If the user manually expands/collapses folders during the 500ms delay, those changes are overwritten when the timer fires. Suggested fix using functional update 🤖 Prompt for AI Agents In `@src/renderer/features/agents/ui/file-tree/FileTreeSidebar.tsx` around lines 416 - 423, The auto-expand timer callback captures the stale expandedFolders set and can overwrite user actions; update the timer to use a functional state update when calling setExpandedFolders so it computes the new Set from the latest state (referencing expandedFolders only to check before scheduling), e.g. inside the timeout use setExpandedFolders(prev => { const next = new Set(prev); next.add(folderPath); return next; }) and keep using autoExpandTimerRef.current to store/clear the timer.
Merged latest changes from upstream while preserving local features: - File Tree Sidebar (local) - Data Viewer Sidebar (local) - File Viewer Sidebar (local) New features from upstream: - Viewed files state for diff review (GitHub-style checkboxes) - Various UI/UX improvements and bug fixes - New queue processing system - Network status and offline handling
Removed aggressive abort in claude.ts to fix issue on follow up when in plan mode.
Member
|
wow |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Filesystem View
New filesystem browser with visual directory/file navigation
Monaco Editor integration for viewing most file types
Drag-and-drop support for file uploads and internal file movement
Copy-paste functionality for files
Git-aware file tracking integration
Data Viewer (Glide)
Interactive data viewer for tabular data formats:
Parquet files (via DuckDB integration)
CSV/TSV
Additional file types
Built-in SQL runner for querying data files directly
Dynamic handling when switching between data files