diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index c92083266..99842de0a 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -42,7 +42,8 @@ const { cancelGeneratePullRequestFieldsLocalMock, getSshFilesystemProviderMock, getSshGitProviderMock, - tryDeleteWslUncPathMock + tryDeleteWslUncPathMock, + recordCrashBreadcrumbMock } = vi.hoisted(() => ({ handleMock: vi.fn(), showSaveDialogMock: vi.fn(), @@ -82,7 +83,8 @@ const { cancelGeneratePullRequestFieldsLocalMock: vi.fn(), getSshFilesystemProviderMock: vi.fn(), getSshGitProviderMock: vi.fn(), - tryDeleteWslUncPathMock: vi.fn() + tryDeleteWslUncPathMock: vi.fn(), + recordCrashBreadcrumbMock: vi.fn() })) vi.mock('electron', () => ({ @@ -116,6 +118,10 @@ vi.mock('../wsl-unc-delete', () => ({ tryDeleteWslUncPath: tryDeleteWslUncPathMock })) +vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({ + recordCrashBreadcrumb: recordCrashBreadcrumbMock +})) + vi.mock('../git/status', () => ({ commitChanges: commitChangesMock, getStatus: getStatusMock, @@ -247,6 +253,7 @@ describe('registerFilesystemHandlers', () => { rmMock, realpathMock, lstatMock, + recordCrashBreadcrumbMock, commitChangesMock, getStatusMock, abortMergeMock, @@ -325,6 +332,65 @@ describe('registerFilesystemHandlers', () => { ) }) + it('records a redacted breadcrumb when fs:readDir throws on a WSL UNC path', async () => { + registerFilesystemHandlers(store as never) + const wslPath = '\\\\wsl.localhost\\Ubuntu\\home\\user\\repo' + // resolveAuthorizedPath authorizes the path, then readdir fails (distro stopped). + realpathMock.mockResolvedValue(wslPath) + registerWorktreeRootsForRepo(store as never, 'repo-1', [wslPath]) + readdirMock.mockRejectedValue(Object.assign(new Error('EIO: i/o error'), { code: 'EIO' })) + + await expect(handlers.get('fs:readDir')!(null, { dirPath: wslPath })).rejects.toThrow(/EIO/) + + expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith('fs_readdir_error', { + throwSite: 'readdir', + errorName: 'Error', + errorCode: 'EIO', + hasConnectionId: false, + isUNC: true, + isWsl: true + }) + // The raw path must never appear in the breadcrumb payload. + const [, breadcrumbData] = recordCrashBreadcrumbMock.mock.calls[0] + expect(JSON.stringify(breadcrumbData)).not.toContain('user') + }) + + it('records a breadcrumb tagged ssh-provider when the SSH provider is gone', async () => { + registerFilesystemHandlers(store as never) + getSshFilesystemProviderMock.mockReturnValue(undefined) + + await expect( + handlers.get('fs:readDir')!(null, { dirPath: '/remote/repo', connectionId: 'ssh-1' }) + ).rejects.toThrow() + + expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith( + 'fs_readdir_error', + expect.objectContaining({ throwSite: 'ssh-provider', hasConnectionId: true }) + ) + }) + + it('records a breadcrumb tagged authorize when the path is denied', async () => { + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('fs:readDir')!(null, { dirPath: path.resolve('/etc/passwd') }) + ).rejects.toThrow() + + expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith( + 'fs_readdir_error', + expect.objectContaining({ throwSite: 'authorize', hasConnectionId: false }) + ) + }) + + it('does not record a breadcrumb when fs:readDir succeeds', async () => { + registerFilesystemHandlers(store as never) + readdirMock.mockResolvedValue([dirEntry({ name: 'file.ts', file: true })]) + + await handlers.get('fs:readDir')!(null, { dirPath: REPO_PATH }) + + expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled() + }) + it('rejects remote downloads with missing required arguments', async () => { registerFilesystemHandlers(store as never) diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 2664d00f9..85c0ce56e 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -115,6 +115,8 @@ import { type CommitMessageAgentEnvironmentResolvers } from '../text-generation/commit-message-agent-environment' import { listRepoWorktrees } from '../repo-worktrees' +import { recordCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store' +import { buildReadDirErrorBreadcrumb, type ReadDirThrowSite } from './readdir-error-diagnostics' import { splitWorktreeId } from '../../shared/worktree-id' import { getRuntimePathBasename } from '../../shared/cross-platform-path' import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options' @@ -483,27 +485,48 @@ export function registerFilesystemHandlers( ipcMain.handle( 'fs:readDir', async (_event, args: { dirPath: string; connectionId?: string }): Promise => { - if (args.connectionId) { - const provider = requireSshFilesystemProvider(args.connectionId) - return provider.readDir(args.dirPath) - } - const dirPath = await resolveAuthorizedPath(args.dirPath, store) - const entries = await readdir(dirPath, { withFileTypes: true }) - const mapped = await Promise.all( - entries.map(async (entry) => ({ - name: entry.name, - isDirectory: await isDirectoryEntry(dirPath, entry, (entryPath) => - resolveAuthorizedPath(entryPath, store) - ), - isSymlink: entry.isSymbolicLink() - })) - ) - return mapped.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 + // Why: a thrown fs:readDir reaches the renderer as the opaque "Error + // invoking remote method 'fs:readDir'" (Windows WSL/UNC realpath/readdir + // failures, dropped SSH providers). Record which throw site fired plus a + // redacted path shape so these are diagnosable without the raw path. + let throwSite: ReadDirThrowSite = 'authorize' + try { + if (args.connectionId) { + throwSite = 'ssh-provider' + const provider = requireSshFilesystemProvider(args.connectionId) + return await provider.readDir(args.dirPath) } - return a.name.localeCompare(b.name) - }) + throwSite = 'authorize' + const dirPath = await resolveAuthorizedPath(args.dirPath, store) + throwSite = 'readdir' + const entries = await readdir(dirPath, { withFileTypes: true }) + const mapped = await Promise.all( + entries.map(async (entry) => ({ + name: entry.name, + isDirectory: await isDirectoryEntry(dirPath, entry, (entryPath) => + resolveAuthorizedPath(entryPath, store) + ), + isSymlink: entry.isSymbolicLink() + })) + ) + return mapped.sort((a, b) => { + if (a.isDirectory !== b.isDirectory) { + return a.isDirectory ? -1 : 1 + } + return a.name.localeCompare(b.name) + }) + } catch (error: unknown) { + recordCrashBreadcrumb( + 'fs_readdir_error', + buildReadDirErrorBreadcrumb({ + dirPath: args.dirPath, + connectionId: args.connectionId, + throwSite, + error + }) + ) + throw error + } } ) diff --git a/src/main/ipc/readdir-error-diagnostics.test.ts b/src/main/ipc/readdir-error-diagnostics.test.ts new file mode 100644 index 000000000..cf599afbd --- /dev/null +++ b/src/main/ipc/readdir-error-diagnostics.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest' +import { buildReadDirErrorBreadcrumb, describeReadDirPathShape } from './readdir-error-diagnostics' + +describe('describeReadDirPathShape', () => { + it('classifies a WSL UNC path without leaking it', () => { + const shape = describeReadDirPathShape('\\\\wsl.localhost\\Ubuntu\\home\\u\\repo', undefined) + expect(shape).toEqual({ hasConnectionId: false, isUNC: true, isWsl: true }) + }) + + it('classifies the legacy \\\\wsl$ root as WSL', () => { + expect(describeReadDirPathShape('\\\\wsl$\\Ubuntu\\home', undefined).isWsl).toBe(true) + }) + + it('classifies a plain network UNC share as UNC but not WSL', () => { + const shape = describeReadDirPathShape('\\\\fileserver\\share\\dir', undefined) + expect(shape).toMatchObject({ isUNC: true, isWsl: false }) + expect(shape.driveLetter).toBeUndefined() + }) + + it('extracts an uppercased drive letter for mapped drives', () => { + expect(describeReadDirPathShape('z:\\projects\\repo', undefined)).toEqual({ + hasConnectionId: false, + isUNC: false, + isWsl: false, + driveLetter: 'Z' + }) + }) + + it('flags the SSH connection without recording it', () => { + const shape = describeReadDirPathShape('/remote/repo', 'ssh-1') + expect(shape).toEqual({ hasConnectionId: true, isUNC: false, isWsl: false }) + }) + + it('never includes the raw path in the shape', () => { + const shape = describeReadDirPathShape('\\\\wsl.localhost\\Ubuntu\\secret\\path', 'ssh-9') + expect(JSON.stringify(shape)).not.toContain('secret') + }) +}) + +describe('buildReadDirErrorBreadcrumb', () => { + it('captures throw site, error code/name, and path shape', () => { + const breadcrumb = buildReadDirErrorBreadcrumb({ + dirPath: '\\\\wsl.localhost\\Ubuntu\\home\\u\\repo', + connectionId: undefined, + throwSite: 'readdir', + error: Object.assign(new Error('EIO: i/o error'), { code: 'EIO' }) + }) + expect(breadcrumb).toEqual({ + throwSite: 'readdir', + errorName: 'Error', + errorCode: 'EIO', + hasConnectionId: false, + isUNC: true, + isWsl: true + }) + }) + + it('omits errorCode when the error has none', () => { + const breadcrumb = buildReadDirErrorBreadcrumb({ + dirPath: '/remote/repo', + connectionId: 'ssh-1', + throwSite: 'ssh-provider', + error: new Error('Remote connection dropped.') + }) + expect(breadcrumb).toMatchObject({ throwSite: 'ssh-provider', errorName: 'Error' }) + expect(breadcrumb.errorCode).toBeUndefined() + expect(breadcrumb.hasConnectionId).toBe(true) + }) +}) diff --git a/src/main/ipc/readdir-error-diagnostics.ts b/src/main/ipc/readdir-error-diagnostics.ts new file mode 100644 index 000000000..dd77dda88 --- /dev/null +++ b/src/main/ipc/readdir-error-diagnostics.ts @@ -0,0 +1,57 @@ +import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting' + +export type ReadDirThrowSite = 'ssh-provider' | 'authorize' | 'readdir' + +/** + * Redacted shape of a directory path for crash diagnostics. + * + * Why shape, not the path: "Error invoking remote method 'fs:readDir'" reaches + * the renderer with no cause. We want to know *what kind* of path failed (WSL + * UNC, network share, drive letter, SSH) without recording the path itself — + * even though breadcrumbs are path-redacted downstream, never collecting the + * raw path is the safer default. + */ +export function describeReadDirPathShape( + dirPath: string, + connectionId: string | undefined +): CrashReportBreadcrumbData { + const isUNC = /^[\\/]{2}/.test(dirPath) + const lower = dirPath.toLowerCase() + // \\wsl$\ and \\wsl.localhost\ (either slash direction) are WSL UNC roots. + const isWsl = isUNC && (lower.includes('wsl$') || lower.includes('wsl.localhost')) + const driveLetterMatch = /^([a-zA-Z]):[\\/]/.exec(dirPath) + return { + hasConnectionId: Boolean(connectionId), + isUNC, + isWsl, + ...(driveLetterMatch ? { driveLetter: driveLetterMatch[1].toUpperCase() } : {}) + } +} + +function errorCode(error: unknown): string | undefined { + if (error && typeof error === 'object' && 'code' in error) { + const code = (error as { code?: unknown }).code + if (typeof code === 'string') { + return code + } + } + return undefined +} + +/** + * Build the redacted breadcrumb payload for a thrown fs:readDir call: which + * throw site fired, the error code/name, and the path shape — no raw path. + */ +export function buildReadDirErrorBreadcrumb(args: { + dirPath: string + connectionId: string | undefined + throwSite: ReadDirThrowSite + error: unknown +}): CrashReportBreadcrumbData { + return { + throwSite: args.throwSite, + errorName: args.error instanceof Error ? args.error.name : typeof args.error, + ...(errorCode(args.error) ? { errorCode: errorCode(args.error)! } : {}), + ...describeReadDirPathShape(args.dirPath, args.connectionId) + } +}