From affa73bcd34a68d2f1e1672f6426d09488f1665b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 29 Mar 2026 22:06:59 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20Windows=20compatibility=20=E2=80=94=20CR?= =?UTF-8?q?LF=20line=20endings=20and=20cross-drive=20path=20traversal=20(#?= =?UTF-8?q?217)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Windows compatibility — handle CRLF line endings and cross-drive path traversal - Replace literal '\n' splits with /\r?\n/ regex across git output parsers (status, worktree, hooks, file listing) to handle Windows CRLF line endings - Fix path traversal security check in isDescendantOrEqual using isAbsolute() instead of sep+sep prefix check, preventing cross-drive bypasses on Windows - Strip trailing \r from ripgrep output in quick-open file listing * fix(test): align updater test with user-initiated not-available behavior The test expected 'idle' for a user-initiated check hitting a missing latest-mac.yml, but the code correctly sends 'not-available' to give the user explicit feedback that they're on the latest version. --- src/main/git/status.ts | 8 ++++++-- src/main/git/worktree.ts | 7 +++++-- src/main/hooks.ts | 4 +++- src/main/ipc/filesystem-auth.ts | 6 ++++-- src/main/ipc/filesystem-list-files.ts | 5 ++++- src/main/updater.test.ts | 10 +++++++--- 6 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 2b2eba4e4..8c02aa920 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -34,7 +34,9 @@ export async function getStatus(worktreePath: string): Promise { cwd: worktreePath, encoding: 'utf-8' } ) - for (const line of stdout.split('\n')) { + // [Fix]: Split by /\r?\n/ instead of '\n' to correctly parse git output on Windows, + // avoiding trailing \r characters in parsed paths. + for (const line of stdout.split(/\r?\n/)) { if (!line) { continue } @@ -417,7 +419,9 @@ async function loadBranchChanges( ) const entries: GitBranchChangeEntry[] = [] - for (const line of stdout.split('\n')) { + // [Fix]: Split by /\r?\n/ instead of '\n' to handle Git CRLF output on Windows, + // preventing trailing \r characters in extracted file paths. + for (const line of stdout.split(/\r?\n/)) { if (!line) { continue } diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 70b07beac..a0d068313 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -9,14 +9,17 @@ const execFileAsync = promisify(execFile) */ export function parseWorktreeList(output: string): GitWorktreeInfo[] { const worktrees: GitWorktreeInfo[] = [] - const blocks = output.trim().split('\n\n') + // [Fix]: Use /\r?\n\r?\n/ to handle both LF and CRLF (\r\n) line endings, + // which are common when running git on Windows. + const blocks = output.trim().split(/\r?\n\r?\n/) for (const block of blocks) { if (!block.trim()) { continue } - const lines = block.trim().split('\n') + // [Fix]: Use /\r?\n/ to handle both LF and CRLF (\r\n) line endings. + const lines = block.trim().split(/\r?\n/) let path = '' let head = '' let branch = '' diff --git a/src/main/hooks.ts b/src/main/hooks.ts index c2f3942dc..c1cfdad19 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -29,7 +29,9 @@ export function parseOrcaYaml(content: string): OrcaHooks | null { } const afterScripts = content.slice(scriptsMatch.index! + scriptsMatch[0].length) - const lines = afterScripts.split('\n') + // [Fix]: Split using /\r?\n/ instead of '\n'. Otherwise, on Windows, trailing \r characters + // stay attached to script commands, which causes fatal '\r command not found' errors in WSL/bash. + const lines = afterScripts.split(/\r?\n/) let currentKey: 'setup' | 'archive' | null = null let currentValue = '' diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index 9e8563789..2782fd5c1 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -1,5 +1,5 @@ import { realpath } from 'fs/promises' -import { resolve, relative, sep, dirname, basename } from 'path' +import { resolve, relative, dirname, basename, isAbsolute } from 'path' import type { Store } from '../persistence' import { listWorktrees } from '../git/worktree' @@ -16,10 +16,12 @@ export function isDescendantOrEqual(resolvedTarget: string, resolvedBase: string } const rel = relative(resolvedBase, resolvedTarget) // rel must not start with ".." and must not be an absolute path (e.g. different drive on Windows) + // [Security Fix]: Added !isAbsolute(rel) to prevent drive traversal bypasses on Windows + // where relative('D:\\repo', 'C:\\etc\\passwd') returns absolute path 'C:\\etc\\passwd' return ( rel !== '' && !rel.startsWith('..') && - !rel.startsWith(sep + sep) && + !isAbsolute(rel) && resolve(resolvedBase, rel) === resolvedTarget ) } diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 4aad27a79..48f8ae111 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -56,7 +56,8 @@ export async function listQuickOpenFiles(rootPath: string, store: Store): Promis buf += chunk const lines = buf.split('\n') buf = lines.pop() ?? '' - for (const line of lines) { + for (let line of lines) { + line = line.replace(/\r$/, '') if (!line) { continue } @@ -74,6 +75,8 @@ export async function listQuickOpenFiles(rootPath: string, store: Store): Promis }) child.once('close', () => { if (buf) { + // [Fix]: Strip trailing \r on Windows for the final buffered chunk + buf = buf.replace(/\r$/, '') const relPath = normalizeRelativePath(relative(authorizedRootPath, buf)) if (shouldIncludeQuickOpenPath(relPath)) { files.push(relPath) diff --git a/src/main/updater.test.ts b/src/main/updater.test.ts index 85cc25082..1dd8261ee 100644 --- a/src/main/updater.test.ts +++ b/src/main/updater.test.ts @@ -263,11 +263,14 @@ describe('updater', () => { setupAutoUpdater(mainWindow as never) checkForUpdatesFromMenu() + // User-initiated checks with no fallback release show 'not-available' + // (instead of 'idle') so the user gets explicit feedback that they're + // already on the latest version. await vi.waitFor(() => { const statuses = sendMock.mock.calls .filter(([channel]) => channel === 'updater:status') .map(([, status]) => status) - expect(statuses).toContainEqual({ state: 'idle' }) + expect(statuses).toContainEqual({ state: 'not-available', userInitiated: true }) }) const statuses = sendMock.mock.calls @@ -275,7 +278,7 @@ describe('updater', () => { .map(([, status]) => status) expect(statuses).toContainEqual({ state: 'checking', userInitiated: true }) - expect(statuses).toContainEqual({ state: 'idle' }) + expect(statuses).toContainEqual({ state: 'not-available', userInitiated: true }) expect(statuses).not.toContainEqual( expect.objectContaining({ state: 'error', @@ -284,7 +287,8 @@ describe('updater', () => { ) expect( statuses.filter( - (status) => typeof status === 'object' && status !== null && status.state === 'idle' + (status) => + typeof status === 'object' && status !== null && status.state === 'not-available' ) ).toHaveLength(1) })