summaryrefslogtreecommitdiff
path: root/packages/core/src
diff options
context:
space:
mode:
Diffstat (limited to 'packages/core/src')
-rw-r--r--packages/core/src/index.ts1
-rw-r--r--packages/core/src/utils/shell-utils.test.ts650
-rw-r--r--packages/core/src/utils/shell-utils.ts181
3 files changed, 299 insertions, 533 deletions
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index b4ce7e85..a49c83fe 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -35,6 +35,7 @@ export * from './utils/editor.js';
export * from './utils/quotaErrorDetection.js';
export * from './utils/fileUtils.js';
export * from './utils/retry.js';
+export * from './utils/shell-utils.js';
export * from './utils/systemEncoding.js';
export * from './utils/textUtils.js';
export * from './utils/formatters.js';
diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts
index a3a2b6c2..9473dee7 100644
--- a/packages/core/src/utils/shell-utils.test.ts
+++ b/packages/core/src/utils/shell-utils.test.ts
@@ -6,578 +6,272 @@
import { expect, describe, it, beforeEach } from 'vitest';
import {
+ checkCommandPermissions,
getCommandRoots,
isCommandAllowed,
stripShellWrapper,
} from './shell-utils.js';
import { Config } from '../config/config.js';
-describe('isCommandAllowed', () => {
- let config: Config;
+let config: Config;
- beforeEach(() => {
- config = {
- getCoreTools: () => undefined,
- getExcludeTools: () => undefined,
- } as unknown as Config;
- });
+beforeEach(() => {
+ config = {
+ getCoreTools: () => [],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+});
- it('should allow a command if no restrictions are provided', async () => {
+describe('isCommandAllowed', () => {
+ it('should allow a command if no restrictions are provided', () => {
const result = isCommandAllowed('ls -l', config);
expect(result.allowed).toBe(true);
});
- it('should allow a command if it is in the allowed list', async () => {
- config = {
- getCoreTools: () => ['ShellTool(ls -l)'],
- getExcludeTools: () => undefined,
- } as unknown as Config;
+ it('should allow a command if it is in the global allowlist', () => {
+ config.getCoreTools = () => ['ShellTool(ls)'];
const result = isCommandAllowed('ls -l', config);
expect(result.allowed).toBe(true);
});
- it('should block a command if it is not in the allowed list', async () => {
- config = {
- getCoreTools: () => ['ShellTool(ls -l)'],
- getExcludeTools: () => undefined,
- } as unknown as Config;
+ it('should block a command if it is not in a strict global allowlist', () => {
+ config.getCoreTools = () => ['ShellTool(ls -l)'];
const result = isCommandAllowed('rm -rf /', config);
expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is not in the allowed commands list",
- );
+ expect(result.reason).toBe(`Command(s) not in the allowed commands list.`);
});
- it('should block a command if it is in the blocked list', async () => {
- config = {
- getCoreTools: () => undefined,
- getExcludeTools: () => ['ShellTool(rm -rf /)'],
- } as unknown as Config;
+ it('should block a command if it is in the blocked list', () => {
+ config.getExcludeTools = () => ['ShellTool(rm -rf /)'];
const result = isCommandAllowed('rm -rf /', config);
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
+ `Command 'rm -rf /' is blocked by configuration`,
);
});
- it('should allow a command if it is not in the blocked list', async () => {
- config = {
- getCoreTools: () => undefined,
- getExcludeTools: () => ['ShellTool(rm -rf /)'],
- } as unknown as Config;
- const result = isCommandAllowed('ls -l', config);
- expect(result.allowed).toBe(true);
- });
-
- it('should block a command if it is in both the allowed and blocked lists', async () => {
- config = {
- getCoreTools: () => ['ShellTool(rm -rf /)'],
- getExcludeTools: () => ['ShellTool(rm -rf /)'],
- } as unknown as Config;
+ it('should prioritize the blocklist over the allowlist', () => {
+ config.getCoreTools = () => ['ShellTool(rm -rf /)'];
+ config.getExcludeTools = () => ['ShellTool(rm -rf /)'];
const result = isCommandAllowed('rm -rf /', config);
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
+ `Command 'rm -rf /' is blocked by configuration`,
);
});
- it('should allow any command when ShellTool is in coreTools without specific commands', async () => {
- config = {
- getCoreTools: () => ['ShellTool'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
+ 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 ShellTool is in excludeTools without specific commands', async () => {
- config = {
- getCoreTools: () => [],
- getExcludeTools: () => ['ShellTool'],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
+ it('should block any command when a wildcard is in excludeTools', () => {
+ config.getExcludeTools = () => ['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 allow a command if it is in the allowed list using the public-facing name', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(ls -l)'],
- getExcludeTools: () => undefined,
- } as unknown as Config;
- const result = isCommandAllowed('ls -l', config);
- expect(result.allowed).toBe(true);
- });
-
- it('should block a command if it is in the blocked list using the public-facing name', async () => {
- config = {
- getCoreTools: () => undefined,
- getExcludeTools: () => ['run_shell_command(rm -rf /)'],
- } as unknown as Config;
+ it('should block a command on the blocklist even with a wildcard allow', () => {
+ config.getCoreTools = () => ['ShellTool'];
+ config.getExcludeTools = () => ['ShellTool(rm -rf /)'];
const result = isCommandAllowed('rm -rf /', config);
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
- );
- });
-
- it('should block any command when ShellTool is in excludeTools using the public-facing name', async () => {
- config = {
- getCoreTools: () => [],
- getExcludeTools: () => ['run_shell_command'],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- 'Shell tool is globally disabled in configuration',
- );
- });
-
- it('should block any command if coreTools contains an empty ShellTool command list using the public-facing name', async () => {
- config = {
- getCoreTools: () => ['run_shell_command()'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'any command' is not in the allowed commands list",
- );
- });
-
- it('should block any command if coreTools contains an empty ShellTool command list', async () => {
- config = {
- getCoreTools: () => ['ShellTool()'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'any command' is not in the allowed commands list",
- );
- });
-
- it('should block a command with extra whitespace if it is in the blocked list', async () => {
- config = {
- getCoreTools: () => undefined,
- getExcludeTools: () => ['ShellTool(rm -rf /)'],
- } as unknown as Config;
- const result = isCommandAllowed(' rm -rf / ', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
+ `Command 'rm -rf /' is blocked by configuration`,
);
});
- it('should allow any command when ShellTool is in present with specific commands', async () => {
- config = {
- getCoreTools: () => ['ShellTool', 'ShellTool(ls)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('any command', config);
+ it('should allow a chained command if all parts are on the global allowlist', () => {
+ config.getCoreTools = () => [
+ 'run_shell_command(echo)',
+ 'run_shell_command(ls)',
+ ];
+ const result = isCommandAllowed('echo "hello" && ls -l', config);
expect(result.allowed).toBe(true);
});
- it('should block a command on the blocklist even with a wildcard allow', async () => {
- config = {
- getCoreTools: () => ['ShellTool'],
- getExcludeTools: () => ['ShellTool(rm -rf /)'],
- } as unknown as Config;
- const result = isCommandAllowed('rm -rf /', config);
+ it('should block a chained command if any part is blocked', () => {
+ config.getExcludeTools = () => ['run_shell_command(rm)'];
+ const result = isCommandAllowed('echo "hello" && rm -rf /', config);
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
+ `Command 'rm -rf /' is blocked by configuration`,
);
});
- it('should allow a command that starts with an allowed command prefix', async () => {
- config = {
- getCoreTools: () => ['ShellTool(gh issue edit)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed(
- 'gh issue edit 1 --add-label "kind/feature"',
- config,
- );
- expect(result.allowed).toBe(true);
- });
+ describe('command substitution', () => {
+ it('should block command substitution using `$(...)`', () => {
+ const result = isCommandAllowed('echo $(rm -rf /)', config);
+ expect(result.allowed).toBe(false);
+ expect(result.reason).toContain('Command substitution');
+ });
- it('should allow a command that starts with an allowed command prefix using the public-facing name', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue edit)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed(
- 'gh issue edit 1 --add-label "kind/feature"',
- config,
- );
- expect(result.allowed).toBe(true);
- });
+ it('should block command substitution using `<(...)`', () => {
+ const result = isCommandAllowed('diff <(ls) <(ls -a)', config);
+ expect(result.allowed).toBe(false);
+ expect(result.reason).toContain('Command substitution');
+ });
- it('should not allow a command that starts with an allowed command prefix but is chained with another command', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue edit)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue edit&&rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is not in the allowed commands list",
- );
- });
+ it('should block command substitution using backticks', () => {
+ const result = isCommandAllowed('echo `rm -rf /`', config);
+ expect(result.allowed).toBe(false);
+ expect(result.reason).toContain('Command substitution');
+ });
- it('should not allow a command that is a prefix of an allowed command', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue edit)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'gh issue' is not in the allowed commands list",
- );
+ 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 not allow a command that is a prefix of a blocked command', async () => {
- config = {
- getCoreTools: () => [],
- getExcludeTools: () => ['run_shell_command(gh issue edit)'],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue', config);
- expect(result.allowed).toBe(true);
- });
+describe('checkCommandPermissions', () => {
+ describe('in "Default Allow" mode (no sessionAllowlist)', () => {
+ it('should return a detailed success object for an allowed command', () => {
+ const result = checkCommandPermissions('ls -l', config);
+ expect(result).toEqual({
+ allAllowed: true,
+ disallowedCommands: [],
+ });
+ });
- it('should not allow a command that is chained with a pipe', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue list)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue list | rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is not in the allowed commands list",
- );
- });
+ it('should return a detailed failure object for a blocked command', () => {
+ config.getExcludeTools = () => ['ShellTool(rm)'];
+ const result = checkCommandPermissions('rm -rf /', config);
+ expect(result).toEqual({
+ allAllowed: false,
+ disallowedCommands: ['rm -rf /'],
+ blockReason: `Command 'rm -rf /' is blocked by configuration`,
+ isHardDenial: true,
+ });
+ });
- it('should not allow a command that is chained with a semicolon', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue list)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue list; rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is not in the allowed commands list",
- );
+ it('should return a detailed failure object for a command not on a strict allowlist', () => {
+ config.getCoreTools = () => ['ShellTool(ls)'];
+ const result = checkCommandPermissions('git status && ls', config);
+ expect(result).toEqual({
+ allAllowed: false,
+ disallowedCommands: ['git status'],
+ blockReason: `Command(s) not in the allowed commands list.`,
+ isHardDenial: false,
+ });
+ });
});
- it('should block a chained command if any part is blocked', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(echo "hello")'],
- getExcludeTools: () => ['run_shell_command(rm)'],
- } as unknown as Config;
- const result = isCommandAllowed('echo "hello" && rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
- );
- });
+ describe('in "Default Deny" mode (with sessionAllowlist)', () => {
+ it('should allow a command on the sessionAllowlist', () => {
+ const result = checkCommandPermissions(
+ 'ls -l',
+ config,
+ new Set(['ls -l']),
+ );
+ expect(result.allAllowed).toBe(true);
+ });
- it('should block a command if its prefix is on the blocklist, even if the command itself is on the allowlist', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(git push)'],
- getExcludeTools: () => ['run_shell_command(git)'],
- } as unknown as Config;
- const result = isCommandAllowed('git push', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'git push' is blocked by configuration",
- );
- });
+ it('should block a command not on the sessionAllowlist or global allowlist', () => {
+ const result = checkCommandPermissions(
+ 'rm -rf /',
+ config,
+ new Set(['ls -l']),
+ );
+ expect(result.allAllowed).toBe(false);
+ expect(result.blockReason).toContain(
+ 'not on the global or session allowlist',
+ );
+ expect(result.disallowedCommands).toEqual(['rm -rf /']);
+ });
- it('should be case-sensitive in its matching', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(echo)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('ECHO "hello"', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- 'Command \'ECHO "hello"\' is not in the allowed commands list',
- );
- });
+ 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(['ls -l']),
+ );
+ expect(result.allAllowed).toBe(true);
+ });
- it('should correctly handle commands with extra whitespace around chaining operators', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(ls -l)'],
- getExcludeTools: () => ['run_shell_command(rm)'],
- } as unknown as Config;
- const result = isCommandAllowed('ls -l ; rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is blocked by configuration",
- );
- });
+ 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 allow a chained command if all parts are allowed', async () => {
- config = {
- getCoreTools: () => [
- 'run_shell_command(echo)',
- 'run_shell_command(ls -l)',
- ],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('echo "hello" && ls -l', config);
- expect(result.allowed).toBe(true);
- });
+ it('should block a command on the sessionAllowlist if it is also globally blocked', () => {
+ config.getExcludeTools = () => ['run_shell_command(rm)'];
+ const result = checkCommandPermissions(
+ 'rm -rf /',
+ config,
+ new Set(['rm -rf /']),
+ );
+ expect(result.allAllowed).toBe(false);
+ expect(result.blockReason).toContain('is blocked by configuration');
+ });
- it('should block a command with command substitution using backticks', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(echo)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('echo `rm -rf /`', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- 'Command substitution using $(), <(), or >() is not allowed for security reasons',
- );
- });
-
- it('should block a command with command substitution using $()', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(echo)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('echo $(rm -rf /)', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- 'Command substitution using $(), <(), or >() is not allowed for security reasons',
- );
- });
-
- it('should block a command with process substitution using <()', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(diff)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('diff <(ls) <(ls -a)', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- 'Command substitution using $(), <(), or >() is not allowed for security reasons',
- );
- });
-
- it('should allow a command with I/O redirection', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(echo)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('echo "hello" > file.txt', config);
- expect(result.allowed).toBe(true);
- });
-
- it('should not allow a command that is chained with a double pipe', async () => {
- config = {
- getCoreTools: () => ['run_shell_command(gh issue list)'],
- getExcludeTools: () => [],
- } as unknown as Config;
- const result = isCommandAllowed('gh issue list || rm -rf /', config);
- expect(result.allowed).toBe(false);
- expect(result.reason).toBe(
- "Command 'rm -rf /' is not in the allowed commands list",
- );
+ 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" && rm -rf /',
+ config,
+ new Set(['echo']),
+ );
+ expect(result.allAllowed).toBe(false);
+ expect(result.disallowedCommands).toEqual(['rm -rf /']);
+ });
});
});
describe('getCommandRoots', () => {
it('should return a single command', () => {
- const result = getCommandRoots('ls -l');
- expect(result).toEqual(['ls']);
+ expect(getCommandRoots('ls -l')).toEqual(['ls']);
});
- it('should return multiple commands', () => {
- const result = getCommandRoots('ls -l | grep "test"');
- expect(result).toEqual(['ls', 'grep']);
+ it('should handle paths and return the binary name', () => {
+ expect(getCommandRoots('/usr/local/bin/node script.js')).toEqual(['node']);
});
- it('should handle multiple commands with &&', () => {
- const result = getCommandRoots('npm run build && npm test');
- expect(result).toEqual(['npm', 'npm']);
- });
-
- it('should handle multiple commands with ;', () => {
- const result = getCommandRoots('echo "hello"; echo "world"');
- expect(result).toEqual(['echo', 'echo']);
+ it('should return an empty array for an empty string', () => {
+ expect(getCommandRoots('')).toEqual([]);
});
it('should handle a mix of operators', () => {
- const result = getCommandRoots(
- 'cat package.json | grep "version" && echo "done"',
- );
- expect(result).toEqual(['cat', 'grep', 'echo']);
- });
-
- it('should handle commands with paths', () => {
- const result = getCommandRoots('/usr/local/bin/node script.js');
- expect(result).toEqual(['node']);
+ const result = getCommandRoots('a;b|c&&d||e&f');
+ expect(result).toEqual(['a', 'b', 'c', 'd', 'e', 'f']);
});
- it('should return an empty array for an empty string', () => {
- const result = getCommandRoots('');
- expect(result).toEqual([]);
+ it('should correctly parse a chained command with quotes', () => {
+ const result = getCommandRoots('echo "hello" && git commit -m "feat"');
+ expect(result).toEqual(['echo', 'git']);
});
});
describe('stripShellWrapper', () => {
- it('should strip sh -c from the beginning of the command', () => {
- const result = stripShellWrapper('sh -c "ls -l"');
- expect(result).toEqual('ls -l');
- });
-
- it('should strip bash -c from the beginning of the command', () => {
- const result = stripShellWrapper('bash -c "ls -l"');
- expect(result).toEqual('ls -l');
- });
-
- it('should strip zsh -c from the beginning of the command', () => {
- const result = stripShellWrapper('zsh -c "ls -l"');
- expect(result).toEqual('ls -l');
- });
-
- it('should not strip anything if the command does not start with a shell wrapper', () => {
- const result = stripShellWrapper('ls -l');
- expect(result).toEqual('ls -l');
- });
-
- it('should handle extra whitespace', () => {
- const result = stripShellWrapper(' sh -c "ls -l" ');
- expect(result).toEqual('ls -l');
+ it('should strip sh -c with quotes', () => {
+ expect(stripShellWrapper('sh -c "ls -l"')).toEqual('ls -l');
});
- it('should handle commands without quotes', () => {
- const result = stripShellWrapper('sh -c ls -l');
- expect(result).toEqual('ls -l');
+ it('should strip bash -c with extra whitespace', () => {
+ expect(stripShellWrapper(' bash -c "ls -l" ')).toEqual('ls -l');
});
- it('should strip cmd.exe /c from the beginning of the command', () => {
- const result = stripShellWrapper('cmd.exe /c "dir"');
- expect(result).toEqual('dir');
+ it('should strip zsh -c without quotes', () => {
+ expect(stripShellWrapper('zsh -c ls -l')).toEqual('ls -l');
});
-});
-
-describe('getCommandRoots', () => {
- it('should handle multiple commands with &', () => {
- const result = getCommandRoots('echo "hello" & echo "world"');
- expect(result).toEqual(['echo', 'echo']);
- });
-});
-
-describe('command substitution', () => {
- let config: Config;
- beforeEach(() => {
- config = {
- getCoreTools: () => ['run_shell_command(echo)', 'run_shell_command(gh)'],
- getExcludeTools: () => [],
- } as unknown as Config;
+ it('should strip cmd.exe /c', () => {
+ expect(stripShellWrapper('cmd.exe /c "dir"')).toEqual('dir');
});
- it('should block unquoted command substitution `$(...)`', () => {
- const result = isCommandAllowed('echo $(pwd)', config);
- expect(result.allowed).toBe(false);
- });
-
- it('should block unquoted command substitution `<(...)`', () => {
- const result = isCommandAllowed('echo <(pwd)', config);
- expect(result.allowed).toBe(false);
- });
-
- it('should allow command substitution in single quotes', () => {
- const result = isCommandAllowed("echo '$(pwd)'", config);
- expect(result.allowed).toBe(true);
- });
-
- it('should allow backticks in single quotes', () => {
- const result = isCommandAllowed("echo '`rm -rf /`'", config);
- expect(result.allowed).toBe(true);
- });
-
- it('should block command substitution in double quotes', () => {
- const result = isCommandAllowed('echo "$(pwd)"', config);
- expect(result.allowed).toBe(false);
- });
-
- it('should allow escaped command substitution', () => {
- const result = isCommandAllowed('echo \\$(pwd)', config);
- expect(result.allowed).toBe(true);
- });
-
- it('should allow complex commands with quoted substitution-like patterns', () => {
- const command =
- "gh pr comment 4795 --body 'This is a test comment with $(pwd) style text'";
- const result = isCommandAllowed(command, config);
- expect(result.allowed).toBe(true);
- });
-
- it('should block complex commands with unquoted substitution-like patterns', () => {
- const command =
- 'gh pr comment 4795 --body "This is a test comment with $(pwd) style text"';
- const result = isCommandAllowed(command, config);
- expect(result.allowed).toBe(false);
- });
-
- it('should allow a command with markdown content using proper quoting', () => {
- // Simple test with safe content in single quotes
- const result = isCommandAllowed(
- "gh pr comment 4795 --body 'This is safe markdown content'",
- config,
- );
- expect(result.allowed).toBe(true);
- });
-});
-
-describe('getCommandRoots with quote handling', () => {
- it('should correctly parse a simple command', () => {
- const result = getCommandRoots('git status');
- expect(result).toEqual(['git']);
- });
-
- it('should correctly parse a command with a quoted argument', () => {
- const result = getCommandRoots('git commit -m "feat: new feature"');
- expect(result).toEqual(['git']);
- });
-
- it('should correctly parse a command with single quotes', () => {
- const result = getCommandRoots("echo 'hello world'");
- expect(result).toEqual(['echo']);
- });
-
- it('should correctly parse a chained command with quotes', () => {
- const result = getCommandRoots('echo "hello" && git status');
- expect(result).toEqual(['echo', 'git']);
- });
-
- it('should correctly parse a complex chained command', () => {
- const result = getCommandRoots(
- 'git commit -m "feat: new feature" && echo "done"',
- );
- expect(result).toEqual(['git', 'echo']);
- });
-
- it('should handle escaped quotes', () => {
- const result = getCommandRoots('echo "this is a "quote""');
- expect(result).toEqual(['echo']);
- });
-
- it('should handle commands with no spaces', () => {
- const result = getCommandRoots('command');
- expect(result).toEqual(['command']);
- });
-
- it('should handle multiple separators', () => {
- const result = getCommandRoots('a;b|c&&d||e&f');
- expect(result).toEqual(['a', 'b', 'c', 'd', 'e', 'f']);
+ it('should not strip anything if no wrapper is present', () => {
+ expect(stripShellWrapper('ls -l')).toEqual('ls -l');
});
});
diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts
index 7008fb1b..c7f1839e 100644
--- a/packages/core/src/utils/shell-utils.ts
+++ b/packages/core/src/utils/shell-utils.ts
@@ -179,38 +179,53 @@ export function detectCommandSubstitution(command: string): boolean {
}
/**
- * Determines whether a given shell command is allowed to execute based on
- * the tool's configuration including allowlists and blocklists.
- * @param command The shell command string to validate
- * @param config The application configuration
- * @returns An object with 'allowed' boolean and optional 'reason' string if not allowed
+ * 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 isCommandAllowed(
+export function checkCommandPermissions(
command: string,
config: Config,
-): { allowed: boolean; reason?: string } {
- // 0. Disallow command substitution
- // Parse the command to check for unquoted/unescaped command substitution
- const hasCommandSubstitution = detectCommandSubstitution(command);
- if (hasCommandSubstitution) {
+ sessionAllowlist?: Set<string>,
+): {
+ allAllowed: boolean;
+ disallowedCommands: string[];
+ blockReason?: string;
+ isHardDenial?: boolean;
+} {
+ // Disallow command substitution for security.
+ if (detectCommandSubstitution(command)) {
return {
- allowed: false,
- reason:
+ allAllowed: false,
+ disallowedCommands: [command],
+ blockReason:
'Command substitution using $(), <(), or >() is not allowed for security reasons',
+ isHardDenial: true,
};
}
const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
-
const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' ');
- /**
- * Checks if a command string starts with a given prefix, ensuring it's a
- * whole word match (i.e., followed by a space or it's an exact match).
- * e.g., `isPrefixedBy('npm install', 'npm')` -> true
- * e.g., `isPrefixedBy('npm', 'npm')` -> true
- * e.g., `isPrefixedBy('npminstall', 'npm')` -> false
- */
const isPrefixedBy = (cmd: string, prefix: string): boolean => {
if (!cmd.startsWith(prefix)) {
return false;
@@ -218,10 +233,6 @@ export function isCommandAllowed(
return cmd.length === prefix.length || cmd[prefix.length] === ' ';
};
- /**
- * Extracts and normalizes shell commands from a list of tool strings.
- * e.g., 'ShellTool("ls -l")' becomes 'ls -l'
- */
const extractCommands = (tools: string[]): string[] =>
tools.flatMap((tool) => {
for (const toolName of SHELL_TOOL_NAMES) {
@@ -234,55 +245,115 @@ export function isCommandAllowed(
const coreTools = config.getCoreTools() || [];
const excludeTools = config.getExcludeTools() || [];
+ const commandsToValidate = splitCommands(command).map(normalize);
- // 1. Check if the shell tool is globally disabled.
+ // 1. Blocklist Check (Highest Priority)
if (SHELL_TOOL_NAMES.some((name) => excludeTools.includes(name))) {
return {
- allowed: false,
- reason: 'Shell tool is globally disabled in configuration',
+ allAllowed: false,
+ disallowedCommands: commandsToValidate,
+ blockReason: 'Shell tool is globally disabled in configuration',
+ isHardDenial: true,
};
}
+ const blockedCommands = extractCommands(excludeTools);
+ for (const cmd of commandsToValidate) {
+ if (blockedCommands.some((blocked) => isPrefixedBy(cmd, blocked))) {
+ return {
+ allAllowed: false,
+ disallowedCommands: [cmd],
+ blockReason: `Command '${cmd}' is blocked by configuration`,
+ isHardDenial: true,
+ };
+ }
+ }
- const blockedCommands = new Set(extractCommands(excludeTools));
- const allowedCommands = new Set(extractCommands(coreTools));
-
- const hasSpecificAllowedCommands = allowedCommands.size > 0;
+ const globallyAllowedCommands = extractCommands(coreTools);
const isWildcardAllowed = SHELL_TOOL_NAMES.some((name) =>
coreTools.includes(name),
);
- const commandsToValidate = splitCommands(command).map(normalize);
+ // 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 blockedCommandsArr = [...blockedCommands];
+ if (sessionAllowlist) {
+ // "DEFAULT DENY" MODE: A session allowlist is provided.
+ // All commands must be in either the session or global allowlist.
+ const disallowedCommands: string[] = [];
+ for (const cmd of commandsToValidate) {
+ const isSessionAllowed = [...sessionAllowlist].some((allowed) =>
+ isPrefixedBy(cmd, normalize(allowed)),
+ );
+ if (isSessionAllowed) continue;
- for (const cmd of commandsToValidate) {
- // 2. Check if the command is on the blocklist.
- const isBlocked = blockedCommandsArr.some((blocked) =>
- isPrefixedBy(cmd, blocked),
- );
- if (isBlocked) {
+ const isGloballyAllowed = globallyAllowedCommands.some((allowed) =>
+ isPrefixedBy(cmd, allowed),
+ );
+ if (isGloballyAllowed) continue;
+
+ disallowedCommands.push(cmd);
+ }
+
+ if (disallowedCommands.length > 0) {
return {
- allowed: false,
- reason: `Command '${cmd}' is blocked by configuration`,
+ allAllowed: false,
+ disallowedCommands,
+ blockReason: `Command(s) not on the global or session allowlist.`,
+ isHardDenial: false, // This is a soft denial; confirmation is possible.
};
}
-
- // 3. If in strict allow-list mode, check if the command is permitted.
- const isStrictAllowlist = hasSpecificAllowedCommands && !isWildcardAllowed;
- const allowedCommandsArr = [...allowedCommands];
- if (isStrictAllowlist) {
- const isAllowed = allowedCommandsArr.some((allowed) =>
- isPrefixedBy(cmd, allowed),
- );
- if (!isAllowed) {
+ } else {
+ // "DEFAULT ALLOW" MODE: No session allowlist.
+ const hasSpecificAllowedCommands = globallyAllowedCommands.length > 0;
+ if (hasSpecificAllowedCommands) {
+ const disallowedCommands: string[] = [];
+ for (const cmd of commandsToValidate) {
+ const isGloballyAllowed = globallyAllowedCommands.some((allowed) =>
+ isPrefixedBy(cmd, allowed),
+ );
+ if (!isGloballyAllowed) {
+ disallowedCommands.push(cmd);
+ }
+ }
+ if (disallowedCommands.length > 0) {
return {
- allowed: false,
- reason: `Command '${cmd}' is not in the allowed commands list`,
+ allAllowed: false,
+ disallowedCommands,
+ blockReason: `Command(s) not in the allowed commands list.`,
+ isHardDenial: false, // This is a soft denial.
};
}
}
+ // If no specific global allowlist exists, and it passed the blocklist,
+ // the command is allowed by default.
}
- // 4. If all checks pass, the command is allowed.
- return { allowed: true };
+ // 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.
+ *
+ * This function operates in "default allow" mode. It is a wrapper around
+ * `checkCommandPermissions`.
+ *
+ * @param command The shell command string to validate.
+ * @param config The application configuration.
+ * @returns An object with 'allowed' boolean and optional 'reason' string if not allowed.
+ */
+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 };
}
generated by cgit v1.2.3 (git 2.51.0) at 2025-11-05 16:31:33 +0000