diff --git a/ui/desktop/src/components/schedule/ScheduleModal.tsx b/ui/desktop/src/components/schedule/ScheduleModal.tsx index ddf06ec08..eb3aeff08 100644 --- a/ui/desktop/src/components/schedule/ScheduleModal.tsx +++ b/ui/desktop/src/components/schedule/ScheduleModal.tsx @@ -5,7 +5,6 @@ import { Button } from '../ui/button'; import { Input } from '../ui/input'; import { CronPicker } from './CronPicker'; import { Recipe, parseDeeplink, parseRecipeFromFile } from '../../recipe'; -import { getStorageDirectory } from '../../recipe/recipe_management'; import ClockIcon from '../../assets/clock-icon.svg'; import { defineMessages, useIntl } from '../../i18n'; @@ -129,15 +128,13 @@ export const ScheduleModal: React.FC = ({ }, [isOpen, schedule, initialDeepLink, handleDeepLinkChange]); const handleBrowseFile = async () => { - const defaultPath = getStorageDirectory(true); - const filePath = await window.electron.selectFileOrDirectory(defaultPath); - if (filePath) { - if (filePath.endsWith('.yaml') || filePath.endsWith('.yml')) { - setRecipeSourcePath(filePath); + const fileResponse = await window.electron.selectRecipeFile(); + if (fileResponse) { + if (fileResponse.filePath.endsWith('.yaml') || fileResponse.filePath.endsWith('.yml')) { + setRecipeSourcePath(fileResponse.filePath); setInternalValidationError(null); try { - const fileResponse = await window.electron.readFile(filePath); if (!fileResponse.found || fileResponse.error) { throw new Error(intl.formatMessage(i18n.failedReadFile)); } diff --git a/ui/desktop/src/components/schedule/__tests__/ScheduleModal.test.tsx b/ui/desktop/src/components/schedule/__tests__/ScheduleModal.test.tsx index 42148f3ca..0f3b30c76 100644 --- a/ui/desktop/src/components/schedule/__tests__/ScheduleModal.test.tsx +++ b/ui/desktop/src/components/schedule/__tests__/ScheduleModal.test.tsx @@ -22,6 +22,18 @@ const baseProps = { }; describe('ScheduleModal', () => { + it('preserves the form when the recipe picker is cancelled', async () => { + const user = userEvent.setup(); + const selectRecipeFile = vi.fn().mockResolvedValue(null); + window.electron.selectRecipeFile = selectRecipeFile; + renderWithIntl(); + + await user.click(screen.getByRole('button', { name: 'Browse for YAML file...' })); + + expect(selectRecipeFile).toHaveBeenCalledOnce(); + expect(screen.queryByText(/Failed to read|Invalid file type/)).not.toBeInTheDocument(); + }); + it('clears a validation error from create mode when reopened to edit a schedule', async () => { const user = userEvent.setup(); const { rerender } = renderWithIntl(); diff --git a/ui/desktop/src/components/settings/chat/GoosehintsModal.tsx b/ui/desktop/src/components/settings/chat/GoosehintsModal.tsx index ba0172676..93f4b7001 100644 --- a/ui/desktop/src/components/settings/chat/GoosehintsModal.tsx +++ b/ui/desktop/src/components/settings/chat/GoosehintsModal.tsx @@ -148,8 +148,6 @@ const FileInfo = ({ filePath, found }: { filePath: string; found: boolean }) => ); }; -const getGoosehintsFile = async (filePath: string) => await window.electron.readFile(filePath); - interface GoosehintsModalProps { directory: string; setIsGoosehintsModalOpen: (isOpen: boolean) => void; @@ -167,23 +165,26 @@ export const GoosehintsModal = ({ directory, setIsGoosehintsModalOpen }: Goosehi useEffect(() => { const fetchGoosehintsFile = async () => { try { - const { file, error, found } = await getGoosehintsFile(goosehintsFilePath); + const { file, error, found } = await window.electron.readGoosehints(); setGoosehintsFile(file); setGoosehintsFileFound(found); - setGoosehintsFileReadError(found && error ? error : ''); + setGoosehintsFileReadError(error ?? ''); } catch (error) { console.error('Error fetching .goosehints file:', error); setGoosehintsFileReadError(intl.formatMessage(i18n.failedToAccess)); } }; if (directory) fetchGoosehintsFile(); - }, [directory, goosehintsFilePath, intl]); + }, [directory, intl]); const writeFile = async () => { setIsSaving(true); setSaveSuccess(false); try { - await window.electron.writeFile(goosehintsFilePath, goosehintsFile); + const saved = await window.electron.writeGoosehints(goosehintsFile); + if (!saved) { + throw new Error('Unable to save .goosehints'); + } setSaveSuccess(true); setGoosehintsFileFound(true); setTimeout(() => setSaveSuccess(false), 3000); diff --git a/ui/desktop/src/desktopFileAccess.test.ts b/ui/desktop/src/desktopFileAccess.test.ts new file mode 100644 index 000000000..d0f16dc36 --- /dev/null +++ b/ui/desktop/src/desktopFileAccess.test.ts @@ -0,0 +1,465 @@ +import fs, { constants as fsConstants } from 'node:fs'; +import fsPromises from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + DesktopFileAccess, + isAppRendererUrl, + isAuthorizedFileAccessRequest, + readSelectedRecipe, +} from './desktopFileAccess'; + +const tempDirectories: string[] = []; + +function makeTempDirectory(): string { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'goose-desktop-file-access-')); + tempDirectories.push(directory); + return directory; +} + +afterEach(() => { + vi.restoreAllMocks(); + while (tempDirectories.length > 0) { + fs.rmSync(tempDirectories.pop()!, { recursive: true, force: true }); + } +}); + +describe('DesktopFileAccess', () => { + it('reads .goosehints from the bound working directory', async () => { + const workingDirectory = makeTempDirectory(); + fs.writeFileSync(path.join(workingDirectory, '.goosehints'), 'project guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const canonicalWorkingDirectory = fs.realpathSync(workingDirectory); + + await expect(access.readGoosehints(7)).resolves.toEqual({ + file: 'project guidance', + filePath: path.join(canonicalWorkingDirectory, '.goosehints'), + error: null, + found: true, + }); + }); + + it('preserves missing-file behavior', async () => { + const workingDirectory = makeTempDirectory(); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const canonicalWorkingDirectory = fs.realpathSync(workingDirectory); + + await expect(access.readGoosehints(7)).resolves.toEqual({ + file: '', + filePath: path.join(canonicalWorkingDirectory, '.goosehints'), + error: null, + found: false, + }); + }); + + it('creates and updates .goosehints in the bound working directory', async () => { + const workingDirectory = makeTempDirectory(); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const filePath = path.join(fs.realpathSync(workingDirectory), '.goosehints'); + + await expect(access.writeGoosehints(7, 'first guidance')).resolves.toBe(true); + expect(fs.readFileSync(filePath, 'utf8')).toBe('first guidance'); + + await expect(access.writeGoosehints(7, 'updated guidance')).resolves.toBe(true); + expect(fs.readFileSync(filePath, 'utf8')).toBe('updated guidance'); + }); + + it.skipIf(process.platform === 'win32')( + 'rejects a canonical working directory replaced by a symlink', + async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const originalDirectory = path.join(root, 'original-project'); + const replacementDirectory = path.join(root, 'replacement-project'); + fs.mkdirSync(workingDirectory); + fs.mkdirSync(replacementDirectory); + fs.writeFileSync(path.join(workingDirectory, '.goosehints'), 'original guidance'); + fs.writeFileSync(path.join(replacementDirectory, '.goosehints'), 'replacement guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + + fs.renameSync(workingDirectory, originalDirectory); + fs.symlinkSync(replacementDirectory, workingDirectory); + + const result = await access.readGoosehints(7); + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(result.found).toBe(false); + expect(result.error).toContain('working directory changed'); + expect(fs.readFileSync(path.join(originalDirectory, '.goosehints'), 'utf8')).toBe( + 'original guidance' + ); + expect(fs.readFileSync(path.join(replacementDirectory, '.goosehints'), 'utf8')).toBe( + 'replacement guidance' + ); + } + ); + + it('rejects a canonical working directory replaced by another directory', async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const originalDirectory = path.join(root, 'original-project'); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(path.join(workingDirectory, '.goosehints'), 'original guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + + fs.renameSync(workingDirectory, originalDirectory); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(path.join(workingDirectory, '.goosehints'), 'replacement guidance'); + + const result = await access.readGoosehints(7); + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(result.found).toBe(false); + expect(result.error).toContain('working directory changed'); + expect(fs.readFileSync(path.join(originalDirectory, '.goosehints'), 'utf8')).toBe( + 'original guidance' + ); + expect(fs.readFileSync(path.join(workingDirectory, '.goosehints'), 'utf8')).toBe( + 'replacement guidance' + ); + }); + + it('rejects a bound working directory that was renamed away', async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const renamedDirectory = path.join(root, 'renamed-project'); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(path.join(workingDirectory, '.goosehints'), 'original guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + + fs.renameSync(workingDirectory, renamedDirectory); + + const result = await access.readGoosehints(7); + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(result.found).toBe(false); + expect(result.error).toContain('working directory changed'); + expect(fs.readFileSync(path.join(renamedDirectory, '.goosehints'), 'utf8')).toBe( + 'original guidance' + ); + }); + + it.skipIf(process.platform === 'win32')( + 'rechecks the working directory before truncating an opened .goosehints', + async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const renamedDirectory = path.join(root, 'renamed-project'); + const filePath = path.join(workingDirectory, '.goosehints'); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(filePath, 'original guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const open = fsPromises.open.bind(fsPromises); + vi.spyOn(fsPromises, 'open').mockImplementationOnce(async (...args) => { + fs.renameSync(workingDirectory, renamedDirectory); + fs.mkdirSync(workingDirectory); + fs.linkSync( + path.join(renamedDirectory, '.goosehints'), + path.join(workingDirectory, '.goosehints') + ); + return open(...args); + }); + + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(fs.readFileSync(path.join(renamedDirectory, '.goosehints'), 'utf8')).toBe( + 'original guidance' + ); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rechecks the working directory before reading an opened .goosehints', + async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const renamedDirectory = path.join(root, 'renamed-project'); + const filePath = path.join(workingDirectory, '.goosehints'); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(filePath, 'original guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const open = fsPromises.open.bind(fsPromises); + vi.spyOn(fsPromises, 'open').mockImplementationOnce(async (...args) => { + fs.renameSync(workingDirectory, renamedDirectory); + fs.mkdirSync(workingDirectory); + fs.linkSync( + path.join(renamedDirectory, '.goosehints'), + path.join(workingDirectory, '.goosehints') + ); + return open(...args); + }); + + const result = await access.readGoosehints(7); + + expect(result.found).toBe(false); + expect(result.file).toBe(''); + expect(result.error).toContain('working directory changed'); + } + ); + + it('rechecks the working directory before creating a missing .goosehints', async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const renamedDirectory = path.join(root, 'renamed-project'); + fs.mkdirSync(workingDirectory); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const lstat = fsPromises.lstat.bind(fsPromises); + vi.spyOn(fsPromises, 'lstat').mockImplementation(async (...args) => { + try { + return await lstat(...args); + } catch (error) { + if (path.basename(args[0].toString()) === '.goosehints') { + fs.renameSync(workingDirectory, renamedDirectory); + fs.mkdirSync(workingDirectory); + } + throw error; + } + }); + const open = vi.spyOn(fsPromises, 'open'); + + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(open).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(workingDirectory, '.goosehints'))).toBe(false); + }); + + it('rejects a renderer without a bound working directory', async () => { + const access = new DesktopFileAccess(); + + await expect(access.readGoosehints(99)).rejects.toThrow('not authorized'); + await expect(access.writeGoosehints(99, 'project guidance')).rejects.toThrow('not authorized'); + }); + + it.skipIf(process.platform === 'win32')( + 'blocks a .goosehints symlink that escapes the working directory', + async () => { + const root = makeTempDirectory(); + const workingDirectory = path.join(root, 'project'); + const secretPath = path.join(root, 'secret'); + fs.mkdirSync(workingDirectory); + fs.writeFileSync(secretPath, 'host secret'); + fs.symlinkSync('../secret', path.join(workingDirectory, '.goosehints')); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + + const result = await access.readGoosehints(7); + const saved = await access.writeGoosehints(7, 'replacement'); + + expect(result.found).toBe(false); + expect(result.file).toBe(''); + expect(result.error).toContain('symbolic link'); + expect(saved).toBe(false); + expect(fs.readFileSync(secretPath, 'utf8')).toBe('host secret'); + } + ); + + it('does not truncate a replacement file opened after validation', async () => { + const workingDirectory = makeTempDirectory(); + const filePath = path.join(workingDirectory, '.goosehints'); + const originalPath = path.join(workingDirectory, 'original.goosehints'); + fs.writeFileSync(filePath, 'original guidance'); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const open = fsPromises.open.bind(fsPromises); + vi.spyOn(fsPromises, 'open').mockImplementationOnce(async (...args) => { + fs.renameSync(filePath, originalPath); + fs.writeFileSync(filePath, 'replacement guidance'); + return open(...args); + }); + + await expect(access.writeGoosehints(7, 'new guidance')).resolves.toBe(false); + expect(fs.readFileSync(filePath, 'utf8')).toBe('replacement guidance'); + expect(fs.readFileSync(originalPath, 'utf8')).toBe('original guidance'); + }); + + it.skipIf(process.platform === 'win32')( + 'keeps a symlinked working directory pinned to its bind-time target', + async () => { + const root = makeTempDirectory(); + const firstProject = path.join(root, 'first-project'); + const secondProject = path.join(root, 'second-project'); + const workingDirectory = path.join(root, 'current-project'); + fs.mkdirSync(firstProject); + fs.mkdirSync(secondProject); + fs.writeFileSync(path.join(firstProject, '.goosehints'), 'first guidance'); + fs.writeFileSync(path.join(secondProject, '.goosehints'), 'second guidance'); + fs.symlinkSync(firstProject, workingDirectory); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + const canonicalFirstProject = fs.realpathSync(firstProject); + + fs.unlinkSync(workingDirectory); + fs.symlinkSync(secondProject, workingDirectory); + + await expect(access.readGoosehints(7)).resolves.toEqual({ + file: 'first guidance', + filePath: path.join(canonicalFirstProject, '.goosehints'), + error: null, + found: true, + }); + await expect(access.writeGoosehints(7, 'updated first guidance')).resolves.toBe(true); + expect(fs.readFileSync(path.join(firstProject, '.goosehints'), 'utf8')).toBe( + 'updated first guidance' + ); + expect(fs.readFileSync(path.join(secondProject, '.goosehints'), 'utf8')).toBe( + 'second guidance' + ); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a non-regular .goosehints target without blocking', + async () => { + const workingDirectory = makeTempDirectory(); + execFileSync('mkfifo', [path.join(workingDirectory, '.goosehints')]); + const access = new DesktopFileAccess(); + await access.bindWindow(7, workingDirectory); + + await expect(access.writeGoosehints(7, 'project guidance')).resolves.toBe(false); + } + ); +}); + +describe('renderer provenance', () => { + const devServerUrl = new URL('http://127.0.0.1:5173/'); + + it('accepts legitimate hash-routed app URLs', () => { + expect(isAppRendererUrl('http://127.0.0.1:5173/#/settings', devServerUrl)).toBe(true); + expect(isAppRendererUrl('http://127.0.0.1:5173/#/schedules?tab=active', devServerUrl)).toBe( + true + ); + expect( + isAppRendererUrl( + 'file:///Applications/Goose.app/Contents/Resources/renderer/main_window/index.html#/settings', + new URL('file:///Applications/Goose.app/Contents/Resources/renderer/main_window/index.html') + ) + ).toBe(true); + }); + + it('rejects sibling paths, foreign origins, and malformed URLs', () => { + expect(isAppRendererUrl('http://127.0.0.1:5173/admin#/settings', devServerUrl)).toBe(false); + expect(isAppRendererUrl('http://localhost:5173/#/settings', devServerUrl)).toBe(false); + expect(isAppRendererUrl('https://attacker.example/#/settings', devServerUrl)).toBe(false); + expect( + isAppRendererUrl( + 'file://attacker/Applications/Goose.app/Contents/Resources/renderer/main_window/index.html', + new URL('file:///Applications/Goose.app/Contents/Resources/renderer/main_window/index.html') + ) + ).toBe(false); + expect(isAppRendererUrl('not a URL', devServerUrl)).toBe(false); + }); + + it('requires a registered top-level Goose window', () => { + const legitimateRequest = { + isRegisteredWindow: true, + isMainFrame: true, + rendererUrl: 'http://127.0.0.1:5173/#/settings', + }; + + expect(isAuthorizedFileAccessRequest(legitimateRequest, devServerUrl)).toBe(true); + expect( + isAuthorizedFileAccessRequest( + { ...legitimateRequest, isRegisteredWindow: false }, + devServerUrl + ) + ).toBe(false); + expect( + isAuthorizedFileAccessRequest({ ...legitimateRequest, isMainFrame: false }, devServerUrl) + ).toBe(false); + }); +}); + +describe('readSelectedRecipe', () => { + it('reads a picker-selected YAML recipe', async () => { + const directory = makeTempDirectory(); + const recipePath = path.join(directory, 'recipe.yaml'); + fs.writeFileSync(recipePath, 'title: Daily summary'); + + await expect(readSelectedRecipe(recipePath)).resolves.toEqual({ + file: 'title: Daily summary', + filePath: recipePath, + error: null, + found: true, + }); + }); + + it('does not read a selected non-recipe file', async () => { + const directory = makeTempDirectory(); + const secretPath = path.join(directory, 'secret.txt'); + fs.writeFileSync(secretPath, 'host secret'); + + const result = await readSelectedRecipe(secretPath); + + expect(result.found).toBe(false); + expect(result.file).toBe(''); + expect(result.error).toContain('YAML'); + }); + + it.skipIf(process.platform === 'win32')('allows a picker-selected YAML symlink', async () => { + const directory = makeTempDirectory(); + const targetPath = path.join(directory, 'target.yaml'); + const recipePath = path.join(directory, 'recipe.yaml'); + fs.writeFileSync(targetPath, 'title: Linked recipe'); + fs.symlinkSync(targetPath, recipePath); + + await expect(readSelectedRecipe(recipePath)).resolves.toEqual({ + file: 'title: Linked recipe', + filePath: recipePath, + error: null, + found: true, + }); + }); + + it.skipIf(process.platform === 'win32')( + 'reads from the opened recipe when a selected symlink is retargeted', + async () => { + const directory = makeTempDirectory(); + const firstTarget = path.join(directory, 'first.yaml'); + const secondTarget = path.join(directory, 'second.yaml'); + const recipePath = path.join(directory, 'recipe.yaml'); + fs.writeFileSync(firstTarget, 'title: First recipe'); + fs.writeFileSync(secondTarget, 'title: Second recipe'); + fs.symlinkSync(firstTarget, recipePath); + const open = fsPromises.open.bind(fsPromises); + const openSpy = vi.spyOn(fsPromises, 'open').mockImplementationOnce(async (...args) => { + const handle = await open(...args); + fs.unlinkSync(recipePath); + fs.symlinkSync(secondTarget, recipePath); + return handle; + }); + + await expect(readSelectedRecipe(recipePath)).resolves.toEqual({ + file: 'title: First recipe', + filePath: recipePath, + error: null, + found: true, + }); + expect(openSpy).toHaveBeenCalledOnce(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a picker-selected FIFO without blocking', + async () => { + const directory = makeTempDirectory(); + const recipePath = path.join(directory, 'recipe.yaml'); + execFileSync('mkfifo', [recipePath]); + const openSpy = vi.spyOn(fsPromises, 'open'); + + const result = await readSelectedRecipe(recipePath); + + expect(result.found).toBe(false); + expect(result.error).toContain('not a regular file'); + expect(openSpy).toHaveBeenCalledWith( + recipePath, + fsConstants.O_RDONLY | fsConstants.O_NONBLOCK + ); + } + ); +}); diff --git a/ui/desktop/src/desktopFileAccess.ts b/ui/desktop/src/desktopFileAccess.ts new file mode 100644 index 000000000..82b9368b9 --- /dev/null +++ b/ui/desktop/src/desktopFileAccess.ts @@ -0,0 +1,279 @@ +import fs from 'node:fs/promises'; +import { constants as fsConstants } from 'node:fs'; +import type { Stats } from 'node:fs'; +import path from 'node:path'; + +export interface FileReadResult { + file: string; + filePath: string; + error: string | null; + found: boolean; +} + +interface FileAccessRequestProvenance { + isRegisteredWindow: boolean; + isMainFrame: boolean; + rendererUrl: string; +} + +export function isAppRendererUrl(rendererUrl: string, expectedUrl: URL): boolean { + try { + const actual = new URL(rendererUrl); + if (expectedUrl.protocol === 'file:') { + return ( + actual.protocol === 'file:' && + actual.host === expectedUrl.host && + actual.pathname === expectedUrl.pathname + ); + } + return actual.origin === expectedUrl.origin && actual.pathname === expectedUrl.pathname; + } catch { + return false; + } +} + +export function isAuthorizedFileAccessRequest( + provenance: FileAccessRequestProvenance, + expectedUrl: URL +): boolean { + return ( + provenance.isRegisteredWindow && + provenance.isMainFrame && + isAppRendererUrl(provenance.rendererUrl, expectedUrl) + ); +} + +function isMissingFile(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error as { code?: unknown }).code === 'ENOENT' + ); +} + +function missingFile(filePath: string): FileReadResult { + return { file: '', filePath, error: null, found: false }; +} + +function failedRead(filePath: string, message: string): FileReadResult { + return { file: '', filePath, error: message, found: false }; +} + +type WorkingDirectoryBinding = + | { status: 'ready'; path: string; dev: bigint; ino: bigint } + | { status: 'missing'; path: string } + | { status: 'error'; path: string }; + +export class DesktopFileAccess { + private readonly workingDirectories = new Map(); + + private bindingForWindow(windowId: number): WorkingDirectoryBinding { + const binding = this.workingDirectories.get(windowId); + if (!binding) { + throw new Error('This window is not authorized to access .goosehints'); + } + return binding; + } + + private async bindingMatchesDirectory(binding: WorkingDirectoryBinding): Promise { + if (binding.status !== 'ready') { + return false; + } + try { + const metadata = await fs.lstat(binding.path, { bigint: true }); + return ( + metadata.isDirectory() && + !metadata.isSymbolicLink() && + metadata.dev === binding.dev && + metadata.ino === binding.ino + ); + } catch { + return false; + } + } + + async bindWindow(windowId: number, workingDirectory: string): Promise { + const resolvedPath = path.resolve(workingDirectory); + try { + const canonicalPath = await fs.realpath(resolvedPath); + const metadata = await fs.lstat(canonicalPath, { bigint: true }); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new Error('Working directory is not a regular directory'); + } + this.workingDirectories.set(windowId, { + status: 'ready', + path: canonicalPath, + dev: metadata.dev, + ino: metadata.ino, + }); + } catch (error) { + this.workingDirectories.set(windowId, { + status: isMissingFile(error) ? 'missing' : 'error', + path: resolvedPath, + }); + } + } + + unbindWindow(windowId: number): void { + this.workingDirectories.delete(windowId); + } + + async readGoosehints(windowId: number): Promise { + const binding = this.bindingForWindow(windowId); + const filePath = path.join(binding.path, '.goosehints'); + if (binding.status === 'missing') { + return missingFile(filePath); + } + if (binding.status === 'error') { + return failedRead(filePath, 'Unable to resolve the working directory'); + } + if (!(await this.bindingMatchesDirectory(binding))) { + return failedRead(filePath, 'The working directory changed after it was authorized'); + } + + try { + const metadata = await fs.lstat(filePath); + if (metadata.isSymbolicLink()) { + return failedRead(filePath, 'Refusing to read a symbolic link as .goosehints'); + } + if (!metadata.isFile()) { + return failedRead(filePath, '.goosehints is not a regular file'); + } + + const canonicalFilePath = await fs.realpath(filePath); + if (path.dirname(canonicalFilePath) !== binding.path) { + return failedRead(filePath, '.goosehints resolves outside the working directory'); + } + + const noFollow = process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW; + const handle = await fs.open(canonicalFilePath, fsConstants.O_RDONLY | noFollow); + try { + const openedMetadata = await handle.stat(); + if (!openedMetadata.isFile()) { + return failedRead(filePath, '.goosehints is not a regular file'); + } + if (openedMetadata.dev !== metadata.dev || openedMetadata.ino !== metadata.ino) { + return failedRead(filePath, '.goosehints changed while it was being opened'); + } + if (!(await this.bindingMatchesDirectory(binding))) { + return failedRead(filePath, 'The working directory changed after it was authorized'); + } + return { + file: await handle.readFile('utf8'), + filePath, + error: null, + found: true, + }; + } finally { + await handle.close(); + } + } catch (error) { + if (isMissingFile(error)) { + return missingFile(filePath); + } + return failedRead(filePath, 'Unable to read .goosehints'); + } + } + + async writeGoosehints(windowId: number, content: string): Promise { + const binding = this.bindingForWindow(windowId); + if (binding.status !== 'ready' || typeof content !== 'string') { + return false; + } + if (!(await this.bindingMatchesDirectory(binding))) { + return false; + } + + const filePath = path.join(binding.path, '.goosehints'); + const noFollow = process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW; + try { + let metadata: Stats; + try { + metadata = await fs.lstat(filePath); + } catch (error) { + if (!isMissingFile(error)) { + return false; + } + if (!(await this.bindingMatchesDirectory(binding))) { + return false; + } + + const handle = await fs.open( + filePath, + fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL | noFollow, + 0o666 + ); + try { + if (!(await handle.stat()).isFile()) { + return false; + } + if (!(await this.bindingMatchesDirectory(binding))) { + return false; + } + await handle.writeFile(content, 'utf8'); + return true; + } finally { + await handle.close(); + } + } + + if (metadata.isSymbolicLink() || !metadata.isFile()) { + return false; + } + + const handle = await fs.open(filePath, fsConstants.O_WRONLY | noFollow); + try { + const openedMetadata = await handle.stat(); + if ( + !openedMetadata.isFile() || + openedMetadata.dev !== metadata.dev || + openedMetadata.ino !== metadata.ino + ) { + return false; + } + if (!(await this.bindingMatchesDirectory(binding))) { + return false; + } + await handle.truncate(0); + await handle.writeFile(content, 'utf8'); + return true; + } finally { + await handle.close(); + } + } catch { + return false; + } + } +} + +export async function readSelectedRecipe(filePath: string): Promise { + const extension = path.extname(filePath).toLowerCase(); + if (extension !== '.yaml' && extension !== '.yml') { + return failedRead(filePath, 'The selected recipe must be a YAML file'); + } + + try { + const nonBlocking = process.platform === 'win32' ? 0 : fsConstants.O_NONBLOCK; + const handle = await fs.open(filePath, fsConstants.O_RDONLY | nonBlocking); + try { + const metadata = await handle.stat(); + if (!metadata.isFile()) { + return failedRead(filePath, 'The selected recipe is not a regular file'); + } + return { + file: await handle.readFile('utf8'), + filePath, + error: null, + found: true, + }; + } finally { + await handle.close(); + } + } catch (error) { + if (isMissingFile(error)) { + return missingFile(filePath); + } + return failedRead(filePath, 'Unable to read the selected recipe'); + } +} diff --git a/ui/desktop/src/main.ts b/ui/desktop/src/main.ts index 9904bbf83..60a5fe3a3 100644 --- a/ui/desktop/src/main.ts +++ b/ui/desktop/src/main.ts @@ -1,4 +1,4 @@ -import type { OpenDialogOptions, OpenDialogReturnValue } from 'electron'; +import type { IpcMainInvokeEvent, OpenDialogOptions, OpenDialogReturnValue } from 'electron'; import { app, App, @@ -57,6 +57,11 @@ import installExtension, { REACT_DEVELOPER_TOOLS } from 'electron-devtools-insta import { BLOCKED_PROTOCOLS, WEB_PROTOCOLS } from './utils/urlSecurity'; import { buildCSP } from './utils/csp'; import { resolveWorkingDir } from './utils/workingDir'; +import { + DesktopFileAccess, + isAuthorizedFileAccessRequest, + readSelectedRecipe, +} from './desktopFileAccess'; function shouldSetupUpdater(): boolean { // Setup updater if either the flag is enabled OR dev updates are enabled @@ -992,6 +997,27 @@ let appConfig = { const windowMap = new Map(); const appWindows = new Map(); +const desktopFileAccess = new DesktopFileAccess(); + +function requireRegularRendererWindow(event: IpcMainInvokeEvent): BrowserWindow { + const senderWindow = BrowserWindow.fromWebContents(event.sender); + const senderFrame = event.senderFrame; + if ( + !senderWindow || + !senderFrame || + !isAuthorizedFileAccessRequest( + { + isRegisteredWindow: windowMap.get(senderWindow.id) === senderWindow, + isMainFrame: senderFrame === event.sender.mainFrame, + rendererUrl: senderFrame.url, + }, + getAppUrl() + ) + ) { + throw new Error('This renderer is not authorized for local file access'); + } + return senderWindow; +} function getRegularWindows(): BrowserWindow[] { return [...windowMap.values()].filter((w) => !w.isDestroyed()); @@ -1460,6 +1486,40 @@ const createChat = async ( mainWindow.show(); } }); + + await desktopFileAccess.bindWindow(windowId, workingDir); + if (mainWindow.isDestroyed()) { + desktopFileAccess.unbindWindow(windowId); + return; + } + windowMap.set(windowId, mainWindow); + + // Handle window closure + mainWindow.on('closed', () => { + windowMap.delete(windowId); + desktopFileAccess.unbindWindow(windowId); + + pendingInitialMessages.delete(windowId); + pendingDeepLinks.delete(windowId); + reactReadyWindows.delete(windowId); + + if (windowPowerSaveBlockers.has(windowId)) { + const blockerId = windowPowerSaveBlockers.get(windowId)!; + try { + powerSaveBlocker.stop(blockerId); + console.log( + `[Main] Stopped power save blocker ${blockerId} for closing window ${windowId}` + ); + } catch (error) { + console.error( + `[Main] Failed to stop power save blocker ${blockerId} for window ${windowId}:`, + error + ); + } + windowPowerSaveBlockers.delete(windowId); + } + }); + mainWindow.loadURL(formattedUrl); // If we have an initial message, store it to send after React is ready @@ -1508,32 +1568,6 @@ const createChat = async ( } }); - windowMap.set(windowId, mainWindow); - - // Handle window closure - mainWindow.on('closed', () => { - windowMap.delete(windowId); - - pendingInitialMessages.delete(windowId); - pendingDeepLinks.delete(windowId); - reactReadyWindows.delete(windowId); - - if (windowPowerSaveBlockers.has(windowId)) { - const blockerId = windowPowerSaveBlockers.get(windowId)!; - try { - powerSaveBlocker.stop(blockerId); - console.log( - `[Main] Stopped power save blocker ${blockerId} for closing window ${windowId}` - ); - } catch (error) { - console.error( - `[Main] Failed to stop power save blocker ${blockerId} for window ${windowId}:`, - error - ); - } - windowPowerSaveBlockers.delete(windowId); - } - }); return mainWindow; }; @@ -2221,6 +2255,43 @@ ipcMain.handle('select-file-or-directory', async (_event, defaultPath?: string) return null; }); +ipcMain.handle('select-recipe-file', async (event) => { + const senderWindow = requireRegularRendererWindow(event); + const pathRoot = appConfig.GOOSE_PATH_ROOT as string | undefined; + const recipeDirectory = pathRoot + ? path.join(pathRoot, 'config', 'recipes') + : path.join(os.homedir(), '.config', 'goose', 'recipes'); + let defaultPath = os.homedir(); + try { + if ((await fs.stat(recipeDirectory)).isDirectory()) { + defaultPath = recipeDirectory; + } + } catch { + // The recipe directory is optional; the native picker falls back to the home directory. + } + + const result = await dialog.showOpenDialog(senderWindow, { + title: 'Select a recipe', + defaultPath, + properties: ['openFile'], + filters: [{ name: 'YAML recipes', extensions: ['yaml', 'yml'] }], + }); + if (result.canceled || result.filePaths.length === 0) { + return null; + } + return readSelectedRecipe(result.filePaths[0]); +}); + +ipcMain.handle('read-goosehints', async (event) => { + const senderWindow = requireRegularRendererWindow(event); + return desktopFileAccess.readGoosehints(senderWindow.id); +}); + +ipcMain.handle('write-goosehints', async (event, content) => { + const senderWindow = requireRegularRendererWindow(event); + return desktopFileAccess.writeGoosehints(senderWindow.id, content); +}); + // Native picker tailored for session imports: shows hidden files (so users can // reach `~/.claude/projects/...` or `~/.pi/agent/sessions/...`), filters for // .json/.jsonl, and returns the file's contents inline so the renderer doesn't @@ -2303,46 +2374,6 @@ ipcMain.handle('check-ollama', async () => { } }); -ipcMain.handle('read-file', async (_event, filePath) => { - try { - const expandedPath = expandTilde(filePath); - if (process.platform === 'win32') { - const buffer = await fs.readFile(expandedPath); - return { file: buffer.toString('utf8'), filePath: expandedPath, error: null, found: true }; - } - // Non-Windows: keep previous behavior via cat for parity - return await new Promise((resolve) => { - const cat = spawn('cat', [expandedPath]); - let output = ''; - let errorOutput = ''; - - cat.stdout.on('data', (data) => { - output += data.toString(); - }); - - cat.stderr.on('data', (data) => { - errorOutput += data.toString(); - }); - - cat.on('close', (code) => { - if (code !== 0) { - resolve({ file: '', filePath: expandedPath, error: errorOutput || null, found: false }); - return; - } - resolve({ file: output, filePath: expandedPath, error: null, found: true }); - }); - - cat.on('error', (error) => { - console.error('Error reading file:', error); - resolve({ file: '', filePath: expandedPath, error, found: false }); - }); - }); - } catch (error) { - console.error('Error reading file:', error); - return { file: '', filePath: expandTilde(filePath), error, found: false }; - } -}); - ipcMain.handle('write-file', async (_event, filePath, content) => { try { // Expand tilde to home directory diff --git a/ui/desktop/src/preload.fileAccess.test.ts b/ui/desktop/src/preload.fileAccess.test.ts new file mode 100644 index 000000000..7e8b6659a --- /dev/null +++ b/ui/desktop/src/preload.fileAccess.test.ts @@ -0,0 +1,43 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +describe('preload file access boundary', () => { + beforeEach(() => { + vi.resetModules(); + }); + + it('exposes only narrow file operations without renderer-supplied paths', async () => { + const exposed: Record = {}; + const invoke = vi.fn(); + vi.doMock('electron', () => ({ + default: {}, + contextBridge: { + exposeInMainWorld: (name: string, api: unknown) => { + exposed[name] = api; + }, + }, + ipcRenderer: { + emit: vi.fn(), + invoke, + off: vi.fn(), + on: vi.fn(), + removeListener: vi.fn(), + send: vi.fn(), + sendSync: vi.fn(), + }, + webUtils: { getPathForFile: vi.fn() }, + })); + + await import('./preload'); + + const electron = exposed.electron as Record unknown>; + expect(electron).not.toHaveProperty('readFile'); + + electron.selectRecipeFile('/etc/passwd'); + electron.readGoosehints('../secret'); + electron.writeGoosehints('project guidance', '../secret'); + + expect(invoke).toHaveBeenNthCalledWith(1, 'select-recipe-file'); + expect(invoke).toHaveBeenNthCalledWith(2, 'read-goosehints'); + expect(invoke).toHaveBeenNthCalledWith(3, 'write-goosehints', 'project guidance'); + }); +}); diff --git a/ui/desktop/src/preload.ts b/ui/desktop/src/preload.ts index 90b365061..0d33f9597 100644 --- a/ui/desktop/src/preload.ts +++ b/ui/desktop/src/preload.ts @@ -118,7 +118,9 @@ type ElectronAPI = { error?: string; } | null>; getBinaryPath: (binaryName: string) => Promise; - readFile: (directory: string) => Promise; + selectRecipeFile: () => Promise; + readGoosehints: () => Promise; + writeGoosehints: (content: string) => Promise; writeFile: (directory: string, content: string) => Promise; ensureDirectory: (dirPath: string) => Promise; listFiles: (dirPath: string, extension?: string) => Promise; @@ -213,7 +215,9 @@ const electronAPI: ElectronAPI = { ipcRenderer.invoke('select-file-or-directory', defaultPath), selectImportSessionFile: () => ipcRenderer.invoke('select-import-session-file'), getBinaryPath: (binaryName: string) => ipcRenderer.invoke('get-binary-path', binaryName), - readFile: (filePath: string) => ipcRenderer.invoke('read-file', filePath), + selectRecipeFile: () => ipcRenderer.invoke('select-recipe-file'), + readGoosehints: () => ipcRenderer.invoke('read-goosehints'), + writeGoosehints: (content: string) => ipcRenderer.invoke('write-goosehints', content), writeFile: (filePath: string, content: string) => ipcRenderer.invoke('write-file', filePath, content), ensureDirectory: (dirPath: string) => ipcRenderer.invoke('ensure-directory', dirPath),