fix(desktop): Harden local file tree paths - #43593
Closed
yucode-kk wants to merge 1 commit into
Closed
Conversation
Normalize Electron local path handling across file tree, preview, media, and git-root flows. Reject malformed and Windows device paths, recheck sensitive files after realpath resolution, and preserve external symlink traversal with stable renderer errors.
alt-glitch
added
type/security
yucode-kk
marked this pull request as ready for review
June 10, 2026 15:09
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves the desktop project tree UX and hardens filesystem IPC by adding explicit error placeholders, normalizing Windows paths, and extracting readDir/gitRoot IPC logic into dedicated, tested modules.
Changes:
- Show an inline “error placeholder” child row when a folder read fails, and update the tree row iconography accordingly.
- Normalize Windows-style paths in the renderer IPC helper and add new renderer tests for Windows-path gitignore filtering.
- Refactor Electron main-process
readDir/gitRootIPC handlers intofs-read-dir.cjs/git-root.cjs, and expandhardening.cjspath validation with comprehensive tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/desktop/src/app/right-sidebar/files/use-project-tree.ts | Adds synthetic error/loading node typing and creates error placeholder children on read failures. |
| apps/desktop/src/app/right-sidebar/files/use-project-tree.test.ts | Updates expectations to validate the new error placeholder child behavior. |
| apps/desktop/src/app/right-sidebar/files/tree.tsx | Renders warning vs loading icons based on the new placeholder flag. |
| apps/desktop/src/app/right-sidebar/files/ipc.ts | Normalizes backslashes to forward slashes when cleaning cache keys/paths. |
| apps/desktop/src/app/right-sidebar/files/ipc.test.ts | Adds coverage for Windows-style paths when filtering gitignored entries. |
| apps/desktop/package.json | Extends platform test script to include new Electron tests. |
| apps/desktop/electron/main.cjs | Switches file path handling to hardening helpers and delegates IPC handlers to new modules. |
| apps/desktop/electron/hardening.test.cjs | Adds tests for invalid/device paths, sensitive files, symlinks, and directory resolution. |
| apps/desktop/electron/hardening.cjs | Introduces structured path validation/errors and new helpers for requested/readable/directory paths. |
| apps/desktop/electron/git-root.test.cjs | Adds tests for the extracted git root resolver. |
| apps/desktop/electron/git-root.cjs | New module implementing gitRootForIpc using hardened path resolution. |
| apps/desktop/electron/fs-read-dir.test.cjs | Adds tests for hidden filtering, sorting, URL inputs, device-path rejection, and symlink handling. |
| apps/desktop/electron/fs-read-dir.cjs | New module implementing hardened directory listing with filtering/statting of uncertain dirents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
729
to
734
| if (parsed.protocol === 'file:') { | ||
| let localPath | ||
| try { | ||
| localPath = fileURLToPath(parsed.toString()) | ||
| localPath = resolveRequestedPathForIpc(parsed.toString(), { purpose: 'Open external file' }) | ||
| } catch { | ||
| return false |
Comment on lines
+75
to
+81
| for (const dirent of dirents) { | ||
| if (FS_READDIR_HIDDEN.has(dirent.name)) { | ||
| continue | ||
| } | ||
|
|
||
| entries.push(await entryForDirent(dirent, resolved, fsImpl)) | ||
| } |
Comment on lines
+52
to
+58
| function errorChild(parentId: string, error: string | undefined): TreeNode { | ||
| return { | ||
| id: `${parentId}::${ERROR_PLACEHOLDER_ID}`, | ||
| isDirectory: false, | ||
| name: `Unable to read (${error || 'read-error'})`, | ||
| placeholder: 'error' | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Harden Hermes Desktop's local file tree and file-reading IPC paths without
restricting symlink or junction traversal outside the selected project.
The file tree previously depended on
Dirent.isDirectory()and directpath.resolve()/fileURLToPath()calls. Windows reparse points could beclassified as neither files nor directories, unreadable child directories
looked like empty folders, and sensitive-file checks could be bypassed through
a safe-named symlink.
This extracts directory and git-root handling into testable Electron helpers,
adds stable directory ordering and per-entry symlink classification, and keeps
unreadable folders visible with an error placeholder. Local path resolution is
now shared across file reads, previews, media, git-root discovery, and
file:URL handling. It rejects blank, NUL, malformed file URL, and Windowsdevice namespace paths, then checks sensitive-file rules against both the
requested path and its realpath.
Workspace containment is intentionally not included. Desktop attachments and
previews may target files outside the active project, and file-tree symlinks or
junctions remain expandable. The remaining validation-to-read TOCTOU window is
left for a separate deeper hardening change.
Validation covered the focused Electron tests (21 passed, 2 skipped because
ordinary symlink creation requires additional Windows privileges), the
renderer file-tree tests (13 passed), TypeScript compilation, and the desktop
production build. The complete desktop platform test command reaches 134
passed and 2 skipped tests, with the existing unrelated
electron/windows-child-process.test.cjsassertion still failing because itcannot find the formatted
execFileSync('reg')call site.