diff --git a/packages/core/src/sandbox/linux/bwrapArgsBuilder.test.ts b/packages/core/src/sandbox/linux/bwrapArgsBuilder.test.ts index 24612bbb09..b9584062bc 100644 --- a/packages/core/src/sandbox/linux/bwrapArgsBuilder.test.ts +++ b/packages/core/src/sandbox/linux/bwrapArgsBuilder.test.ts @@ -339,4 +339,61 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => { const envIndex = args.indexOf(`${includeDir}/.env`); expect(args[envIndex - 2]).toBe('--bind'); }); + + it('binds git worktree directories if present', async () => { + const worktreeGitDir = '/path/to/worktree/.git'; + const mainGitDir = '/path/to/main/.git'; + + const args = await buildBwrapArgs({ + ...defaultOptions, + resolvedPaths: createResolvedPaths({ + gitWorktree: { + worktreeGitDir, + mainGitDir, + }, + }), + }); + + expect(args).toContain(worktreeGitDir); + expect(args).toContain(mainGitDir); + expect(args[args.indexOf(worktreeGitDir) - 1]).toBe('--ro-bind-try'); + expect(args[args.indexOf(mainGitDir) - 1]).toBe('--ro-bind-try'); + }); + + it('enforces read-only binding for git worktrees even if workspaceWrite is true', async () => { + const worktreeGitDir = '/path/to/worktree/.git'; + + const args = await buildBwrapArgs({ + ...defaultOptions, + workspaceWrite: true, + resolvedPaths: createResolvedPaths({ + gitWorktree: { + worktreeGitDir, + }, + }), + }); + + expect(args[args.indexOf(worktreeGitDir) - 1]).toBe('--ro-bind-try'); + }); + + it('git worktree read-only bindings should override previous policyWrite bindings', async () => { + const worktreeGitDir = '/custom/worktree/.git'; + + const args = await buildBwrapArgs({ + ...defaultOptions, + resolvedPaths: createResolvedPaths({ + policyWrite: ['/custom/worktree'], + gitWorktree: { + worktreeGitDir, + }, + }), + }); + + const writeBindIndex = args.indexOf('/custom/worktree'); + const worktreeBindIndex = args.lastIndexOf(worktreeGitDir); + + expect(writeBindIndex).toBeGreaterThan(-1); + expect(worktreeBindIndex).toBeGreaterThan(-1); + expect(worktreeBindIndex).toBeGreaterThan(writeBindIndex); + }); }); diff --git a/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts b/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts index d7172b648e..d7fec044e3 100644 --- a/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts +++ b/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts @@ -11,7 +11,7 @@ import { getSecretFileFindArgs, type ResolvedSandboxPaths, } from '../../services/sandboxManager.js'; -import { resolveGitWorktreePaths, isErrnoException } from '../utils/fsUtils.js'; +import { isErrnoException } from '../utils/fsUtils.js'; import { spawnAsync } from '../../utils/shell-utils.js'; import { debugLogger } from '../../utils/debugLogger.js'; @@ -70,16 +70,6 @@ export async function buildBwrapArgs( bwrapArgs.push(bindFlag, workspace.resolved, workspace.resolved); } - const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths( - workspace.resolved, - ); - if (worktreeGitDir) { - bwrapArgs.push(bindFlag, worktreeGitDir, worktreeGitDir); - } - if (mainGitDir) { - bwrapArgs.push(bindFlag, mainGitDir, mainGitDir); - } - for (const includeDir of resolvedPaths.globalIncludes) { bwrapArgs.push('--ro-bind-try', includeDir, includeDir); } @@ -113,6 +103,18 @@ export async function buildBwrapArgs( } } + // Grant read-only access to git worktrees/submodules. We do this last in order to + // ensure that these rules aren't overwritten by broader write policies. + if (resolvedPaths.gitWorktree) { + const { worktreeGitDir, mainGitDir } = resolvedPaths.gitWorktree; + if (worktreeGitDir) { + bwrapArgs.push('--ro-bind-try', worktreeGitDir, worktreeGitDir); + } + if (mainGitDir) { + bwrapArgs.push('--ro-bind-try', mainGitDir, mainGitDir); + } + } + for (const p of resolvedPaths.forbidden) { if (!fs.existsSync(p)) continue; try { diff --git a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts index 19ba8303ae..e8801b055b 100644 --- a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts +++ b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts @@ -142,5 +142,62 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => { expect(denyIndex).toBeGreaterThan(allowIndex); }); }); + + describe('git worktree paths', () => { + it('enforces read-only binding for git worktrees even if workspaceWrite is true', () => { + const worktreeGitDir = '/path/to/worktree/.git'; + const mainGitDir = '/path/to/main/.git'; + + const profile = buildSeatbeltProfile({ + resolvedPaths: { + ...defaultResolvedPaths, + gitWorktree: { + worktreeGitDir, + mainGitDir, + }, + }, + workspaceWrite: true, + }); + + // Should grant read access + expect(profile).toContain( + `(allow file-read* (subpath "${worktreeGitDir}"))`, + ); + expect(profile).toContain( + `(allow file-read* (subpath "${mainGitDir}"))`, + ); + + // Should NOT grant write access + expect(profile).not.toContain( + `(allow file-read* file-write* (subpath "${worktreeGitDir}"))`, + ); + expect(profile).not.toContain( + `(allow file-read* file-write* (subpath "${mainGitDir}"))`, + ); + }); + + it('git worktree read-only rules should override previous policyAllowed write paths', () => { + const worktreeGitDir = '/custom/worktree/.git'; + const profile = buildSeatbeltProfile({ + resolvedPaths: { + ...defaultResolvedPaths, + policyAllowed: ['/custom/worktree'], + gitWorktree: { + worktreeGitDir, + }, + }, + }); + + const allowString = `(allow file-read* file-write* (subpath "/custom/worktree"))`; + const denyString = `(deny file-write* (subpath "${worktreeGitDir}"))`; + + expect(profile).toContain(allowString); + expect(profile).toContain(denyString); + + const allowIndex = profile.indexOf(allowString); + const denyIndex = profile.indexOf(denyString); + expect(denyIndex).toBeGreaterThan(allowIndex); + }); + }); }); }); diff --git a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts index 967cd8f183..abbf1a6d92 100644 --- a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts +++ b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts @@ -16,7 +16,7 @@ import { SECRET_FILES, type ResolvedSandboxPaths, } from '../../services/sandboxManager.js'; -import { tryRealpath, resolveGitWorktreePaths } from '../utils/fsUtils.js'; +import { resolveToRealPath } from '../../utils/paths.js'; /** * Options for building macOS Seatbelt profile. @@ -52,21 +52,21 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string { profile += `(allow file-write* (subpath "${escapeSchemeString(resolvedPaths.workspace.resolved)}"))\n`; } - const tmpPath = tryRealpath(os.tmpdir()); + const tmpPath = resolveToRealPath(os.tmpdir()); profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(tmpPath)}"))\n`; - // Auto-detect and support git worktrees by granting read and write access to the underlying git directory - const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths( - resolvedPaths.workspace.resolved, - ); - if (worktreeGitDir) { - profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(worktreeGitDir)}"))\n`; - } - if (mainGitDir) { - profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(mainGitDir)}"))\n`; + // Support git worktrees/submodules; read-only to prevent malicious hook/config modification (RCE). + if (resolvedPaths.gitWorktree) { + const { worktreeGitDir, mainGitDir } = resolvedPaths.gitWorktree; + if (worktreeGitDir) { + profile += `(allow file-read* (subpath "${escapeSchemeString(worktreeGitDir)}"))\n`; + } + if (mainGitDir) { + profile += `(allow file-read* (subpath "${escapeSchemeString(mainGitDir)}"))\n`; + } } - const nodeRootPath = tryRealpath( + const nodeRootPath = resolveToRealPath( path.dirname(path.dirname(process.execPath)), ); profile += `(allow file-read* (subpath "${escapeSchemeString(nodeRootPath)}"))\n`; @@ -79,7 +79,7 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string { for (const p of paths) { if (!p.trim()) continue; try { - let resolved = tryRealpath(p); + let resolved = resolveToRealPath(p); // If this is a 'bin' directory (like /usr/local/bin or homebrew/bin), // also grant read access to its parent directory so that symlinked @@ -148,7 +148,7 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string { resolvedPaths.workspace.resolved, GOVERNANCE_FILES[i].path, ); - const realGovernanceFile = tryRealpath(governanceFile); + const realGovernanceFile = resolveToRealPath(governanceFile); // Determine if it should be treated as a directory (subpath) or a file (literal). // .git is generally a directory, while ignore files are literals. @@ -170,6 +170,18 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string { } } + // Grant read-only access to git worktrees/submodules. We do this last in order to + // ensure that these rules aren't overwritten by broader write policies. + if (resolvedPaths.gitWorktree) { + const { worktreeGitDir, mainGitDir } = resolvedPaths.gitWorktree; + if (worktreeGitDir) { + profile += `(deny file-write* (subpath "${escapeSchemeString(worktreeGitDir)}"))\n`; + } + if (mainGitDir) { + profile += `(deny file-write* (subpath "${escapeSchemeString(mainGitDir)}"))\n`; + } + } + // Add explicit deny rules for secret files (.env, .env.*) in the workspace and allowed paths. // We use regex rules to avoid expensive file discovery scans. // Anchoring to workspace/allowed paths to avoid over-blocking. diff --git a/packages/core/src/sandbox/utils/fsUtils.test.ts b/packages/core/src/sandbox/utils/fsUtils.test.ts index 9439050680..460fb9d26b 100644 --- a/packages/core/src/sandbox/utils/fsUtils.test.ts +++ b/packages/core/src/sandbox/utils/fsUtils.test.ts @@ -4,49 +4,117 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import fs from 'node:fs'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import fsPromises from 'node:fs/promises'; import path from 'node:path'; -import os from 'node:os'; -import { tryRealpath } from './fsUtils.js'; +import { resolveGitWorktreePaths } from './fsUtils.js'; + +vi.mock('node:fs/promises', async () => { + const actual = + await vi.importActual( + 'node:fs/promises', + ); + return { + ...actual, + default: { + ...actual, + lstat: vi.fn(), + readFile: vi.fn(), + }, + lstat: vi.fn(), + readFile: vi.fn(), + }; +}); + +vi.mock('../../utils/paths.js', async () => { + const actual = await vi.importActual( + '../../utils/paths.js', + ); + return { + ...actual, + resolveToRealPath: vi.fn((p) => p), + }; +}); describe('fsUtils', () => { - let tempDir: string; - let realTempDir: string; + describe('resolveGitWorktreePaths', () => { + const workspace = path.resolve('/workspace'); + const gitPath = path.join(workspace, '.git'); - beforeAll(() => { - tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'fs-utils-test-')); - realTempDir = fs.realpathSync(tempDir); - }); + beforeEach(() => { + vi.clearAllMocks(); + }); - afterAll(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); - }); - - describe('tryRealpath', () => { - it('should throw error for paths with null bytes', () => { - expect(() => tryRealpath(path.join(tempDir, 'foo\0bar'))).toThrow( - 'Invalid path', + it('should return empty if .git does not exist', async () => { + vi.mocked(fsPromises.lstat).mockRejectedValue( + Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) as never, ); + const result = await resolveGitWorktreePaths(workspace); + expect(result).toEqual({}); }); - it('should resolve existing paths', () => { - const resolved = tryRealpath(tempDir); - expect(resolved).toBe(realTempDir); + it('should return empty if .git is a directory', async () => { + vi.mocked(fsPromises.lstat).mockResolvedValue({ + isFile: () => false, + } as never); + const result = await resolveGitWorktreePaths(workspace); + expect(result).toEqual({}); }); - it('should handle non-existent paths by resolving parent', () => { - const nonExistentPath = path.join(tempDir, 'non-existent-file-12345'); - const expected = path.join(realTempDir, 'non-existent-file-12345'); - const resolved = tryRealpath(nonExistentPath); - expect(resolved).toBe(expected); + it('should resolve worktree paths from .git file', async () => { + const mainGitDir = path.resolve('/project/.git'); + const worktreeGitDir = path.join(mainGitDir, 'worktrees', 'feature'); + + vi.mocked(fsPromises.lstat).mockResolvedValue({ + isFile: () => true, + } as never); + vi.mocked(fsPromises.readFile).mockImplementation(((p: string) => { + if (p === gitPath) return Promise.resolve(`gitdir: ${worktreeGitDir}`); + if (p === path.join(worktreeGitDir, 'gitdir')) + return Promise.resolve(gitPath); + return Promise.reject(new Error('ENOENT')); + }) as never); + + const result = await resolveGitWorktreePaths(workspace); + expect(result).toEqual({ + worktreeGitDir, + mainGitDir, + }); }); - it('should handle nested non-existent paths', () => { - const nonExistentPath = path.join(tempDir, 'dir1', 'dir2', 'file'); - const expected = path.join(realTempDir, 'dir1', 'dir2', 'file'); - const resolved = tryRealpath(nonExistentPath); - expect(resolved).toBe(expected); + it('should reject worktree if backlink is missing or invalid', async () => { + const worktreeGitDir = path.resolve('/git/worktrees/feature'); + + vi.mocked(fsPromises.lstat).mockResolvedValue({ + isFile: () => true, + } as never); + vi.mocked(fsPromises.readFile).mockImplementation(((p: string) => { + if (p === gitPath) return Promise.resolve(`gitdir: ${worktreeGitDir}`); + return Promise.reject(new Error('ENOENT')); + }) as never); + + const result = await resolveGitWorktreePaths(workspace); + expect(result).toEqual({}); + }); + + it('should support submodules via config check', async () => { + const submoduleGitDir = path.resolve('/project/.git/modules/sub'); + + vi.mocked(fsPromises.lstat).mockResolvedValue({ + isFile: () => true, + } as never); + vi.mocked(fsPromises.readFile).mockImplementation(((p: string) => { + if (p === gitPath) return Promise.resolve(`gitdir: ${submoduleGitDir}`); + if (p === path.join(submoduleGitDir, 'config')) + return Promise.resolve(`[core]\n\tworktree = ${workspace}`); + return Promise.reject(new Error('ENOENT')); + }) as never); + + const result = await resolveGitWorktreePaths(workspace); + expect(result).toEqual({ + worktreeGitDir: submoduleGitDir, + mainGitDir: path.resolve('/project/.git'), + }); }); }); }); diff --git a/packages/core/src/sandbox/utils/fsUtils.ts b/packages/core/src/sandbox/utils/fsUtils.ts index 2e3eda1342..c9729caf26 100644 --- a/packages/core/src/sandbox/utils/fsUtils.ts +++ b/packages/core/src/sandbox/utils/fsUtils.ts @@ -4,68 +4,55 @@ * SPDX-License-Identifier: Apache-2.0 */ -import fs from 'node:fs'; +import fs from 'node:fs/promises'; import path from 'node:path'; -import { assertValidPathString } from '../../utils/paths.js'; +import { resolveToRealPath } from '../../utils/paths.js'; export function isErrnoException(e: unknown): e is NodeJS.ErrnoException { return e instanceof Error && 'code' in e; } -export function tryRealpath(p: string): string { - assertValidPathString(p); - try { - return fs.realpathSync(p); - } catch (e) { - if (isErrnoException(e) && e.code === 'ENOENT') { - const parentDir = path.dirname(p); - if (parentDir === p) { - return p; - } - return path.join(tryRealpath(parentDir), path.basename(p)); - } - throw e; - } -} - -export function resolveGitWorktreePaths(workspacePath: string): { +export async function resolveGitWorktreePaths(workspacePath: string): Promise<{ worktreeGitDir?: string; mainGitDir?: string; -} { +}> { try { const gitPath = path.join(workspacePath, '.git'); - const gitStat = fs.lstatSync(gitPath); + const gitStat = await fs.lstat(gitPath); if (gitStat.isFile()) { - const gitContent = fs.readFileSync(gitPath, 'utf8'); + const gitContent = await fs.readFile(gitPath, 'utf8'); const match = gitContent.match(/^gitdir:\s+(.+)$/m); if (match && match[1]) { let worktreeGitDir = match[1].trim(); if (!path.isAbsolute(worktreeGitDir)) { worktreeGitDir = path.resolve(workspacePath, worktreeGitDir); } - const resolvedWorktreeGitDir = tryRealpath(worktreeGitDir); + const resolvedWorktreeGitDir = resolveToRealPath(worktreeGitDir); // Security check: Verify the bidirectional link to prevent sandbox escape let isValid = false; try { const backlinkPath = path.join(resolvedWorktreeGitDir, 'gitdir'); - const backlink = fs.readFileSync(backlinkPath, 'utf8').trim(); + const backlink = (await fs.readFile(backlinkPath, 'utf8')).trim(); // The backlink must resolve to the workspace's .git file - if (tryRealpath(backlink) === tryRealpath(gitPath)) { + if (resolveToRealPath(backlink) === resolveToRealPath(gitPath)) { isValid = true; } } catch { // Fallback for submodules: check core.worktree in config try { const configPath = path.join(resolvedWorktreeGitDir, 'config'); - const config = fs.readFileSync(configPath, 'utf8'); + const config = await fs.readFile(configPath, 'utf8'); const match = config.match(/^\s*worktree\s*=\s*(.+)$/m); if (match && match[1]) { const worktreePath = path.resolve( resolvedWorktreeGitDir, match[1].trim(), ); - if (tryRealpath(worktreePath) === tryRealpath(workspacePath)) { + if ( + resolveToRealPath(worktreePath) === + resolveToRealPath(workspacePath) + ) { isValid = true; } } @@ -78,7 +65,7 @@ export function resolveGitWorktreePaths(workspacePath: string): { return {}; // Reject: valid worktrees/submodules must have a readable backlink } - const mainGitDir = tryRealpath( + const mainGitDir = resolveToRealPath( path.dirname(path.dirname(resolvedWorktreeGitDir)), ); return { diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts index 40902b9121..b504d92f72 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts @@ -10,6 +10,7 @@ import os from 'node:os'; import path from 'node:path'; import { WindowsSandboxManager } from './WindowsSandboxManager.js'; import * as sandboxManager from '../../services/sandboxManager.js'; +import * as paths from '../../utils/paths.js'; import type { SandboxRequest } from '../../services/sandboxManager.js'; import { spawnAsync } from '../../utils/shell-utils.js'; import type { SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js'; @@ -44,9 +45,7 @@ describe('WindowsSandboxManager', () => { beforeEach(() => { vi.spyOn(os, 'platform').mockReturnValue('win32'); - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => - p.toString(), - ); + vi.spyOn(paths, 'resolveToRealPath').mockImplementation((p) => p); // Mock existsSync to skip the csc.exe auto-compilation of helper during unit tests. const originalExistsSync = fs.existsSync; @@ -299,6 +298,60 @@ describe('WindowsSandboxManager', () => { } }); + it('should NOT grant Low Integrity access to git worktree paths (enforce read-only)', async () => { + const worktreeGitDir = createTempDir('worktree-git'); + const mainGitDir = createTempDir('main-git'); + + try { + vi.spyOn(sandboxManager, 'resolveSandboxPaths').mockResolvedValue({ + workspace: { original: testCwd, resolved: testCwd }, + forbidden: [], + globalIncludes: [], + policyAllowed: [], + policyRead: [], + policyWrite: [], + gitWorktree: { + worktreeGitDir, + mainGitDir, + }, + }); + + const req: SandboxRequest = { + command: 'test', + args: [], + cwd: testCwd, + env: {}, + }; + + await manager.prepareCommand(req); + + const icaclsArgs = vi + .mocked(spawnAsync) + .mock.calls.filter((c) => c[0] === 'icacls') + .map((c) => c[1]); + + // Verify that no icacls grants were issued for the git directories + expect(icaclsArgs).not.toContainEqual([ + worktreeGitDir, + '/grant', + '*S-1-16-4096:(OI)(CI)(M)', + '/setintegritylevel', + '(OI)(CI)Low', + ]); + + expect(icaclsArgs).not.toContainEqual([ + mainGitDir, + '/grant', + '*S-1-16-4096:(OI)(CI)(M)', + '/setintegritylevel', + '(OI)(CI)Low', + ]); + } finally { + fs.rmSync(worktreeGitDir, { recursive: true, force: true }); + fs.rmSync(mainGitDir, { recursive: true, force: true }); + } + }); + it('should grant Low Integrity access to additional write paths', async () => { const extraWritePath = createTempDir('extra-write'); try { diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 86d1eda641..2cf736f865 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -299,7 +299,9 @@ export class WindowsSandboxManager implements SandboxManager { ) : false; - if (!isReadonlyMode || isApproved) { + const workspaceWrite = !isReadonlyMode || isApproved || isYolo; + + if (workspaceWrite) { await this.grantLowIntegrityAccess(resolvedPaths.workspace.resolved); writableRoots.push(resolvedPaths.workspace.resolved); } @@ -345,6 +347,12 @@ export class WindowsSandboxManager implements SandboxManager { } } + // Support git worktrees/submodules; read-only to prevent malicious hook/config modification (RCE). + // Read access is inherited; skip grantLowIntegrityAccess to ensure write protection. + if (resolvedPaths.gitWorktree) { + // No-op for read access. + } + // 2. Collect secret files and apply protective ACLs // On Windows, we explicitly deny access to secret files for Low Integrity // processes to ensure they cannot be read or written. diff --git a/packages/core/src/services/sandboxManager.integration.test.ts b/packages/core/src/services/sandboxManager.integration.test.ts index 1461b6d606..65adeaacbb 100644 --- a/packages/core/src/services/sandboxManager.integration.test.ts +++ b/packages/core/src/services/sandboxManager.integration.test.ts @@ -507,6 +507,102 @@ describe('SandboxManager Integration', () => { }); }); + describe('Git Worktree Support', () => { + it('allows access to git common directory in a worktree', async () => { + const mainRepo = createTempDir('main-repo-'); + const worktreeDir = createTempDir('worktree-'); + + const mainGitDir = path.join(mainRepo, '.git'); + fs.mkdirSync(mainGitDir, { recursive: true }); + fs.writeFileSync( + path.join(mainGitDir, 'config'), + '[core]\n\trepositoryformatversion = 0\n', + ); + + const worktreeGitDir = path.join( + mainGitDir, + 'worktrees', + 'test-worktree', + ); + fs.mkdirSync(worktreeGitDir, { recursive: true }); + + // Create the .git file in the worktree directory pointing to the worktree git dir + fs.writeFileSync( + path.join(worktreeDir, '.git'), + `gitdir: ${worktreeGitDir}\n`, + ); + + // Create the backlink from worktree git dir to the worktree's .git file + const backlinkPath = path.join(worktreeGitDir, 'gitdir'); + fs.writeFileSync(backlinkPath, path.join(worktreeDir, '.git')); + + // Create a file in the worktree git dir that we want to access + const secretFile = path.join(worktreeGitDir, 'secret.txt'); + fs.writeFileSync(secretFile, 'git-secret'); + + const osManager = createSandboxManager( + { enabled: true }, + { workspace: worktreeDir }, + ); + + const { command, args } = Platform.cat(secretFile); + const sandboxed = await osManager.prepareCommand({ + command, + args, + cwd: worktreeDir, + env: process.env, + }); + + const result = await runCommand(sandboxed); + assertResult(result, sandboxed, 'success'); + expect(result.stdout.trim()).toBe('git-secret'); + }); + + it('blocks write access to git common directory in a worktree', async () => { + const mainRepo = createTempDir('main-repo-'); + const worktreeDir = createTempDir('worktree-'); + + const mainGitDir = path.join(mainRepo, '.git'); + fs.mkdirSync(mainGitDir, { recursive: true }); + + const worktreeGitDir = path.join( + mainGitDir, + 'worktrees', + 'test-worktree', + ); + fs.mkdirSync(worktreeGitDir, { recursive: true }); + + fs.writeFileSync( + path.join(worktreeDir, '.git'), + `gitdir: ${worktreeGitDir}\n`, + ); + fs.writeFileSync( + path.join(worktreeGitDir, 'gitdir'), + path.join(worktreeDir, '.git'), + ); + + const targetFile = path.join(worktreeGitDir, 'secret.txt'); + + const osManager = createSandboxManager( + { enabled: true }, + // Use YOLO mode to ensure the workspace is fully writable, but git worktrees should still be read-only + { workspace: worktreeDir, modeConfig: { yolo: true } }, + ); + + const { command, args } = Platform.touch(targetFile); + const sandboxed = await osManager.prepareCommand({ + command, + args, + cwd: worktreeDir, + env: process.env, + }); + + const result = await runCommand(sandboxed); + assertResult(result, sandboxed, 'failure'); + expect(fs.existsSync(targetFile)).toBe(false); + }); + }); + describe('Network Access', () => { let server: http.Server; let url: string; diff --git a/packages/core/src/services/sandboxManager.test.ts b/packages/core/src/services/sandboxManager.test.ts index 134ef167bd..7ff8525f77 100644 --- a/packages/core/src/services/sandboxManager.test.ts +++ b/packages/core/src/services/sandboxManager.test.ts @@ -13,7 +13,6 @@ import { sanitizePaths, findSecretFiles, isSecretFile, - tryRealpath, resolveSandboxPaths, getPathIdentity, type SandboxRequest, @@ -36,10 +35,25 @@ vi.mock('node:fs/promises', async () => { readdir: vi.fn(), realpath: vi.fn(), stat: vi.fn(), + lstat: vi.fn(), + readFile: vi.fn(), }, readdir: vi.fn(), realpath: vi.fn(), stat: vi.fn(), + lstat: vi.fn(), + readFile: vi.fn(), + }; +}); + +vi.mock('../utils/paths.js', async () => { + const actual = + await vi.importActual( + '../utils/paths.js', + ); + return { + ...actual, + resolveToRealPath: vi.fn((p) => p), }; }); @@ -279,104 +293,6 @@ describe('SandboxManager', () => { }); }); - describe('tryRealpath', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('should return the realpath if the file exists', async () => { - const realPath = path.resolve('/real/path/to/file.txt'); - const symlinkPath = path.resolve('/some/symlink/to/file.txt'); - vi.mocked(fsPromises.realpath).mockResolvedValue(realPath as never); - const result = await tryRealpath(symlinkPath); - expect(result).toBe(realPath); - expect(fsPromises.realpath).toHaveBeenCalledWith(symlinkPath); - }); - - it('should fallback to parent directory if file does not exist (ENOENT)', async () => { - const nonexistent = path.resolve('/workspace/nonexistent.txt'); - const workspace = path.resolve('/workspace'); - const realWorkspace = path.resolve('/real/workspace'); - - vi.mocked(fsPromises.realpath).mockImplementation(((p: string) => { - if (p === nonexistent) { - return Promise.reject( - Object.assign(new Error('ENOENT: no such file or directory'), { - code: 'ENOENT', - }), - ); - } - if (p === workspace) { - return Promise.resolve(realWorkspace); - } - return Promise.reject(new Error(`Unexpected path: ${p}`)); - }) as never); - - const result = await tryRealpath(nonexistent); - - // It should combine the real path of the parent with the original basename - expect(result).toBe(path.join(realWorkspace, 'nonexistent.txt')); - }); - - it('should recursively fallback up the directory tree on multiple ENOENT errors', async () => { - const missingFile = path.resolve( - '/workspace/missing_dir/missing_file.txt', - ); - const missingDir = path.resolve('/workspace/missing_dir'); - const workspace = path.resolve('/workspace'); - const realWorkspace = path.resolve('/real/workspace'); - - vi.mocked(fsPromises.realpath).mockImplementation(((p: string) => { - if (p === missingFile) { - return Promise.reject( - Object.assign(new Error('ENOENT'), { code: 'ENOENT' }), - ); - } - if (p === missingDir) { - return Promise.reject( - Object.assign(new Error('ENOENT'), { code: 'ENOENT' }), - ); - } - if (p === workspace) { - return Promise.resolve(realWorkspace); - } - return Promise.reject(new Error(`Unexpected path: ${p}`)); - }) as never); - - const result = await tryRealpath(missingFile); - - // It should resolve '/workspace' to '/real/workspace' and append the missing parts - expect(result).toBe( - path.join(realWorkspace, 'missing_dir', 'missing_file.txt'), - ); - }); - - it('should return the path unchanged if it reaches the root directory and it still does not exist', async () => { - const rootPath = path.resolve('/'); - vi.mocked(fsPromises.realpath).mockImplementation(() => - Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })), - ); - - const result = await tryRealpath(rootPath); - expect(result).toBe(rootPath); - }); - - it('should throw an error if realpath fails with a non-ENOENT error (e.g. EACCES)', async () => { - const secretFile = path.resolve('/secret/file.txt'); - vi.mocked(fsPromises.realpath).mockImplementation(() => - Promise.reject( - Object.assign(new Error('EACCES: permission denied'), { - code: 'EACCES', - }), - ), - ); - - await expect(tryRealpath(secretFile)).rejects.toThrow( - 'EACCES: permission denied', - ); - }); - }); - describe('NoopSandboxManager', () => { const sandboxManager = new NoopSandboxManager(); diff --git a/packages/core/src/services/sandboxManager.ts b/packages/core/src/services/sandboxManager.ts index f7f2944fe7..0191207b16 100644 --- a/packages/core/src/services/sandboxManager.ts +++ b/packages/core/src/services/sandboxManager.ts @@ -15,7 +15,6 @@ import { isKnownSafeCommand as isWindowsSafeCommand, isDangerousCommand as isWindowsDangerousCommand, } from '../sandbox/windows/commandSafety.js'; -import { isNodeError } from '../utils/errors.js'; import { sanitizeEnvironment, getSecureSanitizationConfig, @@ -24,6 +23,7 @@ import { import type { ShellExecutionResult } from './shellExecutionService.js'; import type { SandboxPolicyManager } from '../policy/sandboxPolicyManager.js'; import { resolveToRealPath } from '../utils/paths.js'; +import { resolveGitWorktreePaths } from '../sandbox/utils/fsUtils.js'; /** * A structured result of fully resolved sandbox paths. @@ -48,6 +48,13 @@ export interface ResolvedSandboxPaths { policyRead: string[]; /** Paths granted temporary write access by the current command's dynamic permissions. */ policyWrite: string[]; + /** Auto-detected paths for git worktrees/submodules. */ + gitWorktree?: { + /** The actual .git directory for this worktree. */ + worktreeGitDir: string; + /** The main repository's .git directory (if applicable). */ + mainGitDir?: string; + }; } export interface SandboxPermissions { @@ -392,6 +399,12 @@ export async function resolveSandboxPaths( ); const forbiddenIdentities = new Set(forbidden.map(getPathIdentity)); + const { worktreeGitDir, mainGitDir } = + await resolveGitWorktreePaths(resolvedWorkspace); + const gitWorktree = worktreeGitDir + ? { gitWorktree: { worktreeGitDir, mainGitDir } } + : undefined; + /** * Filters out any paths that are explicitly forbidden or match the workspace root (original or resolved). */ @@ -413,8 +426,10 @@ export async function resolveSandboxPaths( policyAllowed: filter(policyAllowed), policyRead: filter(policyRead), policyWrite: filter(policyWrite), + ...gitWorktree, }; } + /** * Sanitizes an array of paths by deduplicating them and ensuring they are absolute. * Always returns an array (empty if input is null/undefined). @@ -451,24 +466,4 @@ export function getPathIdentity(p: string): string { return isCaseInsensitive ? norm.toLowerCase() : norm; } -/** - * Resolves symlinks for a given path to prevent sandbox escapes. - * If a file does not exist (ENOENT), it recursively resolves the parent directory. - * Other errors (e.g. EACCES) are re-thrown. - */ -export async function tryRealpath(p: string): Promise { - try { - return await fs.realpath(p); - } catch (e) { - if (isNodeError(e) && e.code === 'ENOENT') { - const parentDir = path.dirname(p); - if (parentDir === p) { - return p; - } - return path.join(await tryRealpath(parentDir), path.basename(p)); - } - throw e; - } -} - export { createSandboxManager } from './sandboxManagerFactory.js';