refactor(core): delegate sandbox denial parsing to SandboxManager (#23928)

This commit is contained in:
Tommaso Sciortino
2026-03-26 15:10:15 -07:00
committed by GitHub
parent 73dd7328df
commit 8868b34c75
10 changed files with 272 additions and 148 deletions

View File

@@ -375,6 +375,7 @@ describe('PolicyEngine', () => {
isKnownSafeCommand: vi
.fn()
.mockImplementation((args) => args[0] === 'npm'),
parseDenials: vi.fn().mockReturnValue(undefined),
} as unknown as SandboxManager;
engine = new PolicyEngine({

View File

@@ -16,7 +16,9 @@ import {
GOVERNANCE_FILES,
getSecretFileFindArgs,
sanitizePaths,
type ParsedSandboxDenial,
} from '../../services/sandboxManager.js';
import type { ShellExecutionResult } from '../../services/shellExecutionService.js';
import {
sanitizeEnvironment,
getSecureSanitizationConfig,
@@ -38,6 +40,7 @@ import {
isKnownSafeCommand,
isDangerousCommand,
} from '../utils/commandSafety.js';
import { parsePosixSandboxDenials } from '../utils/sandboxDenialUtils.js';
let cachedBpfPath: string | undefined;
@@ -154,6 +157,10 @@ export class LinuxSandboxManager implements SandboxManager {
return isDangerousCommand(args);
}
parseDenials(result: ShellExecutionResult): ParsedSandboxDenial | undefined {
return parsePosixSandboxDenials(result);
}
private getMaskFilePath(): string {
if (
LinuxSandboxManager.maskFilePath &&

View File

@@ -10,7 +10,9 @@ import {
type SandboxedCommand,
type SandboxPermissions,
type GlobalSandboxOptions,
type ParsedSandboxDenial,
} from '../../services/sandboxManager.js';
import type { ShellExecutionResult } from '../../services/shellExecutionService.js';
import {
sanitizeEnvironment,
getSecureSanitizationConfig,
@@ -27,6 +29,7 @@ import {
} from '../utils/commandSafety.js';
import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js';
import { verifySandboxOverrides } from '../utils/commandUtils.js';
import { parsePosixSandboxDenials } from '../utils/sandboxDenialUtils.js';
export interface MacOsSandboxOptions extends GlobalSandboxOptions {
/** The current sandbox mode behavior from config. */
@@ -59,6 +62,10 @@ export class MacOsSandboxManager implements SandboxManager {
return isDangerousCommand(args);
}
parseDenials(result: ShellExecutionResult): ParsedSandboxDenial | undefined {
return parsePosixSandboxDenials(result);
}
async prepareCommand(req: SandboxRequest): Promise<SandboxedCommand> {
await initializeShellParsers();
const sanitizationConfig = getSecureSanitizationConfig(

View File

@@ -0,0 +1,43 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { parsePosixSandboxDenials } from './sandboxDenialUtils.js';
import type { ShellExecutionResult } from '../../services/shellExecutionService.js';
describe('parsePosixSandboxDenials', () => {
it('should detect file system denial and extract paths', () => {
const parsed = parsePosixSandboxDenials({
output: 'ls: /root: Operation not permitted',
} as unknown as ShellExecutionResult);
expect(parsed).toBeDefined();
expect(parsed?.filePaths).toContain('/root');
});
it('should detect network denial', () => {
const parsed = parsePosixSandboxDenials({
output: 'curl: (6) Could not resolve host: google.com',
} as unknown as ShellExecutionResult);
expect(parsed).toBeDefined();
expect(parsed?.network).toBe(true);
});
it('should use fallback heuristic for absolute paths', () => {
const parsed = parsePosixSandboxDenials({
output:
'operation not permitted\nsome error happened with /some/path/to/file',
} as unknown as ShellExecutionResult);
expect(parsed).toBeDefined();
expect(parsed?.filePaths).toContain('/some/path/to/file');
});
it('should return undefined if no denial detected', () => {
const parsed = parsePosixSandboxDenials({
output: 'hello world',
} as unknown as ShellExecutionResult);
expect(parsed).toBeUndefined();
});
});

View File

@@ -0,0 +1,81 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { type ParsedSandboxDenial } from '../../services/sandboxManager.js';
import type { ShellExecutionResult } from '../../services/shellExecutionService.js';
/**
* Common POSIX-style sandbox denial detection.
* Used by macOS and Linux sandbox managers.
*/
export function parsePosixSandboxDenials(
result: ShellExecutionResult,
): ParsedSandboxDenial | undefined {
const output = result.output || '';
const errorOutput = result.error?.message;
const combined = (output + ' ' + (errorOutput || '')).toLowerCase();
const isFileDenial = [
'operation not permitted',
'vim:e303',
'should be read/write',
'sandbox_apply',
'sandbox: ',
].some((keyword) => combined.includes(keyword));
const isNetworkDenial = [
'error connecting to',
'network is unreachable',
'could not resolve host',
'connection refused',
'no address associated with hostname',
].some((keyword) => combined.includes(keyword));
if (!isFileDenial && !isNetworkDenial) {
return undefined;
}
const filePaths = new Set<string>();
// Extract denied paths (POSIX absolute paths)
const regex =
/(?:^|\s)['"]?(\/[\w.-/]+)['"]?:\s*[Oo]peration not permitted/gi;
let match;
while ((match = regex.exec(output)) !== null) {
filePaths.add(match[1]);
}
if (errorOutput) {
while ((match = regex.exec(errorOutput)) !== null) {
filePaths.add(match[1]);
}
}
// Fallback heuristic: look for any absolute path in the output if it was a file denial
if (isFileDenial && filePaths.size === 0) {
const fallbackRegex =
/(?:^|[\s"'[\]])(\/[a-zA-Z0-9_.-]+(?:\/[a-zA-Z0-9_.-]+)+)(?:$|[\s"'[\]:])/gi;
let m;
while ((m = fallbackRegex.exec(output)) !== null) {
const p = m[1];
if (p && !p.startsWith('/bin/') && !p.startsWith('/usr/bin/')) {
filePaths.add(p);
}
}
if (errorOutput) {
while ((m = fallbackRegex.exec(errorOutput)) !== null) {
const p = m[1];
if (p && !p.startsWith('/bin/') && !p.startsWith('/usr/bin/')) {
filePaths.add(p);
}
}
}
}
return {
network: isNetworkDenial || undefined,
filePaths: filePaths.size > 0 ? Array.from(filePaths) : undefined,
};
}

View File

@@ -18,7 +18,9 @@ import {
sanitizePaths,
tryRealpath,
type SandboxPermissions,
type ParsedSandboxDenial,
} from '../../services/sandboxManager.js';
import type { ShellExecutionResult } from '../../services/shellExecutionService.js';
import {
sanitizeEnvironment,
getSecureSanitizationConfig,
@@ -77,6 +79,10 @@ export class WindowsSandboxManager implements SandboxManager {
return isDangerousCommand(args);
}
parseDenials(_result: ShellExecutionResult): ParsedSandboxDenial | undefined {
return undefined; // TODO: Implement Windows-specific denial parsing
}
/**
* Ensures a file or directory exists.
*/

View File

@@ -21,7 +21,7 @@ import {
getSecureSanitizationConfig,
type EnvironmentSanitizationConfig,
} from './environmentSanitization.js';
import type { ShellExecutionResult } from './shellExecutionService.js';
export interface SandboxPermissions {
/** Filesystem permissions. */
fileSystem?: {
@@ -91,6 +91,16 @@ export interface SandboxedCommand {
cwd?: string;
}
/**
* A structured result from parsing sandbox denials.
*/
export interface ParsedSandboxDenial {
/** If the denial is related to file system access, these are the paths that were blocked. */
filePaths?: string[];
/** If the denial is related to network access. */
network?: boolean;
}
/**
* Interface for a service that prepares commands for sandboxed execution.
*/
@@ -109,6 +119,11 @@ export interface SandboxManager {
* Checks if a command with its arguments is explicitly known to be dangerous for this sandbox.
*/
isDangerousCommand(args: string[]): boolean;
/**
* Parses the output of a command to detect sandbox denials.
*/
parseDenials(result: ShellExecutionResult): ParsedSandboxDenial | undefined;
}
/**
@@ -236,10 +251,14 @@ export class NoopSandboxManager implements SandboxManager {
? isWindowsDangerousCommand(args)
: isMacDangerousCommand(args);
}
parseDenials(): undefined {
return undefined;
}
}
/**
* SandboxManager that implements actual sandboxing.
* A SandboxManager implementation that just runs locally (no sandboxing yet).
*/
export class LocalSandboxManager implements SandboxManager {
async prepareCommand(_req: SandboxRequest): Promise<SandboxedCommand> {
@@ -253,6 +272,10 @@ export class LocalSandboxManager implements SandboxManager {
isDangerousCommand(_args: string[]): boolean {
return false;
}
parseDenials(): undefined {
return undefined;
}
}
/**

View File

@@ -43,6 +43,10 @@ class MockSandboxManager implements SandboxManager {
isDangerousCommand(): boolean {
return false;
}
parseDenials(): undefined {
return undefined;
}
}
describe('SandboxedFileSystemService', () => {

View File

@@ -1914,6 +1914,7 @@ describe('ShellExecutionService environment variables', () => {
}),
isKnownSafeCommand: vi.fn().mockReturnValue(false),
isDangerousCommand: vi.fn().mockReturnValue(false),
parseDenials: vi.fn().mockReturnValue(undefined),
};
const configWithSandbox: ShellExecutionConfig = {

View File

@@ -478,162 +478,113 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
// Heuristic Sandbox Denial Detection
const lowerOutput = (
(result.output || '') +
' ' +
(result.error?.message || '')
).toLowerCase();
const isFileDenial = [
'operation not permitted',
'vim:e303',
'should be read/write',
'sandbox_apply',
'sandbox: ',
].some((keyword) => lowerOutput.includes(keyword));
const isNetworkDenial = [
'error connecting to',
'network is unreachable',
'could not resolve host',
'connection refused',
'no address associated with hostname',
].some((keyword) => lowerOutput.includes(keyword));
// Only trigger heuristic if the command actually failed (exit code != 0 or aborted)
const failed =
if (
!!result.error ||
!!result.signal ||
(result.exitCode !== undefined && result.exitCode !== 0) ||
result.aborted;
result.aborted
) {
const sandboxDenial =
this.context.config.sandboxManager.parseDenials(result);
if (sandboxDenial) {
const strippedCommand = stripShellWrapper(this.params.command);
const rootCommands = getCommandRoots(strippedCommand).filter(
(r) => r !== 'shopt',
);
const rootCommandDisplay =
rootCommands.length > 0 ? rootCommands[0] : 'shell';
if (failed && (isFileDenial || isNetworkDenial)) {
const strippedCommand = stripShellWrapper(this.params.command);
const rootCommands = getCommandRoots(strippedCommand).filter(
(r) => r !== 'shopt',
);
const rootCommandDisplay =
rootCommands.length > 0 ? rootCommands[0] : 'shell';
// Extract denied paths
const deniedPaths = new Set<string>();
const regex =
/(?:^|\s)['"]?(\/[\w.-/]+)['"]?:\s*[Oo]peration not permitted/gi;
let match;
while ((match = regex.exec(result.output || '')) !== null) {
deniedPaths.add(match[1]);
}
while ((match = regex.exec(result.error?.message || '')) !== null) {
deniedPaths.add(match[1]);
}
const readPaths = new Set(
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.read || [],
);
const writePaths = new Set(
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.write || [],
);
if (isFileDenial && deniedPaths.size === 0) {
// Fallback heuristic: look for any absolute path in the output
// Avoid matching simple commands like /bin/sh
const fallbackRegex =
/(?:^|[\s"'[\]])(\/[a-zA-Z0-9_.-]+(?:\/[a-zA-Z0-9_.-]+)+)(?:$|[\s"'[\]:])/gi;
let m;
while ((m = fallbackRegex.exec(result.output || '')) !== null) {
const p = m[1];
if (p && !p.startsWith('/bin/') && !p.startsWith('/usr/bin/')) {
deniedPaths.add(p);
}
}
while (
(m = fallbackRegex.exec(result.error?.message || '')) !== null
) {
const p = m[1];
if (p && !p.startsWith('/bin/') && !p.startsWith('/usr/bin/')) {
deniedPaths.add(p);
}
}
}
const readPaths = new Set(
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.read || [],
);
const writePaths = new Set(
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.write || [],
);
for (const p of deniedPaths) {
try {
// Find an existing parent directory to add instead of a non-existent file
let currentPath = p;
try {
if (
fs.existsSync(currentPath) &&
fs.statSync(currentPath).isFile()
) {
currentPath = path.dirname(currentPath);
}
} catch (_e) {
/* ignore */
}
while (currentPath.length > 1) {
if (fs.existsSync(currentPath)) {
writePaths.add(currentPath);
readPaths.add(currentPath);
break;
}
currentPath = path.dirname(currentPath);
}
} catch (_e) {
// ignore
}
}
const additionalPermissions = {
network:
isNetworkDenial ||
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.network ||
undefined,
fileSystem:
isFileDenial || writePaths.size > 0
? {
read: Array.from(readPaths),
write: Array.from(writePaths),
if (sandboxDenial.filePaths) {
for (const p of sandboxDenial.filePaths) {
try {
// Find an existing parent directory to add instead of a non-existent file
let currentPath = p;
try {
if (
fs.existsSync(currentPath) &&
fs.statSync(currentPath).isFile()
) {
currentPath = path.dirname(currentPath);
}
} catch (_e) {
/* ignore */
}
: undefined,
};
while (currentPath.length > 1) {
if (fs.existsSync(currentPath)) {
writePaths.add(currentPath);
readPaths.add(currentPath);
break;
}
currentPath = path.dirname(currentPath);
}
} catch (_e) {
// ignore
}
}
}
const originalReadSize =
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.read?.length ||
0;
const originalWriteSize =
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.write
?.length || 0;
const originalNetwork =
!!this.params[PARAM_ADDITIONAL_PERMISSIONS]?.network;
const newReadSize = additionalPermissions.fileSystem?.read?.length || 0;
const newWriteSize =
additionalPermissions.fileSystem?.write?.length || 0;
const newNetwork = !!additionalPermissions.network;
const hasNewPermissions =
newReadSize > originalReadSize ||
newWriteSize > originalWriteSize ||
(!originalNetwork && newNetwork);
if (hasNewPermissions) {
const confirmationDetails = {
type: 'sandbox_expansion',
title: 'Sandbox Expansion Request',
command: this.params.command,
rootCommand: rootCommandDisplay,
additionalPermissions,
const additionalPermissions = {
network:
sandboxDenial.network ||
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.network ||
undefined,
fileSystem:
sandboxDenial.filePaths?.length || writePaths.size > 0
? {
read: Array.from(readPaths),
write: Array.from(writePaths),
}
: undefined,
};
return {
llmContent: 'Sandbox expansion required',
returnDisplay: returnDisplayMessage,
error: {
type: ToolErrorType.SANDBOX_EXPANSION_REQUIRED,
message: JSON.stringify(confirmationDetails),
},
};
const originalReadSize =
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.read
?.length || 0;
const originalWriteSize =
this.params[PARAM_ADDITIONAL_PERMISSIONS]?.fileSystem?.write
?.length || 0;
const originalNetwork =
!!this.params[PARAM_ADDITIONAL_PERMISSIONS]?.network;
const newReadSize =
additionalPermissions.fileSystem?.read?.length || 0;
const newWriteSize =
additionalPermissions.fileSystem?.write?.length || 0;
const newNetwork = !!additionalPermissions.network;
const hasNewPermissions =
newReadSize > originalReadSize ||
newWriteSize > originalWriteSize ||
(!originalNetwork && newNetwork);
if (hasNewPermissions) {
const confirmationDetails = {
type: 'sandbox_expansion',
title: 'Sandbox Expansion Request',
command: this.params.command,
rootCommand: rootCommandDisplay,
additionalPermissions,
};
return {
llmContent: 'Sandbox expansion required',
returnDisplay: returnDisplayMessage,
error: {
type: ToolErrorType.SANDBOX_EXPANSION_REQUIRED,
message: JSON.stringify(confirmationDetails),
},
};
}
// If no new permissions were found by heuristic, do not intercept.
// Just return the normal execution error so the LLM can try providing explicit paths itself.
}
// If no new permissions were found by heuristic, do not intercept.
// Just return the normal execution error so the LLM can try providing explicit paths itself.
}
const summarizeConfig =