fix: Windows compatibility — CRLF line endings and cross-drive path traversal (#217)
* 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.
This commit is contained in:
parent
c625ce882e
commit
affa73bcd3
|
|
@ -34,7 +34,9 @@ export async function getStatus(worktreePath: string): Promise<GitStatusResult>
|
|||
{ 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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 = ''
|
||||
|
|
|
|||
|
|
@ -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 = ''
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue