MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-12 14:22:00 -07:00
Code Issues Packages Projects Releases Wiki Activity

refactor(sandbox): use centralized sandbox paths in macOS Seatbelt implementation (#24984)

Browse Source
This commit is contained in:
Emily Hedlund
2026-04-08 18:29:38 -07:00
committed by GitHub
parent 464bac270c
commit 5d589946ad
4 changed files with 190 additions and 250 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts
View File

@@ -64,20 +64,12 @@ describe('MacOsSandboxManager', () => {
policy: mockPolicy,
});
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith({
workspace: mockWorkspace,
allowedPaths: mockAllowedPaths,
forbiddenPaths: [],
networkAccess: mockNetworkAccess,
workspaceWrite: false,
additionalPermissions: {
fileSystem: {
read: [],
write: [],
},
network: true,
},
});
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
networkAccess: true,
workspaceWrite: false,
}),
);
expect(result.program).toBe('/usr/bin/sandbox-exec');
expect(result.args[0]).toBe('-f');
@@ -155,11 +147,10 @@ describe('MacOsSandboxManager', () => {
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
additionalPermissions: expect.objectContaining({
fileSystem: expect.objectContaining({
read: expect.not.arrayContaining(['/']),
write: expect.not.arrayContaining(['/']),
}),
workspaceWrite: true,
resolvedPaths: expect.objectContaining({
policyRead: expect.not.arrayContaining(['/']),
policyWrite: expect.not.arrayContaining(['/']),
}),
}),
);
@@ -213,7 +204,11 @@ describe('MacOsSandboxManager', () => {
// The seatbelt builder internally handles governance files, so we simply verify
// it is invoked correctly with the right workspace.
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({ workspace: mockWorkspace }),
expect.objectContaining({
resolvedPaths: expect.objectContaining({
workspace: { resolved: mockWorkspace, original: mockWorkspace },
}),
}),
);
});
});
@@ -233,10 +228,12 @@ describe('MacOsSandboxManager', () => {
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
allowedPaths: expect.arrayContaining([
'/tmp/allowed1',
'/tmp/allowed2',
]),
resolvedPaths: expect.objectContaining({
policyAllowed: expect.arrayContaining([
'/tmp/allowed1',
'/tmp/allowed2',
]),
}),
}),
);
});
@@ -258,7 +255,9 @@ describe('MacOsSandboxManager', () => {
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
forbiddenPaths: expect.arrayContaining(['/tmp/forbidden1']),
resolvedPaths: expect.objectContaining({
forbidden: expect.arrayContaining(['/tmp/forbidden1']),
}),
}),
);
});
@@ -278,7 +277,9 @@ describe('MacOsSandboxManager', () => {
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
forbiddenPaths: expect.arrayContaining(['/tmp/does-not-exist']),
resolvedPaths: expect.objectContaining({
forbidden: expect.arrayContaining(['/tmp/does-not-exist']),
}),
}),
);
});
@@ -301,8 +302,10 @@ describe('MacOsSandboxManager', () => {
expect(seatbeltArgsBuilder.buildSeatbeltProfile).toHaveBeenCalledWith(
expect.objectContaining({
allowedPaths: [],
forbiddenPaths: expect.arrayContaining(['/tmp/conflict']),
resolvedPaths: expect.objectContaining({
policyAllowed: [],
forbidden: expect.arrayContaining(['/tmp/conflict']),
}),
}),
);
});

24
packages/core/src/sandbox/macos/MacOsSandboxManager.ts
View File

@@ -133,28 +133,26 @@ export class MacOsSandboxManager implements SandboxManager {
false,
};
const { command: finalCommand, args: finalArgs } = handleReadWriteCommands(
req,
mergedAdditional,
this.options.workspace,
[
...(req.policy?.allowedPaths || []),
...(this.options.includeDirectories || []),
],
);
const resolvedPaths = await resolveSandboxPaths(
this.options,
req,
mergedAdditional,
);
const { command: finalCommand, args: finalArgs } = handleReadWriteCommands(
req,
mergedAdditional,
this.options.workspace,
req.policy?.allowedPaths,
);
const sandboxArgs = buildSeatbeltProfile({
workspace: this.options.workspace,
allowedPaths: [
...resolvedPaths.policyAllowed,
...(this.options.includeDirectories || []),
],
forbiddenPaths: resolvedPaths.forbidden,
resolvedPaths,
networkAccess: mergedAdditional.network,
workspaceWrite,
additionalPermissions: mergedAdditional,
});
const tempFile = this.writeProfileToTempFile(sandboxArgs);

144
packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts
View File

@@ -8,18 +8,21 @@ import {
buildSeatbeltProfile,
escapeSchemeString,
} from './seatbeltArgsBuilder.js';
import * as fsUtils from '../utils/fsUtils.js';
import type { ResolvedSandboxPaths } from '../../services/sandboxManager.js';
import fs from 'node:fs';
import os from 'node:os';
vi.mock('../utils/fsUtils.js', async () => {
const actual = await vi.importActual('../utils/fsUtils.js');
return {
...actual,
tryRealpath: vi.fn((p) => p),
resolveGitWorktreePaths: vi.fn(() => ({})),
};
});
const defaultResolvedPaths: ResolvedSandboxPaths = {
workspace: {
resolved: '/Users/test/workspace',
original: '/Users/test/raw-workspace',
},
forbidden: [],
globalIncludes: [],
policyAllowed: [],
policyRead: [],
policyWrite: [],
};
describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
afterEach(() => {
@@ -35,12 +38,8 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
describe('buildSeatbeltProfile', () => {
it('should build a strict allowlist profile allowing the workspace', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
const profile = buildSeatbeltProfile({
workspace: '/Users/test/workspace',
allowedPaths: [],
forbiddenPaths: [],
resolvedPaths: defaultResolvedPaths,
});
expect(profile).toContain('(version 1)');
@@ -51,11 +50,11 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
});
it('should allow network when networkAccess is true', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: [],
forbiddenPaths: [],
resolvedPaths: {
...defaultResolvedPaths,
workspace: { resolved: '/test', original: '/test' },
},
networkAccess: true,
});
expect(profile).toContain('(allow network-outbound)');
@@ -63,7 +62,6 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
describe('governance files', () => {
it('should inject explicit deny rules for governance files', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p.toString());
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
vi.spyOn(fs, 'lstatSync').mockImplementation(
(p) =>
@@ -74,9 +72,13 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
);
const profile = buildSeatbeltProfile({
workspace: '/test/workspace',
allowedPaths: [],
forbiddenPaths: [],
resolvedPaths: {
...defaultResolvedPaths,
workspace: {
resolved: '/test/workspace',
original: '/test/workspace',
},
},
});
expect(profile).toContain(
@@ -87,48 +89,16 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
`(deny file-write* (subpath "/test/workspace/.git"))`,
);
});
it('should protect both the symlink and the real path if they differ', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
if (p === '/test/workspace/.gitignore')
return '/test/real/.gitignore';
return p.toString();
});
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
vi.spyOn(fs, 'lstatSync').mockImplementation(
() =>
({
isDirectory: () => false,
isFile: () => true,
}) as unknown as fs.Stats,
);
const profile = buildSeatbeltProfile({
workspace: '/test/workspace',
allowedPaths: [],
forbiddenPaths: [],
});
expect(profile).toContain(
`(deny file-write* (literal "/test/workspace/.gitignore"))`,
);
expect(profile).toContain(
`(deny file-write* (literal "/test/real/.gitignore"))`,
);
});
});
describe('allowedPaths', () => {
it('should embed allowed paths and normalize them', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
if (p === '/test/symlink') return '/test/real_path';
return p;
});
it('should embed allowed paths', () => {
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: ['/custom/path1', '/test/symlink'],
forbiddenPaths: [],
resolvedPaths: {
...defaultResolvedPaths,
workspace: { resolved: '/test', original: '/test' },
policyAllowed: ['/custom/path1', '/test/real_path'],
},
});
expect(profile).toContain(`(subpath "/custom/path1")`);
@@ -138,12 +108,12 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
describe('forbiddenPaths', () => {
it('should explicitly deny forbidden paths', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: [],
forbiddenPaths: ['/secret/path'],
resolvedPaths: {
...defaultResolvedPaths,
workspace: { resolved: '/test', original: '/test' },
forbidden: ['/secret/path'],
},
});
expect(profile).toContain(
@@ -151,46 +121,14 @@ describe.skipIf(os.platform() === 'win32')('seatbeltArgsBuilder', () => {
);
});
it('resolves forbidden symlink paths to their real paths', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
if (p === '/test/symlink' || p === '/test/missing-dir') {
return '/test/real_path';
}
return p;
});
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: [],
forbiddenPaths: ['/test/symlink'],
});
expect(profile).toContain(
`(deny file-read* file-write* (subpath "/test/real_path"))`,
);
});
it('explicitly denies non-existent forbidden paths to prevent creation', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: [],
forbiddenPaths: ['/test/missing-dir/missing-file.txt'],
});
expect(profile).toContain(
`(deny file-read* file-write* (subpath "/test/missing-dir/missing-file.txt"))`,
);
});
it('should override allowed paths if a path is also in forbidden paths', () => {
vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
const profile = buildSeatbeltProfile({
workspace: '/test',
allowedPaths: ['/custom/path1'],
forbiddenPaths: ['/custom/path1'],
resolvedPaths: {
...defaultResolvedPaths,
workspace: { resolved: '/test', original: '/test' },
policyAllowed: ['/custom/path1'],
forbidden: ['/custom/path1'],
},
});
const allowString = `(allow file-read* file-write* (subpath "/custom/path1"))`;

213
packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts
View File

@@ -12,9 +12,9 @@ import {
NETWORK_SEATBELT_PROFILE,
} from './baseProfile.js';
import {
type SandboxPermissions,
GOVERNANCE_FILES,
SECRET_FILES,
type ResolvedSandboxPaths,
} from '../../services/sandboxManager.js';
import { tryRealpath, resolveGitWorktreePaths } from '../utils/fsUtils.js';
@@ -22,16 +22,10 @@ import { tryRealpath, resolveGitWorktreePaths } from '../utils/fsUtils.js';
* Options for building macOS Seatbelt profile.
*/
export interface SeatbeltArgsOptions {
/** The primary workspace path to allow access to. */
workspace: string;
/** Additional paths to allow access to. */
allowedPaths: string[];
/** Absolute paths to explicitly deny read/write access to (overrides allowlists). */
forbiddenPaths: string[];
/** Fully resolved paths for the sandbox execution. */
resolvedPaths: ResolvedSandboxPaths;
/** Whether to allow network access. */
networkAccess?: boolean;
/** Granular additional permissions. */
additionalPermissions?: SandboxPermissions;
/** Whether to allow write access to the workspace. */
workspaceWrite?: boolean;
}
@@ -49,72 +43,22 @@ export function escapeSchemeString(str: string): string {
*/
export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string {
let profile = BASE_SEATBELT_PROFILE + '\n';
const { resolvedPaths, networkAccess, workspaceWrite } = options;
const workspacePath = tryRealpath(options.workspace);
profile += `(allow file-read* (subpath "${escapeSchemeString(options.workspace)}"))\n`;
profile += `(allow file-read* (subpath "${escapeSchemeString(workspacePath)}"))\n`;
if (options.workspaceWrite) {
profile += `(allow file-write* (subpath "${escapeSchemeString(options.workspace)}"))\n`;
profile += `(allow file-write* (subpath "${escapeSchemeString(workspacePath)}"))\n`;
profile += `(allow file-read* (subpath "${escapeSchemeString(resolvedPaths.workspace.original)}"))\n`;
profile += `(allow file-read* (subpath "${escapeSchemeString(resolvedPaths.workspace.resolved)}"))\n`;
if (workspaceWrite) {
profile += `(allow file-write* (subpath "${escapeSchemeString(resolvedPaths.workspace.original)}"))\n`;
profile += `(allow file-write* (subpath "${escapeSchemeString(resolvedPaths.workspace.resolved)}"))\n`;
}
const tmpPath = tryRealpath(os.tmpdir());
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(tmpPath)}"))\n`;
// Add explicit deny rules for governance files in the workspace.
// These are added after the workspace allow rule 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);
const realGovernanceFile = 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 isDirectory = GOVERNANCE_FILES[i].isDirectory;
try {
if (fs.existsSync(realGovernanceFile)) {
isDirectory = fs.lstatSync(realGovernanceFile).isDirectory();
}
} catch {
// Ignore errors, use default guess
}
const ruleType = isDirectory ? 'subpath' : 'literal';
profile += `(deny file-write* (${ruleType} "${escapeSchemeString(governanceFile)}"))\n`;
if (realGovernanceFile !== governanceFile) {
profile += `(deny file-write* (${ruleType} "${escapeSchemeString(realGovernanceFile)}"))\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.
const searchPaths = [options.workspace, ...options.allowedPaths];
for (const basePath of searchPaths) {
const resolvedBase = tryRealpath(basePath);
for (const secret of SECRET_FILES) {
// Map pattern to Seatbelt regex
let regexPattern: string;
const escapedBase = escapeRegex(resolvedBase);
if (secret.pattern.endsWith('*')) {
// .env.* -> .env\..+ (match .env followed by dot and something)
// We anchor the secret file name to either a directory separator or the start of the relative path.
const basePattern = secret.pattern.slice(0, -1).replace(/\./g, '\\\\.');
regexPattern = `^${escapedBase}/(.*/)?${basePattern}[^/]+$`;
} else {
// .env -> \.env$
const basePattern = secret.pattern.replace(/\./g, '\\\\.');
regexPattern = `^${escapedBase}/(.*/)?${basePattern}$`;
}
profile += `(deny file-read* file-write* (regex #"${regexPattern}"))\n`;
}
}
// Auto-detect and support git worktrees by granting read and write access to the underlying git directory
const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths(workspacePath);
const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths(
resolvedPaths.workspace.resolved,
);
if (worktreeGitDir) {
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(worktreeGitDir)}"))\n`;
}
@@ -154,58 +98,115 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string {
}
}
// Handle allowedPaths
const allowedPaths = options.allowedPaths;
// Handle allowedPaths and globalIncludes
const allowedPaths = [
...resolvedPaths.policyAllowed,
...resolvedPaths.globalIncludes,
];
for (let i = 0; i < allowedPaths.length; i++) {
const allowedPath = tryRealpath(allowedPaths[i]);
const allowedPath = allowedPaths[i];
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(allowedPath)}"))\n`;
}
// Handle granular additional permissions
if (options.additionalPermissions?.fileSystem) {
const { read, write } = options.additionalPermissions.fileSystem;
if (read) {
for (let i = 0; i < read.length; i++) {
const resolved = tryRealpath(read[i]);
let isFile = false;
try {
isFile = fs.statSync(resolved).isFile();
} catch {
// Ignore error
}
if (isFile) {
profile += `(allow file-read* (literal "${escapeSchemeString(resolved)}"))\n`;
} else {
profile += `(allow file-read* (subpath "${escapeSchemeString(resolved)}"))\n`;
}
}
// Handle granular additional read permissions
for (let i = 0; i < resolvedPaths.policyRead.length; i++) {
const resolved = resolvedPaths.policyRead[i];
let isFile = false;
try {
isFile = fs.statSync(resolved).isFile();
} catch {
// Ignore error
}
if (write) {
for (let i = 0; i < write.length; i++) {
const resolved = tryRealpath(write[i]);
let isFile = false;
try {
isFile = fs.statSync(resolved).isFile();
} catch {
// Ignore error
}
if (isFile) {
profile += `(allow file-read* file-write* (literal "${escapeSchemeString(resolved)}"))\n`;
} else {
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(resolved)}"))\n`;
}
if (isFile) {
profile += `(allow file-read* (literal "${escapeSchemeString(resolved)}"))\n`;
} else {
profile += `(allow file-read* (subpath "${escapeSchemeString(resolved)}"))\n`;
}
}
// Handle granular additional write permissions
for (let i = 0; i < resolvedPaths.policyWrite.length; i++) {
const resolved = resolvedPaths.policyWrite[i];
let isFile = false;
try {
isFile = fs.statSync(resolved).isFile();
} catch {
// Ignore error
}
if (isFile) {
profile += `(allow file-read* file-write* (literal "${escapeSchemeString(resolved)}"))\n`;
} else {
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(resolved)}"))\n`;
}
}
// Add explicit deny rules for governance files in the workspace.
// These are added after the workspace allow rule 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(
resolvedPaths.workspace.resolved,
GOVERNANCE_FILES[i].path,
);
const realGovernanceFile = 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 isDirectory = GOVERNANCE_FILES[i].isDirectory;
try {
if (fs.existsSync(realGovernanceFile)) {
isDirectory = fs.lstatSync(realGovernanceFile).isDirectory();
}
} catch {
// Ignore errors, use default guess
}
const ruleType = isDirectory ? 'subpath' : 'literal';
profile += `(deny file-write* (${ruleType} "${escapeSchemeString(governanceFile)}"))\n`;
if (realGovernanceFile !== governanceFile) {
profile += `(deny file-write* (${ruleType} "${escapeSchemeString(realGovernanceFile)}"))\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.
const searchPaths = [
resolvedPaths.workspace.resolved,
resolvedPaths.workspace.original,
...resolvedPaths.policyAllowed,
...resolvedPaths.globalIncludes,
];
for (const basePath of searchPaths) {
for (const secret of SECRET_FILES) {
// Map pattern to Seatbelt regex
let regexPattern: string;
const escapedBase = escapeRegex(basePath);
if (secret.pattern.endsWith('*')) {
// .env.* -> .env\..+ (match .env followed by dot and something)
// We anchor the secret file name to either a directory separator or the start of the relative path.
const basePattern = secret.pattern.slice(0, -1).replace(/\./g, '\\\\.');
regexPattern = `^${escapedBase}/(.*/)?${basePattern}[^/]+$`;
} else {
// .env -> \.env$
const basePattern = secret.pattern.replace(/\./g, '\\\\.');
regexPattern = `^${escapedBase}/(.*/)?${basePattern}$`;
}
profile += `(deny file-read* file-write* (regex #"${regexPattern}"))\n`;
}
}
// Handle forbiddenPaths
const forbiddenPaths = options.forbiddenPaths;
const forbiddenPaths = resolvedPaths.forbidden;
for (let i = 0; i < forbiddenPaths.length; i++) {
const forbiddenPath = tryRealpath(forbiddenPaths[i]);
const forbiddenPath = forbiddenPaths[i];
profile += `(deny file-read* file-write* (subpath "${escapeSchemeString(forbiddenPath)}"))\n`;
}
if (options.networkAccess || options.additionalPermissions?.network) {
if (networkAccess) {
profile += NETWORK_SEATBELT_PROFILE;
}