diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 0796e16e0..d951b20c8 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import path from 'path' const { execFileAsyncMock, readFileMock, rmMock } = vi.hoisted(() => ({ execFileAsyncMock: vi.fn(), @@ -19,7 +20,7 @@ vi.mock('fs/promises', () => ({ rm: rmMock })) -import { discardChanges } from './status' +import { discardChanges, isWithinWorktree } from './status' describe('discardChanges', () => { beforeEach(() => { @@ -75,4 +76,8 @@ describe('discardChanges', () => { expect(execFileAsyncMock).not.toHaveBeenCalled() expect(rmMock).not.toHaveBeenCalled() }) + + it('accepts in-tree Windows paths when resolving containment', async () => { + expect(isWithinWorktree(path.win32, 'C:\\repo', 'C:\\repo\\src\\file.ts')).toBe(true) + }) }) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index ca34989e4..e23615a62 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -1,7 +1,7 @@ import { execFile } from 'child_process' import { readFile, rm } from 'fs/promises' import { promisify } from 'util' -import { join, resolve } from 'path' +import * as path from 'path' import type { GitStatusEntry, GitFileStatus, GitDiffResult } from '../../shared/types' const execFileAsync = promisify(execFile) @@ -127,7 +127,7 @@ export async function getDiff( } else { // Unstaged: modified is the working tree version try { - modifiedContent = await readFile(join(worktreePath, filePath), 'utf-8') + modifiedContent = await readFile(path.join(worktreePath, filePath), 'utf-8') } catch { modifiedContent = '' } @@ -163,9 +163,9 @@ export async function unstageFile(worktreePath: string, filePath: string): Promi * Discard working tree changes for a file. */ export async function discardChanges(worktreePath: string, filePath: string): Promise { - const resolvedWorktree = resolve(worktreePath) - const resolvedTarget = resolve(worktreePath, filePath) - if (!resolvedTarget.startsWith(`${resolvedWorktree}/`)) { + const resolvedWorktree = path.resolve(worktreePath) + const resolvedTarget = path.resolve(worktreePath, filePath) + if (!isWithinWorktree(path, resolvedWorktree, resolvedTarget)) { throw new Error(`Path "${filePath}" resolves outside the worktree`) } @@ -187,3 +187,17 @@ export async function discardChanges(worktreePath: string, filePath: string): Pr }) : rm(resolvedTarget, { force: true, recursive: true })) } + +export function isWithinWorktree( + pathApi: Pick, + resolvedWorktree: string, + resolvedTarget: string +): boolean { + const relativeTarget = pathApi.relative(resolvedWorktree, resolvedTarget) + return !( + relativeTarget === '' || + relativeTarget === '..' || + relativeTarget.startsWith(`..${pathApi.sep}`) || + pathApi.isAbsolute(relativeTarget) + ) +} diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index 2f48bff92..df88cf8cc 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -7,6 +7,14 @@ vi.mock('fs', () => ({ existsSync: vi.fn() })) +const { execMock } = vi.hoisted(() => ({ + execMock: vi.fn() +})) + +vi.mock('child_process', () => ({ + exec: execMock +})) + describe('parseOrcaYaml', () => { it('parses YAML with setup script only', () => { const yaml = `scripts:\n setup: |\n echo "setting up"\n npm install\n` @@ -161,3 +169,113 @@ describe('getEffectiveHooks', () => { expect(result).toBeNull() }) }) + +describe('runHook', () => { + const makeRepo = (hookSettings?: { + mode: 'auto' | 'override' + scripts: { setup: string; archive: string } + }) => ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now(), + hookSettings + }) + + it('uses the Windows command shell when running hooks', async () => { + execMock.mockImplementation((_script, _options, callback) => { + callback?.(null, '', '') + return {} as never + }) + + const originalPlatform = process.platform + const originalComSpec = process.env.ComSpec + + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe' + + try { + const { runHook } = await import('./hooks') + const result = await runHook( + 'setup', + 'C:\\repo\\worktree', + makeRepo({ + mode: 'override', + scripts: { setup: 'echo hello', archive: '' } + }) + ) + + expect(result).toEqual({ success: true, output: '' }) + expect(execMock).toHaveBeenCalledWith( + 'echo hello', + expect.objectContaining({ + cwd: 'C:\\repo\\worktree', + shell: 'C:\\Windows\\System32\\cmd.exe' + }), + expect.any(Function) + ) + } finally { + Object.defineProperty(process, 'platform', { + configurable: true, + value: originalPlatform + }) + if (originalComSpec === undefined) { + delete process.env.ComSpec + } else { + process.env.ComSpec = originalComSpec + } + } + }) + + it('keeps bash as the hook shell on non-Windows platforms', async () => { + execMock.mockImplementation((_script, _options, callback) => { + callback?.(null, '', '') + return {} as never + }) + + const originalPlatform = process.platform + const originalShell = process.env.SHELL + + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'linux' + }) + process.env.SHELL = '/opt/homebrew/bin/fish' + + try { + const { runHook } = await import('./hooks') + const result = await runHook( + 'setup', + '/repo/worktree', + makeRepo({ + mode: 'override', + scripts: { setup: 'echo hello', archive: '' } + }) + ) + + expect(result).toEqual({ success: true, output: '' }) + expect(execMock).toHaveBeenCalledWith( + 'echo hello', + expect.objectContaining({ + cwd: '/repo/worktree', + shell: '/bin/bash' + }), + expect.any(Function) + ) + } finally { + Object.defineProperty(process, 'platform', { + configurable: true, + value: originalPlatform + }) + if (originalShell === undefined) { + delete process.env.SHELL + } else { + process.env.SHELL = originalShell + } + } + }) +}) diff --git a/src/main/hooks.ts b/src/main/hooks.ts index 932840875..c2f3942dc 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -7,6 +7,14 @@ import type { OrcaHooks, Repo } from '../shared/types' const HOOK_TIMEOUT = 120_000 // 2 minutes type HookName = keyof OrcaHooks['scripts'] +function getHookShell(): string | undefined { + if (process.platform === 'win32') { + return process.env.ComSpec || 'cmd.exe' + } + + return '/bin/bash' +} + /** * Parse a simple orca.yaml file. Handles only the `scripts:` block with * multiline string values (YAML block scalar `|`). @@ -138,7 +146,7 @@ export function runHook( { cwd, timeout: HOOK_TIMEOUT, - shell: '/bin/bash', + shell: getHookShell(), env: { ...process.env, ORCA_ROOT_PATH: repo.path, diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index 78fef32b7..fd975d514 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -31,7 +31,7 @@ describe('createMainWindow', () => { openExternalMock.mockReset() }) - it('enables renderer sandboxing and only opens http(s) URLs externally', () => { + it('enables renderer sandboxing and opens external links safely', () => { const windowHandlers: Record void> = {} const webContents = { on: vi.fn((event, handler) => { @@ -64,10 +64,35 @@ describe('createMainWindow', () => { ) expect(windowHandlers.windowOpen({ url: 'https://example.com' })).toEqual({ action: 'deny' }) + expect(windowHandlers.windowOpen({ url: 'localhost:3000' })).toEqual({ action: 'deny' }) expect(windowHandlers.windowOpen({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' }) expect(windowHandlers.windowOpen({ url: 'not a url' })).toEqual({ action: 'deny' }) - expect(openExternalMock).toHaveBeenCalledTimes(1) + expect(openExternalMock).toHaveBeenCalledTimes(2) expect(openExternalMock).toHaveBeenCalledWith('https://example.com') + expect(openExternalMock).toHaveBeenCalledWith('http://localhost:3000/') + + const preventDefault = vi.fn() + windowHandlers['will-navigate']({ preventDefault } as never, 'https://example.com/docs') + expect(preventDefault).toHaveBeenCalledTimes(1) + expect(openExternalMock).toHaveBeenCalledTimes(3) + expect(openExternalMock).toHaveBeenLastCalledWith('https://example.com/docs') + + const localhostPreventDefault = vi.fn() + windowHandlers['will-navigate']( + { preventDefault: localhostPreventDefault } as never, + 'localhost:3000' + ) + expect(localhostPreventDefault).toHaveBeenCalledTimes(1) + expect(openExternalMock).toHaveBeenCalledTimes(4) + expect(openExternalMock).toHaveBeenLastCalledWith('http://localhost:3000/') + + const fileNavigationPreventDefault = vi.fn() + windowHandlers['will-navigate']( + { preventDefault: fileNavigationPreventDefault } as never, + 'file:///etc/passwd' + ) + expect(fileNavigationPreventDefault).toHaveBeenCalledTimes(1) + expect(openExternalMock).toHaveBeenCalledTimes(4) }) }) diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index e8378d1cb..ad3426d5b 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -5,6 +5,26 @@ import icon from '../../../resources/icon.png?asset' import devIcon from '../../../resources/icon-dev.png?asset' import type { Store } from '../persistence' +const LOCAL_ADDRESS_PATTERN = + /^(?:localhost|127(?:\.\d{1,3}){3}|0\.0\.0\.0|\[[0-9a-f:]+\])(?::\d+)?(?:\/.*)?$/i + +function normalizeExternalUrl(rawUrl: string): string | null { + if (LOCAL_ADDRESS_PATTERN.test(rawUrl)) { + try { + return new URL(`http://${rawUrl}`).toString() + } catch { + return null + } + } + + try { + const parsed = new URL(rawUrl) + return parsed.protocol === 'https:' || parsed.protocol === 'http:' ? rawUrl : null + } catch { + return null + } +} + export function createMainWindow(store: Store | null): BrowserWindow { const mainWindow = new BrowserWindow({ width: 1200, @@ -33,21 +53,36 @@ export function createMainWindow(store: Store | null): BrowserWindow { }) mainWindow.webContents.setWindowOpenHandler((details) => { - try { - const parsed = new URL(details.url) - if (parsed.protocol === 'https:' || parsed.protocol === 'http:') { - shell.openExternal(details.url) - } - } catch { - // ignore malformed URLs + const externalUrl = normalizeExternalUrl(details.url) + if (externalUrl) { + shell.openExternal(externalUrl) } return { action: 'deny' } }) + // Block ALL in-window navigations to prevent remote pages from inheriting + // the privileged preload bridge (PTY, filesystem, etc.). + // In dev mode, allow navigations to the local dev server (e.g. HMR reloads). mainWindow.webContents.on('will-navigate', (event, url) => { - if (url.startsWith('file://')) { - event.preventDefault() + const externalUrl = normalizeExternalUrl(url) + + if (externalUrl) { + const target = new URL(externalUrl) + if (is.dev && process.env.ELECTRON_RENDERER_URL) { + try { + const allowed = new URL(process.env.ELECTRON_RENDERER_URL) + if (target.origin === allowed.origin) { + return // allow dev server navigations (HMR, etc.) + } + } catch { + // fall through to prevent + } + } + + shell.openExternal(externalUrl) } + + event.preventDefault() }) mainWindow.webContents.on('before-input-event', (event, input) => {