From b1af653bf6bda75efdec10225f5adeced30d34bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=B1=BC?= <2081789787@qq.com> Date: Thu, 11 Jun 2026 22:05:59 +0800 Subject: [PATCH] fix(desktop): Harden local file tree paths (#43618) * fix(desktop): Harden local file tree paths 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. * fix(desktop): Address file tree review feedback --- apps/desktop/electron/fs-read-dir.cjs | 109 ++++++ apps/desktop/electron/fs-read-dir.test.cjs | 364 ++++++++++++++++++ apps/desktop/electron/git-root.cjs | 54 +++ apps/desktop/electron/git-root.test.cjs | 40 ++ apps/desktop/electron/hardening.cjs | 145 +++++-- apps/desktop/electron/hardening.test.cjs | 150 ++++++++ apps/desktop/electron/main.cjs | 127 ++---- apps/desktop/package.json | 2 +- .../src/app/right-sidebar/files/ipc.test.ts | 100 +++++ .../src/app/right-sidebar/files/ipc.ts | 2 +- .../src/app/right-sidebar/files/tree.tsx | 7 +- .../files/use-project-tree.test.ts | 11 +- .../right-sidebar/files/use-project-tree.ts | 16 +- 13 files changed, 988 insertions(+), 139 deletions(-) create mode 100644 apps/desktop/electron/fs-read-dir.cjs create mode 100644 apps/desktop/electron/fs-read-dir.test.cjs create mode 100644 apps/desktop/electron/git-root.cjs create mode 100644 apps/desktop/electron/git-root.test.cjs create mode 100644 apps/desktop/src/app/right-sidebar/files/ipc.test.ts diff --git a/apps/desktop/electron/fs-read-dir.cjs b/apps/desktop/electron/fs-read-dir.cjs new file mode 100644 index 00000000000..52d182ad567 --- /dev/null +++ b/apps/desktop/electron/fs-read-dir.cjs @@ -0,0 +1,109 @@ +'use strict' + +const fs = require('node:fs') +const path = require('node:path') +const { resolveDirectoryForIpc } = require('./hardening.cjs') + +const FS_READDIR_STAT_CONCURRENCY = 16 + +// Always-hidden noise (covers non-git projects too; gitignore catches many of +// these, but the project tree should keep the same hygiene without one). +const FS_READDIR_HIDDEN = new Set([ + '.git', + '.hg', + '.svn', + '.cache', + '.next', + '.turbo', + '.venv', + '__pycache__', + 'build', + 'dist', + 'node_modules', + 'target', + 'venv' +]) + +function direntIsDirectory(dirent) { + return typeof dirent.isDirectory === 'function' && dirent.isDirectory() +} + +function direntIsFile(dirent) { + return typeof dirent.isFile === 'function' && dirent.isFile() +} + +function direntIsSymbolicLink(dirent) { + return typeof dirent.isSymbolicLink === 'function' && dirent.isSymbolicLink() +} + +function shouldStatDirent(dirent) { + if (direntIsDirectory(dirent)) return false + + return direntIsSymbolicLink(dirent) || !direntIsFile(dirent) +} + +async function entryForDirent(dirent, resolved, fsImpl) { + const fullPath = path.join(resolved, dirent.name) + let isDirectory = direntIsDirectory(dirent) + + if (!isDirectory && shouldStatDirent(dirent)) { + try { + isDirectory = (await fsImpl.promises.stat(fullPath)).isDirectory() + } catch { + isDirectory = false + } + } + + return { name: dirent.name, path: fullPath, isDirectory } +} + +async function mapWithStatConcurrency(items, mapper) { + const results = new Array(items.length) + let nextIndex = 0 + + async function runWorker() { + while (nextIndex < items.length) { + const index = nextIndex + nextIndex += 1 + results[index] = await mapper(items[index]) + } + } + + const workerCount = Math.min(FS_READDIR_STAT_CONCURRENCY, items.length) + const workers = Array.from({ length: workerCount }, () => runWorker()) + await Promise.all(workers) + + return results +} + +async function readDirForIpc(dirPath, options = {}) { + const fsImpl = options.fs || fs + let resolved + + try { + ;({ resolvedPath: resolved } = await resolveDirectoryForIpc(dirPath, { + fs: fsImpl, + purpose: 'Directory read' + })) + } catch (error) { + return { entries: [], error: error?.code || 'read-error' } + } + + try { + const dirents = await fsImpl.promises.readdir(resolved, { withFileTypes: true }) + const visibleDirents = dirents.filter(dirent => !FS_READDIR_HIDDEN.has(dirent.name)) + const entries = await mapWithStatConcurrency(visibleDirents, dirent => + entryForDirent(dirent, resolved, fsImpl) + ) + + entries.sort((a, b) => Number(b.isDirectory) - Number(a.isDirectory) || a.name.localeCompare(b.name)) + + return { entries } + } catch (error) { + return { entries: [], error: error?.code || 'read-error' } + } +} + +module.exports = { + readDirForIpc +} diff --git a/apps/desktop/electron/fs-read-dir.test.cjs b/apps/desktop/electron/fs-read-dir.test.cjs new file mode 100644 index 00000000000..42e80af3489 --- /dev/null +++ b/apps/desktop/electron/fs-read-dir.test.cjs @@ -0,0 +1,364 @@ +'use strict' + +const assert = require('node:assert/strict') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') +const test = require('node:test') +const { pathToFileURL } = require('node:url') + +const { readDirForIpc } = require('./fs-read-dir.cjs') + +function mkTmpDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-fs-read-dir-')) +} + +function fakeDirent(name, flags = {}) { + return { + name, + isDirectory: () => Boolean(flags.directory), + isFile: () => Boolean(flags.file), + isSymbolicLink: () => Boolean(flags.symlink) + } +} + +test('readDirForIpc hides noisy directories and files from the project tree', async () => { + const root = mkTmpDir() + + try { + fs.mkdirSync(path.join(root, 'node_modules')) + fs.mkdirSync(path.join(root, 'src')) + fs.writeFileSync(path.join(root, 'target'), 'hidden file') + fs.writeFileSync(path.join(root, 'README.md'), 'visible file') + + const result = await readDirForIpc(root) + + assert.equal(result.error, undefined) + assert.deepEqual( + result.entries.map(entry => entry.name), + ['src', 'README.md'] + ) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc filters a hidden basename whether it is a file or directory', async () => { + const dirRoot = mkTmpDir() + const fileRoot = mkTmpDir() + + try { + fs.mkdirSync(path.join(dirRoot, 'node_modules')) + fs.writeFileSync(path.join(dirRoot, 'visible.txt'), 'visible') + fs.writeFileSync(path.join(fileRoot, 'node_modules'), 'hidden file') + fs.writeFileSync(path.join(fileRoot, 'visible.txt'), 'visible') + + assert.deepEqual( + (await readDirForIpc(dirRoot)).entries.map(entry => entry.name), + ['visible.txt'] + ) + assert.deepEqual( + (await readDirForIpc(fileRoot)).entries.map(entry => entry.name), + ['visible.txt'] + ) + } finally { + fs.rmSync(dirRoot, { recursive: true, force: true }) + fs.rmSync(fileRoot, { recursive: true, force: true }) + } +}) + +test('readDirForIpc returns directories before files and sorts by name within groups', async () => { + const root = mkTmpDir() + + try { + fs.writeFileSync(path.join(root, 'z.txt'), 'z') + fs.mkdirSync(path.join(root, 'src')) + fs.writeFileSync(path.join(root, 'a.txt'), 'a') + fs.mkdirSync(path.join(root, 'lib')) + + const result = await readDirForIpc(root) + + assert.equal(result.error, undefined) + assert.deepEqual( + result.entries.map(entry => entry.name), + ['lib', 'src', 'a.txt', 'z.txt'] + ) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc accepts file URLs for directories', async () => { + const root = mkTmpDir() + + try { + fs.mkdirSync(path.join(root, 'src')) + fs.writeFileSync(path.join(root, 'README.md'), 'visible file') + + const result = await readDirForIpc(pathToFileURL(root).toString()) + + assert.equal(result.error, undefined) + assert.deepEqual( + result.entries.map(entry => entry.name), + ['src', 'README.md'] + ) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc returns invalid-path for blank or non-string input', async () => { + let readdirCalls = 0 + const fsImpl = { + promises: { + readdir: async () => { + readdirCalls += 1 + return [] + } + } + } + + assert.deepEqual(await readDirForIpc('', { fs: fsImpl }), { entries: [], error: 'invalid-path' }) + assert.deepEqual(await readDirForIpc(' ', { fs: fsImpl }), { entries: [], error: 'invalid-path' }) + assert.deepEqual(await readDirForIpc(null, { fs: fsImpl }), { entries: [], error: 'invalid-path' }) + assert.equal(readdirCalls, 0) +}) + +test('readDirForIpc rejects Windows device paths before readdir', async () => { + let readdirCalls = 0 + const fsImpl = { + promises: { + readdir: async () => { + readdirCalls += 1 + return [] + } + } + } + + assert.deepEqual(await readDirForIpc('\\\\?\\C:\\secret', { fs: fsImpl }), { + entries: [], + error: 'device-path' + }) + assert.equal(readdirCalls, 0) +}) + +test('readDirForIpc returns filesystem error codes instead of throwing', async () => { + const root = mkTmpDir() + + try { + const result = await readDirForIpc(path.join(root, 'missing')) + + assert.deepEqual(result, { entries: [], error: 'ENOENT' }) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc marks a symlink to a directory as a directory', async t => { + const root = mkTmpDir() + + try { + fs.mkdirSync(path.join(root, 'actual-dir')) + + try { + fs.symlinkSync(path.join(root, 'actual-dir'), path.join(root, 'linked-dir'), 'dir') + } catch (error) { + if (error?.code === 'EPERM' || error?.code === 'EACCES') { + t.skip(`symlink creation is not permitted on this platform (${error.code})`) + + return + } + + throw error + } + + const result = await readDirForIpc(root) + const linked = result.entries.find(entry => entry.name === 'linked-dir') + + assert.equal(result.error, undefined) + assert.equal(linked?.isDirectory, true) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc marks a Windows junction to a directory as a directory', async t => { + if (process.platform !== 'win32') { + t.skip('junctions are a Windows-specific symlink type') + + return + } + + const root = mkTmpDir() + + try { + fs.mkdirSync(path.join(root, 'actual-dir')) + + try { + fs.symlinkSync(path.join(root, 'actual-dir'), path.join(root, 'junction-dir'), 'junction') + } catch (error) { + if (error?.code === 'EPERM' || error?.code === 'EACCES') { + t.skip(`junction creation is not permitted on this platform (${error.code})`) + + return + } + + throw error + } + + const result = await readDirForIpc(root) + const junction = result.entries.find(entry => entry.name === 'junction-dir') + + assert.equal(result.error, undefined) + assert.equal(junction?.isDirectory, true) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + } +}) + +test('readDirForIpc allows expanding symlink or junction directories outside the project root', async t => { + const root = mkTmpDir() + const outside = mkTmpDir() + + try { + fs.writeFileSync(path.join(outside, 'outside.txt'), 'ok') + + const linkPath = path.join(root, 'outside-link') + try { + fs.symlinkSync(outside, linkPath, process.platform === 'win32' ? 'junction' : 'dir') + } catch (error) { + if (error?.code === 'EPERM' || error?.code === 'EACCES') { + t.skip(`directory symlink creation is not permitted on this platform (${error.code})`) + + return + } + + throw error + } + + const result = await readDirForIpc(linkPath) + + assert.equal(result.error, undefined) + assert.deepEqual(result.entries, [ + { name: 'outside.txt', path: path.join(linkPath, 'outside.txt'), isDirectory: false } + ]) + } finally { + fs.rmSync(root, { recursive: true, force: true }) + fs.rmSync(outside, { recursive: true, force: true }) + } +}) + +test('readDirForIpc stats symbolic links and unknown entries without dropping the whole listing', async () => { + const input = path.join('virtual-root') + const resolved = path.resolve(input) + const statCalls = [] + const fsImpl = { + promises: { + readdir: async () => [ + fakeDirent('unknown-entry'), + fakeDirent('linked-dir', { symlink: true }), + fakeDirent('broken-link', { symlink: true }), + fakeDirent('plain.txt', { file: true }) + ], + stat: async fullPath => { + if (fullPath === resolved) { + return { isDirectory: () => true } + } + + statCalls.push(fullPath) + if (fullPath.endsWith(`${path.sep}linked-dir`)) { + return { isDirectory: () => true } + } + throw Object.assign(new Error('gone'), { code: 'ENOENT' }) + } + } + } + + const result = await readDirForIpc(input, { fs: fsImpl }) + + assert.equal(result.error, undefined) + assert.deepEqual( + statCalls.sort(), + [path.join(resolved, 'broken-link'), path.join(resolved, 'linked-dir'), path.join(resolved, 'unknown-entry')].sort() + ) + assert.deepEqual(result.entries, [ + { name: 'linked-dir', path: path.join(resolved, 'linked-dir'), isDirectory: true }, + { name: 'broken-link', path: path.join(resolved, 'broken-link'), isDirectory: false }, + { name: 'plain.txt', path: path.join(resolved, 'plain.txt'), isDirectory: false }, + { name: 'unknown-entry', path: path.join(resolved, 'unknown-entry'), isDirectory: false } + ]) +}) + +test('readDirForIpc bounds concurrent stats while preserving complete sorted output', async () => { + const input = path.join('virtual-root') + const resolved = path.resolve(input) + const names = Array.from({ length: 105 }, (_, index) => `entry-${String(104 - index).padStart(3, '0')}`) + const failedName = 'entry-100' + const directoryNames = new Set(names.filter((_, index) => index % 10 === 4)) + const successfulDirectoryNames = new Set([...directoryNames].filter(name => name !== failedName)) + const statCalls = [] + let active = 0 + let peak = 0 + let releaseStats + let markFirstStatStarted + const statsReleased = new Promise(resolve => { + releaseStats = resolve + }) + const firstStatStarted = new Promise(resolve => { + markFirstStatStarted = resolve + }) + const fsImpl = { + promises: { + readdir: async () => [ + fakeDirent('node_modules', { symlink: true }), + ...names.map((name, index) => fakeDirent(name, { symlink: index % 2 === 0 })) + ], + stat: async fullPath => { + if (fullPath === resolved) { + return { isDirectory: () => true } + } + + statCalls.push(fullPath) + active += 1 + peak = Math.max(peak, active) + markFirstStatStarted() + await statsReleased + active -= 1 + + const name = path.basename(fullPath) + if (name === failedName) { + throw Object.assign(new Error('gone'), { code: 'ENOENT' }) + } + + return { isDirectory: () => successfulDirectoryNames.has(name) } + } + } + } + + const resultPromise = readDirForIpc(input, { fs: fsImpl }) + await firstStatStarted + await new Promise(resolve => setImmediate(resolve)) + releaseStats() + const result = await resultPromise + + const expectedNames = [ + ...names.filter(name => successfulDirectoryNames.has(name)).sort(), + ...names.filter(name => !successfulDirectoryNames.has(name)).sort() + ] + + assert.equal(result.error, undefined) + assert.equal(result.entries.length, names.length) + assert.equal(statCalls.length, names.length) + assert.equal(statCalls.some(fullPath => fullPath.endsWith(`${path.sep}node_modules`)), false) + assert.ok(peak > 1, `expected concurrent stats, observed peak ${peak}`) + assert.ok(peak <= 16, `expected at most 16 concurrent stats, observed peak ${peak}`) + assert.deepEqual( + result.entries.map(entry => entry.name), + expectedNames + ) + assert.equal(result.entries.find(entry => entry.name === failedName)?.isDirectory, false) + assert.equal( + result.entries.filter(entry => entry.isDirectory).length, + successfulDirectoryNames.size + ) +}) diff --git a/apps/desktop/electron/git-root.cjs b/apps/desktop/electron/git-root.cjs new file mode 100644 index 00000000000..593d3531ebc --- /dev/null +++ b/apps/desktop/electron/git-root.cjs @@ -0,0 +1,54 @@ +'use strict' + +const fs = require('node:fs') +const path = require('node:path') +const { resolveRequestedPathForIpc } = require('./hardening.cjs') + +function findGitRoot(start, fsImpl = fs) { + let dir = start + + for (let i = 0; i < 50; i += 1) { + try { + if (fsImpl.existsSync(path.join(dir, '.git'))) { + return dir + } + } catch { + return null + } + + const parent = path.dirname(dir) + + if (parent === dir) { + return null + } + + dir = parent + } + + return null +} + +async function gitRootForIpc(startPath, options = {}) { + const fsImpl = options.fs || fs + let resolved + + try { + resolved = resolveRequestedPathForIpc(startPath, { purpose: 'Git root' }) + } catch { + return null + } + + try { + const stat = await fsImpl.promises.stat(resolved) + const start = stat.isDirectory() ? resolved : path.dirname(resolved) + + return findGitRoot(start, fsImpl) + } catch { + return findGitRoot(resolved, fsImpl) + } +} + +module.exports = { + findGitRoot, + gitRootForIpc +} diff --git a/apps/desktop/electron/git-root.test.cjs b/apps/desktop/electron/git-root.test.cjs new file mode 100644 index 00000000000..ba649b259f3 --- /dev/null +++ b/apps/desktop/electron/git-root.test.cjs @@ -0,0 +1,40 @@ +'use strict' + +const assert = require('node:assert/strict') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') +const test = require('node:test') +const { pathToFileURL } = require('node:url') + +const { gitRootForIpc } = require('./git-root.cjs') + +function mkTmpDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-git-root-')) +} + +test('gitRootForIpc returns null for invalid and device paths', async () => { + assert.equal(await gitRootForIpc(''), null) + assert.equal(await gitRootForIpc(' '), null) + assert.equal(await gitRootForIpc(null), null) + assert.equal(await gitRootForIpc('\\\\?\\C:\\secret'), null) + assert.equal(await gitRootForIpc('file:///%E0%A4%A'), null) +}) + +test('gitRootForIpc resolves directories files missing descendants and file URLs', async t => { + const root = mkTmpDir() + t.after(() => fs.rmSync(root, { recursive: true, force: true })) + + const gitDir = path.join(root, '.git') + const srcDir = path.join(root, 'src') + const filePath = path.join(srcDir, 'index.ts') + fs.mkdirSync(gitDir) + fs.mkdirSync(srcDir) + fs.writeFileSync(filePath, 'export {}\n', 'utf8') + + assert.equal(await gitRootForIpc(root), root) + assert.equal(await gitRootForIpc(srcDir), root) + assert.equal(await gitRootForIpc(filePath), root) + assert.equal(await gitRootForIpc(pathToFileURL(filePath).toString()), root) + assert.equal(await gitRootForIpc(path.join(srcDir, 'missing.ts')), root) +}) diff --git a/apps/desktop/electron/hardening.cjs b/apps/desktop/electron/hardening.cjs index 4ffdea051b5..812dc3f77c7 100644 --- a/apps/desktop/electron/hardening.cjs +++ b/apps/desktop/electron/hardening.cjs @@ -106,71 +106,155 @@ function sensitiveFileBlockReason(filePath) { return null } -function resolveRequestedFilePath(filePath, baseDir = process.cwd(), purpose = 'File read') { - const raw = String(filePath || '').trim() +function ipcPathError(code, message) { + const error = new Error(message) + error.code = code + return error +} + +function rejectUnsafePathSyntax(filePath, purpose = 'File read') { + if (typeof filePath !== 'string') { + throw ipcPathError('invalid-path', `${purpose} failed: file path is required.`) + } + + const raw = filePath.trim() if (!raw) { - throw new Error(`${purpose} failed: file path is required.`) + throw ipcPathError('invalid-path', `${purpose} failed: file path is required.`) } if (raw.includes('\0')) { - throw new Error(`${purpose} failed: file path is invalid.`) + throw ipcPathError('invalid-path', `${purpose} failed: file path is invalid.`) } + const normalized = raw.replace(/\\/g, '/').toLowerCase() + if ( + normalized.startsWith('//?/') || + normalized.startsWith('//./') || + normalized.startsWith('globalroot/device/') || + normalized.includes('/globalroot/device/') + ) { + throw ipcPathError('device-path', `${purpose} blocked: Windows device paths are not allowed.`) + } + + return raw +} + +function resolveRequestedPathForIpc(filePath, options = {}) { + const purpose = String(options.purpose || 'File read') + const raw = rejectUnsafePathSyntax(filePath, purpose) + if (/^file:/i.test(raw)) { + let resolvedPath try { - return fileURLToPath(raw) + const parsed = new URL(raw) + if (parsed.protocol !== 'file:') { + throw new Error('not a file URL') + } + resolvedPath = fileURLToPath(parsed) } catch { - throw new Error(`${purpose} failed: file URL is invalid.`) + throw ipcPathError('invalid-path', `${purpose} failed: file URL is invalid.`) } + + rejectUnsafePathSyntax(resolvedPath, purpose) + return path.resolve(resolvedPath) } - const resolvedBase = path.resolve(String(baseDir || process.cwd())) - return path.resolve(resolvedBase, raw) + const baseInput = typeof options.baseDir === 'string' && options.baseDir.trim() ? options.baseDir : process.cwd() + const safeBaseInput = rejectUnsafePathSyntax(baseInput, purpose) + const resolvedBase = path.resolve(safeBaseInput) + rejectUnsafePathSyntax(resolvedBase, purpose) + const resolvedPath = path.resolve(resolvedBase, raw) + rejectUnsafePathSyntax(resolvedPath, purpose) + + return resolvedPath +} + +async function statForIpc(fsImpl, resolvedPath, purpose, typeLabel) { + try { + return await fsImpl.promises.stat(resolvedPath) + } catch (error) { + const code = error && typeof error === 'object' ? error.code : '' + if (code === 'ENOENT' || code === 'ENOTDIR') { + throw ipcPathError(code || 'ENOENT', `${purpose} failed: ${typeLabel} does not exist.`) + } + throw ipcPathError(code || 'read-error', `${purpose} failed: ${error instanceof Error ? error.message : String(error)}`) + } +} + +async function realpathForIpc(fsImpl, resolvedPath, purpose) { + if (typeof fsImpl.promises.realpath !== 'function') { + return resolvedPath + } + + try { + const realPath = await fsImpl.promises.realpath(resolvedPath) + rejectUnsafePathSyntax(realPath, purpose) + return realPath + } catch (error) { + const code = error && typeof error === 'object' ? error.code : '' + throw ipcPathError(code || 'read-error', `${purpose} failed: ${error instanceof Error ? error.message : String(error)}`) + } +} + +function rejectSensitiveFilePath(filePath, purpose) { + const blockReason = sensitiveFileBlockReason(filePath) + if (blockReason) { + throw ipcPathError('sensitive-file', `${purpose} blocked for sensitive file: ${blockReason}`) + } +} + +async function resolveDirectoryForIpc(dirPath, options = {}) { + const purpose = String(options.purpose || 'Directory read') + const fsImpl = options.fs || fs + const resolvedPath = resolveRequestedPathForIpc(dirPath, { baseDir: options.baseDir, purpose }) + const stat = await statForIpc(fsImpl, resolvedPath, purpose, 'directory') + + if (!stat.isDirectory()) { + throw ipcPathError('ENOTDIR', `${purpose} failed: path is not a directory.`) + } + + const realPath = await realpathForIpc(fsImpl, resolvedPath, purpose) + + return { realPath, resolvedPath, stat } } async function resolveReadableFileForIpc(filePath, options = {}) { const purpose = String(options.purpose || 'File read') - const resolvedPath = resolveRequestedFilePath(filePath, options.baseDir, purpose) + const fsImpl = options.fs || fs + const resolvedPath = resolveRequestedPathForIpc(filePath, { baseDir: options.baseDir, purpose }) if (options.blockSensitive !== false) { - const blockReason = sensitiveFileBlockReason(resolvedPath) - if (blockReason) { - throw new Error(`${purpose} blocked for sensitive file: ${blockReason}`) - } + rejectSensitiveFilePath(resolvedPath, purpose) } - let stat - try { - stat = await fs.promises.stat(resolvedPath) - } catch (error) { - const code = error && typeof error === 'object' ? error.code : '' - if (code === 'ENOENT' || code === 'ENOTDIR') { - throw new Error(`${purpose} failed: file does not exist.`) - } - throw new Error(`${purpose} failed: ${error instanceof Error ? error.message : String(error)}`) - } + const stat = await statForIpc(fsImpl, resolvedPath, purpose, 'file') if (stat.isDirectory()) { - throw new Error(`${purpose} failed: path points to a directory.`) + throw ipcPathError('EISDIR', `${purpose} failed: path points to a directory.`) } if (!stat.isFile()) { - throw new Error(`${purpose} failed: only regular files can be read.`) + throw ipcPathError('EINVAL', `${purpose} failed: only regular files can be read.`) + } + + const realPath = await realpathForIpc(fsImpl, resolvedPath, purpose) + if (options.blockSensitive !== false) { + rejectSensitiveFilePath(realPath, purpose) } const maxBytes = Number.isFinite(options.maxBytes) && Number(options.maxBytes) > 0 ? Number(options.maxBytes) : null if (maxBytes && stat.size > maxBytes) { - throw new Error(`${purpose} failed: file is too large (${stat.size} bytes; limit ${maxBytes} bytes).`) + throw ipcPathError('EFBIG', `${purpose} failed: file is too large (${stat.size} bytes; limit ${maxBytes} bytes).`) } try { - await fs.promises.access(resolvedPath, fs.constants.R_OK) + await fsImpl.promises.access(resolvedPath, fs.constants.R_OK) } catch { - throw new Error(`${purpose} failed: file is not readable.`) + throw ipcPathError('EACCES', `${purpose} failed: file is not readable.`) } - return { resolvedPath, stat } + return { realPath, resolvedPath, stat } } module.exports = { @@ -178,7 +262,10 @@ module.exports = { DEFAULT_FETCH_TIMEOUT_MS, TEXT_PREVIEW_SOURCE_MAX_BYTES, encryptDesktopSecret, + rejectUnsafePathSyntax, + resolveDirectoryForIpc, resolveReadableFileForIpc, + resolveRequestedPathForIpc, resolveTimeoutMs, sensitiveFileBlockReason } diff --git a/apps/desktop/electron/hardening.test.cjs b/apps/desktop/electron/hardening.test.cjs index 865da8fe797..a52ee27c830 100644 --- a/apps/desktop/electron/hardening.test.cjs +++ b/apps/desktop/electron/hardening.test.cjs @@ -8,11 +8,20 @@ const { pathToFileURL } = require('node:url') const { DEFAULT_FETCH_TIMEOUT_MS, encryptDesktopSecret, + resolveDirectoryForIpc, resolveReadableFileForIpc, + resolveRequestedPathForIpc, resolveTimeoutMs, sensitiveFileBlockReason } = require('./hardening.cjs') +async function rejectsWithCode(promise, code) { + await assert.rejects(promise, error => { + assert.equal(error?.code, code) + return true + }) +} + test('resolveTimeoutMs falls back to defaults and accepts overrides', () => { assert.equal(resolveTimeoutMs(undefined), DEFAULT_FETCH_TIMEOUT_MS) assert.equal(resolveTimeoutMs(0), DEFAULT_FETCH_TIMEOUT_MS) @@ -51,6 +60,52 @@ test('sensitiveFileBlockReason blocks obvious secret file patterns', () => { assert.match(String(sensitiveFileBlockReason('/tmp/server-cert.pem')), /\.pem/) }) +test('path helpers reject blank non-string NUL and Windows device syntax', async () => { + await rejectsWithCode(resolveReadableFileForIpc('', { purpose: 'File preview' }), 'invalid-path') + await rejectsWithCode(resolveReadableFileForIpc(' ', { purpose: 'File preview' }), 'invalid-path') + await rejectsWithCode(resolveReadableFileForIpc(null, { purpose: 'File preview' }), 'invalid-path') + await rejectsWithCode(resolveReadableFileForIpc(`safe${String.fromCharCode(0)}name.txt`), 'invalid-path') + + const devicePaths = [ + '\\\\?\\C:\\secret.txt', + '\\\\.\\C:\\secret.txt', + '\\\\?\\UNC\\server\\share\\secret.txt', + 'GLOBALROOT/Device/HarddiskVolumeShadowCopy1/secret.txt' + ] + + for (const devicePath of devicePaths) { + assert.throws( + () => resolveRequestedPathForIpc(devicePath, { purpose: 'File preview' }), + error => { + assert.equal(error?.code, 'device-path') + return true + } + ) + await rejectsWithCode(resolveReadableFileForIpc(devicePath, { purpose: 'File preview' }), 'device-path') + } + + assert.throws( + () => resolveRequestedPathForIpc('file:///%E0%A4%A', { purpose: 'File preview' }), + error => { + assert.equal(error?.code, 'invalid-path') + return true + } + ) + await rejectsWithCode(resolveReadableFileForIpc('file:///%E0%A4%A', { purpose: 'File preview' }), 'invalid-path') +}) + +test('resolveRequestedPathForIpc resolves relative paths from the trimmed base directory', () => { + const baseDir = path.join(os.tmpdir(), 'hermes-desktop-base') + + assert.equal( + resolveRequestedPathForIpc('notes.txt', { + baseDir: ` ${baseDir} `, + purpose: 'File preview' + }), + path.resolve(baseDir, 'notes.txt') + ) +}) + test('resolveReadableFileForIpc validates existence type size and sensitivity', async t => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-hardening-')) t.after(() => fs.rmSync(tempDir, { recursive: true, force: true })) @@ -71,6 +126,13 @@ test('resolveReadableFileForIpc validates existence type size and sensitivity', }) assert.equal(fromFileUrl.resolvedPath, textPath) + const spacedPath = path.join(tempDir, 'notes with spaces.txt') + fs.writeFileSync(spacedPath, 'space ok', 'utf8') + const fromSpacedFileUrl = await resolveReadableFileForIpc(pathToFileURL(spacedPath).toString(), { + purpose: 'File preview' + }) + assert.equal(fromSpacedFileUrl.resolvedPath, spacedPath) + await assert.rejects( resolveReadableFileForIpc('missing.txt', { baseDir: tempDir, @@ -114,3 +176,91 @@ test('resolveReadableFileForIpc validates existence type size and sensitivity', }) assert.equal(envTemplate.resolvedPath, envTemplatePath) }) + +test('resolveReadableFileForIpc blocks common sensitive files', async t => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-sensitive-')) + t.after(() => fs.rmSync(tempDir, { recursive: true, force: true })) + + const sshDir = path.join(tempDir, '.ssh') + fs.mkdirSync(sshDir) + + const blockedFiles = [ + path.join(tempDir, '.env'), + path.join(tempDir, '.npmrc'), + path.join(sshDir, 'id_ed25519'), + path.join(tempDir, 'cert.pem'), + path.join(tempDir, 'cert.p12'), + path.join(tempDir, 'cert.pfx') + ] + + for (const filePath of blockedFiles) { + fs.writeFileSync(filePath, 'secret', 'utf8') + await rejectsWithCode(resolveReadableFileForIpc(filePath, { purpose: 'File preview' }), 'sensitive-file') + } + + const allowed = path.join(tempDir, '.env.example') + fs.writeFileSync(allowed, 'EXAMPLE_TOKEN=value', 'utf8') + assert.equal((await resolveReadableFileForIpc(allowed, { purpose: 'File preview' })).resolvedPath, allowed) +}) + +test('resolveReadableFileForIpc blocks symlinks whose realpath is sensitive', async t => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-realpath-')) + t.after(() => fs.rmSync(tempDir, { recursive: true, force: true })) + + const envPath = path.join(tempDir, '.env') + const linkPath = path.join(tempDir, 'safe-name.txt') + fs.writeFileSync(envPath, 'SECRET_TOKEN=123', 'utf8') + + try { + fs.symlinkSync(envPath, linkPath, 'file') + } catch (error) { + if (error?.code === 'EPERM' || error?.code === 'EACCES') { + t.skip(`symlink creation is not permitted on this platform (${error.code})`) + return + } + throw error + } + + await rejectsWithCode(resolveReadableFileForIpc(linkPath, { purpose: 'File preview' }), 'sensitive-file') +}) + +test('resolveDirectoryForIpc accepts directories and rejects invalid directory targets', async t => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-dir-')) + t.after(() => fs.rmSync(tempDir, { recursive: true, force: true })) + + const directory = path.join(tempDir, 'project') + const filePath = path.join(tempDir, 'file.txt') + fs.mkdirSync(directory) + fs.writeFileSync(filePath, 'not a directory', 'utf8') + + const resolved = await resolveDirectoryForIpc(directory) + assert.equal(resolved.resolvedPath, directory) + assert.equal(resolved.stat.isDirectory(), true) + + await rejectsWithCode(resolveDirectoryForIpc(filePath), 'ENOTDIR') + await rejectsWithCode(resolveDirectoryForIpc(path.join(tempDir, 'missing')), 'ENOENT') + await rejectsWithCode(resolveDirectoryForIpc('\\\\?\\C:\\secret'), 'device-path') +}) + +test('resolveDirectoryForIpc accepts directory symlinks or junctions', async t => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-dir-link-')) + t.after(() => fs.rmSync(tempDir, { recursive: true, force: true })) + + const directory = path.join(tempDir, 'actual-project') + const linkPath = path.join(tempDir, 'linked-project') + fs.mkdirSync(directory) + + try { + fs.symlinkSync(directory, linkPath, process.platform === 'win32' ? 'junction' : 'dir') + } catch (error) { + if (error?.code === 'EPERM' || error?.code === 'EACCES') { + t.skip(`directory symlink creation is not permitted on this platform (${error.code})`) + return + } + throw error + } + + const resolved = await resolveDirectoryForIpc(linkPath) + assert.equal(resolved.resolvedPath, linkPath) + assert.equal(resolved.stat.isDirectory(), true) +}) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index 5af4b5605ce..9abfc216e56 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -22,7 +22,7 @@ const http = require('node:http') const https = require('node:https') const net = require('node:net') const path = require('node:path') -const { fileURLToPath, pathToFileURL } = require('node:url') +const { pathToFileURL } = require('node:url') const { execFileSync, spawn } = require('node:child_process') const { detectRemoteDisplay, isWindowsBinaryPathInWsl, isWslEnvironment } = require('./bootstrap-platform.cjs') const { runBootstrap } = require('./bootstrap-runner.cjs') @@ -31,6 +31,8 @@ const { canImportHermesCli, verifyHermesCli } = require('./backend-probes.cjs') const { probeGatewayWebSocket } = require('./gateway-ws-probe.cjs') const { serializeJsonBody, setJsonRequestHeaders } = require('./oauth-net-request.cjs') const { fetchMarketplaceThemes, searchMarketplaceThemes } = require('./vscode-marketplace.cjs') +const { readDirForIpc } = require('./fs-read-dir.cjs') +const { gitRootForIpc } = require('./git-root.cjs') const { OFFICIAL_REPO_HTTPS_URL, isOfficialSshRemote @@ -65,6 +67,7 @@ const { TEXT_PREVIEW_SOURCE_MAX_BYTES, encryptDesktopSecret: encryptDesktopSecretStrict, resolveReadableFileForIpc, + resolveRequestedPathForIpc, resolveTimeoutMs } = require('./hardening.cjs') @@ -730,7 +733,7 @@ function openExternalUrl(rawUrl) { if (parsed.protocol === 'file:') { let localPath try { - localPath = fileURLToPath(parsed.toString()) + localPath = resolveRequestedPathForIpc(parsed.toString(), { purpose: 'Open external file' }) } catch { return false } @@ -2878,10 +2881,10 @@ async function resourceBufferFromUrl(rawUrl) { const buffer = match[2] ? Buffer.from(encoded, 'base64') : Buffer.from(decodeURIComponent(encoded), 'utf8') return { buffer, mimeType } } - if (rawUrl.startsWith('file:')) { - const filePath = fileURLToPath(rawUrl) - const buffer = await fs.promises.readFile(filePath) - return { buffer, mimeType: mimeTypeForPath(filePath) } + if (/^file:/i.test(rawUrl)) { + const { resolvedPath } = await resolveReadableFileForIpc(rawUrl, { purpose: 'Image file' }) + const buffer = await fs.promises.readFile(resolvedPath) + return { buffer, mimeType: mimeTypeForPath(resolvedPath) } } const parsed = new URL(rawUrl) @@ -2959,11 +2962,13 @@ function expandUserPath(filePath) { return value } -function previewFileTarget(rawTarget, baseDir) { +async function previewFileTarget(rawTarget, baseDir) { const raw = String(rawTarget || '').trim() const base = baseDir ? path.resolve(expandUserPath(baseDir)) : resolveHermesCwd() - const filePath = raw.startsWith('file:') ? fileURLToPath(raw) : path.resolve(base, expandUserPath(raw)) - let resolved = filePath + let resolved = resolveRequestedPathForIpc(/^file:/i.test(raw) ? raw : expandUserPath(raw), { + baseDir: base, + purpose: 'Preview target' + }) if (directoryExists(resolved)) { resolved = path.join(resolved, 'index.html') @@ -2974,6 +2979,8 @@ function previewFileTarget(rawTarget, baseDir) { return null } + ;({ resolvedPath: resolved } = await resolveReadableFileForIpc(resolved, { purpose: 'Preview target' })) + const mimeType = mimeTypeForPath(resolved) const metadata = previewFileMetadata(resolved, mimeType) const isHtml = PREVIEW_HTML_EXTENSIONS.has(ext) @@ -3019,7 +3026,7 @@ function previewUrlTarget(rawTarget) { } } -function normalizePreviewTarget(rawTarget, baseDir) { +async function normalizePreviewTarget(rawTarget, baseDir) { const raw = String(rawTarget || '').trim() if (!raw) { @@ -3031,20 +3038,15 @@ function normalizePreviewTarget(rawTarget, baseDir) { return previewUrlTarget(raw) } - return previewFileTarget(raw, baseDir) + return await previewFileTarget(raw, baseDir) } catch { return null } } -function filePathFromPreviewUrl(rawUrl) { - const filePath = fileURLToPath(String(rawUrl || '')) - - if (!fileExists(filePath)) { - throw new Error('Preview file is not readable') - } - - return filePath +async function filePathFromPreviewUrl(rawUrl) { + const { resolvedPath } = await resolveReadableFileForIpc(String(rawUrl || ''), { purpose: 'Preview file' }) + return resolvedPath } function sendPreviewFileChanged(payload) { @@ -3054,8 +3056,8 @@ function sendPreviewFileChanged(payload) { webContents.send('hermes:preview-file-changed', payload) } -function watchPreviewFile(rawUrl) { - const filePath = filePathFromPreviewUrl(rawUrl) +async function watchPreviewFile(rawUrl) { + const filePath = await filePathFromPreviewUrl(rawUrl) const watchDir = path.dirname(filePath) const targetName = path.basename(filePath) const id = crypto.randomBytes(12).toString('base64url') @@ -5587,48 +5589,6 @@ ipcMain.handle('hermes:logs:reveal', async () => { ipcMain.handle('hermes:logs:recent', async () => ({ path: DESKTOP_LOG_PATH, lines: hermesLog.slice(-200) })) -// Always-hidden noise (covers non-git projects too — gitignore would catch -// these anyway when present, but we want the same hygiene without one). -const FS_READDIR_HIDDEN = new Set([ - '.git', - '.hg', - '.svn', - '.cache', - '.next', - '.turbo', - '.venv', - '__pycache__', - 'build', - 'dist', - 'node_modules', - 'target', - 'venv' -]) - -function findGitRoot(start) { - let dir = start - - for (let i = 0; i < 50; i += 1) { - try { - if (fs.existsSync(path.join(dir, '.git'))) { - return dir - } - } catch { - return null - } - - const parent = path.dirname(dir) - - if (parent === dir) { - return null - } - - dir = parent - } - - return null -} - function isExecutableFile(filePath) { if (!filePath || !path.isAbsolute(filePath)) { return false @@ -5811,46 +5771,9 @@ function disposeTerminalSession(id) { return true } -ipcMain.handle('hermes:fs:readDir', async (_event, dirPath) => { - const resolved = path.resolve(String(dirPath || '')) +ipcMain.handle('hermes:fs:readDir', async (_event, dirPath) => readDirForIpc(dirPath)) - if (!resolved) { - return { entries: [], error: 'invalid-path' } - } - - try { - const dirents = await fs.promises.readdir(resolved, { withFileTypes: true }) - - const entries = dirents - .filter(d => { - if (FS_READDIR_HIDDEN.has(d.name)) { - return false - } - - return true - }) - .map(d => ({ name: d.name, path: path.join(resolved, d.name), isDirectory: d.isDirectory() })) - .sort((a, b) => Number(b.isDirectory) - Number(a.isDirectory) || a.name.localeCompare(b.name)) - - return { entries } - } catch (error) { - return { entries: [], error: error?.code || 'read-error' } - } -}) - -ipcMain.handle('hermes:fs:gitRoot', async (_event, startPath) => { - const input = String(startPath || '') - const resolved = input.startsWith('file:') ? fileURLToPath(input) : path.resolve(input) - - try { - const stat = await fs.promises.stat(resolved) - const start = stat.isDirectory() ? resolved : path.dirname(resolved) - - return findGitRoot(start) - } catch { - return findGitRoot(resolved) - } -}) +ipcMain.handle('hermes:fs:gitRoot', async (_event, startPath) => gitRootForIpc(startPath)) ipcMain.handle('hermes:terminal:start', async (event, payload = {}) => { if (!nodePty) { diff --git a/apps/desktop/package.json b/apps/desktop/package.json index e45ec8e804b..d03bd7cd0ad 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -35,7 +35,7 @@ "test:desktop:nsis": "node scripts/test-desktop.mjs nsis", "test:desktop:existing": "node scripts/test-desktop.mjs existing", "test:desktop:fresh": "node scripts/test-desktop.mjs fresh", - "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs", + "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs", "typecheck": "tsc -p . --noEmit", "lint": "eslint src/ electron/", "lint:fix": "eslint src/ electron/ --fix", diff --git a/apps/desktop/src/app/right-sidebar/files/ipc.test.ts b/apps/desktop/src/app/right-sidebar/files/ipc.test.ts new file mode 100644 index 00000000000..bcaddad55b5 --- /dev/null +++ b/apps/desktop/src/app/right-sidebar/files/ipc.test.ts @@ -0,0 +1,100 @@ +/// + +import { Buffer } from 'node:buffer' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { HermesReadDirEntry, HermesReadDirResult } from '@/global' + +import { clearProjectDirCache, readProjectDir } from './ipc' + +const readDir = vi.fn<(path: string) => Promise>() +const readFileDataUrl = vi.fn<(path: string) => Promise>() +const gitRoot = vi.fn<(path: string) => Promise>() + +function ok(entries: HermesReadDirEntry[]): HermesReadDirResult { + return { entries } +} + +function dataUrl(text: string) { + return `data:text/plain;base64,${Buffer.from(text, 'utf8').toString('base64')}` +} + +function installBridge() { + ;( + window as unknown as { + hermesDesktop: { + gitRoot: typeof gitRoot + readDir: typeof readDir + readFileDataUrl: typeof readFileDataUrl + } + } + ).hermesDesktop = { gitRoot, readDir, readFileDataUrl } +} + +describe('readProjectDir', () => { + beforeEach(() => { + clearProjectDirCache() + readDir.mockReset() + readFileDataUrl.mockReset() + gitRoot.mockReset() + installBridge() + }) + + afterEach(() => { + clearProjectDirCache() + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + }) + + it('returns no-bridge when the desktop bridge is unavailable', async () => { + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + + await expect(readProjectDir('/repo')).resolves.toEqual({ entries: [], error: 'no-bridge' }) + }) + + it('filters gitignored entries when readDir returns Windows-style paths', async () => { + gitRoot.mockResolvedValue('C:\\repo') + readDir.mockImplementation(async path => { + if (path === 'C:\\repo\\src') { + return ok([ + { name: 'debug.log', path: 'C:\\repo\\src\\debug.log', isDirectory: false }, + { name: '临时.txt', path: 'C:\\repo\\src\\临时.txt', isDirectory: false }, + { name: 'keep.ts', path: 'C:\\repo\\src\\keep.ts', isDirectory: false } + ]) + } + + if (path === 'C:/repo') { + return ok([{ name: '.gitignore', path: 'C:/repo/.gitignore', isDirectory: false }]) + } + + if (path === 'C:/repo/src') { + return ok([]) + } + + return ok([]) + }) + readFileDataUrl.mockResolvedValue(dataUrl('# Unicode 路径规则\nsrc/*.log\nsrc/临时.txt\n')) + + const result = await readProjectDir('C:\\repo\\src', 'C:\\repo') + + expect(result.entries.map(entry => entry.name)).toEqual(['keep.ts']) + expect(gitRoot).toHaveBeenCalledWith('C:/repo') + expect(readFileDataUrl).toHaveBeenCalledWith('C:/repo/.gitignore') + }) + + it('does not fetch .gitignore contents when listings do not contain .gitignore', async () => { + gitRoot.mockResolvedValue('/repo') + readDir.mockImplementation(async path => { + if (path === '/repo/src') { + return ok([{ name: 'debug.log', path: '/repo/src/debug.log', isDirectory: false }]) + } + + return ok([]) + }) + + const result = await readProjectDir('/repo/src', '/repo') + + expect(result.entries.map(entry => entry.name)).toEqual(['debug.log']) + expect(readFileDataUrl).not.toHaveBeenCalled() + }) +}) diff --git a/apps/desktop/src/app/right-sidebar/files/ipc.ts b/apps/desktop/src/app/right-sidebar/files/ipc.ts index 843ebe761cd..078f0baab1e 100644 --- a/apps/desktop/src/app/right-sidebar/files/ipc.ts +++ b/apps/desktop/src/app/right-sidebar/files/ipc.ts @@ -27,7 +27,7 @@ function decodeDataUrl(dataUrl: string) { } function clean(path: string) { - return path.replace(/\/+$/, '') || '/' + return path.replace(/\\/g, '/').replace(/\/+$/, '') || '/' } /** Strict POSIX-style relative path; null if `child` is not inside `root`. */ diff --git a/apps/desktop/src/app/right-sidebar/files/tree.tsx b/apps/desktop/src/app/right-sidebar/files/tree.tsx index 6421581ca8c..49cd72a8d27 100644 --- a/apps/desktop/src/app/right-sidebar/files/tree.tsx +++ b/apps/desktop/src/app/right-sidebar/files/tree.tsx @@ -145,7 +145,8 @@ function ProjectTreeRow({ } const isFolder = node.data.isDirectory - const isPlaceholder = node.data.id.endsWith('::__loading__') + const isPlaceholder = Boolean(node.data.placeholder) + const isErrorPlaceholder = node.data.placeholder === 'error' return (
} - {isPlaceholder ? ( + {isPlaceholder && !isErrorPlaceholder ? ( + ) : isErrorPlaceholder ? ( + ) : isFolder ? ( ) : ( diff --git a/apps/desktop/src/app/right-sidebar/files/use-project-tree.test.ts b/apps/desktop/src/app/right-sidebar/files/use-project-tree.test.ts index a0ecd409f4a..d1c0018bf2e 100644 --- a/apps/desktop/src/app/right-sidebar/files/use-project-tree.test.ts +++ b/apps/desktop/src/app/right-sidebar/files/use-project-tree.test.ts @@ -106,7 +106,7 @@ describe('useProjectTree', () => { expect(readDir).toHaveBeenCalledTimes(1) }) - it('captures per-folder error code and leaves the folder expandable but empty', async () => { + it('captures per-folder error code and shows an error placeholder child', async () => { readDir.mockResolvedValueOnce(ok([{ name: 'priv', path: '/p/priv', isDirectory: true }])) readDir.mockResolvedValueOnce({ entries: [], error: 'EACCES' }) @@ -119,7 +119,14 @@ describe('useProjectTree', () => { }) expect(result.current.data[0].error).toBe('EACCES') - expect(result.current.data[0].children).toEqual([]) + expect(result.current.data[0].children).toEqual([ + { + id: '/p/priv::__error__', + isDirectory: false, + name: 'Unable to read (EACCES)', + placeholder: 'error' + } + ]) }) it('dedupes concurrent loadChildren calls for the same id', async () => { diff --git a/apps/desktop/src/app/right-sidebar/files/use-project-tree.ts b/apps/desktop/src/app/right-sidebar/files/use-project-tree.ts index 23fb5efe2dc..3e022c19fd3 100644 --- a/apps/desktop/src/app/right-sidebar/files/use-project-tree.ts +++ b/apps/desktop/src/app/right-sidebar/files/use-project-tree.ts @@ -14,11 +14,14 @@ export interface TreeNode { children?: TreeNode[] /** True while a readDir for this folder is in flight. */ loading?: boolean + /** Synthetic loading/error rows are not real filesystem entries. */ + placeholder?: 'error' | 'loading' /** Last error code from readDir (e.g. EACCES). Cleared on next successful load. */ error?: string } const PLACEHOLDER_ID = '__loading__' +const ERROR_PLACEHOLDER_ID = '__error__' function makeNode(path: string, name: string, isDirectory: boolean): TreeNode { return { id: path, isDirectory, name } @@ -43,7 +46,16 @@ function patchNode(nodes: TreeNode[] | undefined | null, id: string, patch: (n: } function placeholderChild(parentId: string): TreeNode { - return { id: `${parentId}::${PLACEHOLDER_ID}`, isDirectory: false, name: 'Loading…' } + return { id: `${parentId}::${PLACEHOLDER_ID}`, isDirectory: false, name: 'Loading…', placeholder: 'loading' } +} + +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' + } } export interface UseProjectTreeResult { @@ -227,7 +239,7 @@ export function useProjectTree(cwd: string): UseProjectTreeResult { ...n, loading: false, error: error || undefined, - children: error ? [] : entries.map(e => makeNode(e.path, e.name, e.isDirectory)) + children: error ? [errorChild(n.id, error)] : entries.map(e => makeNode(e.path, e.name, e.isDirectory)) })) } })