summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--docs/tools/shell.md77
-rw-r--r--packages/core/src/tools/shell.test.ts167
-rw-r--r--packages/core/src/tools/shell.ts91
3 files changed, 251 insertions, 84 deletions
diff --git a/docs/tools/shell.md b/docs/tools/shell.md
index cc65b013..021cede1 100644
--- a/docs/tools/shell.md
+++ b/docs/tools/shell.md
@@ -64,96 +64,71 @@ run_shell_command(command="npm run dev &", description="Start development server
You can restrict the commands that can be executed by the `run_shell_command` tool by using the `coreTools` and `excludeTools` settings in your configuration file.
-- `coreTools`: If you want to restrict the `run_shell_command` tool to a specific set of commands, you can add entries to the `coreTools` list in the format `ShellTool(<command>)`. For example, `"coreTools": ["ShellTool(ls -l)"]` will only allow the `ls -l` command to be executed. If you include `ShellTool` as a general entry in the `coreTools` list, it will act as a wildcard and allow any command to be executed, even if you have other specific commands in the list.
-- `excludeTools`: If you want to block specific commands, you can add entries to the `excludeTools` list in the format `ShellTool(<command>)`. For example, `"excludeTools": ["ShellTool(rm -rf /)"]` will block the `rm -rf /` command.
+- `coreTools`: To restrict `run_shell_command` to a specific set of commands, add entries to the `coreTools` list in the format `run_shell_command(<command>)`. For example, `"coreTools": ["run_shell_command(git)"]` will only allow `git` commands. Including the generic `run_shell_command` acts as a wildcard, allowing any command not explicitly blocked.
+- `excludeTools`: To block specific commands, add entries to the `excludeTools` list in the format `run_shell_command(<command>)`. For example, `"excludeTools": ["run_shell_command(rm)"]` will block `rm` commands.
-### Command Restriction Examples
-
-Here are some examples of how to use the `coreTools` and `excludeTools` settings to control which commands can be executed.
-
-**Allow only specific commands**
-
-To allow only `ls -l` and `git status`, and block all other commands:
-
-```json
-{
- "coreTools": ["ShellTool(ls -l)", "ShellTool(git status)"]
-}
-```
-
-- `ls -l`: Allowed
-- `git status`: Allowed
-- `npm install`: Blocked
-
-**Block specific commands**
-
-To block `rm -rf /` and `npm install`, and allow all other commands:
+The validation logic is designed to be secure and flexible:
-```json
-{
- "excludeTools": ["ShellTool(rm -rf /)", "ShellTool(npm install)"]
-}
-```
+1. **Command Chaining Disabled**: The tool automatically splits commands chained with `&&`, `||`, or `;` and validates each part separately. If any part of the chain is disallowed, the entire command is blocked.
+2. **Prefix Matching**: The tool uses prefix matching. For example, if you allow `git`, you can run `git status` or `git log`.
+3. **Blocklist Precedence**: The `excludeTools` list is always checked first. If a command matches a blocked prefix, it will be denied, even if it also matches an allowed prefix in `coreTools`.
-- `rm -rf /`: Blocked
-- `npm install`: Blocked
-- `ls -l`: Allowed
+### Command Restriction Examples
-**Allow all commands**
+**Allow only specific command prefixes**
-To allow any command to be executed, you can use the `ShellTool` wildcard in `coreTools`:
+To allow only `git` and `npm` commands, and block all others:
```json
{
- "coreTools": ["ShellTool"]
+ "coreTools": ["run_shell_command(git)", "run_shell_command(npm)"]
}
```
-- `ls -l`: Allowed
+- `git status`: Allowed
- `npm install`: Allowed
-- `any other command`: Allowed
+- `ls -l`: Blocked
-**Wildcard with specific allowed commands**
+**Block specific command prefixes**
-If you include the `ShellTool` wildcard along with specific commands, the wildcard takes precedence, and all commands are allowed.
+To block `rm` and allow all other commands:
```json
{
- "coreTools": ["ShellTool", "ShellTool(ls -l)"]
+ "coreTools": ["run_shell_command"],
+ "excludeTools": ["run_shell_command(rm)"]
}
```
-- `ls -l`: Allowed
+- `rm -rf /`: Blocked
+- `git status`: Allowed
- `npm install`: Allowed
-- `any other command`: Allowed
-**Wildcard with a blocklist**
+**Blocklist takes precedence**
-You can use the `ShellTool` wildcard to allow all commands, while still blocking specific commands using `excludeTools`.
+If a command prefix is in both `coreTools` and `excludeTools`, it will be blocked.
```json
{
- "coreTools": ["ShellTool"],
- "excludeTools": ["ShellTool(rm -rf /)"]
+ "coreTools": ["run_shell_command(git)"],
+ "excludeTools": ["run_shell_command(git push)"]
}
```
-- `rm -rf /`: Blocked
-- `ls -l`: Allowed
-- `npm install`: Allowed
+- `git push origin main`: Blocked
+- `git status`: Allowed
**Block all shell commands**
-To block all shell commands, you can add the `ShellTool` wildcard to `excludeTools`:
+To block all shell commands, add the `run_shell_command` wildcard to `excludeTools`:
```json
{
- "excludeTools": ["ShellTool"]
+ "excludeTools": ["run_shell_command"]
}
```
- `ls -l`: Blocked
-- `npm install`: Blocked
- `any other command`: Blocked
## Security Note for `excludeTools`
diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts
index 2cbd0ff4..936c1f77 100644
--- a/packages/core/src/tools/shell.test.ts
+++ b/packages/core/src/tools/shell.test.ts
@@ -168,4 +168,171 @@ describe('ShellTool', () => {
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
});
+
+ it('should allow a command that starts with an allowed command prefix', async () => {
+ const config = {
+ getCoreTools: () => ['ShellTool(gh issue edit)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed(
+ 'gh issue edit 1 --add-label "kind/feature"',
+ );
+ expect(isAllowed).toBe(true);
+ });
+
+ it('should allow a command that starts with an allowed command prefix using the public-facing name', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue edit)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed(
+ 'gh issue edit 1 --add-label "kind/feature"',
+ );
+ expect(isAllowed).toBe(true);
+ });
+
+ it('should not allow a command that starts with an allowed command prefix but is chained with another command', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue edit)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue edit&&rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should not allow a command that is a prefix of an allowed command', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue edit)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should not allow a command that is a prefix of a blocked command', async () => {
+ const config = {
+ getCoreTools: () => [],
+ getExcludeTools: () => ['run_shell_command(gh issue edit)'],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue');
+ expect(isAllowed).toBe(true);
+ });
+
+ it('should not allow a command that is chained with a pipe', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue list)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue list | rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should not allow a command that is chained with a semicolon', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue list)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue list; rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should block a chained command if any part is blocked', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(echo "hello")'],
+ getExcludeTools: () => ['run_shell_command(rm)'],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('echo "hello" && rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should block a command if its prefix is on the blocklist, even if the command itself is on the allowlist', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(git push)'],
+ getExcludeTools: () => ['run_shell_command(git)'],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('git push');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should be case-sensitive in its matching', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(echo)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('ECHO "hello"');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should correctly handle commands with extra whitespace around chaining operators', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(ls -l)'],
+ getExcludeTools: () => ['run_shell_command(rm)'],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('ls -l ; rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should allow a chained command if all parts are allowed', async () => {
+ const config = {
+ getCoreTools: () => [
+ 'run_shell_command(echo)',
+ 'run_shell_command(ls -l)',
+ ],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('echo "hello" && ls -l');
+ expect(isAllowed).toBe(true);
+ });
+
+ it('should block a command with command substitution using backticks', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(echo)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('echo `rm -rf /`');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should block a command with command substitution using $()', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(echo)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('echo $(rm -rf /)');
+ expect(isAllowed).toBe(false);
+ });
+
+ it('should allow a command with I/O redirection', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(echo)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('echo "hello" > file.txt');
+ expect(isAllowed).toBe(true);
+ });
+
+ it('should not allow a command that is chained with a double pipe', async () => {
+ const config = {
+ getCoreTools: () => ['run_shell_command(gh issue list)'],
+ getExcludeTools: () => [],
+ } as unknown as Config;
+ const shellTool = new ShellTool(config);
+ const isAllowed = shellTool.isCommandAllowed('gh issue list || rm -rf /');
+ expect(isAllowed).toBe(false);
+ });
});
diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts
index a2fa5ce4..bbb998d5 100644
--- a/packages/core/src/tools/shell.ts
+++ b/packages/core/src/tools/shell.ts
@@ -99,17 +99,39 @@ Process Group PGID: Process group started or \`(none)\``,
}
isCommandAllowed(command: string): boolean {
- const normalize = (cmd: string) => cmd.trim().replace(/\s+/g, ' ');
+ // 0. Disallow command substitution
+ if (command.includes('$(') || command.includes('`')) {
+ return false;
+ }
+
+ const SHELL_TOOL_NAMES = [ShellTool.name, ShellTool.Name];
+
+ 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;
+ }
+ 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) => {
- if (tool.startsWith(`${ShellTool.name}(`) && tool.endsWith(')')) {
- return [normalize(tool.slice(ShellTool.name.length + 1, -1))];
- } else if (
- tool.startsWith(`${ShellTool.Name}(`) &&
- tool.endsWith(')')
- ) {
- return [normalize(tool.slice(ShellTool.Name.length + 1, -1))];
+ for (const toolName of SHELL_TOOL_NAMES) {
+ if (tool.startsWith(`${toolName}(`) && tool.endsWith(')')) {
+ return [normalize(tool.slice(toolName.length + 1, -1))];
+ }
}
return [];
});
@@ -117,41 +139,44 @@ Process Group PGID: Process group started or \`(none)\``,
const coreTools = this.config.getCoreTools() || [];
const excludeTools = this.config.getExcludeTools() || [];
- if (
- excludeTools.includes(ShellTool.name) ||
- excludeTools.includes(ShellTool.Name)
- ) {
+ // 1. Check if the shell tool is globally disabled.
+ if (SHELL_TOOL_NAMES.some((name) => excludeTools.includes(name))) {
return false;
}
- const blockedCommands = extractCommands(excludeTools);
- const normalizedCommand = normalize(command);
+ const blockedCommands = new Set(extractCommands(excludeTools));
+ const allowedCommands = new Set(extractCommands(coreTools));
- if (blockedCommands.includes(normalizedCommand)) {
- return false;
- }
-
- const hasSpecificCommands = coreTools.some(
- (tool) =>
- (tool.startsWith(`${ShellTool.name}(`) && tool.endsWith(')')) ||
- (tool.startsWith(`${ShellTool.Name}(`) && tool.endsWith(')')),
+ const hasSpecificAllowedCommands = allowedCommands.size > 0;
+ const isWildcardAllowed = SHELL_TOOL_NAMES.some((name) =>
+ coreTools.includes(name),
);
- if (hasSpecificCommands) {
- // If the generic `ShellTool` is also present, it acts as a wildcard,
- // allowing all commands (that are not explicitly blocked).
- if (
- coreTools.includes(ShellTool.name) ||
- coreTools.includes(ShellTool.Name)
- ) {
- return true;
+ const commandsToValidate = command.split(/&&|\|\||\||;/).map(normalize);
+
+ for (const cmd of commandsToValidate) {
+ // 2. Check if the command is on the blocklist.
+ const isBlocked = [...blockedCommands].some((blocked) =>
+ isPrefixedBy(cmd, blocked),
+ );
+ if (isBlocked) {
+ return false;
}
- // Otherwise, we are in strict allow-list mode.
- const allowedCommands = extractCommands(coreTools);
- return allowedCommands.includes(normalizedCommand);
+ // 3. If in strict allow-list mode, check if the command is permitted.
+ const isStrictAllowlist =
+ hasSpecificAllowedCommands && !isWildcardAllowed;
+ if (isStrictAllowlist) {
+ const isAllowed = [...allowedCommands].some((allowed) =>
+ isPrefixedBy(cmd, allowed),
+ );
+ if (!isAllowed) {
+ return false;
+ }
+ }
}
+ // 4. If all checks pass, the command is allowed.
return true;
}