Skip to content

Commit 0a25464

Browse files
committed
refactor: eliminate code redundancy between extractCommandPatterns and parseCommand
- Refactored extractCommandPatterns to use the existing parseCommand function - Ensures consistent command parsing behavior across the codebase - Maintains security by removing subshell contents before parsing - All existing tests continue to pass
1 parent f6642f6 commit 0a25464

File tree

1 file changed

+39
-57
lines changed

1 file changed

+39
-57
lines changed

webview-ui/src/utils/commandPatterns.ts

Lines changed: 39 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { parse } from "shell-quote"
1+
import { parseCommand } from "./command-validation"
22

33
export interface CommandPattern {
44
pattern: string
@@ -15,75 +15,57 @@ export function extractCommandPatterns(command: string): string[] {
1515

1616
const patterns = new Set<string>()
1717

18-
try {
19-
// First, remove subshell expressions to avoid extracting their contents
20-
const cleanedCommand = command
21-
.replace(/\$\([^)]*\)/g, "") // Remove $() subshells
22-
.replace(/`[^`]*`/g, "") // Remove backtick subshells
18+
// First, check if the command contains subshells and remove them
19+
// This is important for security - we don't want to extract patterns from subshell contents
20+
const cleanedCommand = command
21+
.replace(/\$\([^)]*\)/g, "") // Remove $() subshells
22+
.replace(/`[^`]*`/g, "") // Remove backtick subshells
2323

24-
const parsed = parse(cleanedCommand)
24+
// Use parseCommand to split the cleaned command into sub-commands
25+
// This ensures consistent parsing behavior with command-validation
26+
const subCommands = parseCommand(cleanedCommand)
2527

26-
const commandSeparators = new Set(["|", "&&", "||", ";"])
27-
let current: any[] = []
28+
// Process each sub-command to extract patterns
29+
for (const subCommand of subCommands) {
30+
// Skip empty commands
31+
if (!subCommand.trim()) continue
2832

29-
for (const token of parsed) {
30-
if (typeof token === "object" && "op" in token && token.op && commandSeparators.has(token.op)) {
31-
if (current.length) processCommand(current, patterns)
32-
current = []
33-
} else {
34-
current.push(token)
35-
}
36-
}
33+
// Split the command into tokens
34+
const tokens = subCommand.trim().split(/\s+/)
3735

38-
if (current.length) processCommand(current, patterns)
39-
} catch (_error) {
40-
// If parsing fails, try to extract at least the main command
41-
const mainCommand = command.trim().split(/\s+/)[0]
42-
43-
// Apply same validation as in processCommand
44-
if (
45-
mainCommand &&
46-
!/^\d+$/.test(mainCommand) && // Skip pure numbers
47-
!["total", "error", "warning", "failed", "success", "done"].includes(mainCommand.toLowerCase()) &&
48-
(/[a-zA-Z]/.test(mainCommand) || mainCommand.includes("/"))
49-
) {
50-
patterns.add(mainCommand)
51-
}
52-
}
36+
if (tokens.length === 0) continue
5337

54-
return Array.from(patterns).sort()
55-
}
38+
const mainCmd = tokens[0]
5639

57-
function processCommand(cmd: any[], patterns: Set<string>) {
58-
if (!cmd.length || typeof cmd[0] !== "string") return
40+
// Skip if it's just a number (like "0" from "0 total")
41+
if (/^\d+$/.test(mainCmd)) continue
5942

60-
const mainCmd = cmd[0]
43+
// Skip common output patterns that aren't commands
44+
const skipWords = ["total", "error", "warning", "failed", "success", "done"]
45+
if (skipWords.includes(mainCmd.toLowerCase())) continue
6146

62-
// Skip if it's just a number (like "0" from "0 total")
63-
if (/^\d+$/.test(mainCmd)) return
47+
// Only add if it contains at least one letter or is a valid path
48+
if (/[a-zA-Z]/.test(mainCmd) || mainCmd.includes("/")) {
49+
patterns.add(mainCmd)
6450

65-
// Skip common output patterns that aren't commands
66-
const skipWords = ["total", "error", "warning", "failed", "success", "done"]
67-
if (skipWords.includes(mainCmd.toLowerCase())) return
51+
// Build up patterns progressively (e.g., "npm", "npm install", "npm install express")
52+
// Stop at flags or special characters
53+
const stopPatterns = [/^-/, /[\\/.~]/]
6854

69-
// Only add if it contains at least one letter or is a valid path
70-
if (/[a-zA-Z]/.test(mainCmd) || mainCmd.includes("/")) {
71-
patterns.add(mainCmd)
72-
} else {
73-
return // Don't process further if main command is invalid
74-
}
75-
76-
// Patterns that indicate we should stop looking for subcommands
77-
const stopPatterns = [/^-/, /[\\/.~ ]/]
55+
for (let i = 1; i < tokens.length; i++) {
56+
const token = tokens[i]
7857

79-
// Build up patterns progressively
80-
for (let i = 1; i < cmd.length; i++) {
81-
const arg = cmd[i]
82-
if (typeof arg !== "string" || stopPatterns.some((re) => re.test(arg))) break
58+
// Stop if we hit a flag or special character
59+
if (stopPatterns.some((re) => re.test(token))) break
8360

84-
const pattern = cmd.slice(0, i + 1).join(" ")
85-
patterns.add(pattern)
61+
// Build the pattern up to this point
62+
const pattern = tokens.slice(0, i + 1).join(" ")
63+
patterns.add(pattern)
64+
}
65+
}
8666
}
67+
68+
return Array.from(patterns).sort()
8769
}
8870

8971
export function detectSecurityIssues(command: string): SecurityWarning[] {

0 commit comments

Comments
 (0)