Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/artifacts-summary.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/mcp-inspector.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/research.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot.firewall.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .github/workflows/smoke-copilot.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 48 additions & 2 deletions pkg/workflow/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@
// shellEscapeCommandString escapes a complete command string (which may already contain
// quoted arguments) for passing as a single argument to another command.
// It wraps the command in double quotes and escapes any double quotes, dollar signs,
// backticks, and backslashes within the command.
// backticks, backslashes, and parentheses within the command.
// This is useful when passing a command to wrapper programs like awf that expect
// the command as a single quoted argument.
//
// Special case: Parentheses immediately following $ (i.e., $(...)) are NOT escaped
// to preserve command substitution syntax.
func shellEscapeCommandString(cmd string) string {
// Escape backslashes first (must be done before other escapes)
escaped := strings.ReplaceAll(cmd, "\\", "\\\\")
Expand All @@ -49,7 +52,50 @@
escaped = strings.ReplaceAll(escaped, "$", "\\$")
// Escape backticks (to prevent command substitution)
escaped = strings.ReplaceAll(escaped, "`", "\\`")

Check failure on line 55 in pkg/workflow/shell.go

View workflow job for this annotation

GitHub Actions / Lint Code

File is not properly formatted (gofmt)
// Escape parentheses (to prevent subshell interpretation inside double quotes)
// BUT preserve command substitution syntax: \$(...) should remain as \$(...)
// We need to escape ( and ) except when they immediately follow \$ (which was $ before escaping)
result := make([]byte, 0, len(escaped)*2)

Check failure

Code scanning / CodeQL

Size computation for allocation may overflow High

This operation, which is used in an
allocation
, involves a
potentially large value
and might overflow.
This operation, which is used in an
allocation
, involves a
potentially large value
and might overflow.
This operation, which is used in an
allocation
, involves a
potentially large value
and might overflow.
This operation, which is used in an
allocation
, involves a
potentially large value
and might overflow.
This operation, which is used in an
allocation
, involves a
potentially large value
and might overflow.

Copilot Autofix

AI 17 days ago

The best solution is to introduce a length check in shellEscapeCommandString before making the allocation for result. We should defensively check that len(escaped) is below a reasonable safe maximum (e.g., 64MB, which is standard for preventing overflow like in the CodeQL example), and fail gracefully if it exceeds that value. The check should return an empty quoted shell string or some visually obvious error string if the length is excessive, or, better, panic with a clear, descriptive error (or perhaps log and return a safe error). Since only pkg/workflow/shell.go is shown and permitted for editing, these checks must be implemented there.

Specifically:

  • Edit shellEscapeCommandString in pkg/workflow/shell.go to check that len(escaped) is below a threshold (e.g., 6410241024); otherwise, return an error string.
  • Optionally, you could also log the error to aid future diagnostics, but as logging facilities may not be imported here, prefer minimal intervention.
  • You might define a const for the threshold for clarity and future maintainability.
Suggested changeset 1
pkg/workflow/shell.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/pkg/workflow/shell.go b/pkg/workflow/shell.go
--- a/pkg/workflow/shell.go
+++ b/pkg/workflow/shell.go
@@ -44,6 +44,7 @@
 // Special case: Parentheses immediately following $ (i.e., $(...)) are NOT escaped
 // to preserve command substitution syntax.
 func shellEscapeCommandString(cmd string) string {
+	const MaxCmdLength = 64 * 1024 * 1024 // 64MB safeguard
 	// Escape backslashes first (must be done before other escapes)
 	escaped := strings.ReplaceAll(cmd, "\\", "\\\\")
 	// Escape double quotes
@@ -56,6 +57,10 @@
 	// Escape parentheses (to prevent subshell interpretation inside double quotes)
 	// BUT preserve command substitution syntax: \$(...) should remain as \$(...)
 	// We need to escape ( and ) except when they immediately follow \$ (which was $ before escaping)
+	if len(escaped) > MaxCmdLength {
+		// Defensive fail: do not attempt to allocate, return a safe shell string
+		return "\"\"" // Or optionally return an error string
+	}
 	result := make([]byte, 0, len(escaped)*2)
 	for i := 0; i < len(escaped); i++ {
 		ch := escaped[i]
EOF
@@ -44,6 +44,7 @@
// Special case: Parentheses immediately following $ (i.e., $(...)) are NOT escaped
// to preserve command substitution syntax.
func shellEscapeCommandString(cmd string) string {
const MaxCmdLength = 64 * 1024 * 1024 // 64MB safeguard
// Escape backslashes first (must be done before other escapes)
escaped := strings.ReplaceAll(cmd, "\\", "\\\\")
// Escape double quotes
@@ -56,6 +57,10 @@
// Escape parentheses (to prevent subshell interpretation inside double quotes)
// BUT preserve command substitution syntax: \$(...) should remain as \$(...)
// We need to escape ( and ) except when they immediately follow \$ (which was $ before escaping)
if len(escaped) > MaxCmdLength {
// Defensive fail: do not attempt to allocate, return a safe shell string
return "\"\"" // Or optionally return an error string
}
result := make([]byte, 0, len(escaped)*2)
for i := 0; i < len(escaped); i++ {
ch := escaped[i]
Copilot is powered by AI and may make mistakes. Always verify output.
for i := 0; i < len(escaped); i++ {
ch := escaped[i]
if ch == '(' {
// Don't escape opening paren if it follows \$
if i >= 2 && escaped[i-2] == '\\' && escaped[i-1] == '$' {
result = append(result, ch)
} else {
result = append(result, '\\', ch)
}
} else if ch == ')' {
// Don't escape closing paren if we're inside a \$(...) construct
// We need to track by looking backward for matching \$(
inCommandSubst := false
depth := 1 // We're currently at a ')', so depth starts at 1
for j := i - 1; j >= 0; j-- {
if escaped[j] == ')' {
depth++
} else if escaped[j] == '(' {
depth--
if depth == 0 {
// Found the matching opening paren
// Check if it's a command substitution \$(
if j >= 2 && escaped[j-2] == '\\' && escaped[j-1] == '$' {
inCommandSubst = true
}
break
}
}
}
if inCommandSubst {
result = append(result, ch)
} else {
result = append(result, '\\', ch)
}
} else {
result = append(result, ch)
}
}

// Wrap in double quotes
return "\"" + escaped + "\""
return "\"" + string(result) + "\""
}
Loading
Loading