fix(pkg):do regex precompile insteadd on the fly (#911)

* fix(pkg/providers):do regex precompile insteadd on the fly

* fix(providers): replace HTTP-specific regex with standalone status code matcher

The precompiled HTTP regex used uppercase "HTTP" which never matched
because ClassifyError lowercases the input. Replace it with a
case-insensitive word-boundary pattern that matches any standalone
3-digit status code (300-599), which also subsumes the HTTP/x.x case.

Add test case for standalone status code extraction.

* fix(providers): restore http regex and add standalone status code matcher

Restore the http-prefixed regex (without unnecessary (?i) flag since
input is already lowercased by ClassifyError) as a mid-priority pattern
to reduce false positives. Add a standalone word-boundary matcher as a
fallback for bare status codes like "429". Fix test to use lowercased
input matching the actual calling convention.

* perf(tools): move path regex compilation from per-call to package init

The path regex in guardCommand was compiled on every call. Hoist it
to a package-level var (absolutePathPattern) alongside defaultDenyPatterns
in a single var block, so it is compiled once at init time.

* style(tools): move inline comment to fix golines formatting error
This commit is contained in:
Tong Niu 2026-03-01 23:06:31 +11:00 committed by GitHub
parent 71bdeb41c9
commit d6e88da8ba
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 64 additions and 56 deletions

View file

@ -6,6 +6,13 @@ import (
"strings" "strings"
) )
// Common patterns in Go HTTP error messages
var httpStatusPatterns = []*regexp.Regexp{
regexp.MustCompile(`status[:\s]+(\d{3})`),
regexp.MustCompile(`http[/\s]+\d*\.?\d*\s+(\d{3})`),
regexp.MustCompile(`\b([3-5]\d{2})\b`),
}
// errorPattern defines a single pattern (string or regex) for error classification. // errorPattern defines a single pattern (string or regex) for error classification.
type errorPattern struct { type errorPattern struct {
substring string substring string
@ -198,20 +205,13 @@ func classifyByMessage(msg string) FailoverReason {
} }
// extractHTTPStatus extracts an HTTP status code from an error message. // extractHTTPStatus extracts an HTTP status code from an error message.
// Looks for patterns like "status: 429", "status 429", "HTTP 429", or standalone "429". // Looks for patterns like "status: 429", "status 429", "http/1.1 429", "http 429", or standalone "429".
func extractHTTPStatus(msg string) int { func extractHTTPStatus(msg string) int {
// Common patterns in Go HTTP error messages for _, p := range httpStatusPatterns {
patterns := []*regexp.Regexp{
regexp.MustCompile(`status[:\s]+(\d{3})`),
regexp.MustCompile(`HTTP[/\s]+\d*\.?\d*\s+(\d{3})`),
}
for _, p := range patterns {
if m := p.FindStringSubmatch(msg); len(m) > 1 { if m := p.FindStringSubmatch(msg); len(m) > 1 {
return parseDigits(m[1]) return parseDigits(m[1])
} }
} }
return 0 return 0
} }

View file

@ -305,7 +305,8 @@ func TestExtractHTTPStatus(t *testing.T) {
}{ }{
{"status: 429 rate limited", 429}, {"status: 429 rate limited", 429},
{"status 401 unauthorized", 401}, {"status 401 unauthorized", 401},
{"HTTP/1.1 502 Bad Gateway", 502}, {"http/1.1 502 bad gateway", 502},
{"error 429", 429},
{"no status code here", 0}, {"no status code here", 0},
{"random number 12345", 0}, {"random number 12345", 0},
} }

View file

@ -24,11 +24,15 @@ type ExecTool struct {
restrictToWorkspace bool restrictToWorkspace bool
} }
var defaultDenyPatterns = []*regexp.Regexp{ var (
defaultDenyPatterns = []*regexp.Regexp{
regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`), regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`),
regexp.MustCompile(`\bdel\s+/[fq]\b`), regexp.MustCompile(`\bdel\s+/[fq]\b`),
regexp.MustCompile(`\brmdir\s+/s\b`), regexp.MustCompile(`\brmdir\s+/s\b`),
regexp.MustCompile(`\b(format|mkfs|diskpart)\b\s`), // Match disk wiping commands (must be followed by space/args) // Match disk wiping commands (must be followed by space/args)
regexp.MustCompile(
`\b(format|mkfs|diskpart)\b\s`,
),
regexp.MustCompile(`\bdd\s+if=`), regexp.MustCompile(`\bdd\s+if=`),
regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null) regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null)
regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`), regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`),
@ -67,7 +71,11 @@ var defaultDenyPatterns = []*regexp.Regexp{
regexp.MustCompile(`\bssh\b.*@`), regexp.MustCompile(`\bssh\b.*@`),
regexp.MustCompile(`\beval\b`), regexp.MustCompile(`\beval\b`),
regexp.MustCompile(`\bsource\s+.*\.sh\b`), regexp.MustCompile(`\bsource\s+.*\.sh\b`),
} }
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
)
func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) { func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) {
return NewExecToolWithConfig(workingDir, restrict, nil) return NewExecToolWithConfig(workingDir, restrict, nil)
@ -287,8 +295,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
return "" return ""
} }
pathPattern := regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) matches := absolutePathPattern.FindAllString(cmd, -1)
matches := pathPattern.FindAllString(cmd, -1)
for _, raw := range matches { for _, raw := range matches {
p, err := filepath.Abs(raw) p, err := filepath.Abs(raw)