Skip to content

Commit 721cc59

Browse files
mrubensmrubens
committed
PR feedback
1 parent 93780ae commit 721cc59

File tree

2 files changed

+74
-43
lines changed

2 files changed

+74
-43
lines changed

‎webview-ui/src/utils/__tests__/command-validation.spec.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@ describe("Command Validation", () => {
3939
// Test $[] arithmetic expansion detection
4040
expect(parseCommand("echo $[1 + 2]")).toEqual(["echo $[1 + 2]"])
4141

42-
// Verify containsSubshell detects $[] pattern
43-
expect(containsSubshell("echo $[1 + 2]")).toBe(true)
44-
expect(containsSubshell("echo $(date)")).toBe(true)
45-
expect(containsSubshell("echo `date`")).toBe(true)
46-
expect(containsSubshell("diff <(sort f1) <(sort f2)")).toBe(true)
47-
expect(containsSubshell("echo hello")).toBe(false)
42+
// Verify containsSubshell detects all subshell patterns
43+
expect(containsSubshell("echo $[1 + 2]")).toBe(true) // $[] arithmetic expansion
44+
expect(containsSubshell("echo $((1 + 2))")).toBe(true) // $(()) arithmetic expansion
45+
expect(containsSubshell("echo $(date)")).toBe(true) // $() command substitution
46+
expect(containsSubshell("echo `date`")).toBe(true) // backtick substitution
47+
expect(containsSubshell("diff <(sort f1) <(sort f2)")).toBe(true) // process substitution
48+
expect(containsSubshell("echo hello")).toBe(false) // no subshells
4849
})
4950

5051
it("handles empty and whitespace input", () => {
@@ -968,9 +969,9 @@ describe("Unified Command Decision Functions", () => {
968969
// Multiple subshells, one with denied prefix
969970
expect(validator.validateCommand("echo $(date) $(rm file)")).toBe("auto_deny")
970971

971-
// Nested subshells - inner commands are extracted and not in allowlist
972+
// Nested subshells - validates individual parsed commands
972973
expect(validator.validateCommand("echo $(echo $(date))")).toBe("ask_user")
973-
expect(validator.validateCommand("echo $(echo $(rm file))")).toBe("ask_user") // complex nested parsing results in mixed decisions
974+
expect(validator.validateCommand("echo $(echo $(rm file))")).toBe("ask_user") // complex nested parsing with mixed validation results
974975
})
975976

976977
it("handles complex commands with subshells", () => {

‎webview-ui/src/utils/command-validation.ts‎

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,19 @@ type ShellToken = string | { op: string } | { command: string }
6666
* - >() - process substitution (output)
6767
* - $(()) - arithmetic expansion
6868
* - $[] - arithmetic expansion (alternative syntax)
69+
*
70+
* @example
71+
* ```typescript
72+
* containsSubshell("echo $(date)") // true - command substitution
73+
* containsSubshell("echo `date`") // true - backtick substitution
74+
* containsSubshell("diff <(sort f1)") // true - process substitution
75+
* containsSubshell("echo $((1+2))") // true - arithmetic expansion
76+
* containsSubshell("echo $[1+2]") // true - arithmetic expansion (alt)
77+
* containsSubshell("echo hello") // false - no subshells
78+
* ```
6979
*/
7080
export function containsSubshell(source: string): boolean {
71-
return /(\$\()|`|<\(|>\(|\$\[/.test(source)
81+
return /(\$\()|`|(<\(|>\()|(\$\(\()|(\$\[)/.test(source)
7282
}
7383

7484
/**
@@ -102,6 +112,36 @@ export function parseCommand(command: string): string[] {
102112
return allCommands
103113
}
104114

115+
/**
116+
* Helper function to restore placeholders in a command string
117+
*/
118+
function restorePlaceholders(
119+
command: string,
120+
quotes: string[],
121+
redirections: string[],
122+
arrayIndexing: string[],
123+
arithmeticExpressions: string[],
124+
parameterExpansions: string[],
125+
variables: string[],
126+
subshells: string[],
127+
): string {
128+
let result = command
129+
// Restore quotes
130+
result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)])
131+
// Restore redirections
132+
result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)])
133+
// Restore array indexing expressions
134+
result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)])
135+
// Restore arithmetic expressions
136+
result = result.replace(/__ARITH_(\d+)__/g, (_, i) => arithmeticExpressions[parseInt(i)])
137+
// Restore parameter expansions
138+
result = result.replace(/__PARAM_(\d+)__/g, (_, i) => parameterExpansions[parseInt(i)])
139+
// Restore variable references
140+
result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
141+
result = result.replace(/__SUBSH_(\d+)__/g, (_, i) => subshells[parseInt(i)])
142+
return result
143+
}
144+
105145
/**
106146
* Parse a single line of commands (internal helper function)
107147
*/
@@ -193,23 +233,18 @@ function parseCommandLine(command: string): string[] {
193233
.filter((cmd) => cmd.length > 0)
194234

195235
// Restore all placeholders for each command
196-
return fallbackCommands.map((cmd) => {
197-
let result = cmd
198-
// Restore quotes
199-
result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)])
200-
// Restore redirections
201-
result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)])
202-
// Restore array indexing expressions
203-
result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)])
204-
// Restore arithmetic expressions
205-
result = result.replace(/__ARITH_(\d+)__/g, (_, i) => arithmeticExpressions[parseInt(i)])
206-
// Restore parameter expansions
207-
result = result.replace(/__PARAM_(\d+)__/g, (_, i) => parameterExpansions[parseInt(i)])
208-
// Restore variable references
209-
result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
210-
result = result.replace(/__SUBSH_(\d+)__/g, (_, i) => subshells[parseInt(i)])
211-
return result
212-
})
236+
return fallbackCommands.map((cmd) =>
237+
restorePlaceholders(
238+
cmd,
239+
quotes,
240+
redirections,
241+
arrayIndexing,
242+
arithmeticExpressions,
243+
parameterExpansions,
244+
variables,
245+
subshells,
246+
),
247+
)
213248
}
214249

215250
const commands: string[] = []
@@ -248,23 +283,18 @@ function parseCommandLine(command: string): string[] {
248283
}
249284

250285
// Restore quotes and redirections
251-
return commands.map((cmd) => {
252-
let result = cmd
253-
// Restore quotes
254-
result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)])
255-
// Restore redirections
256-
result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)])
257-
// Restore array indexing expressions
258-
result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)])
259-
// Restore arithmetic expressions
260-
result = result.replace(/__ARITH_(\d+)__/g, (_, i) => arithmeticExpressions[parseInt(i)])
261-
// Restore parameter expansions
262-
result = result.replace(/__PARAM_(\d+)__/g, (_, i) => parameterExpansions[parseInt(i)])
263-
// Restore variable references
264-
result = result.replace(/__VAR_(\d+)__/g, (_, i) => variables[parseInt(i)])
265-
result = result.replace(/__SUBSH_(\d+)__/g, (_, i) => subshells[parseInt(i)])
266-
return result
267-
})
286+
return commands.map((cmd) =>
287+
restorePlaceholders(
288+
cmd,
289+
quotes,
290+
redirections,
291+
arrayIndexing,
292+
arithmeticExpressions,
293+
parameterExpansions,
294+
variables,
295+
subshells,
296+
),
297+
)
268298
}
269299

270300
/**

0 commit comments

Comments
 (0)