feat(core): populate sandbox forbidden paths with project ignore file contents (#24038)

This commit is contained in:
Emily Hedlund
2026-04-01 12:27:55 -04:00
committed by GitHub
parent 066da2a1d1
commit 6a8a0d4faa
13 changed files with 309 additions and 79 deletions
@@ -274,7 +274,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [forbiddenDir] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [forbiddenDir],
},
);
const { command, args } = Platform.touch(testFile);
@@ -306,7 +309,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [forbiddenDir] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [forbiddenDir],
},
);
const { command, args } = Platform.cat(nestedFile);
@@ -335,7 +341,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [conflictDir] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [conflictDir],
},
);
const { command, args } = Platform.touch(testFile);
@@ -365,7 +374,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [nonExistentPath] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [nonExistentPath],
},
);
const { command, args } = Platform.echo('survived');
const sandboxed = await osManager.prepareCommand({
@@ -397,7 +409,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [nonExistentFile] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [nonExistentFile],
},
);
// We use touch to attempt creation of the file
@@ -436,7 +451,10 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
{ workspace: tempWorkspace, forbiddenPaths: [symlinkFile] },
{
workspace: tempWorkspace,
forbiddenPaths: async () => [symlinkFile],
},
);
// Attempt to read the target file directly
@@ -14,6 +14,9 @@ import {
findSecretFiles,
isSecretFile,
tryRealpath,
resolveSandboxPaths,
getPathIdentity,
type SandboxRequest,
} from './sandboxManager.js';
import { createSandboxManager } from './sandboxManagerFactory.js';
import { LinuxSandboxManager } from '../sandbox/linux/LinuxSandboxManager.js';
@@ -121,8 +124,10 @@ describe('SandboxManager', () => {
afterEach(() => vi.restoreAllMocks());
describe('sanitizePaths', () => {
it('should return undefined if no paths are provided', () => {
expect(sanitizePaths(undefined)).toBeUndefined();
it('should return an empty array if no paths are provided', () => {
expect(sanitizePaths(undefined)).toEqual([]);
expect(sanitizePaths(null)).toEqual([]);
expect(sanitizePaths([])).toEqual([]);
});
it('should deduplicate paths and return them', () => {
@@ -133,6 +138,20 @@ describe('SandboxManager', () => {
]);
});
it('should deduplicate case-insensitively on Windows and macOS', () => {
vi.spyOn(os, 'platform').mockReturnValue('win32');
const paths = ['/workspace/foo', '/WORKSPACE/FOO'];
expect(sanitizePaths(paths)).toEqual(['/workspace/foo']);
vi.spyOn(os, 'platform').mockReturnValue('darwin');
const macPaths = ['/tmp/foo', '/tmp/FOO'];
expect(sanitizePaths(macPaths)).toEqual(['/tmp/foo']);
vi.spyOn(os, 'platform').mockReturnValue('linux');
const linuxPaths = ['/tmp/foo', '/tmp/FOO'];
expect(sanitizePaths(linuxPaths)).toEqual(['/tmp/foo', '/tmp/FOO']);
});
it('should throw an error if a path is not absolute', () => {
const paths = ['/workspace/foo', 'relative/path'];
expect(() => sanitizePaths(paths)).toThrow(
@@ -141,6 +160,110 @@ describe('SandboxManager', () => {
});
});
describe('getPathIdentity', () => {
it('should normalize slashes and strip trailing slashes', () => {
expect(getPathIdentity('/foo/bar//baz/')).toBe(
path.normalize('/foo/bar/baz'),
);
});
it('should handle case sensitivity correctly per platform', () => {
vi.spyOn(os, 'platform').mockReturnValue('win32');
expect(getPathIdentity('/Workspace/Foo')).toBe('/workspace/foo');
vi.spyOn(os, 'platform').mockReturnValue('darwin');
expect(getPathIdentity('/Tmp/Foo')).toBe('/tmp/foo');
vi.spyOn(os, 'platform').mockReturnValue('linux');
expect(getPathIdentity('/Tmp/Foo')).toBe('/Tmp/Foo');
});
});
describe('resolveSandboxPaths', () => {
it('should resolve allowed and forbidden paths', async () => {
const options = {
workspace: '/workspace',
forbiddenPaths: async () => ['/workspace/forbidden'],
};
const req = {
command: 'ls',
args: [],
cwd: '/workspace',
env: {},
policy: {
allowedPaths: ['/workspace/allowed'],
},
};
const result = await resolveSandboxPaths(options, req as SandboxRequest);
expect(result.allowed).toEqual(['/workspace/allowed']);
expect(result.forbidden).toEqual(['/workspace/forbidden']);
});
it('should filter out workspace from allowed paths', async () => {
const options = {
workspace: '/workspace',
};
const req = {
command: 'ls',
args: [],
cwd: '/workspace',
env: {},
policy: {
allowedPaths: ['/workspace', '/workspace/', '/other/path'],
},
};
const result = await resolveSandboxPaths(options, req as SandboxRequest);
expect(result.allowed).toEqual(['/other/path']);
});
it('should prioritize forbidden paths over allowed paths', async () => {
const options = {
workspace: '/workspace',
forbiddenPaths: async () => ['/workspace/secret'],
};
const req = {
command: 'ls',
args: [],
cwd: '/workspace',
env: {},
policy: {
allowedPaths: ['/workspace/secret', '/workspace/normal'],
},
};
const result = await resolveSandboxPaths(options, req as SandboxRequest);
expect(result.allowed).toEqual(['/workspace/normal']);
expect(result.forbidden).toEqual(['/workspace/secret']);
});
it('should handle case-insensitive conflicts on supported platforms', async () => {
vi.spyOn(os, 'platform').mockReturnValue('darwin');
const options = {
workspace: '/workspace',
forbiddenPaths: async () => ['/workspace/SECRET'],
};
const req = {
command: 'ls',
args: [],
cwd: '/workspace',
env: {},
policy: {
allowedPaths: ['/workspace/secret'],
},
};
const result = await resolveSandboxPaths(options, req as SandboxRequest);
expect(result.allowed).toEqual([]);
expect(result.forbidden).toEqual(['/workspace/SECRET']);
});
});
describe('tryRealpath', () => {
beforeEach(() => {
vi.clearAllMocks();
+50 -21
View File
@@ -63,15 +63,12 @@ export interface SandboxModeConfig {
* Global configuration options used to initialize a SandboxManager.
*/
export interface GlobalSandboxOptions {
/**
* The primary workspace path the sandbox is anchored to.
* This directory is granted full read and write access.
*/
/** The absolute path to the primary workspace directory, granted full read/write access. */
workspace: string;
/** Absolute paths to explicitly include in the workspace context. */
includeDirectories?: string[];
/** Absolute paths to explicitly deny read/write access to (overrides allowlists). */
forbiddenPaths?: string[];
/** An optional asynchronous resolver function for paths that should be explicitly denied. */
forbiddenPaths?: () => Promise<string[]>;
/** The current sandbox mode behavior from config. */
modeConfig?: SandboxModeConfig;
/** The policy manager for persistent approvals. */
@@ -298,29 +295,47 @@ export class LocalSandboxManager implements SandboxManager {
}
/**
* Sanitizes an array of paths by deduplicating them and ensuring they are absolute.
* Resolves sanitized allowed and forbidden paths for a request.
* Filters the workspace from allowed paths and ensures forbidden paths take precedence.
*/
export function sanitizePaths(paths?: string[]): string[] | undefined {
if (!paths) return undefined;
export async function resolveSandboxPaths(
options: GlobalSandboxOptions,
req: SandboxRequest,
): Promise<{
allowed: string[];
forbidden: string[];
}> {
const forbidden = sanitizePaths(await options.forbiddenPaths?.());
const allowed = sanitizePaths(req.policy?.allowedPaths);
const workspaceIdentity = getPathIdentity(options.workspace);
const forbiddenIdentities = new Set(forbidden.map(getPathIdentity));
const filteredAllowed = allowed.filter((p) => {
const identity = getPathIdentity(p);
return identity !== workspaceIdentity && !forbiddenIdentities.has(identity);
});
return {
allowed: filteredAllowed,
forbidden,
};
}
/**
* Sanitizes an array of paths by deduplicating them and ensuring they are absolute.
* Always returns an array (empty if input is null/undefined).
*/
export function sanitizePaths(paths?: string[] | null): string[] {
if (!paths || paths.length === 0) return [];
// We use a Map to deduplicate paths based on their normalized,
// platform-specific identity e.g. handling case-insensitivity on Windows)
// while preserving the original string casing.
const uniquePathsMap = new Map<string, string>();
for (const p of paths) {
if (!path.isAbsolute(p)) {
throw new Error(`Sandbox path must be absolute: ${p}`);
}
// Normalize the path (resolves slashes and redundant components)
let key = path.normalize(p);
// Windows file systems are case-insensitive, so we lowercase the key for
// deduplication
if (os.platform() === 'win32') {
key = key.toLowerCase();
}
const key = getPathIdentity(p);
if (!uniquePathsMap.has(key)) {
uniquePathsMap.set(key, p);
}
@@ -329,6 +344,20 @@ export function sanitizePaths(paths?: string[]): string[] | undefined {
return Array.from(uniquePathsMap.values());
}
/** Returns a normalized identity for a path, stripping trailing slashes and handling case sensitivity. */
export function getPathIdentity(p: string): string {
let norm = path.normalize(p);
// Strip trailing slashes (except for root paths)
if (norm.length > 1 && (norm.endsWith('/') || norm.endsWith('\\'))) {
norm = norm.slice(0, -1);
}
const platform = os.platform();
const isCaseInsensitive = platform === 'win32' || platform === 'darwin';
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.