diff --git a/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts b/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts index 74a14706efd..e79257225aa 100644 --- a/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts +++ b/src/main/runtime/orca-runtime-files-repository-admin-path.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, mkdir, readFile, readdir, rm, writeFile } from 'node:fs/promises' +import { mkdtemp, mkdir, readFile, readdir, rm, symlink, writeFile } from 'node:fs/promises' import { existsSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -31,13 +31,16 @@ vi.mock('../git/worktree', () => { let repoPath = '' +let repoConnectionId: string | undefined + function makeStore() { const repo = { id: 'repo-1', path: repoPath, displayName: 'repo', badgeColor: 'blue', - addedAt: 1 + addedAt: 1, + ...(repoConnectionId ? { connectionId: repoConnectionId } : {}) } return { getRepo: (id: string) => (id === 'repo-1' ? repo : undefined), @@ -95,7 +98,11 @@ function dispatchFileMethod(method: string, params: Record): Pr id: 'req-1', authToken: 'tok', method, - params: { worktree: `path:${repoPath}`, expectedExecutionHostId: 'local', ...params } + params: { + worktree: `path:${repoPath}`, + expectedExecutionHostId: repoConnectionId ? `ssh:${repoConnectionId}` : 'local', + ...params + } }) } @@ -416,3 +423,218 @@ describe('files.* RPCs refuse a linked worktree .git pointer file', () => { ) }) }) + +// Why: the relative spelling is not what the filesystem touches. A symlinked ancestor makes a path +// with no `.git` segment resolve straight into `.git`, so segment matching alone is not a guard. +describe.skipIf(process.platform === 'win32')( + 'files.* RPCs refuse a .git aliased through a symlinked ancestor', + () => { + beforeEach(async () => { + await buildRepo() + await symlink(join(repoPath, '.git'), join(repoPath, 'foo'), 'dir') + }) + + afterEach(async () => { + await rm(repoPath, { recursive: true, force: true }) + }) + + it('files.write refuses foo/config when foo is a symlink to .git', async () => { + const response = await dispatchFileMethod('files.write', { + relativePath: 'foo/config', + content: '[core]\n\thooksPath = /tmp/evil\n' + }) + + expectRefused(response) + expect(await readFile(join(repoPath, '.git', 'config'), 'utf-8')).toBe('[core]\n') + }) + + it('files.delete refuses foo/config when foo is a symlink to .git', async () => { + const response = await dispatchFileMethod('files.delete', { + relativePath: 'foo/config', + recursive: false + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'config'))).toBe(true) + }) + + it('files.rename refuses foo/config as the source', async () => { + const response = await dispatchFileMethod('files.rename', { + oldRelativePath: 'foo/config', + newRelativePath: 'stolen-config' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'config'))).toBe(true) + expect(existsSync(join(repoPath, 'stolen-config'))).toBe(false) + }) + + it('files.rename refuses foo/hooks/pre-commit as the destination', async () => { + const response = await dispatchFileMethod('files.rename', { + oldRelativePath: 'tracked.txt', + newRelativePath: 'foo/hooks/pre-commit' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + it('files.copy refuses foo/hooks/pre-commit as the destination', async () => { + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'tracked.txt', + destinationRelativePath: 'foo/hooks/pre-commit' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + it('files.createDir refuses foo/hooks', async () => { + const response = await dispatchFileMethod('files.createDir', { + relativePath: 'foo/hooks' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + it.each([ + ['files.writeBase64', { relativePath: 'foo/config', contentBase64: 'ZXZpbA==' }], + [ + 'files.writeBase64Chunk', + { relativePath: 'foo/config', contentBase64: 'ZXZpbA==', append: true } + ] + ])('%s refuses a symlinked .git target', async (method, params) => { + const response = await dispatchFileMethod(method, params) + + expectRefused(response) + expect(await readFile(join(repoPath, '.git', 'config'), 'utf-8')).toBe('[core]\n') + }) + + it('files.createFile refuses foo/hooks/pre-commit', async () => { + const response = await dispatchFileMethod('files.createFile', { + relativePath: 'foo/hooks/pre-commit' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + it('files.createDirNoClobber refuses foo/hooks', async () => { + const response = await dispatchFileMethod('files.createDirNoClobber', { + relativePath: 'foo/hooks' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + it('files.copy refuses foo/config as the source', async () => { + const response = await dispatchFileMethod('files.copy', { + sourceRelativePath: 'foo/config', + destinationRelativePath: 'stolen-config' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, 'stolen-config'))).toBe(false) + }) + + it('files.commitUpload refuses foo/config as the temp path', async () => { + const response = await dispatchFileMethod('files.commitUpload', { + tempRelativePath: 'foo/config', + finalRelativePath: 'stolen-config' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'config'))).toBe(true) + expect(existsSync(join(repoPath, 'stolen-config'))).toBe(false) + }) + + it('files.commitUpload refuses foo/hooks/pre-commit as the final path', async () => { + const response = await dispatchFileMethod('files.commitUpload', { + tempRelativePath: 'tracked.txt', + finalRelativePath: 'foo/hooks/pre-commit' + }) + + expectRefused(response) + expect(existsSync(join(repoPath, '.git', 'hooks'))).toBe(false) + }) + + // Why: deleting the link itself is legitimate and must keep working — only following it in is not. + it('files.delete still removes the symlink itself, leaving .git intact', async () => { + const response = await dispatchFileMethod('files.delete', { + relativePath: 'foo', + recursive: false + }) + + expect(response.ok).toBe(true) + expect(existsSync(join(repoPath, 'foo'))).toBe(false) + expect(existsSync(join(repoPath, '.git', 'HEAD'))).toBe(true) + }) + } +) + +describe('resolved-path classification', () => { + afterEach(async () => { + await rm(repoPath, { recursive: true, force: true }) + }) + + // Known limitation, documented deliberately: classification runs on the whole resolved path, so a + // workspace that itself lives under a directory named `.git` is refused. It fails safe, and no + // real workspace layout puts a checkout inside a `.git` directory. + it('refuses a workspace that itself lives under a .git segment', async () => { + const base = await mkdtemp(join(tmpdir(), 'orca-admin-path-base-')) + const workspacePath = join(base, '.git', 'workspace') + await mkdir(workspacePath, { recursive: true }) + await writeFile(join(workspacePath, 'tracked.txt'), 'working tree content\n', 'utf-8') + repoPath = workspacePath + listedWorktrees.splice(0, listedWorktrees.length, { path: workspacePath }) + + const response = await dispatchFileMethod('files.delete', { + relativePath: 'tracked.txt', + recursive: false + }) + + expectRefused(response) + expect(existsSync(join(workspacePath, 'tracked.txt'))).toBe(true) + repoPath = base + }) +}) + +// Why: the SSH branch returns before the local gate, so the canonical-path check never runs there. +// The relative-path guard at the RPC boundary is the only thing covering it. +describe('files.* RPCs refuse repository admin paths on the SSH branch', () => { + beforeEach(async () => { + await buildRepo() + repoConnectionId = 'conn-1' + }) + + afterEach(async () => { + repoConnectionId = undefined + await rm(repoPath, { recursive: true, force: true }) + }) + + it.each(['files.delete', 'files.write', 'files.createDir'])( + '%s refuses .git before reaching the SSH provider', + async (method) => { + const response = await dispatchFileMethod( + method, + method === 'files.write' + ? { relativePath: '.git/config', content: 'evil' } + : { relativePath: '.git/config', recursive: false } + ) + + // Without the guard this reaches getSshFilesystemProvider and reports a dropped connection. + expectRefused(response) + } + ) + + it('files.rename refuses a .git destination before reaching the SSH provider', async () => { + const response = await dispatchFileMethod('files.rename', { + oldRelativePath: 'tracked.txt', + newRelativePath: '.git/hooks/pre-commit' + }) + + expectRefused(response) + }) +}) diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index 7f73de87109..612c1aef584 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -52,6 +52,7 @@ import { import { wslAwareSpawn } from '../git/runner' import { parseWslPath, toWindowsWslPath } from '../wsl' import { resolveAuthorizedPath } from '../ipc/filesystem-auth' +import { resolveAuthorizedMutablePath } from './repository-admin-path-authorization' import { isENOENT } from '../ipc/filesystem-path-containment' import { listQuickOpenFiles } from '../ipc/filesystem-list-files' import { searchQuickOpenFilePaths as searchHostQuickOpenFilePaths } from '../ipc/filesystem-search-file-paths' @@ -1804,7 +1805,7 @@ export class RuntimeFileCommands { return { ok: true } } - const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const filePath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) try { const fileStats = await lstat(filePath) if (fileStats.isDirectory()) { @@ -1844,7 +1845,7 @@ export class RuntimeFileCommands { return { ok: true } } - const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const filePath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) await mkdir(dirname(filePath), { recursive: true }) await writeFile(filePath, content, { flag: 'wx' }) return { ok: true } @@ -1876,7 +1877,7 @@ export class RuntimeFileCommands { return { ok: true } } - const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const filePath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) await mkdir(dirname(filePath), { recursive: true }) await writeFile(filePath, content, { flag: append ? 'a' : 'wx' }) return { ok: true } @@ -1905,7 +1906,7 @@ export class RuntimeFileCommands { return { ok: true } } - const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const filePath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) await mkdir(dirname(filePath), { recursive: true }) try { await writeFile(filePath, '', { encoding: 'utf-8', flag: 'wx' }) @@ -1938,7 +1939,7 @@ export class RuntimeFileCommands { return { ok: true } } - const dirPath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const dirPath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) await assertRuntimePathDoesNotExist(dirPath) await mkdir(dirPath, { recursive: false }) return { ok: true } @@ -1967,7 +1968,7 @@ export class RuntimeFileCommands { return { ok: true } } - const dirPath = await resolveAuthorizedPath(target.path, this.host.requireStore()) + const dirPath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore()) await mkdir(dirPath, { recursive: false }) return { ok: true } } @@ -2003,8 +2004,8 @@ export class RuntimeFileCommands { } const store = this.host.requireStore() - const tempPath = await resolveAuthorizedPath(tempTarget.path, store) - const finalPath = await resolveAuthorizedPath(finalTarget.path, store) + const tempPath = await resolveAuthorizedMutablePath(tempTarget.path, store) + const finalPath = await resolveAuthorizedMutablePath(finalTarget.path, store) await mkdir(dirname(finalPath), { recursive: true }) await copyFile(tempPath, finalPath, constants.COPYFILE_EXCL) await rm(tempPath, { force: true }) @@ -2041,8 +2042,12 @@ export class RuntimeFileCommands { } const store = this.host.requireStore() - const oldPath = await resolveAuthorizedPath(oldTarget.path, store, { preserveSymlink: true }) - const newPath = await resolveAuthorizedPath(newTarget.path, store, { preserveSymlink: true }) + const oldPath = await resolveAuthorizedMutablePath(oldTarget.path, store, { + preserveSymlink: true + }) + const newPath = await resolveAuthorizedMutablePath(newTarget.path, store, { + preserveSymlink: true + }) await renameLocalPathSerializedByDestination(oldPath, newPath) return { ok: true } } @@ -2077,10 +2082,10 @@ export class RuntimeFileCommands { } const store = this.host.requireStore() - const sourcePath = await resolveAuthorizedPath(sourceTarget.path, store, { + const sourcePath = await resolveAuthorizedMutablePath(sourceTarget.path, store, { preserveSymlink: true }) - const destinationPath = await resolveAuthorizedPath(destinationTarget.path, store, { + const destinationPath = await resolveAuthorizedMutablePath(destinationTarget.path, store, { preserveSymlink: true }) await mkdir(dirname(destinationPath), { recursive: true }) @@ -2113,7 +2118,7 @@ export class RuntimeFileCommands { return { ok: true } } - const targetPath = await resolveAuthorizedPath(target.path, this.host.requireStore(), { + const targetPath = await resolveAuthorizedMutablePath(target.path, this.host.requireStore(), { preserveSymlink: true }) // Why: a non-local runtime has no client Trash; this delete is permanent, so the renderer confirms before calling. diff --git a/src/main/runtime/repository-admin-path-authorization.ts b/src/main/runtime/repository-admin-path-authorization.ts new file mode 100644 index 00000000000..3f7eb8c0e98 --- /dev/null +++ b/src/main/runtime/repository-admin-path-authorization.ts @@ -0,0 +1,29 @@ +import type { Store } from '../persistence' +import { resolveAuthorizedPath, type ResolveAuthorizedPathOptions } from '../ipc/filesystem-auth' +import { + isRepositoryAdminPath, + REPOSITORY_ADMIN_PATH_DENIED_MESSAGE +} from '../../shared/repository-admin-path' + +/** + * Authorizes a file-explorer mutation, then refuses repository admin state on the path the + * filesystem will actually touch. + * + * The caller's relative spelling is not enough on its own: a symlinked ancestor (`foo -> .git`) + * carries no `.git` segment, yet `resolveAuthorizedPath` canonicalizes it — including through the + * nearest existing ancestor of a not-yet-created path — straight into `.git`. + * + * Fails closed: `resolveAuthorizedPath` throws when it cannot canonicalize, so an unclassifiable + * path never reaches the check. + */ +export async function resolveAuthorizedMutablePath( + targetPath: string, + store: Store, + options: ResolveAuthorizedPathOptions = {} +): Promise { + const resolvedPath = await resolveAuthorizedPath(targetPath, store, options) + if (isRepositoryAdminPath(resolvedPath, process.platform === 'win32' ? 'win32' : 'posix')) { + throw new Error(REPOSITORY_ADMIN_PATH_DENIED_MESSAGE) + } + return resolvedPath +}