mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-07-22 07:41:23 -07:00
refactor(core): standardize OS-specific sandbox tests and extract linux helper methods (#23715)
This commit is contained in:
Emily Hedlund
@@ -55,7 +55,7 @@ describe('MacOsSandboxManager', () => {
|
||||
});
|
||||
|
||||
describe('prepareCommand', () => {
|
||||
it('should correctly orchestrate Seatbelt args and format the final command', async () => {
|
||||
it('should correctly format the base command and args', async () => {
|
||||
const result = await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: ['hello'],
|
||||
@@ -118,5 +118,119 @@ describe('MacOsSandboxManager', () => {
|
||||
expect(result.env['SAFE_VAR']).toBe('1');
|
||||
expect(result.env['GITHUB_TOKEN']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should allow network when networkAccess is true', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: ['hello'],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: { ...mockPolicy, networkAccess: true },
|
||||
});
|
||||
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ networkAccess: true }),
|
||||
);
|
||||
});
|
||||
|
||||
describe('governance files', () => {
|
||||
it('should ensure governance files exist', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: mockPolicy,
|
||||
});
|
||||
|
||||
// The seatbelt builder internally handles governance files, so we simply verify
|
||||
// it is invoked correctly with the right workspace.
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ workspace: mockWorkspace }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('allowedPaths', () => {
|
||||
it('should parameterize allowed paths and normalize them', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: {
|
||||
...mockPolicy,
|
||||
allowedPaths: ['/tmp/allowed1', '/tmp/allowed2'],
|
||||
},
|
||||
});
|
||||
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
allowedPaths: ['/tmp/allowed1', '/tmp/allowed2'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('forbiddenPaths', () => {
|
||||
it('should parameterize forbidden paths and explicitly deny them', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: {
|
||||
...mockPolicy,
|
||||
forbiddenPaths: ['/tmp/forbidden1'],
|
||||
},
|
||||
});
|
||||
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
forbiddenPaths: ['/tmp/forbidden1'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('explicitly denies non-existent forbidden paths to prevent creation', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: {
|
||||
...mockPolicy,
|
||||
forbiddenPaths: ['/tmp/does-not-exist'],
|
||||
},
|
||||
});
|
||||
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
forbiddenPaths: ['/tmp/does-not-exist'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should override allowed paths if a path is also in forbidden paths', async () => {
|
||||
await manager.prepareCommand({
|
||||
command: 'echo',
|
||||
args: [],
|
||||
cwd: mockWorkspace,
|
||||
env: {},
|
||||
policy: {
|
||||
...mockPolicy,
|
||||
allowedPaths: ['/tmp/conflict'],
|
||||
forbiddenPaths: ['/tmp/conflict'],
|
||||
},
|
||||
});
|
||||
|
||||
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
allowedPaths: ['/tmp/conflict'],
|
||||
forbiddenPaths: ['/tmp/conflict'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,201 +14,224 @@ describe('seatbeltArgsBuilder', () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should build a strict allowlist profile allowing the workspace via param', async () => {
|
||||
// Mock tryRealpath to just return the path for testing
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => p);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/Users/test/workspace',
|
||||
});
|
||||
|
||||
expect(args[0]).toBe('-p');
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(version 1)');
|
||||
expect(profile).toContain('(deny default)');
|
||||
expect(profile).toContain('(allow process-exec)');
|
||||
expect(profile).toContain('(subpath (param "WORKSPACE"))');
|
||||
expect(profile).not.toContain('(allow network*)');
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('WORKSPACE=/Users/test/workspace');
|
||||
expect(args).toContain(`TMPDIR=${os.tmpdir()}`);
|
||||
});
|
||||
|
||||
it('should allow network when networkAccess is true', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => p);
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
networkAccess: true,
|
||||
});
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(allow network-outbound)');
|
||||
});
|
||||
|
||||
it('should parameterize allowed paths and normalize them', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => {
|
||||
if (p === '/test/symlink') return '/test/real_path';
|
||||
return p;
|
||||
});
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
allowedPaths: ['/custom/path1', '/test/symlink'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(subpath (param "ALLOWED_PATH_0"))');
|
||||
expect(profile).toContain('(subpath (param "ALLOWED_PATH_1"))');
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('ALLOWED_PATH_0=/custom/path1');
|
||||
expect(args).toContain('ALLOWED_PATH_1=/test/real_path');
|
||||
});
|
||||
|
||||
it('should parameterize forbidden paths and explicitly deny them', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => p);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/secret/path'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('FORBIDDEN_PATH_0=/secret/path');
|
||||
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('explicitly denies non-existent forbidden paths to prevent creation', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => p);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/test/missing-dir/missing-file.txt'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain(
|
||||
'FORBIDDEN_PATH_0=/test/missing-dir/missing-file.txt',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('resolves forbidden symlink paths to their real paths', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => {
|
||||
if (p === '/test/symlink') return '/test/real_path';
|
||||
return p;
|
||||
});
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/test/symlink'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
// The builder should resolve the symlink and explicitly deny the real target path
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('FORBIDDEN_PATH_0=/test/real_path');
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('should override allowed paths if a path is also in forbidden paths', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => p);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
allowedPaths: ['/custom/path1'],
|
||||
forbiddenPaths: ['/custom/path1'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
const allowString =
|
||||
'(allow file-read* file-write* (subpath (param "ALLOWED_PATH_0")))';
|
||||
const denyString =
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))';
|
||||
|
||||
expect(profile).toContain(allowString);
|
||||
expect(profile).toContain(denyString);
|
||||
|
||||
// Verify ordering: The explicit deny must appear AFTER the explicit allow in the profile string
|
||||
// Seatbelt rules are evaluated in order where the latest rule matching a path wins
|
||||
const allowIndex = profile.indexOf(allowString);
|
||||
const denyIndex = profile.indexOf(denyString);
|
||||
expect(denyIndex).toBeGreaterThan(allowIndex);
|
||||
});
|
||||
|
||||
describe('governance files', () => {
|
||||
it('should inject explicit deny rules for governance files', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
|
||||
p.toString(),
|
||||
);
|
||||
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
|
||||
vi.spyOn(fs, 'lstatSync').mockImplementation(
|
||||
(p) =>
|
||||
({
|
||||
isDirectory: () => p.toString().endsWith('.git'),
|
||||
isFile: () => !p.toString().endsWith('.git'),
|
||||
}) as unknown as fs.Stats,
|
||||
describe('buildSeatbeltArgs', () => {
|
||||
it('should build a strict allowlist profile allowing the workspace via param', async () => {
|
||||
// Mock tryRealpath to just return the path for testing
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => p,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/Users/test/workspace',
|
||||
});
|
||||
|
||||
expect(args[0]).toBe('-p');
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(version 1)');
|
||||
expect(profile).toContain('(deny default)');
|
||||
expect(profile).toContain('(allow process-exec)');
|
||||
expect(profile).toContain('(subpath (param "WORKSPACE"))');
|
||||
expect(profile).not.toContain('(allow network*)');
|
||||
|
||||
// .gitignore should be a literal deny
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain(
|
||||
'GOVERNANCE_FILE_0=/Users/test/workspace/.gitignore',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
|
||||
// .git should be a subpath deny
|
||||
expect(args).toContain('GOVERNANCE_FILE_2=/Users/test/workspace/.git');
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (subpath (param "GOVERNANCE_FILE_2")))',
|
||||
);
|
||||
expect(args).toContain('WORKSPACE=/Users/test/workspace');
|
||||
expect(args).toContain(`TMPDIR=${os.tmpdir()}`);
|
||||
});
|
||||
|
||||
it('should protect both the symlink and the real path if they differ', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => {
|
||||
if (p === '/test/workspace/.gitignore') return '/test/real/.gitignore';
|
||||
return p.toString();
|
||||
it('should allow network when networkAccess is true', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => p,
|
||||
);
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
networkAccess: true,
|
||||
});
|
||||
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
|
||||
vi.spyOn(fs, 'lstatSync').mockImplementation(
|
||||
() =>
|
||||
({
|
||||
isDirectory: () => false,
|
||||
isFile: () => true,
|
||||
}) as unknown as fs.Stats,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({ workspace: '/test/workspace' });
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(allow network-outbound)');
|
||||
});
|
||||
|
||||
expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore');
|
||||
expect(args).toContain('REAL_GOVERNANCE_FILE_0=/test/real/.gitignore');
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "REAL_GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
describe('governance files', () => {
|
||||
it('should inject explicit deny rules for governance files', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
|
||||
p.toString(),
|
||||
);
|
||||
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
|
||||
vi.spyOn(fs, 'lstatSync').mockImplementation(
|
||||
(p) =>
|
||||
({
|
||||
isDirectory: () => p.toString().endsWith('.git'),
|
||||
isFile: () => !p.toString().endsWith('.git'),
|
||||
}) as unknown as fs.Stats,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/Users/test/workspace',
|
||||
});
|
||||
const profile = args[1];
|
||||
|
||||
// .gitignore should be a literal deny
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain(
|
||||
'GOVERNANCE_FILE_0=/Users/test/workspace/.gitignore',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
|
||||
// .git should be a subpath deny
|
||||
expect(args).toContain('GOVERNANCE_FILE_2=/Users/test/workspace/.git');
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (subpath (param "GOVERNANCE_FILE_2")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('should protect both the symlink and the real path if they differ', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (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 args = await buildSeatbeltArgs({ workspace: '/test/workspace' });
|
||||
const profile = args[1];
|
||||
|
||||
expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore');
|
||||
expect(args).toContain('REAL_GOVERNANCE_FILE_0=/test/real/.gitignore');
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-write* (literal (param "REAL_GOVERNANCE_FILE_0")))',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('allowedPaths', () => {
|
||||
it('should parameterize allowed paths and normalize them', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => {
|
||||
if (p === '/test/symlink') return '/test/real_path';
|
||||
return p;
|
||||
},
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
allowedPaths: ['/custom/path1', '/test/symlink'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
expect(profile).toContain('(subpath (param "ALLOWED_PATH_0"))');
|
||||
expect(profile).toContain('(subpath (param "ALLOWED_PATH_1"))');
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('ALLOWED_PATH_0=/custom/path1');
|
||||
expect(args).toContain('ALLOWED_PATH_1=/test/real_path');
|
||||
});
|
||||
});
|
||||
|
||||
describe('forbiddenPaths', () => {
|
||||
it('should parameterize forbidden paths and explicitly deny them', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => p,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/secret/path'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('FORBIDDEN_PATH_0=/secret/path');
|
||||
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('resolves forbidden symlink paths to their real paths', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => {
|
||||
if (p === '/test/symlink') return '/test/real_path';
|
||||
return p;
|
||||
},
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/test/symlink'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
// The builder should resolve the symlink and explicitly deny the real target path
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain('FORBIDDEN_PATH_0=/test/real_path');
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('explicitly denies non-existent forbidden paths to prevent creation', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => p,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
forbiddenPaths: ['/test/missing-dir/missing-file.txt'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
expect(args).toContain('-D');
|
||||
expect(args).toContain(
|
||||
'FORBIDDEN_PATH_0=/test/missing-dir/missing-file.txt',
|
||||
);
|
||||
expect(profile).toContain(
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))',
|
||||
);
|
||||
});
|
||||
|
||||
it('should override allowed paths if a path is also in forbidden paths', async () => {
|
||||
vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
|
||||
async (p) => p,
|
||||
);
|
||||
|
||||
const args = await buildSeatbeltArgs({
|
||||
workspace: '/test',
|
||||
allowedPaths: ['/custom/path1'],
|
||||
forbiddenPaths: ['/custom/path1'],
|
||||
});
|
||||
|
||||
const profile = args[1];
|
||||
|
||||
const allowString =
|
||||
'(allow file-read* file-write* (subpath (param "ALLOWED_PATH_0")))';
|
||||
const denyString =
|
||||
'(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_0")))';
|
||||
|
||||
expect(profile).toContain(allowString);
|
||||
expect(profile).toContain(denyString);
|
||||
|
||||
// Verify ordering: The explicit deny must appear AFTER the explicit allow in the profile string
|
||||
// Seatbelt rules are evaluated in order where the latest rule matching a path wins
|
||||
const allowIndex = profile.indexOf(allowString);
|
||||
const denyIndex = profile.indexOf(denyString);
|
||||
expect(denyIndex).toBeGreaterThan(allowIndex);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user