Skip to content

Commit 41fa9d3

Browse files
mrubenscte
mrubens
authored and
cte
committed
Fix Gemini command escaping (#2120)
1 parent 2afc771 commit 41fa9d3

File tree

2 files changed

+273
-1
lines changed

2 files changed

+273
-1
lines changed

‎src/core/tools/__tests__/executeCommandTool.test.ts‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
// npx jest src/core/tools/__tests__/executeCommandTool.test.ts
2+
3+
import { describe, expect, it, jest, beforeEach } from "@jest/globals"
4+
import { executeCommandTool } from "../executeCommandTool"
5+
import { Cline } from "../../Cline"
6+
import { ToolUse } from "../../assistant-message"
7+
import { formatResponse } from "../../prompts/responses"
8+
import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../types"
9+
import { ClineAsk } from "../../../schemas"
10+
11+
// Mock dependencies
12+
jest.mock("../../Cline")
13+
jest.mock("../../prompts/responses")
14+
15+
describe("executeCommandTool", () => {
16+
// Setup common test variables
17+
let mockCline: jest.Mocked<Partial<Cline>> & { consecutiveMistakeCount: number; didRejectTool: boolean }
18+
let mockAskApproval: jest.Mock
19+
let mockHandleError: jest.Mock
20+
let mockPushToolResult: jest.Mock
21+
let mockRemoveClosingTag: jest.Mock
22+
let mockToolUse: ToolUse
23+
24+
beforeEach(() => {
25+
// Reset mocks
26+
jest.clearAllMocks()
27+
28+
// Create mock implementations with eslint directives to handle the type issues
29+
mockCline = {
30+
// @ts-expect-error - Jest mock function type issues
31+
ask: jest.fn().mockResolvedValue(undefined),
32+
// @ts-expect-error - Jest mock function type issues
33+
say: jest.fn().mockResolvedValue(undefined),
34+
// @ts-expect-error - Jest mock function type issues
35+
sayAndCreateMissingParamError: jest.fn().mockResolvedValue("Missing parameter error"),
36+
// @ts-expect-error - Jest mock function type issues
37+
executeCommandTool: jest.fn().mockResolvedValue([false, "Command executed"]),
38+
consecutiveMistakeCount: 0,
39+
didRejectTool: false,
40+
rooIgnoreController: {
41+
// @ts-expect-error - Jest mock function type issues
42+
validateCommand: jest.fn().mockReturnValue(null),
43+
},
44+
}
45+
46+
// @ts-expect-error - Jest mock function type issues
47+
mockAskApproval = jest.fn().mockResolvedValue(true)
48+
// @ts-expect-error - Jest mock function type issues
49+
mockHandleError = jest.fn().mockResolvedValue(undefined)
50+
mockPushToolResult = jest.fn()
51+
mockRemoveClosingTag = jest.fn().mockReturnValue("command")
52+
53+
// Create a mock tool use object
54+
mockToolUse = {
55+
type: "tool_use",
56+
name: "execute_command",
57+
params: {
58+
command: "echo test",
59+
},
60+
partial: false,
61+
}
62+
})
63+
64+
/**
65+
* Tests for HTML entity unescaping in commands
66+
* This verifies that HTML entities are properly converted to their actual characters
67+
* before the command is executed
68+
*/
69+
describe("HTML entity unescaping", () => {
70+
it("should unescape &lt; to < character in commands", async () => {
71+
// Setup
72+
mockToolUse.params.command = "echo &lt;test&gt;"
73+
74+
// Execute
75+
await executeCommandTool(
76+
mockCline as unknown as Cline,
77+
mockToolUse,
78+
mockAskApproval as unknown as AskApproval,
79+
mockHandleError as unknown as HandleError,
80+
mockPushToolResult as unknown as PushToolResult,
81+
mockRemoveClosingTag as unknown as RemoveClosingTag,
82+
)
83+
84+
// Verify
85+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo <test>")
86+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo <test>", undefined)
87+
})
88+
89+
it("should unescape &gt; to > character in commands", async () => {
90+
// Setup
91+
mockToolUse.params.command = "echo test &gt; output.txt"
92+
93+
// Execute
94+
await executeCommandTool(
95+
mockCline as unknown as Cline,
96+
mockToolUse,
97+
mockAskApproval as unknown as AskApproval,
98+
mockHandleError as unknown as HandleError,
99+
mockPushToolResult as unknown as PushToolResult,
100+
mockRemoveClosingTag as unknown as RemoveClosingTag,
101+
)
102+
103+
// Verify
104+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test > output.txt")
105+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test > output.txt", undefined)
106+
})
107+
108+
it("should unescape &amp; to & character in commands", async () => {
109+
// Setup
110+
mockToolUse.params.command = "echo foo &amp;&amp; echo bar"
111+
112+
// Execute
113+
await executeCommandTool(
114+
mockCline as unknown as Cline,
115+
mockToolUse,
116+
mockAskApproval as unknown as AskApproval,
117+
mockHandleError as unknown as HandleError,
118+
mockPushToolResult as unknown as PushToolResult,
119+
mockRemoveClosingTag as unknown as RemoveClosingTag,
120+
)
121+
122+
// Verify
123+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo foo && echo bar")
124+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo foo && echo bar", undefined)
125+
})
126+
127+
it("should handle multiple mixed HTML entities in commands", async () => {
128+
// Setup
129+
mockToolUse.params.command = "grep -E 'pattern' &lt;file.txt &gt;output.txt 2&gt;&amp;1"
130+
131+
// Execute
132+
await executeCommandTool(
133+
mockCline as unknown as Cline,
134+
mockToolUse,
135+
mockAskApproval as unknown as AskApproval,
136+
mockHandleError as unknown as HandleError,
137+
mockPushToolResult as unknown as PushToolResult,
138+
mockRemoveClosingTag as unknown as RemoveClosingTag,
139+
)
140+
141+
// Verify
142+
const expectedCommand = "grep -E 'pattern' <file.txt >output.txt 2>&1"
143+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommand)
144+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith(expectedCommand, undefined)
145+
})
146+
})
147+
148+
// Other functionality tests
149+
describe("Basic functionality", () => {
150+
it("should execute a command normally without HTML entities", async () => {
151+
// Setup
152+
mockToolUse.params.command = "echo test"
153+
154+
// Execute
155+
await executeCommandTool(
156+
mockCline as unknown as Cline,
157+
mockToolUse,
158+
mockAskApproval as unknown as AskApproval,
159+
mockHandleError as unknown as HandleError,
160+
mockPushToolResult as unknown as PushToolResult,
161+
mockRemoveClosingTag as unknown as RemoveClosingTag,
162+
)
163+
164+
// Verify
165+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test")
166+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test", undefined)
167+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
168+
})
169+
170+
it("should pass along custom working directory if provided", async () => {
171+
// Setup
172+
mockToolUse.params.command = "echo test"
173+
mockToolUse.params.cwd = "/custom/path"
174+
175+
// Execute
176+
await executeCommandTool(
177+
mockCline as unknown as Cline,
178+
mockToolUse,
179+
mockAskApproval as unknown as AskApproval,
180+
mockHandleError as unknown as HandleError,
181+
mockPushToolResult as unknown as PushToolResult,
182+
mockRemoveClosingTag as unknown as RemoveClosingTag,
183+
)
184+
185+
// Verify
186+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test", "/custom/path")
187+
})
188+
})
189+
190+
describe("Error handling", () => {
191+
it("should handle missing command parameter", async () => {
192+
// Setup
193+
mockToolUse.params.command = undefined
194+
195+
// Execute
196+
await executeCommandTool(
197+
mockCline as unknown as Cline,
198+
mockToolUse,
199+
mockAskApproval as unknown as AskApproval,
200+
mockHandleError as unknown as HandleError,
201+
mockPushToolResult as unknown as PushToolResult,
202+
mockRemoveClosingTag as unknown as RemoveClosingTag,
203+
)
204+
205+
// Verify
206+
expect(mockCline.consecutiveMistakeCount).toBe(1)
207+
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("execute_command", "command")
208+
expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error")
209+
expect(mockAskApproval).not.toHaveBeenCalled()
210+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
211+
})
212+
213+
it("should handle command rejection", async () => {
214+
// Setup
215+
mockToolUse.params.command = "echo test"
216+
// @ts-expect-error - Jest mock function type issues
217+
mockAskApproval.mockResolvedValue(false)
218+
219+
// Execute
220+
await executeCommandTool(
221+
mockCline as unknown as Cline,
222+
mockToolUse,
223+
mockAskApproval as unknown as AskApproval,
224+
mockHandleError as unknown as HandleError,
225+
mockPushToolResult as unknown as PushToolResult,
226+
mockRemoveClosingTag as unknown as RemoveClosingTag,
227+
)
228+
229+
// Verify
230+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test")
231+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
232+
expect(mockPushToolResult).not.toHaveBeenCalled()
233+
})
234+
235+
it("should handle rooignore validation failures", async () => {
236+
// Setup
237+
mockToolUse.params.command = "cat .env"
238+
// Override the validateCommand mock to return a filename
239+
const validateCommandMock = jest.fn().mockReturnValue(".env")
240+
mockCline.rooIgnoreController = {
241+
// @ts-expect-error - Jest mock function type issues
242+
validateCommand: validateCommandMock,
243+
}
244+
245+
const mockRooIgnoreError = "RooIgnore error"
246+
;(formatResponse.rooIgnoreError as jest.Mock).mockReturnValue(mockRooIgnoreError)
247+
;(formatResponse.toolError as jest.Mock).mockReturnValue("Tool error")
248+
249+
// Execute
250+
await executeCommandTool(
251+
mockCline as unknown as Cline,
252+
mockToolUse,
253+
mockAskApproval as unknown as AskApproval,
254+
mockHandleError as unknown as HandleError,
255+
mockPushToolResult as unknown as PushToolResult,
256+
mockRemoveClosingTag as unknown as RemoveClosingTag,
257+
)
258+
259+
// Verify
260+
expect(validateCommandMock).toHaveBeenCalledWith("cat .env")
261+
expect(mockCline.say).toHaveBeenCalledWith("rooignore_error", ".env")
262+
expect(formatResponse.rooIgnoreError).toHaveBeenCalledWith(".env")
263+
expect(formatResponse.toolError).toHaveBeenCalledWith(mockRooIgnoreError)
264+
expect(mockPushToolResult).toHaveBeenCalled()
265+
expect(mockAskApproval).not.toHaveBeenCalled()
266+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
267+
})
268+
})
269+
})

‎src/core/tools/executeCommandTool.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export async function executeCommandTool(
1111
pushToolResult: PushToolResult,
1212
removeClosingTag: RemoveClosingTag,
1313
) {
14-
const command: string | undefined = block.params.command
14+
let command: string | undefined = block.params.command
1515
const customCwd: string | undefined = block.params.cwd
1616
try {
1717
if (block.partial) {
@@ -32,6 +32,9 @@ export async function executeCommandTool(
3232
return
3333
}
3434

35+
// unescape html entities (e.g. &lt; -> <)
36+
command = command.replace(/&lt;/g, "<").replace(/&gt;/g, ">").replace(/&amp;/g, "&")
37+
3538
cline.consecutiveMistakeCount = 0
3639

3740
const didApprove = await askApproval("command", command)

0 commit comments

Comments
 (0)