Skip to content

Commit b54f640

Browse files
committed
feat: add command timeout allowlist with IPC support
- Add commandTimeoutAllowlist configuration option to exclude specific command prefixes from timeout - Update global settings schema to include the new allowlist setting - Modify executeCommandTool to check allowlist before applying timeout restrictions - Add VSCode configuration and localization support for the new setting - Include comprehensive tests for allowlist functionality - IPC support included through global settings integration
1 parent a6e16e8 commit b54f640

File tree

5 files changed

+158
-2
lines changed

5 files changed

+158
-2
lines changed

packages/types/src/global-settings.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export const globalSettingsSchema = z.object({
5151
allowedCommands: z.array(z.string()).optional(),
5252
deniedCommands: z.array(z.string()).optional(),
5353
commandExecutionTimeout: z.number().optional(),
54+
commandTimeoutAllowlist: z.array(z.string()).optional(),
5455
preventCompletionWithOpenTodos: z.boolean().optional(),
5556
allowedMaxRequests: z.number().nullish(),
5657
autoCondenseContext: z.boolean().optional(),
@@ -203,6 +204,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
203204
followupAutoApproveTimeoutMs: 0,
204205
allowedCommands: ["*"],
205206
commandExecutionTimeout: 30_000,
207+
commandTimeoutAllowlist: [],
206208
preventCompletionWithOpenTodos: false,
207209

208210
browserToolEnabled: false,

src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,4 +186,139 @@ describe("Command Execution Timeout Integration", () => {
186186
expect(result[0]).toBe(false) // Not rejected
187187
expect(result[1]).not.toContain("terminated after exceeding")
188188
})
189+
190+
describe("Command Timeout Allowlist", () => {
191+
beforeEach(() => {
192+
// Reset mocks for allowlist tests
193+
vitest.clearAllMocks()
194+
;(fs.access as any).mockResolvedValue(undefined)
195+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal)
196+
})
197+
198+
it("should skip timeout for commands in allowlist", async () => {
199+
// Mock VSCode configuration with timeout and allowlist
200+
const mockGetConfiguration = vitest.fn().mockReturnValue({
201+
get: vitest.fn().mockImplementation((key: string) => {
202+
if (key === "commandExecutionTimeout") return 1 // 1 second timeout
203+
if (key === "commandTimeoutAllowlist") return ["npm", "git"]
204+
return undefined
205+
}),
206+
})
207+
;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
208+
209+
const options: ExecuteCommandOptions = {
210+
executionId: "test-execution",
211+
command: "npm install",
212+
// Should be overridden to 0 due to allowlist
213+
}
214+
215+
// Create a process that would timeout if not allowlisted
216+
const longRunningProcess = new Promise((resolve) => {
217+
setTimeout(resolve, 2000) // 2 seconds, longer than 1 second timeout
218+
})
219+
mockTerminal.runCommand.mockReturnValue(longRunningProcess)
220+
221+
const result = await executeCommand(mockTask as Task, options)
222+
223+
// Should complete successfully without timeout because "npm" is in allowlist
224+
expect(result[0]).toBe(false) // Not rejected
225+
expect(result[1]).not.toContain("terminated after exceeding")
226+
}, 3000)
227+
228+
it("should apply timeout for commands not in allowlist", async () => {
229+
// Mock VSCode configuration with timeout and allowlist
230+
const mockGetConfiguration = vitest.fn().mockReturnValue({
231+
get: vitest.fn().mockImplementation((key: string) => {
232+
if (key === "commandExecutionTimeout") return 1 // 1 second timeout
233+
if (key === "commandTimeoutAllowlist") return ["npm", "git"]
234+
return undefined
235+
}),
236+
})
237+
;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
238+
239+
const options: ExecuteCommandOptions = {
240+
executionId: "test-execution",
241+
command: "sleep 10", // Not in allowlist
242+
}
243+
244+
// Create a process that never resolves
245+
const neverResolvingProcess = new Promise(() => {})
246+
;(neverResolvingProcess as any).abort = vitest.fn()
247+
mockTerminal.runCommand.mockReturnValue(neverResolvingProcess)
248+
249+
const result = await executeCommand(mockTask as Task, options)
250+
251+
// Should timeout because "sleep" is not in allowlist
252+
expect(result[0]).toBe(false) // Not rejected by user
253+
expect(result[1]).toContain("terminated after exceeding")
254+
}, 3000)
255+
256+
it("should handle empty allowlist", async () => {
257+
// Mock VSCode configuration with timeout and empty allowlist
258+
const mockGetConfiguration = vitest.fn().mockReturnValue({
259+
get: vitest.fn().mockImplementation((key: string) => {
260+
if (key === "commandExecutionTimeout") return 1 // 1 second timeout
261+
if (key === "commandTimeoutAllowlist") return []
262+
return undefined
263+
}),
264+
})
265+
;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
266+
267+
const options: ExecuteCommandOptions = {
268+
executionId: "test-execution",
269+
command: "npm install",
270+
}
271+
272+
// Create a process that never resolves
273+
const neverResolvingProcess = new Promise(() => {})
274+
;(neverResolvingProcess as any).abort = vitest.fn()
275+
mockTerminal.runCommand.mockReturnValue(neverResolvingProcess)
276+
277+
const result = await executeCommand(mockTask as Task, options)
278+
279+
// Should timeout because allowlist is empty
280+
expect(result[0]).toBe(false) // Not rejected by user
281+
expect(result[1]).toContain("terminated after exceeding")
282+
}, 3000)
283+
284+
it("should match command prefixes correctly", async () => {
285+
// Mock VSCode configuration with timeout and allowlist
286+
const mockGetConfiguration = vitest.fn().mockReturnValue({
287+
get: vitest.fn().mockImplementation((key: string) => {
288+
if (key === "commandExecutionTimeout") return 1 // 1 second timeout
289+
if (key === "commandTimeoutAllowlist") return ["git log", "npm run"]
290+
return undefined
291+
}),
292+
})
293+
;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
294+
295+
// Test exact prefix match
296+
const options1: ExecuteCommandOptions = {
297+
executionId: "test-execution-1",
298+
command: "git log --oneline",
299+
}
300+
301+
// Test partial prefix match (should not match)
302+
const options2: ExecuteCommandOptions = {
303+
executionId: "test-execution-2",
304+
command: "git status", // "git" alone is not in allowlist, only "git log"
305+
}
306+
307+
const longRunningProcess = new Promise((resolve) => {
308+
setTimeout(resolve, 2000) // 2 seconds
309+
})
310+
const neverResolvingProcess = new Promise(() => {})
311+
;(neverResolvingProcess as any).abort = vitest.fn()
312+
313+
// First command should not timeout (matches "git log" prefix)
314+
mockTerminal.runCommand.mockReturnValueOnce(longRunningProcess)
315+
const result1 = await executeCommand(mockTask as Task, options1)
316+
expect(result1[1]).not.toContain("terminated after exceeding")
317+
318+
// Second command should timeout (doesn't match any prefix exactly)
319+
mockTerminal.runCommand.mockReturnValueOnce(neverResolvingProcess)
320+
const result2 = await executeCommand(mockTask as Task, options2)
321+
expect(result2[1]).toContain("terminated after exceeding")
322+
}, 5000)
323+
})
189324
})

src/core/tools/executeCommandTool.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,18 @@ export async function executeCommandTool(
7070
.getConfiguration(Package.name)
7171
.get<number>("commandExecutionTimeout", 0)
7272

73-
// Convert seconds to milliseconds for internal use
74-
const commandExecutionTimeout = commandExecutionTimeoutSeconds * 1000
73+
// Get command timeout allowlist from VSCode configuration
74+
const commandTimeoutAllowlist = vscode.workspace
75+
.getConfiguration(Package.name)
76+
.get<string[]>("commandTimeoutAllowlist", [])
77+
78+
// Check if command matches any prefix in the allowlist
79+
const isCommandAllowlisted = commandTimeoutAllowlist.some(prefix =>
80+
command.startsWith(prefix.trim())
81+
)
82+
83+
// Convert seconds to milliseconds for internal use, but skip timeout if command is allowlisted
84+
const commandExecutionTimeout = isCommandAllowlisted ? 0 : commandExecutionTimeoutSeconds * 1000
7585

7686
const options: ExecuteCommandOptions = {
7787
executionId,

src/package.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,14 @@
345345
"maximum": 600,
346346
"description": "%commands.commandExecutionTimeout.description%"
347347
},
348+
"roo-cline.commandTimeoutAllowlist": {
349+
"type": "array",
350+
"items": {
351+
"type": "string"
352+
},
353+
"default": [],
354+
"description": "%commands.commandTimeoutAllowlist.description%"
355+
},
348356
"roo-cline.preventCompletionWithOpenTodos": {
349357
"type": "boolean",
350358
"default": false,

src/package.nls.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"commands.allowedCommands.description": "Commands that can be auto-executed when 'Always approve execute operations' is enabled",
3030
"commands.deniedCommands.description": "Command prefixes that will be automatically denied without asking for approval. In case of conflicts with allowed commands, the longest prefix match takes precedence. Add * to deny all commands.",
3131
"commands.commandExecutionTimeout.description": "Maximum time in seconds to wait for command execution to complete before timing out (0 = no timeout, 1-600s, default: 0s)",
32+
"commands.commandTimeoutAllowlist.description": "Command prefixes that are excluded from the command execution timeout. Commands matching these prefixes will run without timeout restrictions.",
3233
"commands.preventCompletionWithOpenTodos.description": "Prevent task completion when there are incomplete todos in the todo list",
3334
"settings.vsCodeLmModelSelector.description": "Settings for VSCode Language Model API",
3435
"settings.vsCodeLmModelSelector.vendor.description": "The vendor of the language model (e.g. copilot)",

0 commit comments

Comments
 (0)