Implementation of sandbox "Write-Protected" Governance Files (#23139)

Co-authored-by: Abhi <43648792+abhipatel12@users.noreply.github.com>
This commit is contained in:
David Pierce
2026-03-24 04:04:17 +00:00
committed by GitHub
parent a833d350a4
commit 37c8de3c06
7 changed files with 365 additions and 51 deletions

View File

@@ -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<typeof import('node:fs')>('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`,
]);
});
});

View File

@@ -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(
/\/$/,

View File

@@ -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({

View File

@@ -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.

View File

@@ -76,6 +76,16 @@ export interface SandboxManager {
prepareCommand(req: SandboxRequest): Promise<SandboxedCommand>;
}
/**
* 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.

View File

@@ -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 });
}
});
});

View File

@@ -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<void> {
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 <network:0|1> <cwd> <command> [args...]
const program = this.helperPath;