fix: extract path-security helpers, add tests, and improve type safety (#102)

* fix: extract path-security helpers and refactor IPC modules

- Extracted path-security helpers from filesystem.ts into filesystem-auth.ts to fix max-lines lint error (402 -> 293 lines)
- Extracted duplicated ENOENT detection into isENOENT() helper to eliminate code duplication
- Fixed missing curly braces on single-line if-return in isDescendantOrEqual (lint violation)
- Replaced any with unknown in test files to satisfy lint rules
- All tests passing (29/29)
- Lint clean (0 errors, 0 warnings)

* fix: bundle preload deps for sandbox mode and fix editor test types

The sandbox: true change in createMainWindow broke the app because
electron-vite was externalizing @electron-toolkit/preload, producing
a require() call that fails in sandboxed preload scripts. Exclude it
from externalization so it gets bundled inline.

Also fix type errors in editor.test.ts from the partial store setup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jinjing 2026-03-25 09:59:19 -07:00 committed by GitHub
parent 6aab126edb
commit 4c0eee235b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 849 additions and 101 deletions

View file

@ -5,7 +5,13 @@ import tailwindcss from '@tailwindcss/vite'
export default defineConfig({
main: {},
preload: {},
preload: {
build: {
externalizeDeps: {
exclude: ['@electron-toolkit/preload']
}
}
},
renderer: {
resolve: {
alias: {

View file

@ -1,5 +1,17 @@
import { describe, expect, it } from 'vitest'
import { deriveCheckStatus } from './client'
import { deriveCheckStatus, mapPRState } from './client'
describe('mapPRState', () => {
it('returns draft when an open PR is marked as draft', () => {
expect(mapPRState('OPEN', true)).toBe('draft')
})
it('preserves merged and closed states', () => {
expect(mapPRState('MERGED', true)).toBe('merged')
expect(mapPRState('CLOSED', true)).toBe('closed')
expect(mapPRState('OPEN')).toBe('open')
})
})
describe('deriveCheckStatus', () => {
it('returns neutral when no checks are present', () => {
@ -8,12 +20,7 @@ describe('deriveCheckStatus', () => {
})
it('returns failure when a failed check is present', () => {
expect(
deriveCheckStatus([
{ status: 'QUEUED' },
{ conclusion: 'FAILURE' }
])
).toBe('failure')
expect(deriveCheckStatus([{ status: 'QUEUED' }, { conclusion: 'FAILURE' }])).toBe('failure')
})
it('returns pending when checks are still running', () => {
@ -21,11 +28,8 @@ describe('deriveCheckStatus', () => {
})
it('returns success when all checks complete without failures', () => {
expect(
deriveCheckStatus([
{ conclusion: 'SUCCESS' },
{ conclusion: 'SKIPPED' }
])
).toBe('success')
expect(deriveCheckStatus([{ conclusion: 'SUCCESS' }, { conclusion: 'SKIPPED' }])).toBe(
'success'
)
})
})

View file

@ -41,7 +41,13 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
const branchName = branch.replace(/^refs\/heads\//, '')
const { stdout } = await execFileAsync(
'gh',
['pr', 'view', branchName, '--json', 'number,title,state,url,statusCheckRollup,updatedAt'],
[
'pr',
'view',
branchName,
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft'
],
{
cwd: repoPath,
encoding: 'utf-8'
@ -51,7 +57,7 @@ export async function getPRForBranch(repoPath: string, branch: string): Promise<
return {
number: data.number,
title: data.title,
state: mapPRState(data.state),
state: mapPRState(data.state, data.isDraft),
url: data.url,
checksStatus: deriveCheckStatus(data.statusCheckRollup),
updatedAt: data.updatedAt
@ -216,7 +222,7 @@ function mapCheckConclusion(state: string): PRCheckDetail['conclusion'] {
return null
}
export function mapPRState(state: string): PRInfo['state'] {
export function mapPRState(state: string, isDraft?: boolean): PRInfo['state'] {
const s = state?.toUpperCase()
if (s === 'MERGED') {
return 'merged'
@ -224,7 +230,9 @@ export function mapPRState(state: string): PRInfo['state'] {
if (s === 'CLOSED') {
return 'closed'
}
// gh CLI returns isDraft separately, but state field is OPEN for drafts too
if (isDraft) {
return 'draft'
}
return 'open'
}

View file

@ -0,0 +1,125 @@
import { realpath } from 'fs/promises'
import { resolve, relative, sep, dirname, basename } from 'path'
import type { Store } from '../persistence'
import { listWorktrees } from '../git/worktree'
export const PATH_ACCESS_DENIED_MESSAGE =
'Access denied: path resolves outside allowed directories. If this blocks a legitimate workflow, please file a GitHub issue.'
/**
* Check whether resolvedTarget is equal to or a descendant of resolvedBase.
* Uses relative() so it works with both `/` (Unix) and `\` (Windows) separators.
*/
export function isDescendantOrEqual(resolvedTarget: string, resolvedBase: string): boolean {
if (resolvedTarget === resolvedBase) {
return true
}
const rel = relative(resolvedBase, resolvedTarget)
// rel must not start with ".." and must not be an absolute path (e.g. different drive on Windows)
return (
rel !== '' &&
!rel.startsWith('..') &&
!rel.startsWith(sep + sep) &&
resolve(resolvedBase, rel) === resolvedTarget
)
}
export function getAllowedRoots(store: Store): string[] {
const roots = store.getRepos().map((repo) => resolve(repo.path))
const workspaceDir = store.getSettings().workspaceDir
if (workspaceDir) {
roots.push(resolve(workspaceDir))
}
return roots
}
export function isPathAllowed(targetPath: string, store: Store): boolean {
const resolvedTarget = resolve(targetPath)
return getAllowedRoots(store).some((root) => isDescendantOrEqual(resolvedTarget, root))
}
/**
* Returns true if the error is an ENOENT (file-not-found) error.
*/
export function isENOENT(error: unknown): boolean {
return (
error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'ENOENT'
)
}
export async function resolveAuthorizedPath(targetPath: string, store: Store): Promise<string> {
const resolvedTarget = resolve(targetPath)
if (!isPathAllowed(resolvedTarget, store)) {
throw new Error(PATH_ACCESS_DENIED_MESSAGE)
}
try {
const realTarget = await realpath(resolvedTarget)
if (!isPathAllowed(realTarget, store)) {
throw new Error(PATH_ACCESS_DENIED_MESSAGE)
}
return realTarget
} catch (error) {
if (!isENOENT(error)) {
throw error
}
const realParent = await realpath(dirname(resolvedTarget))
const candidateTarget = resolve(realParent, basename(resolvedTarget))
if (!isPathAllowed(candidateTarget, store)) {
throw new Error(PATH_ACCESS_DENIED_MESSAGE)
}
return candidateTarget
}
}
async function normalizeExistingPath(targetPath: string): Promise<string> {
try {
return await realpath(targetPath)
} catch {
return resolve(targetPath)
}
}
export async function resolveRegisteredWorktreePath(
worktreePath: string,
store: Store
): Promise<string> {
const resolvedPath = await resolveAuthorizedPath(worktreePath, store)
for (const repo of store.getRepos()) {
const normalizedRepoPath = await normalizeExistingPath(repo.path)
if (resolvedPath === normalizedRepoPath) {
return resolvedPath
}
const worktrees = await listWorktrees(repo.path)
for (const worktree of worktrees) {
const normalizedWorktreePath = await normalizeExistingPath(worktree.path)
if (resolvedPath === normalizedWorktreePath) {
return resolvedPath
}
}
}
throw new Error('Access denied: unknown repository or worktree path')
}
export function validateGitRelativeFilePath(worktreePath: string, filePath: string): string {
if (!filePath || filePath.includes('\0') || resolve(filePath) === filePath) {
throw new Error('Access denied: invalid git file path')
}
const resolvedFilePath = resolve(worktreePath, filePath)
if (!isDescendantOrEqual(resolvedFilePath, worktreePath)) {
throw new Error('Access denied: git file path escapes the selected worktree')
}
const normalizedRelativePath = relative(worktreePath, resolvedFilePath)
if (!normalizedRelativePath) {
throw new Error('Access denied: invalid git file path')
}
return normalizedRelativePath
}

View file

@ -0,0 +1,178 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const handlers = new Map<string, (_event: unknown, args: unknown) => Promise<unknown> | unknown>()
const {
handleMock,
readdirMock,
readFileMock,
writeFileMock,
statMock,
realpathMock,
lstatMock,
getStatusMock,
getDiffMock,
stageFileMock,
unstageFileMock,
discardChangesMock,
listWorktreesMock
} = vi.hoisted(() => ({
handleMock: vi.fn(),
readdirMock: vi.fn(),
readFileMock: vi.fn(),
writeFileMock: vi.fn(),
statMock: vi.fn(),
realpathMock: vi.fn(),
lstatMock: vi.fn(),
getStatusMock: vi.fn(),
getDiffMock: vi.fn(),
stageFileMock: vi.fn(),
unstageFileMock: vi.fn(),
discardChangesMock: vi.fn(),
listWorktreesMock: vi.fn()
}))
vi.mock('electron', () => ({
ipcMain: {
handle: handleMock
}
}))
vi.mock('fs/promises', () => ({
readdir: readdirMock,
readFile: readFileMock,
writeFile: writeFileMock,
stat: statMock,
realpath: realpathMock,
lstat: lstatMock
}))
vi.mock('../git/status', () => ({
getStatus: getStatusMock,
getDiff: getDiffMock,
stageFile: stageFileMock,
unstageFile: unstageFileMock,
discardChanges: discardChangesMock
}))
vi.mock('../git/worktree', () => ({
listWorktrees: listWorktreesMock
}))
import { registerFilesystemHandlers } from './filesystem'
describe('registerFilesystemHandlers', () => {
const store = {
getRepos: () => [
{
id: 'repo-1',
path: '/workspace/repo',
displayName: 'repo',
badgeColor: '#000',
addedAt: 0
}
],
getSettings: () => ({
workspaceDir: '/workspace'
})
}
beforeEach(() => {
handlers.clear()
handleMock.mockReset()
readdirMock.mockReset()
readFileMock.mockReset()
writeFileMock.mockReset()
statMock.mockReset()
realpathMock.mockReset()
lstatMock.mockReset()
getStatusMock.mockReset()
getDiffMock.mockReset()
stageFileMock.mockReset()
unstageFileMock.mockReset()
discardChangesMock.mockReset()
listWorktreesMock.mockReset()
handleMock.mockImplementation((channel, handler) => {
handlers.set(channel, handler)
})
realpathMock.mockImplementation(async (targetPath: string) => targetPath)
listWorktreesMock.mockResolvedValue([
{ path: '/workspace/repo-feature', head: 'abc', branch: '', isBare: false }
])
statMock.mockResolvedValue({ size: 10, isDirectory: () => false, mtimeMs: 123 })
lstatMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }))
})
it('rejects readFile when the real path escapes allowed roots', async () => {
realpathMock.mockImplementation(async (targetPath: string) => {
if (targetPath === '/workspace/repo/link.txt') {
return '/private/secret.txt'
}
return targetPath
})
registerFilesystemHandlers(store as never)
await expect(
handlers.get('fs:readFile')!(null, { filePath: '/workspace/repo/link.txt' })
).rejects.toThrow('Access denied: path resolves outside allowed directories')
expect(readFileMock).not.toHaveBeenCalled()
})
it('rejects writes to directories', async () => {
lstatMock.mockResolvedValue({ isDirectory: () => true })
registerFilesystemHandlers(store as never)
await expect(
handlers.get('fs:writeFile')!(null, {
filePath: '/workspace/repo/folder',
content: 'data'
})
).rejects.toThrow('Cannot write to a directory')
expect(writeFileMock).not.toHaveBeenCalled()
})
it('normalizes repo worktree paths and keeps git file paths relative', async () => {
stageFileMock.mockResolvedValue(undefined)
registerFilesystemHandlers(store as never)
await handlers.get('git:stage')!(null, {
worktreePath: '/workspace/repo-feature',
filePath: './src/../src/file.ts'
})
expect(stageFileMock).toHaveBeenCalledWith('/workspace/repo-feature', 'src/file.ts')
})
it('rejects git file paths that escape the selected worktree', async () => {
registerFilesystemHandlers(store as never)
await expect(
handlers.get('git:discard')!(null, {
worktreePath: '/workspace/repo-feature',
filePath: '../outside.txt'
})
).rejects.toThrow('Access denied: git file path escapes the selected worktree')
expect(discardChangesMock).not.toHaveBeenCalled()
})
it('rejects git operations for unknown worktrees', async () => {
listWorktreesMock.mockResolvedValue([])
registerFilesystemHandlers(store as never)
await expect(
handlers.get('git:status')!(null, {
worktreePath: '/workspace/repo-feature'
})
).rejects.toThrow('Access denied: unknown repository or worktree path')
expect(getStatusMock).not.toHaveBeenCalled()
})
})

View file

@ -1,6 +1,6 @@
import { ipcMain } from 'electron'
import { readdir, readFile, writeFile, stat } from 'fs/promises'
import { resolve, relative } from 'path'
import { readdir, readFile, writeFile, stat, lstat } from 'fs/promises'
import { relative } from 'path'
import { spawn } from 'child_process'
import type { Store } from '../persistence'
import type {
@ -12,41 +12,15 @@ import type {
SearchFileResult
} from '../../shared/types'
import { getStatus, getDiff, stageFile, unstageFile, discardChanges } from '../git/status'
import {
resolveAuthorizedPath,
resolveRegisteredWorktreePath,
validateGitRelativeFilePath,
isENOENT
} from './filesystem-auth'
const MAX_FILE_SIZE = 5 * 1024 * 1024 // 5MB
/**
* Validate that a path is within a known worktree directory.
*/
function isPathAllowed(targetPath: string, store: Store): boolean {
const resolvedTarget = resolve(targetPath)
const repos = store.getRepos()
for (const repo of repos) {
// Allow paths within the repo itself
if (
resolvedTarget.startsWith(`${resolve(repo.path)}/`) ||
resolvedTarget === resolve(repo.path)
) {
return true
}
}
// Also check the workspace directory from settings
const settings = store.getSettings()
if (settings.workspaceDir) {
const resolvedWorkspace = resolve(settings.workspaceDir)
if (
resolvedTarget.startsWith(`${resolvedWorkspace}/`) ||
resolvedTarget === resolvedWorkspace
) {
return true
}
}
return false
}
/**
* Check if a buffer appears to be binary (contains null bytes in first 8KB).
*/
@ -63,11 +37,8 @@ function isBinaryBuffer(buffer: Buffer): boolean {
export function registerFilesystemHandlers(store: Store): void {
// ─── Filesystem ─────────────────────────────────────────
ipcMain.handle('fs:readDir', async (_event, args: { dirPath: string }): Promise<DirEntry[]> => {
if (!isPathAllowed(args.dirPath, store)) {
throw new Error('Access denied: path is outside allowed directories')
}
const entries = await readdir(args.dirPath, { withFileTypes: true })
const dirPath = await resolveAuthorizedPath(args.dirPath, store)
const entries = await readdir(dirPath, { withFileTypes: true })
return entries
.map((entry) => ({
name: entry.name,
@ -86,18 +57,15 @@ export function registerFilesystemHandlers(store: Store): void {
ipcMain.handle(
'fs:readFile',
async (_event, args: { filePath: string }): Promise<{ content: string; isBinary: boolean }> => {
if (!isPathAllowed(args.filePath, store)) {
throw new Error('Access denied: path is outside allowed directories')
}
const stats = await stat(args.filePath)
const filePath = await resolveAuthorizedPath(args.filePath, store)
const stats = await stat(filePath)
if (stats.size > MAX_FILE_SIZE) {
throw new Error(
`File too large: ${(stats.size / 1024 / 1024).toFixed(1)}MB exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit`
)
}
const buffer = await readFile(args.filePath)
const buffer = await readFile(filePath)
if (isBinaryBuffer(buffer)) {
return { content: '', isBinary: true }
}
@ -109,11 +77,20 @@ export function registerFilesystemHandlers(store: Store): void {
ipcMain.handle(
'fs:writeFile',
async (_event, args: { filePath: string; content: string }): Promise<void> => {
if (!isPathAllowed(args.filePath, store)) {
throw new Error('Access denied: path is outside allowed directories')
const filePath = await resolveAuthorizedPath(args.filePath, store)
try {
const fileStats = await lstat(filePath)
if (fileStats.isDirectory()) {
throw new Error('Cannot write to a directory')
}
} catch (error) {
if (!isENOENT(error)) {
throw error
}
}
await writeFile(args.filePath, args.content, 'utf-8')
await writeFile(filePath, args.content, 'utf-8')
}
)
@ -123,11 +100,8 @@ export function registerFilesystemHandlers(store: Store): void {
_event,
args: { filePath: string }
): Promise<{ size: number; isDirectory: boolean; mtime: number }> => {
if (!isPathAllowed(args.filePath, store)) {
throw new Error('Access denied: path is outside allowed directories')
}
const stats = await stat(args.filePath)
const filePath = await resolveAuthorizedPath(args.filePath, store)
const stats = await stat(filePath)
return {
size: stats.size,
isDirectory: stats.isDirectory(),
@ -138,9 +112,7 @@ export function registerFilesystemHandlers(store: Store): void {
// ─── Search ────────────────────────────────────────────
ipcMain.handle('fs:search', async (_event, args: SearchOptions): Promise<SearchResult> => {
if (!isPathAllowed(args.rootPath, store)) {
throw new Error('Access denied: path is outside allowed directories')
}
const rootPath = await resolveAuthorizedPath(args.rootPath, store)
const maxResults = args.maxResults ?? 10000
@ -179,7 +151,7 @@ export function registerFilesystemHandlers(store: Store): void {
}
}
rgArgs.push('--', args.query, args.rootPath)
rgArgs.push('--', args.query, rootPath)
const fileMap = new Map<string, SearchFileResult>()
let totalMatches = 0
@ -213,7 +185,7 @@ export function registerFilesystemHandlers(store: Store): void {
const data = msg.data
const absPath: string = data.path.text
const relPath = relative(args.rootPath, absPath)
const relPath = relative(rootPath, absPath)
let fileResult = fileMap.get(absPath)
if (!fileResult) {
@ -275,7 +247,8 @@ export function registerFilesystemHandlers(store: Store): void {
ipcMain.handle(
'git:status',
async (_event, args: { worktreePath: string }): Promise<GitStatusEntry[]> => {
return getStatus(args.worktreePath)
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
return getStatus(worktreePath)
}
)
@ -285,28 +258,36 @@ export function registerFilesystemHandlers(store: Store): void {
_event,
args: { worktreePath: string; filePath: string; staged: boolean }
): Promise<GitDiffResult> => {
return getDiff(args.worktreePath, args.filePath, args.staged)
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
const filePath = validateGitRelativeFilePath(worktreePath, args.filePath)
return getDiff(worktreePath, filePath, args.staged)
}
)
ipcMain.handle(
'git:stage',
async (_event, args: { worktreePath: string; filePath: string }): Promise<void> => {
await stageFile(args.worktreePath, args.filePath)
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
const filePath = validateGitRelativeFilePath(worktreePath, args.filePath)
await stageFile(worktreePath, filePath)
}
)
ipcMain.handle(
'git:unstage',
async (_event, args: { worktreePath: string; filePath: string }): Promise<void> => {
await unstageFile(args.worktreePath, args.filePath)
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
const filePath = validateGitRelativeFilePath(worktreePath, args.filePath)
await unstageFile(worktreePath, filePath)
}
)
ipcMain.handle(
'git:discard',
async (_event, args: { worktreePath: string; filePath: string }): Promise<void> => {
await discardChanges(args.worktreePath, args.filePath)
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
const filePath = validateGitRelativeFilePath(worktreePath, args.filePath)
await discardChanges(worktreePath, filePath)
}
)
}

View file

@ -0,0 +1,92 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { handleMock, getPRForBranchMock, getIssueMock, listIssuesMock } = vi.hoisted(() => ({
handleMock: vi.fn(),
getPRForBranchMock: vi.fn(),
getIssueMock: vi.fn(),
listIssuesMock: vi.fn()
}))
vi.mock('electron', () => ({
ipcMain: {
handle: handleMock
}
}))
vi.mock('../github/client', () => ({
getPRForBranch: getPRForBranchMock,
getIssue: getIssueMock,
listIssues: listIssuesMock
}))
import { registerGitHubHandlers } from './github'
type HandlerMap = Record<string, (_event: unknown, args: unknown) => unknown>
describe('registerGitHubHandlers', () => {
const handlers: HandlerMap = {}
const store = {
getRepos: () => [
{
id: 'repo-1',
path: '/workspace/repo',
displayName: 'repo',
badgeColor: '#000',
addedAt: 0
}
]
}
beforeEach(() => {
handleMock.mockReset()
getPRForBranchMock.mockReset()
getIssueMock.mockReset()
listIssuesMock.mockReset()
for (const key of Object.keys(handlers)) {
delete handlers[key]
}
handleMock.mockImplementation((channel, handler) => {
handlers[channel] = handler
})
})
it('normalizes registered repo paths before invoking github clients', async () => {
getPRForBranchMock.mockResolvedValue({ number: 42 })
registerGitHubHandlers(store as never)
await handlers['gh:prForBranch'](null, {
repoPath: '/workspace/repo/../repo',
branch: 'feature/test'
})
expect(getPRForBranchMock).toHaveBeenCalledWith('/workspace/repo', 'feature/test')
})
it('rejects unknown repository paths', async () => {
registerGitHubHandlers(store as never)
expect(() =>
handlers['gh:issue'](null, {
repoPath: '/workspace/other',
number: 7
})
).toThrow('Access denied: unknown repository path')
expect(getIssueMock).not.toHaveBeenCalled()
})
it('forwards listIssues for registered repositories', async () => {
listIssuesMock.mockResolvedValue([])
registerGitHubHandlers(store as never)
await handlers['gh:listIssues'](null, {
repoPath: '/workspace/repo',
limit: 5
})
expect(listIssuesMock).toHaveBeenCalledWith('/workspace/repo', 5)
})
})

View file

@ -1,27 +1,43 @@
import { ipcMain } from 'electron'
import { resolve } from 'path'
import type { Store } from '../persistence'
import { getPRForBranch, getIssue, listIssues, getPRChecks, updatePRTitle } from '../github/client'
export function registerGitHubHandlers(): void {
function assertRegisteredRepoPath(repoPath: string, store: Store): string {
const resolvedRepoPath = resolve(repoPath)
const registeredRepo = store.getRepos().find((repo) => resolve(repo.path) === resolvedRepoPath)
if (!registeredRepo) {
throw new Error('Access denied: unknown repository path')
}
return registeredRepo.path
}
export function registerGitHubHandlers(store: Store): void {
ipcMain.handle('gh:prForBranch', (_event, args: { repoPath: string; branch: string }) => {
return getPRForBranch(args.repoPath, args.branch)
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
return getPRForBranch(repoPath, args.branch)
})
ipcMain.handle('gh:issue', (_event, args: { repoPath: string; number: number }) => {
return getIssue(args.repoPath, args.number)
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
return getIssue(repoPath, args.number)
})
ipcMain.handle('gh:listIssues', (_event, args: { repoPath: string; limit?: number }) => {
return listIssues(args.repoPath, args.limit)
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
return listIssues(repoPath, args.limit)
})
ipcMain.handle('gh:prChecks', (_event, args: { repoPath: string; prNumber: number }) => {
return getPRChecks(args.repoPath, args.prNumber)
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
return getPRChecks(repoPath, args.prNumber)
})
ipcMain.handle(
'gh:updatePRTitle',
(_event, args: { repoPath: string; prNumber: number; title: string }) => {
return updatePRTitle(args.repoPath, args.prNumber, args.title)
const repoPath = assertRegisteredRepoPath(args.repoPath, store)
return updatePRTitle(repoPath, args.prNumber, args.title)
}
)
}

View file

@ -0,0 +1,80 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
registerGitHubHandlersMock,
registerSettingsHandlersMock,
registerShellHandlersMock,
registerSessionHandlersMock,
registerUIHandlersMock,
registerFilesystemHandlersMock,
registerClipboardHandlersMock,
registerUpdaterHandlersMock
} = vi.hoisted(() => ({
registerGitHubHandlersMock: vi.fn(),
registerSettingsHandlersMock: vi.fn(),
registerShellHandlersMock: vi.fn(),
registerSessionHandlersMock: vi.fn(),
registerUIHandlersMock: vi.fn(),
registerFilesystemHandlersMock: vi.fn(),
registerClipboardHandlersMock: vi.fn(),
registerUpdaterHandlersMock: vi.fn()
}))
vi.mock('./github', () => ({
registerGitHubHandlers: registerGitHubHandlersMock
}))
vi.mock('./settings', () => ({
registerSettingsHandlers: registerSettingsHandlersMock
}))
vi.mock('./shell', () => ({
registerShellHandlers: registerShellHandlersMock
}))
vi.mock('./session', () => ({
registerSessionHandlers: registerSessionHandlersMock
}))
vi.mock('./ui', () => ({
registerUIHandlers: registerUIHandlersMock
}))
vi.mock('./filesystem', () => ({
registerFilesystemHandlers: registerFilesystemHandlersMock
}))
vi.mock('../window/attach-main-window-services', () => ({
registerClipboardHandlers: registerClipboardHandlersMock,
registerUpdaterHandlers: registerUpdaterHandlersMock
}))
import { registerCoreHandlers } from './register-core-handlers'
describe('registerCoreHandlers', () => {
beforeEach(() => {
registerGitHubHandlersMock.mockReset()
registerSettingsHandlersMock.mockReset()
registerShellHandlersMock.mockReset()
registerSessionHandlersMock.mockReset()
registerUIHandlersMock.mockReset()
registerFilesystemHandlersMock.mockReset()
registerClipboardHandlersMock.mockReset()
registerUpdaterHandlersMock.mockReset()
})
it('passes the store through to handler registrars that need it', () => {
const store = { marker: 'store' }
registerCoreHandlers(store as never)
expect(registerGitHubHandlersMock).toHaveBeenCalledWith(store)
expect(registerSettingsHandlersMock).toHaveBeenCalledWith(store)
expect(registerSessionHandlersMock).toHaveBeenCalledWith(store)
expect(registerUIHandlersMock).toHaveBeenCalledWith(store)
expect(registerFilesystemHandlersMock).toHaveBeenCalledWith(store)
expect(registerShellHandlersMock).toHaveBeenCalled()
expect(registerClipboardHandlersMock).toHaveBeenCalled()
expect(registerUpdaterHandlersMock).toHaveBeenCalled()
})
})

View file

@ -12,7 +12,7 @@ import {
} from '../window/attach-main-window-services'
export function registerCoreHandlers(store: Store): void {
registerGitHubHandlers()
registerGitHubHandlers(store)
registerSettingsHandlers(store)
registerShellHandlers()
registerSessionHandlers(store)

View file

@ -1,6 +1,6 @@
import type { BrowserWindow } from 'electron'
import { ipcMain } from 'electron'
import { join, basename } from 'path'
import { join, basename, resolve, relative, isAbsolute } from 'path'
import type { Store } from '../persistence'
import type { Worktree, WorktreeMeta } from '../../shared/types'
import { listWorktrees, addWorktree, removeWorktree } from '../git/worktree'
@ -58,9 +58,7 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store
const settings = store.getSettings()
const requestedName = args.name
// Sanitize name for use in branch names and directory paths
// (git branch names cannot contain spaces; collapse runs of spaces to a single hyphen)
const sanitizedName = args.name.replace(/\s+/g, '-')
const sanitizedName = sanitizeWorktreeName(args.name)
// Compute branch name with prefix
let branchName = sanitizedName
@ -83,6 +81,7 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store
} else {
worktreePath = join(settings.workspaceDir, sanitizedName)
}
worktreePath = ensurePathWithinWorkspace(worktreePath, settings.workspaceDir)
// Determine base branch
const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path)
@ -196,6 +195,34 @@ function mergeWorktree(
}
}
function sanitizeWorktreeName(input: string): string {
const sanitized = input
.trim()
.replace(/[\\/]+/g, '-')
.replace(/\s+/g, '-')
.replace(/[^A-Za-z0-9._-]+/g, '-')
.replace(/-+/g, '-')
.replace(/^[.-]+|[.-]+$/g, '')
if (!sanitized || sanitized === '.' || sanitized === '..') {
throw new Error('Invalid worktree name')
}
return sanitized
}
function ensurePathWithinWorkspace(targetPath: string, workspaceDir: string): string {
const resolvedWorkspaceDir = resolve(workspaceDir)
const resolvedTargetPath = resolve(targetPath)
const rel = relative(resolvedWorkspaceDir, resolvedTargetPath)
if (isAbsolute(rel) || rel.startsWith('..')) {
throw new Error('Invalid worktree path')
}
return resolvedTargetPath
}
function parseWorktreeId(worktreeId: string): { repoId: string; worktreePath: string } {
const sepIdx = worktreeId.indexOf('::')
if (sepIdx === -1) {

View file

@ -0,0 +1,87 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
onMock,
removeAllListenersMock,
setPermissionRequestHandlerMock,
registerRepoHandlersMock,
registerWorktreeHandlersMock,
registerPtyHandlersMock,
setupAutoUpdaterMock
} = vi.hoisted(() => ({
onMock: vi.fn(),
removeAllListenersMock: vi.fn(),
setPermissionRequestHandlerMock: vi.fn(),
registerRepoHandlersMock: vi.fn(),
registerWorktreeHandlersMock: vi.fn(),
registerPtyHandlersMock: vi.fn(),
setupAutoUpdaterMock: vi.fn()
}))
vi.mock('electron', () => ({
app: {},
clipboard: {},
ipcMain: {
on: onMock,
removeAllListeners: removeAllListenersMock,
removeHandler: vi.fn(),
handle: vi.fn()
}
}))
vi.mock('../ipc/repos', () => ({
registerRepoHandlers: registerRepoHandlersMock
}))
vi.mock('../ipc/worktrees', () => ({
registerWorktreeHandlers: registerWorktreeHandlersMock
}))
vi.mock('../ipc/pty', () => ({
registerPtyHandlers: registerPtyHandlersMock
}))
vi.mock('../updater', () => ({
checkForUpdates: vi.fn(),
getUpdateStatus: vi.fn(),
quitAndInstall: vi.fn(),
setupAutoUpdater: setupAutoUpdaterMock
}))
import { attachMainWindowServices } from './attach-main-window-services'
describe('attachMainWindowServices', () => {
beforeEach(() => {
onMock.mockReset()
removeAllListenersMock.mockReset()
setPermissionRequestHandlerMock.mockReset()
registerRepoHandlersMock.mockReset()
registerWorktreeHandlersMock.mockReset()
registerPtyHandlersMock.mockReset()
setupAutoUpdaterMock.mockReset()
})
it('only allows the explicit permission allowlist', () => {
const mainWindow = {
webContents: {
session: {
setPermissionRequestHandler: setPermissionRequestHandlerMock
}
}
}
const store = { flush: vi.fn() }
attachMainWindowServices(mainWindow as never, store as never)
expect(setPermissionRequestHandlerMock).toHaveBeenCalledTimes(1)
const permissionHandler = setPermissionRequestHandlerMock.mock.calls[0][0]
const callback = vi.fn()
permissionHandler(null, 'media', callback)
permissionHandler(null, 'fullscreen', callback)
permissionHandler(null, 'pointerLock', callback)
permissionHandler(null, 'clipboard-read', callback)
expect(callback.mock.calls).toEqual([[true], [true], [true], [false]])
})
})

View file

@ -13,14 +13,10 @@ export function attachMainWindowServices(mainWindow: BrowserWindow, store: Store
registerFileDropRelay(mainWindow)
setupAutoUpdater(mainWindow, { onBeforeQuit: () => store.flush() })
const allowedPermissions = new Set(['media', 'fullscreen', 'pointerLock'])
mainWindow.webContents.session.setPermissionRequestHandler(
(_webContents, permission, callback) => {
if (permission === 'clipboard-read' || permission === 'clipboard-sanitized-write') {
callback(false)
return
}
callback(true)
callback(allowedPermissions.has(permission))
}
)
}

View file

@ -0,0 +1,73 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { browserWindowMock, openExternalMock } = vi.hoisted(() => ({
browserWindowMock: vi.fn(),
openExternalMock: vi.fn()
}))
vi.mock('electron', () => ({
BrowserWindow: browserWindowMock,
nativeTheme: { shouldUseDarkColors: false },
shell: { openExternal: openExternalMock }
}))
vi.mock('@electron-toolkit/utils', () => ({
is: { dev: false }
}))
vi.mock('../../../resources/icon.png?asset', () => ({
default: 'icon'
}))
vi.mock('../../../resources/icon-dev.png?asset', () => ({
default: 'icon-dev'
}))
import { createMainWindow } from './createMainWindow'
describe('createMainWindow', () => {
beforeEach(() => {
browserWindowMock.mockReset()
openExternalMock.mockReset()
})
it('enables renderer sandboxing and only opens http(s) URLs externally', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setWindowOpenHandler: vi.fn((handler) => {
windowHandlers.windowOpen = handler
}),
send: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
expect(browserWindowMock).toHaveBeenCalledWith(
expect.objectContaining({
webPreferences: expect.objectContaining({ sandbox: true })
})
)
expect(windowHandlers.windowOpen({ url: 'https://example.com' })).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).toHaveBeenCalledWith('https://example.com')
})
})

View file

@ -19,7 +19,7 @@ export function createMainWindow(store: Store | null): BrowserWindow {
icon: is.dev ? devIcon : icon,
webPreferences: {
preload: join(__dirname, '../preload/index.js'),
sandbox: false
sandbox: true
}
})
@ -33,7 +33,14 @@ export function createMainWindow(store: Store | null): BrowserWindow {
})
mainWindow.webContents.setWindowOpenHandler((details) => {
shell.openExternal(details.url)
try {
const parsed = new URL(details.url)
if (parsed.protocol === 'https:' || parsed.protocol === 'http:') {
shell.openExternal(details.url)
}
} catch {
// ignore malformed URLs
}
return { action: 'deny' }
})

View file

@ -0,0 +1,60 @@
import { createStore, type StoreApi } from 'zustand/vanilla'
import { describe, expect, it } from 'vitest'
import { createEditorSlice } from './editor'
import type { AppState } from '../types'
function createEditorStore(): StoreApi<AppState> {
// Only the editor slice + activeWorktreeId are needed for these tests.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return createStore<any>()((...args: any[]) => ({
activeWorktreeId: 'wt-1',
...createEditorSlice(...(args as Parameters<typeof createEditorSlice>))
})) as unknown as StoreApi<AppState>
}
describe('createEditorSlice openDiff', () => {
it('keeps staged and unstaged diffs in separate tabs', () => {
const store = createEditorStore()
store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false)
store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', true)
expect(store.getState().openFiles.map((file) => file.id)).toEqual([
'/repo/file.ts::unstaged',
'/repo/file.ts::staged'
])
})
it('repairs an existing diff tab entry to the correct mode and staged state', () => {
const store = createEditorStore()
store.setState({
openFiles: [
{
id: '/repo/file.ts::staged',
filePath: '/repo/file.ts',
relativePath: 'file.ts',
worktreeId: 'wt-1',
language: 'typescript',
isDirty: false,
mode: 'edit'
}
],
activeFileId: null,
activeFileIdByWorktree: {},
activeTabTypeByWorktree: {},
activeTabType: 'terminal'
})
store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', true)
expect(store.getState().openFiles).toEqual([
expect.objectContaining({
id: '/repo/file.ts::staged',
mode: 'diff',
diffStaged: true
})
])
expect(store.getState().activeFileId).toBe('/repo/file.ts::staged')
})
})

View file

@ -279,10 +279,18 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
openDiff: (worktreeId, filePath, relativePath, language, staged) =>
set((s) => {
const id = `${filePath}${staged ? '::staged' : ''}`
const id = `${filePath}::${staged ? 'staged' : 'unstaged'}`
const existing = s.openFiles.find((f) => f.id === id)
if (existing) {
// Ensure mode and diffStaged are up-to-date (e.g. if a plain edit tab
// previously occupied this id before the suffix scheme changed).
const needsUpdate = existing.mode !== 'diff' || existing.diffStaged !== staged
return {
openFiles: needsUpdate
? s.openFiles.map((f) =>
f.id === id ? { ...f, mode: 'diff' as const, diffStaged: staged } : f
)
: s.openFiles,
activeFileId: id,
activeTabType: 'editor',
activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id },