Merge pull request #5 from RooVetGit/feature/autoCommandAllowList

Adding allow-list for auto-executable commands
This commit is contained in:
John Stearns
2024-11-05 13:24:16 -08:00
committed by GitHub
6 changed files with 134 additions and 12 deletions

View File

@@ -4,15 +4,18 @@
### Packaging ### Packaging
1. Bump the version in `package.json` 1. Bump the version in `package.json`
2. Build the VSIX file: 2. Remove the old VSIX file:
```bash
rm bin/roo-cline-*.vsix
```
3. Build the VSIX file:
```bash ```bash
npm run vsix npm run vsix
``` ```
3. The new VSIX file will be created in the `bin/` directory 4. The new VSIX file will be created in the `bin/` directory
4. Commit the new VSIX file to git and remove the old one: 5. Commit the new VSIX file to git:
```bash ```bash
git rm bin/roo-cline-*.vsix git add bin/*.vsix
git add bin/roo-cline-<new_version>.vsix
git commit -m "chore: update VSIX to version <new_version>" git commit -m "chore: update VSIX to version <new_version>"
``` ```

Binary file not shown.

BIN
bin/roo-cline-1.0.3.vsix Normal file

Binary file not shown.

View File

@@ -2,7 +2,7 @@
"name": "roo-cline", "name": "roo-cline",
"displayName": "Roo Cline", "displayName": "Roo Cline",
"description": "Autonomous coding agent right in your IDE, capable of creating/editing files, running commands, using the browser, and more with your permission every step of the way.", "description": "Autonomous coding agent right in your IDE, capable of creating/editing files, running commands, using the browser, and more with your permission every step of the way.",
"version": "1.0.2", "version": "1.0.3",
"icon": "assets/icons/icon.png", "icon": "assets/icons/icon.png",
"galleryBanner": { "galleryBanner": {
"color": "#617A91", "color": "#617A91",

View File

@@ -56,6 +56,16 @@ type UserContent = Array<
Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolUseBlockParam | Anthropic.ToolResultBlockParam Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolUseBlockParam | Anthropic.ToolResultBlockParam
> >
// Add near the top of the file, after imports:
const ALLOWED_AUTO_EXECUTE_COMMANDS = [
'npm',
'npx',
'tsc',
'git log',
'git diff',
'list'
] as const
export class Cline { export class Cline {
readonly taskId: string readonly taskId: string
api: ApiHandler api: ApiHandler
@@ -124,6 +134,25 @@ export class Cline {
} }
} }
protected isAllowedCommand(command?: string): boolean {
if (!command) {
return false;
}
// Check for command chaining characters
if (command.includes('&&') ||
command.includes(';') ||
command.includes('||') ||
command.includes('|') ||
command.includes('$(') ||
command.includes('`')) {
return false;
}
const trimmedCommand = command.trim().toLowerCase();
return ALLOWED_AUTO_EXECUTE_COMMANDS.some(prefix =>
trimmedCommand.startsWith(prefix.toLowerCase())
);
}
// Storing task to disk for history // Storing task to disk for history
private async ensureTaskDirectoryExists(): Promise<string> { private async ensureTaskDirectoryExists(): Promise<string> {
@@ -555,8 +584,8 @@ export class Cline {
: [{ type: "text", text: lastMessage.content }] : [{ type: "text", text: lastMessage.content }]
if (previousAssistantMessage && previousAssistantMessage.role === "assistant") { if (previousAssistantMessage && previousAssistantMessage.role === "assistant") {
const assistantContent = Array.isArray(previousAssistantMessage.content) const assistantContent = Array.isArray(previousAssistantMessage.content)
? previousAssistantMessage.content ? previousAssistantMessage.content
: [{ type: "text", text: previousAssistantMessage.content }] : [{ type: "text", text: previousAssistantMessage.content }]
const toolUseBlocks = assistantContent.filter( const toolUseBlocks = assistantContent.filter(
(block) => block.type === "tool_use" (block) => block.type === "tool_use"
@@ -839,7 +868,7 @@ export class Cline {
// (have to do this for partial and complete since sending content in thinking tags to markdown renderer will automatically be removed) // (have to do this for partial and complete since sending content in thinking tags to markdown renderer will automatically be removed)
// Remove end substrings of <thinking or </thinking (below xml parsing is only for opening tags) // Remove end substrings of <thinking or </thinking (below xml parsing is only for opening tags)
// (this is done with the xml parsing below now, but keeping here for reference) // (this is done with the xml parsing below now, but keeping here for reference)
// content = content.replace(/<\/?t(?:h(?:i(?:n(?:k(?:i(?:n(?:g)?)?)?)?)?)?)?$/, "") // content = content.replace(/<\/?t(?:h(?:i(?:n(?:k(?:i(?:n(?:g)?)?)?)?)?$/, "")
// Remove all instances of <thinking> (with optional line break after) and </thinking> (with optional line break before) // Remove all instances of <thinking> (with optional line break after) and </thinking> (with optional line break before)
// - Needs to be separate since we dont want to remove the line break before the first tag // - Needs to be separate since we dont want to remove the line break before the first tag
// - Needs to happen before the xml parsing below // - Needs to happen before the xml parsing below
@@ -1503,7 +1532,7 @@ export class Cline {
const command: string | undefined = block.params.command const command: string | undefined = block.params.command
try { try {
if (block.partial) { if (block.partial) {
if (this.alwaysAllowExecute) { if (this.alwaysAllowExecute && this.isAllowedCommand(command)) {
await this.say("command", command, undefined, block.partial) await this.say("command", command, undefined, block.partial)
} else { } else {
await this.ask("command", removeClosingTag("command", command), block.partial).catch( await this.ask("command", removeClosingTag("command", command), block.partial).catch(
@@ -1520,7 +1549,9 @@ export class Cline {
break break
} }
this.consecutiveMistakeCount = 0 this.consecutiveMistakeCount = 0
const didApprove = this.alwaysAllowExecute || (await askApproval("command", command))
const didApprove = (this.alwaysAllowExecute && this.isAllowedCommand(command)) ||
(await askApproval("command", command))
if (!didApprove) { if (!didApprove) {
break break
} }

View File

@@ -318,5 +318,93 @@ describe('Cline', () => {
expect(writeDisabledCline.alwaysAllowWrite).toBe(false); expect(writeDisabledCline.alwaysAllowWrite).toBe(false);
// The write operation would require approval in actual implementation // The write operation would require approval in actual implementation
}); });
}); });
describe('isAllowedCommand', () => {
let cline: any
beforeEach(() => {
// Create a more complete mock provider
const mockProvider = {
context: {
globalStorageUri: { fsPath: '/mock/path' }
},
postStateToWebview: jest.fn(),
postMessageToWebview: jest.fn(),
updateTaskHistory: jest.fn()
}
// Mock the required dependencies
const mockApiConfig = {
getModel: () => ({
id: 'claude-3-sonnet',
info: { supportsComputerUse: true }
})
}
// Create test instance with mocked constructor params
cline = new Cline(
mockProvider as any,
mockApiConfig as any,
undefined, // customInstructions
false, // alwaysAllowReadOnly
false, // alwaysAllowWrite
false, // alwaysAllowExecute
'test task' // task
)
// Mock internal methods that are called during initialization
cline.initiateTaskLoop = jest.fn()
cline.say = jest.fn()
cline.addToClineMessages = jest.fn()
cline.overwriteClineMessages = jest.fn()
cline.addToApiConversationHistory = jest.fn()
cline.overwriteApiConversationHistory = jest.fn()
})
test('returns true for allowed commands', () => {
expect(cline.isAllowedCommand('npm install')).toBe(true)
expect(cline.isAllowedCommand('npx create-react-app')).toBe(true)
expect(cline.isAllowedCommand('tsc --watch')).toBe(true)
expect(cline.isAllowedCommand('git log --oneline')).toBe(true)
expect(cline.isAllowedCommand('git diff main')).toBe(true)
})
test('returns true regardless of case or whitespace', () => {
expect(cline.isAllowedCommand('NPM install')).toBe(true)
expect(cline.isAllowedCommand(' npm install')).toBe(true)
expect(cline.isAllowedCommand('GIT DIFF')).toBe(true)
})
test('returns false for non-allowed commands', () => {
expect(cline.isAllowedCommand('rm -rf /')).toBe(false)
expect(cline.isAllowedCommand('git push')).toBe(false)
expect(cline.isAllowedCommand('git commit')).toBe(false)
expect(cline.isAllowedCommand('curl http://example.com')).toBe(false)
})
test('returns false for undefined or empty commands', () => {
expect(cline.isAllowedCommand()).toBe(false)
expect(cline.isAllowedCommand('')).toBe(false)
expect(cline.isAllowedCommand(' ')).toBe(false)
})
test('returns false for commands with chaining operators', () => {
const maliciousCommands = [
'npm install && rm -rf /',
'git status; dangerous-command',
'git log || evil-script',
'git status | malicious-pipe',
'git log $(evil-command)',
'git status `rm -rf /`',
'npm install && echo "malicious"',
'git status; curl http://evil.com',
'tsc --watch || wget malware',
];
maliciousCommands.forEach(cmd => {
expect(cline.isAllowedCommand(cmd)).toBe(false);
});
});
})
}); });