abbyswag/qwen-code
1
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2025-12-19 09:33:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(core): improve error messages in isCommandAllowed (#3349)

Browse Source
This commit is contained in:
Yuki Okita
2025-07-07 14:03:36 +09:00
committed by GitHub
parent 20825e4114
commit 87a44ec468
2 changed files with 169 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

191
packages/core/src/tools/shell.test.ts
View File

@@ -15,8 +15,8 @@ describe('ShellTool', () => {
getExcludeTools: () => undefined,
} as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('ls -l');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('ls -l');
expect(result.allowed).toBe(true);
});
it('should allow a command if it is in the allowed list', async () => {
@@ -25,8 +25,8 @@ describe('ShellTool', () => {
getExcludeTools: () => undefined,
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('ls -l');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('ls -l');
expect(result.allowed).toBe(true);
});
it('should block a command if it is not in the allowed list', async () => {
@@ -35,8 +35,11 @@ describe('ShellTool', () => {
getExcludeTools: () => undefined,
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is not in the allowed commands list",
);
});
it('should block a command if it is in the blocked list', async () => {
@@ -45,8 +48,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should allow a command if it is not in the blocked list', async () => {
@@ -55,8 +61,8 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('ls -l');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('ls -l');
expect(result.allowed).toBe(true);
});
it('should block a command if it is in both the allowed and blocked lists', async () => {
@@ -65,8 +71,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should allow any command when ShellTool is in coreTools without specific commands', async () => {
@@ -75,8 +84,8 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('any command');
expect(result.allowed).toBe(true);
});
it('should block any command when ShellTool is in excludeTools without specific commands', async () => {
@@ -85,8 +94,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('any command');
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 () => {
@@ -95,8 +107,8 @@ describe('ShellTool', () => {
getExcludeTools: () => undefined,
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('ls -l');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('ls -l');
expect(result.allowed).toBe(true);
});
it('should block a command if it is in the blocked list using the public-facing name', async () => {
@@ -105,8 +117,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['run_shell_command(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('rm -rf /');
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 () => {
@@ -115,8 +130,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['run_shell_command'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('any command');
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 () => {
@@ -125,8 +143,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('any command');
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 () => {
@@ -135,8 +156,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('any command');
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 () => {
@@ -145,8 +169,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed(' rm -rf / ');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed(' rm -rf / ');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should allow any command when ShellTool is present with specific commands', async () => {
@@ -155,8 +182,8 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('any command');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('any command');
expect(result.allowed).toBe(true);
});
it('should block a command on the blocklist even with a wildcard allow', async () => {
@@ -165,8 +192,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['ShellTool(rm -rf /)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should allow a command that starts with an allowed command prefix', async () => {
@@ -175,10 +205,10 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed(
const result = shellTool.isCommandAllowed(
'gh issue edit 1 --add-label "kind/feature"',
);
expect(isAllowed).toBe(true);
expect(result.allowed).toBe(true);
});
it('should allow a command that starts with an allowed command prefix using the public-facing name', async () => {
@@ -187,10 +217,10 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed(
const result = shellTool.isCommandAllowed(
'gh issue edit 1 --add-label "kind/feature"',
);
expect(isAllowed).toBe(true);
expect(result.allowed).toBe(true);
});
it('should not allow a command that starts with an allowed command prefix but is chained with another command', async () => {
@@ -199,8 +229,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('gh issue edit&&rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('gh issue edit&&rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is not in the allowed commands list",
);
});
it('should not allow a command that is a prefix of an allowed command', async () => {
@@ -209,8 +242,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('gh issue');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('gh issue');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'gh issue' is not in the allowed commands list",
);
});
it('should not allow a command that is a prefix of a blocked command', async () => {
@@ -219,8 +255,8 @@ describe('ShellTool', () => {
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);
const result = shellTool.isCommandAllowed('gh issue');
expect(result.allowed).toBe(true);
});
it('should not allow a command that is chained with a pipe', async () => {
@@ -229,8 +265,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('gh issue list | rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('gh issue list | rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is not in the allowed commands list",
);
});
it('should not allow a command that is chained with a semicolon', async () => {
@@ -239,8 +278,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('gh issue list; rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('gh issue list; rm -rf /');
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 any part is blocked', async () => {
@@ -249,8 +291,11 @@ describe('ShellTool', () => {
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);
const result = shellTool.isCommandAllowed('echo "hello" && rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should block a command if its prefix is on the blocklist, even if the command itself is on the allowlist', async () => {
@@ -259,8 +304,11 @@ describe('ShellTool', () => {
getExcludeTools: () => ['run_shell_command(git)'],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('git push');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('git push');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'git push' is blocked by configuration",
);
});
it('should be case-sensitive in its matching', async () => {
@@ -269,8 +317,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('ECHO "hello"');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('ECHO "hello"');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
'Command \'ECHO "hello"\' is not in the allowed commands list',
);
});
it('should correctly handle commands with extra whitespace around chaining operators', async () => {
@@ -279,8 +330,11 @@ describe('ShellTool', () => {
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);
const result = shellTool.isCommandAllowed('ls -l ; rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is blocked by configuration",
);
});
it('should allow a chained command if all parts are allowed', async () => {
@@ -292,8 +346,8 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('echo "hello" && ls -l');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('echo "hello" && ls -l');
expect(result.allowed).toBe(true);
});
it('should block a command with command substitution using backticks', async () => {
@@ -302,8 +356,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('echo `rm -rf /`');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('echo `rm -rf /`');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
'Command substitution using backticks is not allowed for security reasons',
);
});
it('should block a command with command substitution using $()', async () => {
@@ -312,8 +369,11 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('echo $(rm -rf /)');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('echo $(rm -rf /)');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
'Command substitution using $() is not allowed for security reasons',
);
});
it('should allow a command with I/O redirection', async () => {
@@ -322,8 +382,8 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('echo "hello" > file.txt');
expect(isAllowed).toBe(true);
const result = shellTool.isCommandAllowed('echo "hello" > file.txt');
expect(result.allowed).toBe(true);
});
it('should not allow a command that is chained with a double pipe', async () => {
@@ -332,7 +392,10 @@ describe('ShellTool', () => {
getExcludeTools: () => [],
} as unknown as Config;
const shellTool = new ShellTool(config);
const isAllowed = shellTool.isCommandAllowed('gh issue list || rm -rf /');
expect(isAllowed).toBe(false);
const result = shellTool.isCommandAllowed('gh issue list || rm -rf /');
expect(result.allowed).toBe(false);
expect(result.reason).toBe(
"Command 'rm -rf /' is not in the allowed commands list",
);
});
});