diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index c536721817..ed4a91442f 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -41,7 +41,7 @@ import { import * as modifiableToolModule from '../tools/modifiable-tool.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; -import { isShellInvocationAllowlisted } from '../utils/shell-utils.js'; +import { isShellInvocationAllowlisted } from '../utils/shell-permissions.js'; vi.mock('fs/promises', () => ({ writeFile: vi.fn(), diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 6464854e9b..20936675f9 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -38,11 +38,9 @@ import { import * as Diff from 'diff'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; -import { - isShellInvocationAllowlisted, - SHELL_TOOL_NAMES, -} from '../utils/shell-utils.js'; +import { SHELL_TOOL_NAMES } from '../utils/shell-utils.js'; import { doesToolInvocationMatch } from '../utils/tool-utils.js'; +import { isShellInvocationAllowlisted } from '../utils/shell-permissions.js'; import levenshtein from 'fast-levenshtein'; import { ShellToolInvocation } from '../tools/shell.js'; import type { ToolConfirmationRequest } from '../confirmation-bus/types.js'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3ff58417c6..876a7bf731 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -63,6 +63,7 @@ export * from './utils/googleQuotaErrors.js'; export * from './utils/fileUtils.js'; export * from './utils/retry.js'; export * from './utils/shell-utils.js'; +export * from './utils/shell-permissions.js'; export * from './utils/terminalSerializer.js'; export * from './utils/systemEncoding.js'; export * from './utils/textUtils.js'; diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 295270d156..71a4ff232b 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -19,6 +19,11 @@ import { debugLogger } from '../utils/debugLogger.js'; import type { CheckerRunner } from '../safety/checker-runner.js'; import { SafetyCheckDecision } from '../safety/protocol.js'; import type { HookExecutionRequest } from '../confirmation-bus/types.js'; +import { + SHELL_TOOL_NAMES, + initializeShellParsers, + splitCommands, +} from '../utils/shell-utils.js'; function ruleMatches( rule: PolicyRule | SafetyCheckerRule, @@ -144,8 +149,67 @@ export class PolicyEngine { debugLogger.debug( `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`, ); + + // Special handling for shell commands: check sub-commands if present + if ( + toolCall.name && + SHELL_TOOL_NAMES.includes(toolCall.name) && + rule.decision === PolicyDecision.ALLOW + ) { + const command = (toolCall.args as { command?: string })?.command; + if (command) { + await initializeShellParsers(); + const subCommands = splitCommands(command); + + // If there are multiple sub-commands, we must verify EACH of them matches an ALLOW rule. + // If any sub-command results in DENY -> the whole thing is DENY. + // If any sub-command results in ASK_USER -> the whole thing is ASK_USER (unless one is DENY). + // Only if ALL sub-commands are ALLOW do we proceed with ALLOW. + if (subCommands.length === 0) { + // This case occurs if the command is non-empty but parsing fails. + // An ALLOW rule for a prefix might have matched, but since the rest of + // the command is un-parseable, it's unsafe to proceed. + // Fall back to a safe decision. + debugLogger.debug( + `[PolicyEngine.check] Command parsing failed for: ${command}. Falling back to safe decision because implicit ALLOW is unsafe.`, + ); + decision = this.applyNonInteractiveMode(PolicyDecision.ASK_USER); + } else if (subCommands.length > 1) { + debugLogger.debug( + `[PolicyEngine.check] Compound command detected: ${subCommands.length} parts`, + ); + let aggregateDecision = PolicyDecision.ALLOW; + + for (const subCmd of subCommands) { + // Recursively check each sub-command + const subCall = { + name: toolCall.name, + args: { command: subCmd }, + }; + const subResult = await this.check(subCall, serverName); + + if (subResult.decision === PolicyDecision.DENY) { + aggregateDecision = PolicyDecision.DENY; + break; // Fail fast + } else if (subResult.decision === PolicyDecision.ASK_USER) { + aggregateDecision = PolicyDecision.ASK_USER; + // efficient: we can only strictly downgrade from ALLOW to ASK_USER, + // but we must continue looking for DENY. + } + } + + decision = aggregateDecision; + } else { + // Single command, rule match is valid + decision = this.applyNonInteractiveMode(rule.decision); + } + } else { + decision = this.applyNonInteractiveMode(rule.decision); + } + } else { + decision = this.applyNonInteractiveMode(rule.decision); + } matchedRule = rule; - decision = this.applyNonInteractiveMode(rule.decision); break; } } diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts new file mode 100644 index 0000000000..89c8362f5d --- /dev/null +++ b/packages/core/src/policy/shell-safety.test.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { PolicyEngine } from './policy-engine.js'; +import { PolicyDecision } from './types.js'; +import type { FunctionCall } from '@google/genai'; + +describe('Shell Safety Policy', () => { + let policyEngine: PolicyEngine; + + beforeEach(() => { + policyEngine = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + // Mimic the regex generated by toml-loader for commandPrefix = ["git log"] + // Regex: "command":"git log(?:[\s"]|$) + argsPattern: /"command":"git log(?:[\s"]|$)/, + decision: PolicyDecision.ALLOW, + priority: 1.01, // Higher priority than default + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + }); + + it('SHOULD match "git log" exactly', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('SHOULD match "git log" with arguments', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log --oneline' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('SHOULD NOT match "git logout" when prefix is "git log" (strict word boundary)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git logout' }, + }; + + // Desired behavior: Should NOT match "git log" prefix. + // If it doesn't match, it should fall back to default decision (ASK_USER). + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow "git log && rm -rf /" completely when prefix is "git log" (compound command safety)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log && rm -rf /' }, + }; + + // Desired behavior: Should inspect all parts. "rm -rf /" is not allowed. + // The "git log" part is ALLOW, but "rm -rf /" is ASK_USER (default). + // Aggregate should be ASK_USER. + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + it('SHOULD NOT allow "git log &&& rm -rf /" when prefix is "git log" (parse failure)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log &&& rm -rf /' }, + }; + + // Desired behavior: Should fail safe (ASK_USER or DENY) because parsing failed. + // If we let it pass as "single command" that matches prefix, it's dangerous. + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); +}); diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 4f5ca8b976..79454c2e98 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -349,7 +349,7 @@ export async function loadPoliciesFromToml( const argsPatterns: Array = commandPrefixes.length > 0 ? commandPrefixes.map( - (prefix) => `"command":"${escapeRegex(prefix)}`, + (prefix) => `"command":"${escapeRegex(prefix)}(?:[\\s"]|$)`, ) : [effectiveArgsPattern]; @@ -434,7 +434,7 @@ export async function loadPoliciesFromToml( const argsPatterns: Array = commandPrefixes.length > 0 ? commandPrefixes.map( - (prefix) => `"command":"${escapeRegex(prefix)}`, + (prefix) => `"command":"${escapeRegex(prefix)}(?:[\\s"]|$)`, ) : [effectiveArgsPattern]; diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 84f41b43fe..09a2ce0750 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -70,6 +70,9 @@ vi.mock('../utils/getPty.js', () => ({ vi.mock('../utils/terminalSerializer.js', () => ({ serializeTerminalToObject: mockSerializeTerminalToObject, })); +vi.mock('../utils/systemEncoding.js', () => ({ + getCachedEncodingForBuffer: vi.fn().mockReturnValue('utf-8'), +})); const mockProcessKill = vi .spyOn(process, 'kill') diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 0a948a4ad8..c5ec9ce289 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -36,10 +36,8 @@ vi.mock('node:os', async (importOriginal) => { vi.mock('crypto'); vi.mock('../utils/summarizer.js'); -import { - initializeShellParsers, - isCommandAllowed, -} from '../utils/shell-utils.js'; +import { initializeShellParsers } from '../utils/shell-utils.js'; +import { isCommandAllowed } from '../utils/shell-permissions.js'; import { ShellTool } from './shell.js'; import { type Config } from '../config/config.js'; import { diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index feca545ef9..e6387dbee5 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -38,10 +38,12 @@ import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { getCommandRoots, initializeShellParsers, - isCommandAllowed, - isShellInvocationAllowlisted, stripShellWrapper, } from '../utils/shell-utils.js'; +import { + isCommandAllowed, + isShellInvocationAllowlisted, +} from '../utils/shell-permissions.js'; import { SHELL_TOOL_NAME } from './tool-names.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; diff --git a/packages/core/src/utils/shell-permissions.test.ts b/packages/core/src/utils/shell-permissions.test.ts new file mode 100644 index 0000000000..7f7cf1f46e --- /dev/null +++ b/packages/core/src/utils/shell-permissions.test.ts @@ -0,0 +1,520 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + expect, + describe, + it, + beforeEach, + beforeAll, + vi, + afterEach, +} from 'vitest'; +import { initializeShellParsers } from './shell-utils.js'; +import { + checkCommandPermissions, + isCommandAllowed, + isShellInvocationAllowlisted, +} from './shell-permissions.js'; +import type { Config } from '../config/config.js'; +import type { AnyToolInvocation } from '../index.js'; + +const mockPlatform = vi.hoisted(() => vi.fn()); +const mockHomedir = vi.hoisted(() => vi.fn()); +vi.mock('os', () => ({ + default: { + platform: mockPlatform, + homedir: mockHomedir, + }, + platform: mockPlatform, + homedir: mockHomedir, +})); + +const mockQuote = vi.hoisted(() => vi.fn()); +vi.mock('shell-quote', () => ({ + quote: mockQuote, +})); + +let config: Config; +const isWindowsRuntime = process.platform === 'win32'; +const describeWindowsOnly = isWindowsRuntime ? describe : describe.skip; + +beforeAll(async () => { + mockPlatform.mockReturnValue('linux'); + await initializeShellParsers(); +}); + +beforeEach(() => { + mockPlatform.mockReturnValue('linux'); + mockQuote.mockImplementation((args: string[]) => + args.map((arg) => `'${arg}'`).join(' '), + ); + config = { + getCoreTools: () => [], + getExcludeTools: () => new Set([]), + getAllowedTools: () => [], + getApprovalMode: () => 'strict', + isInteractive: () => false, + } as unknown as Config; +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe('isCommandAllowed', () => { + it('should allow a command if no restrictions are provided', () => { + const result = isCommandAllowed('goodCommand --safe', config); + expect(result.allowed).toBe(true); + }); + + it('should allow a command if it is in the global allowlist', () => { + config.getCoreTools = () => ['ShellTool(goodCommand)']; + const result = isCommandAllowed('goodCommand --safe', config); + expect(result.allowed).toBe(true); + }); + + it('should block a command if it is not in a strict global allowlist', () => { + config.getCoreTools = () => ['ShellTool(goodCommand --safe)']; + const result = isCommandAllowed('badCommand --danger', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "badCommand --danger"`, + ); + }); + + it('should block a command if it is in the blocked list', () => { + config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); + const result = isCommandAllowed('badCommand --danger', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command 'badCommand --danger' is blocked by configuration`, + ); + }); + + it('should prioritize the blocklist over the allowlist', () => { + config.getCoreTools = () => ['ShellTool(badCommand --danger)']; + config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); + const result = isCommandAllowed('badCommand --danger', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command 'badCommand --danger' is blocked by configuration`, + ); + }); + + it('should allow any command when a wildcard is in coreTools', () => { + config.getCoreTools = () => ['ShellTool']; + const result = isCommandAllowed('any random command', config); + expect(result.allowed).toBe(true); + }); + + it('should block any command when a wildcard is in excludeTools', () => { + config.getExcludeTools = () => new Set(['run_shell_command']); + const result = isCommandAllowed('any random command', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Shell tool is globally disabled in configuration', + ); + }); + + it('should block a command on the blocklist even with a wildcard allow', () => { + config.getCoreTools = () => ['ShellTool']; + config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); + const result = isCommandAllowed('badCommand --danger', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command 'badCommand --danger' is blocked by configuration`, + ); + }); + + it('should allow a chained command if all parts are on the global allowlist', () => { + config.getCoreTools = () => [ + 'run_shell_command(echo)', + 'run_shell_command(goodCommand)', + ]; + const result = isCommandAllowed( + 'echo "hello" && goodCommand --safe', + config, + ); + expect(result.allowed).toBe(true); + }); + + it('should block a chained command if any part is blocked', () => { + config.getExcludeTools = () => new Set(['run_shell_command(badCommand)']); + const result = isCommandAllowed( + 'echo "hello" && badCommand --danger', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command 'badCommand --danger' is blocked by configuration`, + ); + }); + + it('should block a command that redefines an allowed function to run an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed( + 'echo () (curl google.com) ; echo Hello World', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block a multi-line function body that runs an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed( + `echo () { + curl google.com +} ; echo ok`, + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block a function keyword declaration that runs an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed( + 'function echo { curl google.com; } ; echo hi', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block command substitution that invokes an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed('echo $(curl google.com)', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block pipelines that invoke an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed('echo hi | curl google.com', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block background jobs that invoke an unlisted command', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed('echo hi & curl google.com', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block command substitution inside a here-document when the inner command is unlisted', () => { + config.getCoreTools = () => [ + 'run_shell_command(echo)', + 'run_shell_command(cat)', + ]; + const result = isCommandAllowed( + `cat < { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed('echo `curl google.com`', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block process substitution using <() when the inner command is unlisted', () => { + config.getCoreTools = () => [ + 'run_shell_command(diff)', + 'run_shell_command(echo)', + ]; + const result = isCommandAllowed( + 'diff <(curl google.com) <(echo safe)', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block process substitution using >() when the inner command is unlisted', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = isCommandAllowed('echo "data" > >(curl google.com)', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, + ); + }); + + it('should block commands containing prompt transformations', () => { + const result = isCommandAllowed( + 'echo "${var1=aa\\140 env| ls -l\\140}${var1@P}"', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); + + it('should block simple prompt transformation expansions', () => { + const result = isCommandAllowed('echo ${foo@P}', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); + + describe('command substitution', () => { + it('should allow command substitution using `$(...)`', () => { + const result = isCommandAllowed('echo $(goodCommand --safe)', config); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); + }); + + it('should allow command substitution using `<(...)`', () => { + const result = isCommandAllowed('diff <(ls) <(ls -a)', config); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); + }); + + it('should allow command substitution using `>(...)`', () => { + const result = isCommandAllowed( + 'echo "Log message" > >(tee log.txt)', + config, + ); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); + }); + + it('should allow command substitution using backticks', () => { + const result = isCommandAllowed('echo `goodCommand --safe`', config); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); + }); + + it('should allow substitution-like patterns inside single quotes', () => { + config.getCoreTools = () => ['ShellTool(echo)']; + const result = isCommandAllowed("echo '$(pwd)'", config); + expect(result.allowed).toBe(true); + }); + + it('should block a command when parsing fails', () => { + const result = isCommandAllowed('ls &&', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); + }); +}); + +describe('checkCommandPermissions', () => { + describe('in "Default Allow" mode (no sessionAllowlist)', () => { + it('should return a detailed success object for an allowed command', () => { + const result = checkCommandPermissions('goodCommand --safe', config); + expect(result).toEqual({ + allAllowed: true, + disallowedCommands: [], + }); + }); + + it('should block commands that cannot be parsed safely', () => { + const result = checkCommandPermissions('ls &&', config); + expect(result).toEqual({ + allAllowed: false, + disallowedCommands: ['ls &&'], + blockReason: 'Command rejected because it could not be parsed safely', + isHardDenial: true, + }); + }); + + it('should return a detailed failure object for a blocked command', () => { + config.getExcludeTools = () => new Set(['ShellTool(badCommand)']); + const result = checkCommandPermissions('badCommand --danger', config); + expect(result).toEqual({ + allAllowed: false, + disallowedCommands: ['badCommand --danger'], + blockReason: `Command 'badCommand --danger' is blocked by configuration`, + isHardDenial: true, + }); + }); + + it('should return a detailed failure object for a command not on a strict allowlist', () => { + config.getCoreTools = () => ['ShellTool(goodCommand)']; + const result = checkCommandPermissions( + 'git status && goodCommand', + config, + ); + expect(result).toEqual({ + allAllowed: false, + disallowedCommands: ['git status'], + blockReason: `Command(s) not in the allowed commands list. Disallowed commands: "git status"`, + isHardDenial: false, + }); + }); + }); + + describe('in "Default Deny" mode (with sessionAllowlist)', () => { + it('should allow a command on the sessionAllowlist', () => { + const result = checkCommandPermissions( + 'goodCommand --safe', + config, + new Set(['goodCommand --safe']), + ); + expect(result.allAllowed).toBe(true); + }); + + it('should block a command not on the sessionAllowlist or global allowlist', () => { + const result = checkCommandPermissions( + 'badCommand --danger', + config, + new Set(['goodCommand --safe']), + ); + expect(result.allAllowed).toBe(false); + expect(result.blockReason).toContain( + 'not on the global or session allowlist', + ); + expect(result.disallowedCommands).toEqual(['badCommand --danger']); + }); + + it('should allow a command on the global allowlist even if not on the session allowlist', () => { + config.getCoreTools = () => ['ShellTool(git status)']; + const result = checkCommandPermissions( + 'git status', + config, + new Set(['goodCommand --safe']), + ); + expect(result.allAllowed).toBe(true); + }); + + it('should allow a chained command if parts are on different allowlists', () => { + config.getCoreTools = () => ['ShellTool(git status)']; + const result = checkCommandPermissions( + 'git status && git commit', + config, + new Set(['git commit']), + ); + expect(result.allAllowed).toBe(true); + }); + + it('should block a command on the sessionAllowlist if it is also globally blocked', () => { + config.getExcludeTools = () => new Set(['run_shell_command(badCommand)']); + const result = checkCommandPermissions( + 'badCommand --danger', + config, + new Set(['badCommand --danger']), + ); + expect(result.allAllowed).toBe(false); + expect(result.blockReason).toContain('is blocked by configuration'); + }); + + it('should block a chained command if one part is not on any allowlist', () => { + config.getCoreTools = () => ['run_shell_command(echo)']; + const result = checkCommandPermissions( + 'echo "hello" && badCommand --danger', + config, + new Set(['echo']), + ); + expect(result.allAllowed).toBe(false); + expect(result.disallowedCommands).toEqual(['badCommand --danger']); + }); + }); +}); + +describeWindowsOnly('PowerShell integration', () => { + const originalComSpec = process.env['ComSpec']; + + beforeEach(() => { + mockPlatform.mockReturnValue('win32'); + const systemRoot = process.env['SystemRoot'] || 'C:\\Windows'; + process.env['ComSpec'] = + `${systemRoot}\\System32\\WindowsPowerShell\\v1.0\\powershell.exe`; + }); + + afterEach(() => { + if (originalComSpec === undefined) { + delete process.env['ComSpec']; + } else { + process.env['ComSpec'] = originalComSpec; + } + }); + + it('should block commands when PowerShell parser reports errors', () => { + const { allowed, reason } = isCommandAllowed('Get-ChildItem |', config); + expect(allowed).toBe(false); + expect(reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); +}); + +describe('isShellInvocationAllowlisted', () => { + function createInvocation(command: string): AnyToolInvocation { + return { params: { command } } as unknown as AnyToolInvocation; + } + + it('should return false when any chained command segment is not allowlisted', () => { + const invocation = createInvocation( + 'git status && rm -rf /tmp/should-not-run', + ); + expect( + isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']), + ).toBe(false); + }); + + it('should return true when every segment is explicitly allowlisted', () => { + const invocation = createInvocation( + 'git status && rm -rf /tmp/should-run && git diff', + ); + expect( + isShellInvocationAllowlisted(invocation, [ + 'run_shell_command(git)', + 'run_shell_command(rm -rf)', + ]), + ).toBe(true); + }); + + it('should return true when the allowlist contains a wildcard shell entry', () => { + const invocation = createInvocation('git status && rm -rf /tmp/should-run'); + expect( + isShellInvocationAllowlisted(invocation, ['run_shell_command']), + ).toBe(true); + }); + + it('should treat piped commands as separate segments that must be allowlisted', () => { + const invocation = createInvocation('git status | tail -n 1'); + expect( + isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']), + ).toBe(false); + expect( + isShellInvocationAllowlisted(invocation, [ + 'run_shell_command(git)', + 'run_shell_command(tail)', + ]), + ).toBe(true); + }); +}); diff --git a/packages/core/src/utils/shell-permissions.ts b/packages/core/src/utils/shell-permissions.ts new file mode 100644 index 0000000000..29f28b5410 --- /dev/null +++ b/packages/core/src/utils/shell-permissions.ts @@ -0,0 +1,270 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { AnyToolInvocation } from '../index.js'; +import type { Config } from '../config/config.js'; +import { doesToolInvocationMatch } from './tool-utils.js'; +import { + parseCommandDetails, + SHELL_TOOL_NAMES, + type ParsedCommandDetail, +} from './shell-utils.js'; + +/** + * Checks a shell command against security policies and allowlists. + * + * This function operates in one of two modes depending on the presence of + * the `sessionAllowlist` parameter: + * + * 1. **"Default Deny" Mode (sessionAllowlist is provided):** This is the + * strictest mode, used for user-defined scripts like custom commands. + * A command is only permitted if it is found on the global `coreTools` + * allowlist OR the provided `sessionAllowlist`. It must not be on the + * global `excludeTools` blocklist. + * + * 2. **"Default Allow" Mode (sessionAllowlist is NOT provided):** This mode + * is used for direct tool invocations (e.g., by the model). If a strict + * global `coreTools` allowlist exists, commands must be on it. Otherwise, + * any command is permitted as long as it is not on the `excludeTools` + * blocklist. + * + * @param command The shell command string to validate. + * @param config The application configuration. + * @param sessionAllowlist A session-level list of approved commands. Its + * presence activates "Default Deny" mode. + * @returns An object detailing which commands are not allowed. + */ +export function checkCommandPermissions( + command: string, + config: Config, + sessionAllowlist?: Set, +): { + allAllowed: boolean; + disallowedCommands: string[]; + blockReason?: string; + isHardDenial?: boolean; +} { + const parseResult = parseCommandDetails(command); + if (!parseResult || parseResult.hasError) { + return { + allAllowed: false, + disallowedCommands: [command], + blockReason: 'Command rejected because it could not be parsed safely', + isHardDenial: true, + }; + } + + const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' '); + const commandsToValidate = parseResult.details + .map((detail: ParsedCommandDetail) => normalize(detail.text)) + .filter(Boolean); + const invocation: AnyToolInvocation & { params: { command: string } } = { + params: { command: '' }, + } as AnyToolInvocation & { params: { command: string } }; + + // 1. Blocklist Check (Highest Priority) + const excludeTools = config.getExcludeTools() || new Set([]); + const isWildcardBlocked = SHELL_TOOL_NAMES.some((name) => + excludeTools.has(name), + ); + + if (isWildcardBlocked) { + return { + allAllowed: false, + disallowedCommands: commandsToValidate, + blockReason: 'Shell tool is globally disabled in configuration', + isHardDenial: true, + }; + } + + for (const cmd of commandsToValidate) { + invocation.params['command'] = cmd; + if ( + doesToolInvocationMatch('run_shell_command', invocation, [ + ...excludeTools, + ]) + ) { + return { + allAllowed: false, + disallowedCommands: [cmd], + blockReason: `Command '${cmd}' is blocked by configuration`, + isHardDenial: true, + }; + } + } + + const coreTools = config.getCoreTools() || []; + const isWildcardAllowed = SHELL_TOOL_NAMES.some((name) => + coreTools.includes(name), + ); + + // If there's a global wildcard, all commands are allowed at this point + // because they have already passed the blocklist check. + if (isWildcardAllowed) { + return { allAllowed: true, disallowedCommands: [] }; + } + + const disallowedCommands: string[] = []; + + if (sessionAllowlist) { + // "DEFAULT DENY" MODE: A session allowlist is provided. + // All commands must be in either the session or global allowlist. + const normalizedSessionAllowlist = new Set( + [...sessionAllowlist].flatMap((cmd) => + SHELL_TOOL_NAMES.map((name) => `${name}(${cmd})`), + ), + ); + + for (const cmd of commandsToValidate) { + invocation.params['command'] = cmd; + const isSessionAllowed = doesToolInvocationMatch( + 'run_shell_command', + invocation, + [...normalizedSessionAllowlist], + ); + if (isSessionAllowed) continue; + + const isGloballyAllowed = doesToolInvocationMatch( + 'run_shell_command', + invocation, + coreTools, + ); + if (isGloballyAllowed) continue; + + disallowedCommands.push(cmd); + } + + if (disallowedCommands.length > 0) { + return { + allAllowed: false, + disallowedCommands, + blockReason: `Command(s) not on the global or session allowlist. Disallowed commands: ${disallowedCommands + .map((c) => JSON.stringify(c)) + .join(', ')}`, + isHardDenial: false, // This is a soft denial; confirmation is possible. + }; + } + } else { + // "DEFAULT ALLOW" MODE: No session allowlist. + const hasSpecificAllowedCommands = + coreTools.filter((tool) => + SHELL_TOOL_NAMES.some((name) => tool.startsWith(`${name}(`)), + ).length > 0; + + if (hasSpecificAllowedCommands) { + for (const cmd of commandsToValidate) { + invocation.params['command'] = cmd; + const isGloballyAllowed = doesToolInvocationMatch( + 'run_shell_command', + invocation, + coreTools, + ); + if (!isGloballyAllowed) { + disallowedCommands.push(cmd); + } + } + if (disallowedCommands.length > 0) { + return { + allAllowed: false, + disallowedCommands, + blockReason: `Command(s) not in the allowed commands list. Disallowed commands: ${disallowedCommands + .map((c) => JSON.stringify(c)) + .join(', ')}`, + isHardDenial: false, + }; + } + } + // If no specific global allowlist exists, and it passed the blocklist, + // the command is allowed by default. + } + + // If all checks for the current mode pass, the command is allowed. + return { allAllowed: true, disallowedCommands: [] }; +} + +export function isCommandAllowed( + command: string, + config: Config, +): { allowed: boolean; reason?: string } { + // By not providing a sessionAllowlist, we invoke "default allow" behavior. + const { allAllowed, blockReason } = checkCommandPermissions(command, config); + if (allAllowed) { + return { allowed: true }; + } + return { allowed: false, reason: blockReason }; +} + +/** + * Determines whether a shell invocation should be auto-approved based on an allowlist. + * + * This reuses the same parsing logic as command-permission enforcement so that + * chained commands must be individually covered by the allowlist. + * + * @param invocation The shell tool invocation being evaluated. + * @param allowedPatterns The configured allowlist patterns (e.g. `run_shell_command(git)`). + * @returns True if every parsed command segment is allowed by the patterns; false otherwise. + */ +export function isShellInvocationAllowlisted( + invocation: AnyToolInvocation, + allowedPatterns: string[], +): boolean { + if (!allowedPatterns.length) { + return false; + } + + const hasShellWildcard = allowedPatterns.some((pattern) => + SHELL_TOOL_NAMES.includes(pattern), + ); + const hasShellSpecificPattern = allowedPatterns.some((pattern) => + SHELL_TOOL_NAMES.some((name) => pattern.startsWith(`${name}(`)), + ); + + if (!hasShellWildcard && !hasShellSpecificPattern) { + return false; + } + + if (hasShellWildcard) { + return true; + } + + if ( + !('params' in invocation) || + typeof invocation.params !== 'object' || + invocation.params === null || + !('command' in invocation.params) + ) { + return false; + } + + const commandValue = (invocation.params as { command?: unknown }).command; + if (typeof commandValue !== 'string' || !commandValue.trim()) { + return false; + } + + const command = commandValue.trim(); + + const parseResult = parseCommandDetails(command); + if (!parseResult || parseResult.hasError) { + return false; + } + + const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' '); + const commandsToValidate = parseResult.details + .map((detail: ParsedCommandDetail) => normalize(detail.text)) + .filter(Boolean); + + if (commandsToValidate.length === 0) { + return false; + } + + return commandsToValidate.every((commandSegment: string) => + doesToolInvocationMatch( + SHELL_TOOL_NAMES[0], + { params: { command: commandSegment } } as AnyToolInvocation, + allowedPatterns, + ), + ); +} diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 6414664e8d..a7abbfb43d 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -14,17 +14,12 @@ import { afterEach, } from 'vitest'; import { - checkCommandPermissions, escapeShellArg, getCommandRoots, getShellConfiguration, - isCommandAllowed, initializeShellParsers, stripShellWrapper, - isShellInvocationAllowlisted, } from './shell-utils.js'; -import type { Config } from '../config/config.js'; -import type { AnyToolInvocation } from '../index.js'; const mockPlatform = vi.hoisted(() => vi.fn()); const mockHomedir = vi.hoisted(() => vi.fn()); @@ -42,7 +37,6 @@ vi.mock('shell-quote', () => ({ quote: mockQuote, })); -let config: Config; const isWindowsRuntime = process.platform === 'win32'; const describeWindowsOnly = isWindowsRuntime ? describe : describe.skip; @@ -56,397 +50,12 @@ beforeEach(() => { mockQuote.mockImplementation((args: string[]) => args.map((arg) => `'${arg}'`).join(' '), ); - config = { - getCoreTools: () => [], - getExcludeTools: () => new Set([]), - getAllowedTools: () => [], - } as unknown as Config; }); afterEach(() => { vi.clearAllMocks(); }); -describe('isCommandAllowed', () => { - it('should allow a command if no restrictions are provided', () => { - const result = isCommandAllowed('goodCommand --safe', config); - expect(result.allowed).toBe(true); - }); - - it('should allow a command if it is in the global allowlist', () => { - config.getCoreTools = () => ['ShellTool(goodCommand)']; - const result = isCommandAllowed('goodCommand --safe', config); - expect(result.allowed).toBe(true); - }); - - it('should block a command if it is not in a strict global allowlist', () => { - config.getCoreTools = () => ['ShellTool(goodCommand --safe)']; - const result = isCommandAllowed('badCommand --danger', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "badCommand --danger"`, - ); - }); - - it('should block a command if it is in the blocked list', () => { - config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); - const result = isCommandAllowed('badCommand --danger', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command 'badCommand --danger' is blocked by configuration`, - ); - }); - - it('should prioritize the blocklist over the allowlist', () => { - config.getCoreTools = () => ['ShellTool(badCommand --danger)']; - config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); - const result = isCommandAllowed('badCommand --danger', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command 'badCommand --danger' is blocked by configuration`, - ); - }); - - it('should allow any command when a wildcard is in coreTools', () => { - config.getCoreTools = () => ['ShellTool']; - const result = isCommandAllowed('any random command', config); - expect(result.allowed).toBe(true); - }); - - it('should block any command when a wildcard is in excludeTools', () => { - config.getExcludeTools = () => new Set(['run_shell_command']); - const result = isCommandAllowed('any random command', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - 'Shell tool is globally disabled in configuration', - ); - }); - - it('should block a command on the blocklist even with a wildcard allow', () => { - config.getCoreTools = () => ['ShellTool']; - config.getExcludeTools = () => new Set(['ShellTool(badCommand --danger)']); - const result = isCommandAllowed('badCommand --danger', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command 'badCommand --danger' is blocked by configuration`, - ); - }); - - it('should allow a chained command if all parts are on the global allowlist', () => { - config.getCoreTools = () => [ - 'run_shell_command(echo)', - 'run_shell_command(goodCommand)', - ]; - const result = isCommandAllowed( - 'echo "hello" && goodCommand --safe', - config, - ); - expect(result.allowed).toBe(true); - }); - - it('should block a chained command if any part is blocked', () => { - config.getExcludeTools = () => new Set(['run_shell_command(badCommand)']); - const result = isCommandAllowed( - 'echo "hello" && badCommand --danger', - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command 'badCommand --danger' is blocked by configuration`, - ); - }); - - it('should block a command that redefines an allowed function to run an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed( - 'echo () (curl google.com) ; echo Hello World', - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block a multi-line function body that runs an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed( - `echo () { - curl google.com -} ; echo ok`, - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block a function keyword declaration that runs an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed( - 'function echo { curl google.com; } ; echo hi', - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block command substitution that invokes an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed('echo $(curl google.com)', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block pipelines that invoke an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed('echo hi | curl google.com', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block background jobs that invoke an unlisted command', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed('echo hi & curl google.com', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block command substitution inside a here-document when the inner command is unlisted', () => { - config.getCoreTools = () => [ - 'run_shell_command(echo)', - 'run_shell_command(cat)', - ]; - const result = isCommandAllowed( - `cat < { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed('echo `curl google.com`', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block process substitution using <() when the inner command is unlisted', () => { - config.getCoreTools = () => [ - 'run_shell_command(diff)', - 'run_shell_command(echo)', - ]; - const result = isCommandAllowed( - 'diff <(curl google.com) <(echo safe)', - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block process substitution using >() when the inner command is unlisted', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = isCommandAllowed('echo "data" > >(curl google.com)', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "curl google.com"`, - ); - }); - - it('should block commands containing prompt transformations', () => { - const result = isCommandAllowed( - 'echo "${var1=aa\\140 env| ls -l\\140}${var1@P}"', - config, - ); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - 'Command rejected because it could not be parsed safely', - ); - }); - - it('should block simple prompt transformation expansions', () => { - const result = isCommandAllowed('echo ${foo@P}', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - 'Command rejected because it could not be parsed safely', - ); - }); - - describe('command substitution', () => { - it('should allow command substitution using `$(...)`', () => { - const result = isCommandAllowed('echo $(goodCommand --safe)', config); - expect(result.allowed).toBe(true); - expect(result.reason).toBeUndefined(); - }); - - it('should allow command substitution using `<(...)`', () => { - const result = isCommandAllowed('diff <(ls) <(ls -a)', config); - expect(result.allowed).toBe(true); - expect(result.reason).toBeUndefined(); - }); - - it('should allow command substitution using `>(...)`', () => { - const result = isCommandAllowed( - 'echo "Log message" > >(tee log.txt)', - config, - ); - expect(result.allowed).toBe(true); - expect(result.reason).toBeUndefined(); - }); - - it('should allow command substitution using backticks', () => { - const result = isCommandAllowed('echo `goodCommand --safe`', config); - expect(result.allowed).toBe(true); - expect(result.reason).toBeUndefined(); - }); - - it('should allow substitution-like patterns inside single quotes', () => { - config.getCoreTools = () => ['ShellTool(echo)']; - const result = isCommandAllowed("echo '$(pwd)'", config); - expect(result.allowed).toBe(true); - }); - - it('should block a command when parsing fails', () => { - const result = isCommandAllowed('ls &&', config); - expect(result.allowed).toBe(false); - expect(result.reason).toBe( - 'Command rejected because it could not be parsed safely', - ); - }); - }); -}); - -describe('checkCommandPermissions', () => { - describe('in "Default Allow" mode (no sessionAllowlist)', () => { - it('should return a detailed success object for an allowed command', () => { - const result = checkCommandPermissions('goodCommand --safe', config); - expect(result).toEqual({ - allAllowed: true, - disallowedCommands: [], - }); - }); - - it('should block commands that cannot be parsed safely', () => { - const result = checkCommandPermissions('ls &&', config); - expect(result).toEqual({ - allAllowed: false, - disallowedCommands: ['ls &&'], - blockReason: 'Command rejected because it could not be parsed safely', - isHardDenial: true, - }); - }); - - it('should return a detailed failure object for a blocked command', () => { - config.getExcludeTools = () => new Set(['ShellTool(badCommand)']); - const result = checkCommandPermissions('badCommand --danger', config); - expect(result).toEqual({ - allAllowed: false, - disallowedCommands: ['badCommand --danger'], - blockReason: `Command 'badCommand --danger' is blocked by configuration`, - isHardDenial: true, - }); - }); - - it('should return a detailed failure object for a command not on a strict allowlist', () => { - config.getCoreTools = () => ['ShellTool(goodCommand)']; - const result = checkCommandPermissions( - 'git status && goodCommand', - config, - ); - expect(result).toEqual({ - allAllowed: false, - disallowedCommands: ['git status'], - blockReason: `Command(s) not in the allowed commands list. Disallowed commands: "git status"`, - isHardDenial: false, - }); - }); - }); - - describe('in "Default Deny" mode (with sessionAllowlist)', () => { - it('should allow a command on the sessionAllowlist', () => { - const result = checkCommandPermissions( - 'goodCommand --safe', - config, - new Set(['goodCommand --safe']), - ); - expect(result.allAllowed).toBe(true); - }); - - it('should block a command not on the sessionAllowlist or global allowlist', () => { - const result = checkCommandPermissions( - 'badCommand --danger', - config, - new Set(['goodCommand --safe']), - ); - expect(result.allAllowed).toBe(false); - expect(result.blockReason).toContain( - 'not on the global or session allowlist', - ); - expect(result.disallowedCommands).toEqual(['badCommand --danger']); - }); - - it('should allow a command on the global allowlist even if not on the session allowlist', () => { - config.getCoreTools = () => ['ShellTool(git status)']; - const result = checkCommandPermissions( - 'git status', - config, - new Set(['goodCommand --safe']), - ); - expect(result.allAllowed).toBe(true); - }); - - it('should allow a chained command if parts are on different allowlists', () => { - config.getCoreTools = () => ['ShellTool(git status)']; - const result = checkCommandPermissions( - 'git status && git commit', - config, - new Set(['git commit']), - ); - expect(result.allAllowed).toBe(true); - }); - - it('should block a command on the sessionAllowlist if it is also globally blocked', () => { - config.getExcludeTools = () => new Set(['run_shell_command(badCommand)']); - const result = checkCommandPermissions( - 'badCommand --danger', - config, - new Set(['badCommand --danger']), - ); - expect(result.allAllowed).toBe(false); - expect(result.blockReason).toContain('is blocked by configuration'); - }); - - it('should block a chained command if one part is not on any allowlist', () => { - config.getCoreTools = () => ['run_shell_command(echo)']; - const result = checkCommandPermissions( - 'echo "hello" && badCommand --danger', - config, - new Set(['echo']), - ); - expect(result.allAllowed).toBe(false); - expect(result.disallowedCommands).toEqual(['badCommand --danger']); - }); - }); -}); - describe('getCommandRoots', () => { it('should return a single command', () => { expect(getCommandRoots('ls -l')).toEqual(['ls']); @@ -521,14 +130,6 @@ describeWindowsOnly('PowerShell integration', () => { expect(roots.length).toBeGreaterThan(0); expect(roots).toContain('Get-ChildItem'); }); - - it('should block commands when PowerShell parser reports errors', () => { - const { allowed, reason } = isCommandAllowed('Get-ChildItem |', config); - expect(allowed).toBe(false); - expect(reason).toBe( - 'Command rejected because it could not be parsed safely', - ); - }); }); describe('stripShellWrapper', () => { @@ -568,53 +169,6 @@ describe('stripShellWrapper', () => { }); }); -describe('isShellInvocationAllowlisted', () => { - function createInvocation(command: string): AnyToolInvocation { - return { params: { command } } as unknown as AnyToolInvocation; - } - - it('should return false when any chained command segment is not allowlisted', () => { - const invocation = createInvocation( - 'git status && rm -rf /tmp/should-not-run', - ); - expect( - isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']), - ).toBe(false); - }); - - it('should return true when every segment is explicitly allowlisted', () => { - const invocation = createInvocation( - 'git status && rm -rf /tmp/should-run && git diff', - ); - expect( - isShellInvocationAllowlisted(invocation, [ - 'run_shell_command(git)', - 'run_shell_command(rm -rf)', - ]), - ).toBe(true); - }); - - it('should return true when the allowlist contains a wildcard shell entry', () => { - const invocation = createInvocation('git status && rm -rf /tmp/should-run'); - expect( - isShellInvocationAllowlisted(invocation, ['run_shell_command']), - ).toBe(true); - }); - - it('should treat piped commands as separate segments that must be allowlisted', () => { - const invocation = createInvocation('git status | tail -n 1'); - expect( - isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']), - ).toBe(false); - expect( - isShellInvocationAllowlisted(invocation, [ - 'run_shell_command(git)', - 'run_shell_command(tail)', - ]), - ).toBe(true); - }); -}); - describe('escapeShellArg', () => { describe('POSIX (bash)', () => { it('should use shell-quote for escaping', () => { diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index d26122b705..ac152a88c2 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -4,11 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { AnyToolInvocation } from '../index.js'; -import type { Config } from '../config/config.js'; import os from 'node:os'; import { quote } from 'shell-quote'; -import { doesToolInvocationMatch } from './tool-utils.js'; import { spawn, spawnSync, @@ -107,7 +104,7 @@ export async function initializeShellParsers(): Promise { await treeSitterInitialization; } -interface ParsedCommandDetail { +export interface ParsedCommandDetail { name: string; text: string; } @@ -399,7 +396,9 @@ function parsePowerShellCommandDetails( } } -function parseCommandDetails(command: string): CommandParseResult | null { +export function parseCommandDetails( + command: string, +): CommandParseResult | null { const configuration = getShellConfiguration(); if (configuration.shell === 'powershell') { @@ -552,178 +551,6 @@ export function stripShellWrapper(command: string): string { * @param command The shell command string to check * @returns true if command substitution would be executed by bash */ -/** - * Checks a shell command against security policies and allowlists. - * - * This function operates in one of two modes depending on the presence of - * the `sessionAllowlist` parameter: - * - * 1. **"Default Deny" Mode (sessionAllowlist is provided):** This is the - * strictest mode, used for user-defined scripts like custom commands. - * A command is only permitted if it is found on the global `coreTools` - * allowlist OR the provided `sessionAllowlist`. It must not be on the - * global `excludeTools` blocklist. - * - * 2. **"Default Allow" Mode (sessionAllowlist is NOT provided):** This mode - * is used for direct tool invocations (e.g., by the model). If a strict - * global `coreTools` allowlist exists, commands must be on it. Otherwise, - * any command is permitted as long as it is not on the `excludeTools` - * blocklist. - * - * @param command The shell command string to validate. - * @param config The application configuration. - * @param sessionAllowlist A session-level list of approved commands. Its - * presence activates "Default Deny" mode. - * @returns An object detailing which commands are not allowed. - */ -export function checkCommandPermissions( - command: string, - config: Config, - sessionAllowlist?: Set, -): { - allAllowed: boolean; - disallowedCommands: string[]; - blockReason?: string; - isHardDenial?: boolean; -} { - const parseResult = parseCommandDetails(command); - if (!parseResult || parseResult.hasError) { - return { - allAllowed: false, - disallowedCommands: [command], - blockReason: 'Command rejected because it could not be parsed safely', - isHardDenial: true, - }; - } - - const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' '); - const commandsToValidate = parseResult.details - .map((detail) => normalize(detail.text)) - .filter(Boolean); - const invocation: AnyToolInvocation & { params: { command: string } } = { - params: { command: '' }, - } as AnyToolInvocation & { params: { command: string } }; - - // 1. Blocklist Check (Highest Priority) - const excludeTools = config.getExcludeTools() || new Set([]); - const isWildcardBlocked = SHELL_TOOL_NAMES.some((name) => - excludeTools.has(name), - ); - - if (isWildcardBlocked) { - return { - allAllowed: false, - disallowedCommands: commandsToValidate, - blockReason: 'Shell tool is globally disabled in configuration', - isHardDenial: true, - }; - } - - for (const cmd of commandsToValidate) { - invocation.params['command'] = cmd; - if ( - doesToolInvocationMatch('run_shell_command', invocation, [ - ...excludeTools, - ]) - ) { - return { - allAllowed: false, - disallowedCommands: [cmd], - blockReason: `Command '${cmd}' is blocked by configuration`, - isHardDenial: true, - }; - } - } - - const coreTools = config.getCoreTools() || []; - const isWildcardAllowed = SHELL_TOOL_NAMES.some((name) => - coreTools.includes(name), - ); - - // If there's a global wildcard, all commands are allowed at this point - // because they have already passed the blocklist check. - if (isWildcardAllowed) { - return { allAllowed: true, disallowedCommands: [] }; - } - - const disallowedCommands: string[] = []; - - if (sessionAllowlist) { - // "DEFAULT DENY" MODE: A session allowlist is provided. - // All commands must be in either the session or global allowlist. - const normalizedSessionAllowlist = new Set( - [...sessionAllowlist].flatMap((cmd) => - SHELL_TOOL_NAMES.map((name) => `${name}(${cmd})`), - ), - ); - - for (const cmd of commandsToValidate) { - invocation.params['command'] = cmd; - const isSessionAllowed = doesToolInvocationMatch( - 'run_shell_command', - invocation, - [...normalizedSessionAllowlist], - ); - if (isSessionAllowed) continue; - - const isGloballyAllowed = doesToolInvocationMatch( - 'run_shell_command', - invocation, - coreTools, - ); - if (isGloballyAllowed) continue; - - disallowedCommands.push(cmd); - } - - if (disallowedCommands.length > 0) { - return { - allAllowed: false, - disallowedCommands, - blockReason: `Command(s) not on the global or session allowlist. Disallowed commands: ${disallowedCommands - .map((c) => JSON.stringify(c)) - .join(', ')}`, - isHardDenial: false, // This is a soft denial; confirmation is possible. - }; - } - } else { - // "DEFAULT ALLOW" MODE: No session allowlist. - const hasSpecificAllowedCommands = - coreTools.filter((tool) => - SHELL_TOOL_NAMES.some((name) => tool.startsWith(`${name}(`)), - ).length > 0; - - if (hasSpecificAllowedCommands) { - for (const cmd of commandsToValidate) { - invocation.params['command'] = cmd; - const isGloballyAllowed = doesToolInvocationMatch( - 'run_shell_command', - invocation, - coreTools, - ); - if (!isGloballyAllowed) { - disallowedCommands.push(cmd); - } - } - if (disallowedCommands.length > 0) { - return { - allAllowed: false, - disallowedCommands, - blockReason: `Command(s) not in the allowed commands list. Disallowed commands: ${disallowedCommands - .map((c) => JSON.stringify(c)) - .join(', ')}`, - isHardDenial: false, // This is a soft denial. - }; - } - } - // If no specific global allowlist exists, and it passed the blocklist, - // the command is allowed by default. - } - - // If all checks for the current mode pass, the command is allowed. - return { allAllowed: true, disallowedCommands: [] }; -} - /** * Determines whether a given shell command is allowed to execute based on * the tool's configuration including allowlists and blocklists. @@ -765,87 +592,3 @@ export const spawnAsync = ( reject(err); }); }); - -export function isCommandAllowed( - command: string, - config: Config, -): { allowed: boolean; reason?: string } { - // By not providing a sessionAllowlist, we invoke "default allow" behavior. - const { allAllowed, blockReason } = checkCommandPermissions(command, config); - if (allAllowed) { - return { allowed: true }; - } - return { allowed: false, reason: blockReason }; -} - -/** - * Determines whether a shell invocation should be auto-approved based on an allowlist. - * - * This reuses the same parsing logic as command-permission enforcement so that - * chained commands must be individually covered by the allowlist. - * - * @param invocation The shell tool invocation being evaluated. - * @param allowedPatterns The configured allowlist patterns (e.g. `run_shell_command(git)`). - * @returns True if every parsed command segment is allowed by the patterns; false otherwise. - */ -export function isShellInvocationAllowlisted( - invocation: AnyToolInvocation, - allowedPatterns: string[], -): boolean { - if (!allowedPatterns.length) { - return false; - } - - const hasShellWildcard = allowedPatterns.some((pattern) => - SHELL_TOOL_NAMES.includes(pattern), - ); - const hasShellSpecificPattern = allowedPatterns.some((pattern) => - SHELL_TOOL_NAMES.some((name) => pattern.startsWith(`${name}(`)), - ); - - if (!hasShellWildcard && !hasShellSpecificPattern) { - return false; - } - - if (hasShellWildcard) { - return true; - } - - if ( - !('params' in invocation) || - typeof invocation.params !== 'object' || - invocation.params === null || - !('command' in invocation.params) - ) { - return false; - } - - const commandValue = (invocation.params as { command?: unknown }).command; - if (typeof commandValue !== 'string' || !commandValue.trim()) { - return false; - } - - const command = commandValue.trim(); - - const parseResult = parseCommandDetails(command); - if (!parseResult || parseResult.hasError) { - return false; - } - - const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' '); - const commandsToValidate = parseResult.details - .map((detail) => normalize(detail.text)) - .filter(Boolean); - - if (commandsToValidate.length === 0) { - return false; - } - - return commandsToValidate.every((commandSegment) => - doesToolInvocationMatch( - SHELL_TOOL_NAMES[0], - { params: { command: commandSegment } } as AnyToolInvocation, - allowedPatterns, - ), - ); -}