Revert "Shell approval rework" (#11143)

This commit is contained in:
cornmander
2025-10-14 18:55:28 -04:00
committed by GitHub
parent 6f0107e7b7
commit bd5c158a62
12 changed files with 279 additions and 661 deletions
+7 -32
View File
@@ -9,7 +9,6 @@ import {
describe,
it,
expect,
beforeAll,
beforeEach,
afterEach,
type Mock,
@@ -24,10 +23,7 @@ vi.mock('os');
vi.mock('crypto');
vi.mock('../utils/summarizer.js');
import {
initializeShellParsers,
isCommandAllowed,
} from '../utils/shell-utils.js';
import { isCommandAllowed } from '../utils/shell-utils.js';
import { ShellTool } from './shell.js';
import { type Config } from '../config/config.js';
import {
@@ -45,9 +41,6 @@ import { ToolConfirmationOutcome } from './tools.js';
import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
const originalComSpec = process.env['ComSpec'];
const itWindowsOnly = process.platform === 'win32' ? it : it.skip;
describe('ShellTool', () => {
let shellTool: ShellTool;
let mockConfig: Config;
@@ -78,8 +71,6 @@ describe('ShellTool', () => {
(vi.mocked(crypto.randomBytes) as Mock).mockReturnValue(
Buffer.from('abcdef', 'hex'),
);
process.env['ComSpec'] =
'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe';
// Capture the output callback to simulate streaming events from the service
mockShellExecutionService.mockImplementation((_cmd, _cwd, callback) => {
@@ -93,36 +84,23 @@ describe('ShellTool', () => {
});
});
afterEach(() => {
if (originalComSpec === undefined) {
delete process.env['ComSpec'];
} else {
process.env['ComSpec'] = originalComSpec;
}
});
describe('isCommandAllowed', () => {
it('should allow a command if no restrictions are provided', () => {
(mockConfig.getCoreTools as Mock).mockReturnValue(undefined);
(mockConfig.getExcludeTools as Mock).mockReturnValue(undefined);
expect(isCommandAllowed('goodCommand --safe', mockConfig).allowed).toBe(
true,
);
expect(isCommandAllowed('ls -l', mockConfig).allowed).toBe(true);
});
it('should allow a command with command substitution using $()', () => {
const evaluation = isCommandAllowed(
'echo $(goodCommand --safe)',
mockConfig,
it('should block a command with command substitution using $()', () => {
expect(isCommandAllowed('echo $(rm -rf /)', mockConfig).allowed).toBe(
false,
);
expect(evaluation.allowed).toBe(true);
expect(evaluation.reason).toBeUndefined();
});
});
describe('build', () => {
it('should return an invocation for a valid command', () => {
const invocation = shellTool.build({ command: 'goodCommand --safe' });
const invocation = shellTool.build({ command: 'ls -l' });
expect(invocation).toBeDefined();
});
@@ -229,7 +207,7 @@ describe('ShellTool', () => {
);
});
itWindowsOnly('should not wrap command on windows', async () => {
it('should not wrap command on windows', async () => {
vi.mocked(os.platform).mockReturnValue('win32');
const invocation = shellTool.build({ command: 'dir' });
const promise = invocation.execute(mockAbortSignal);
@@ -448,6 +426,3 @@ describe('ShellTool', () => {
});
});
});
beforeAll(async () => {
await initializeShellParsers();
});