summaryrefslogtreecommitdiff
path: root/packages/core/src
diff options
context:
space:
mode:
Diffstat (limited to 'packages/core/src')
-rw-r--r--packages/core/src/tools/shell.test.ts167
-rw-r--r--packages/core/src/tools/shell.ts91
2 files changed, 225 insertions, 33 deletions
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;
}
generated by cgit v1.2.3 (git 2.51.0) at 2025-11-12 13:03:34 +0000