diff --git a/pkg/agentdrain/mask.go b/pkg/agentdrain/mask.go index bc1158165a2..cd405b01c52 100644 --- a/pkg/agentdrain/mask.go +++ b/pkg/agentdrain/mask.go @@ -30,7 +30,10 @@ func NewMasker(rules []MaskRule) (*Masker, error) { maskLog.Printf("Compiling %d mask rules", len(rules)) compiled := make([]compiledRule, 0, len(rules)) for _, r := range rules { - re, err := regexp.Compile(r.Pattern) + // Patterns come from trusted configuration (defaults or admin-supplied + // mask rules), not from untrusted runtime input, and compile errors are + // surfaced to the caller instead of panicking. + re, err := regexp.Compile(r.Pattern) //nolint:regexpdynamicpattern if err != nil { maskLog.Printf("Failed to compile mask rule %q: %v", r.Name, err) return nil, fmt.Errorf("agentdrain: mask rule %q: %w", r.Name, err) diff --git a/pkg/cli/codemod_activation_outputs.go b/pkg/cli/codemod_activation_outputs.go index 59562e82f8e..645170a27dc 100644 --- a/pkg/cli/codemod_activation_outputs.go +++ b/pkg/cli/codemod_activation_outputs.go @@ -8,6 +8,15 @@ import ( var activationOutputsCodemodLog = logger.New("cli:codemod_activation_outputs") +// activationOutputPatterns maps each activation output name to its precompiled, +// word-boundary-anchored replacement pattern. Using precompiled constant patterns +// avoids building regexps from dynamic strings. +var activationOutputPatterns = map[string]*regexp.Regexp{ + "text": regexp.MustCompile(`needs\.activation\.outputs\.text\b`), + "title": regexp.MustCompile(`needs\.activation\.outputs\.title\b`), + "body": regexp.MustCompile(`needs\.activation\.outputs\.body\b`), +} + // getActivationOutputsCodemod creates a codemod for transforming needs.activation.outputs.* to steps.sanitized.outputs.* func getActivationOutputsCodemod() Codemod { return Codemod{ @@ -26,10 +35,15 @@ func getActivationOutputsCodemod() Codemod { for _, output := range outputs { newReplacement := "steps.sanitized.outputs." + output - // Use regex with word boundary to prevent partial matches + // Use the precompiled pattern with a word boundary to prevent partial matches // This ensures we don't match things like "needs.activation.outputs.text_custom" - // The pattern matches the old expression followed by a non-word character or end of string - pattern := regexp.MustCompile(`needs\.activation\.outputs\.` + output + `\b`) + pattern, ok := activationOutputPatterns[output] + if !ok { + // This should never happen: activationOutputPatterns covers exactly the outputs slice. + // Log a warning so any future drift is visible rather than silently skipped. + activationOutputsCodemodLog.Printf("WARNING: no precompiled pattern for output %q; skipping", output) + continue + } // Check if pattern exists in content if pattern.MatchString(result) { diff --git a/pkg/cli/firewall_policy.go b/pkg/cli/firewall_policy.go index 7a9c58fc528..e567a0d3921 100644 --- a/pkg/cli/firewall_policy.go +++ b/pkg/cli/firewall_policy.go @@ -213,7 +213,9 @@ func containsRegexMeta(s string) bool { // domainMatchesRegex checks if a domain matches any regex pattern in the list. func domainMatchesRegex(domain string, patterns []string) bool { for _, pattern := range patterns { - re, err := regexp.Compile(pattern) + // Patterns come from trusted firewall policy configuration, and compile + // errors are logged and safely skipped rather than causing a panic. + re, err := regexp.Compile(pattern) //nolint:regexpdynamicpattern if err != nil { firewallPolicyLog.Printf("Invalid regex pattern %q: %v", pattern, err) continue diff --git a/pkg/parser/frontmatter_content.go b/pkg/parser/frontmatter_content.go index a4692fd5ee7..cf0718bbf11 100644 --- a/pkg/parser/frontmatter_content.go +++ b/pkg/parser/frontmatter_content.go @@ -250,8 +250,10 @@ func ExtractMarkdownSection(content, sectionName string) (string, error) { inSection := false var sectionLevel int - // Create regex pattern to match headers at any level (H1-H3) with flexible spacing - headerPattern := regexp.MustCompile(`^(#{1,3})[\s\t]+` + regexp.QuoteMeta(sectionName) + `[\s\t]*$`) + // Create regex pattern to match headers at any level (H1-H3) with flexible spacing. + // sectionName is escaped with regexp.QuoteMeta, so the compiled pattern only ever + // matches sectionName literally; it cannot introduce ReDoS or invalid syntax. + headerPattern := regexp.MustCompile(`^(#{1,3})[\s\t]+` + regexp.QuoteMeta(sectionName) + `[\s\t]*$`) //nolint:regexpdynamicpattern for scanner.Scan() { line := scanner.Text() diff --git a/pkg/parser/json_path_locator.go b/pkg/parser/json_path_locator.go index 5dc876cd717..caf38dbdb0b 100644 --- a/pkg/parser/json_path_locator.go +++ b/pkg/parser/json_path_locator.go @@ -181,8 +181,7 @@ func matchesPathAtLevel(line string, pathSegments []PathSegment, level int, arra switch segment.Type { case "key": // Look for "key:" pattern - keyPattern := regexp.MustCompile(`^` + regexp.QuoteMeta(segment.Value) + `\s*:`) - if keyPattern.MatchString(trimmedLine) { + if matchesPathSegmentKey(trimmedLine, segment.Value) { // Found the key - return position after the colon colonIndex := strings.Index(line, ":") if colonIndex != -1 { @@ -279,8 +278,7 @@ func findFirstAdditionalProperty(yamlContent string, propertyNames []string) JSO // Check if this line contains any of the additional properties for _, propName := range propertyNames { // Look for "propName:" pattern at the start of the trimmed line - keyPattern := regexp.MustCompile(`^` + regexp.QuoteMeta(propName) + `\s*:`) - if keyPattern.MatchString(trimmedLine) { + if matchesPathSegmentKey(trimmedLine, propName) { // Found the property - return position of the property name propIndex := strings.Index(line, propName) if propIndex != -1 { @@ -409,8 +407,11 @@ func findNestedSectionStart(lines []string, pathSegments []PathSegment) (int, in } func matchesPathSegmentKey(trimmedLine, key string) bool { - keyPattern := regexp.MustCompile(`^` + regexp.QuoteMeta(key) + `\s*:`) - return keyPattern.MatchString(trimmedLine) + if !strings.HasPrefix(trimmedLine, key) { + return false + } + rest := strings.TrimLeft(trimmedLine[len(key):], " \t") + return strings.HasPrefix(rest, ":") } func findNestedSectionEnd(lines []string, foundLine, baseIndentLevel int) int { diff --git a/pkg/parser/json_path_locator_test.go b/pkg/parser/json_path_locator_test.go index d8756daa095..99ede0a2952 100644 --- a/pkg/parser/json_path_locator_test.go +++ b/pkg/parser/json_path_locator_test.go @@ -251,3 +251,30 @@ func TestParseJSONPath(t *testing.T) { }) } } + +func TestMatchesPathSegmentKey(t *testing.T) { + tests := []struct { + name string + trimmedLine string + key string + want bool + }{ + {"exact match", "foo: bar", "foo", true}, + {"space before colon", "foo : bar", "foo", true}, + {"tab before colon", "foo\t: bar", "foo", true}, + {"multiple spaces before colon", "foo : bar", "foo", true}, + {"no value after colon", "foo:", "foo", true}, + {"key prefix should not match", "foobar: x", "foo", false}, + {"no colon", "foobar", "foo", false}, + {"empty key", ":val", "", true}, + {"different key", "bar: x", "foo", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := matchesPathSegmentKey(tt.trimmedLine, tt.key) + if got != tt.want { + t.Errorf("matchesPathSegmentKey(%q, %q) = %v, want %v", tt.trimmedLine, tt.key, got, tt.want) + } + }) + } +} diff --git a/pkg/parser/schema_suggestions.go b/pkg/parser/schema_suggestions.go index f58ba673fd6..12c2077ba1b 100644 --- a/pkg/parser/schema_suggestions.go +++ b/pkg/parser/schema_suggestions.go @@ -503,17 +503,17 @@ func extractTopLevelYAMLValue(yamlContent, fieldName string) string { escapedField := regexp.QuoteMeta(fieldName) // Try single-quoted value: field: 'value' (anchored to column 0, no leading whitespace) - reSingle := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*'([^'\n]+)'`) + reSingle := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*'([^'\n]+)'`) //nolint:regexpdynamicpattern // fieldName is escaped via regexp.QuoteMeta above if match := reSingle.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } // Try double-quoted value: field: "value" - reDouble := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*"([^"\n]+)"`) + reDouble := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*"([^"\n]+)"`) //nolint:regexpdynamicpattern // fieldName is escaped via regexp.QuoteMeta above if match := reDouble.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } // Try unquoted value: field: value - reUnquoted := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) + reUnquoted := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) //nolint:regexpdynamicpattern // fieldName is escaped via regexp.QuoteMeta above if match := reUnquoted.FindStringSubmatch(yamlContent); len(match) >= 2 { return strings.TrimSpace(match[1]) } @@ -528,7 +528,7 @@ func extractNestedYAMLValue(yamlContent, parentKey, childKey string) string { lines := strings.Split(yamlContent, "\n") escapedParent := regexp.QuoteMeta(parentKey) - parentPattern := regexp.MustCompile(`^(\s*)` + escapedParent + `[ \t]*:`) + parentPattern := regexp.MustCompile(`^(\s*)` + escapedParent + `[ \t]*:`) //nolint:regexpdynamicpattern // parentKey is escaped via regexp.QuoteMeta above escapedChild := regexp.QuoteMeta(childKey) parentIndent := -1 @@ -567,15 +567,15 @@ func extractNestedYAMLValue(yamlContent, parentKey, childKey string) string { // Try to match child key with its value (single-quoted, double-quoted, unquoted). childPrefix := `^\s+` + escapedChild + `[ \t]*:[ \t]*` - reSingle := regexp.MustCompile(childPrefix + `'([^'\n]+)'`) + reSingle := regexp.MustCompile(childPrefix + `'([^'\n]+)'`) //nolint:regexpdynamicpattern // childPrefix is built from regexp.QuoteMeta(childKey) above if match := reSingle.FindStringSubmatch(line); len(match) >= 2 { return strings.TrimSpace(match[1]) } - reDouble := regexp.MustCompile(childPrefix + `"([^"\n]+)"`) + reDouble := regexp.MustCompile(childPrefix + `"([^"\n]+)"`) //nolint:regexpdynamicpattern // childPrefix is built from regexp.QuoteMeta(childKey) above if match := reDouble.FindStringSubmatch(line); len(match) >= 2 { return strings.TrimSpace(match[1]) } - reUnquoted := regexp.MustCompile(childPrefix + `([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) + reUnquoted := regexp.MustCompile(childPrefix + `([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) //nolint:regexpdynamicpattern // childPrefix is built from regexp.QuoteMeta(childKey) above if match := reUnquoted.FindStringSubmatch(line); len(match) >= 2 { return strings.TrimSpace(match[1]) } diff --git a/pkg/stringutil/sanitize.go b/pkg/stringutil/sanitize.go index 05d2d3bc569..9cce7b6b329 100644 --- a/pkg/stringutil/sanitize.go +++ b/pkg/stringutil/sanitize.go @@ -163,9 +163,16 @@ func buildSanitizePreservePattern(opts *SanitizeOptions) string { // When the caller has requested preservation of special chars, unwanted chars are // replaced with hyphens; otherwise they are removed entirely. func applySanitizePattern(result, allowedChars string, preserveSpecialChars bool) string { + // allowedChars is always produced by buildSanitizePreservePattern, which only + // ever returns one of the combinations already precompiled in sanitizePatterns. + // Fall back to the base alphanumeric-and-hyphen pattern for safety. pattern, ok := sanitizePatterns[allowedChars] if !ok { - pattern = regexp.MustCompile(`[^` + allowedChars + `]+`) + // allowedChars is always produced by buildSanitizePreservePattern; this branch + // means a new combination was added there but not to sanitizePatterns. Log a + // warning so the gap is visible rather than silently using the wrong pattern. + sanitizeLog.Printf("WARNING: no precompiled sanitize pattern for %q; falling back to a-z0-9-", allowedChars) + pattern = sanitizePatterns["a-z0-9-"] } if preserveSpecialChars { return pattern.ReplaceAllString(result, "-") diff --git a/pkg/workflow/gh_cli_permissions.go b/pkg/workflow/gh_cli_permissions.go index 306fcdbe32f..d6ffc981e16 100644 --- a/pkg/workflow/gh_cli_permissions.go +++ b/pkg/workflow/gh_cli_permissions.go @@ -100,7 +100,7 @@ var getCompiledGHCLIPermissions = sync.OnceValues(func() (compiledGHCLIPermissio subcommandPattern := `(?m)(?:^|[\s|;])gh\s+(` + strings.Join(groups, "|") + `)\s+([\w][\w-]*)\b` // Defensive check: the pattern is built from embedded JSON keys quoted with // regexp.QuoteMeta, so a compile error would indicate unexpected data corruption. - subcommandRE, err := regexp.Compile(subcommandPattern) + subcommandRE, err := regexp.Compile(subcommandPattern) //nolint:regexpdynamicpattern if err != nil { return compiledGHCLIPermissions{}, fmt.Errorf("invalid gh subcommand pattern %q: %w", subcommandPattern, err) } @@ -141,7 +141,9 @@ var getCompiledGHCLIPermissions = sync.OnceValues(func() (compiledGHCLIPermissio } for _, ap := range data.APIPathPatterns { - re, err := regexp.Compile(ap.Pattern) + // Pattern comes from the embedded, build-time gh_cli_permissions.json config, + // not from untrusted runtime input. + re, err := regexp.Compile(ap.Pattern) //nolint:regexpdynamicpattern if err != nil { return compiledGHCLIPermissions{}, fmt.Errorf("invalid gh API path pattern %q in gh_cli_permissions.json: %w", ap.Pattern, err) } diff --git a/pkg/workflow/mcp_renderer_guard.go b/pkg/workflow/mcp_renderer_guard.go index 587aee8f2ae..67a9e2a54f4 100644 --- a/pkg/workflow/mcp_renderer_guard.go +++ b/pkg/workflow/mcp_renderer_guard.go @@ -37,7 +37,9 @@ const sinkVisibilityRuntimeExpr = "${" + sinkVisibilityEnvVar + "}" // Expressions are always of the form ${{ ... }} and must not contain double quotes // (our generated expressions use single-quoted strings inside the GitHub Actions expression, // so this invariant holds for all compiler-generated fallback values). -var guardExprRE = regexp.MustCompile(`"` + regexp.QuoteMeta(guardExprSentinel) + `(\$\{\{[^"]+\}\})"`) +// guardExprSentinel is a package-level constant, so regexp.QuoteMeta here operates on a +// fixed, trusted value rather than untrusted input. +var guardExprRE = regexp.MustCompile(`"` + regexp.QuoteMeta(guardExprSentinel) + `(\$\{\{[^"]+\}\})"`) //nolint:regexpdynamicpattern // renderGuardPoliciesJSON renders a "guard-policies" JSON field at the given indent level. // The policies map contains policy names (e.g., "allow-only") mapped to their configurations. diff --git a/pkg/workflow/observability_otlp.go b/pkg/workflow/observability_otlp.go index 4277af4a97e..3eb848416db 100644 --- a/pkg/workflow/observability_otlp.go +++ b/pkg/workflow/observability_otlp.go @@ -15,7 +15,10 @@ import ( var otlpLog = logger.New("workflow:observability_otlp") -var sentryEndpointExpressionPattern = regexp.MustCompile(`(?i)^\$\{\{\s*secrets\.` + regexp.QuoteMeta(constants.OTELSentryEndpointSecretName) + `\s*\}\}$`) +// sentryEndpointExpressionPattern matches the expected sentry endpoint secret expression. +// constants.OTELSentryEndpointSecretName is a fixed package constant, so regexp.QuoteMeta +// here operates on a trusted, non-user-controlled value. +var sentryEndpointExpressionPattern = regexp.MustCompile(`(?i)^\$\{\{\s*secrets\.` + regexp.QuoteMeta(constants.OTELSentryEndpointSecretName) + `\s*\}\}$`) //nolint:regexpdynamicpattern var otlpResourceAttributeSecretRefPattern = regexp.MustCompile(`\$\{\{\s*(secrets|vars)\.`) var otelServiceNameKeyPattern = regexp.MustCompile(`(?m)^\s*OTEL_SERVICE_NAME:`) diff --git a/pkg/workflow/template_injection_utils.go b/pkg/workflow/template_injection_utils.go index 947d0139ce5..72a6f49713f 100644 --- a/pkg/workflow/template_injection_utils.go +++ b/pkg/workflow/template_injection_utils.go @@ -49,21 +49,46 @@ type heredocPattern struct { // Each entry covers one of the common delimiter suffixes used by heredocs in shell scripts. // Since Go regex doesn't support backreferences, we match common heredoc delimiter suffixes explicitly. // Matches both exact delimiters (EOF) and prefixed delimiters (GH_AW_SAFE_OUTPUTS_CONFIG_EOF). -var heredocPatterns = func() []heredocPattern { - suffixes := []string{"EOF", "EOL", "END", "HEREDOC", "JSON", "YAML", "SQL"} - patterns := make([]heredocPattern, len(suffixes)) - for i, suffix := range suffixes { - // Pattern for quoted delimiter ending with suffix: << 'PREFIX_SUFFIX' or << "PREFIX_SUFFIX" - // \w* matches zero or more word characters (allowing both exact match and prefixes) - // (?ms) enables multiline and dotall modes, .*? is non-greedy - // \s*\w*%s\s*$ allows for leading/trailing whitespace on the closing delimiter - patterns[i] = heredocPattern{ - quoted: regexp.MustCompile(fmt.Sprintf(`(?ms)<<\s*['"]\w*%s['"].*?\n\s*\w*%s\s*$`, suffix, suffix)), - unquoted: regexp.MustCompile(fmt.Sprintf(`(?ms)<<\s*\w*%s.*?\n\s*\w*%s\s*$`, suffix, suffix)), - } - } - return patterns -}() +// Patterns are written out per fixed suffix (rather than built with fmt.Sprintf in a loop) so +// that each regexp is a compile-time constant string. +var heredocPatterns = []heredocPattern{ + // Pattern for quoted delimiter ending with suffix: << 'PREFIX_SUFFIX' or << "PREFIX_SUFFIX" + // \w* matches zero or more word characters (allowing both exact match and prefixes) + // (?ms) enables multiline and dotall modes, .*? is non-greedy + // \s*\w*SUFFIX\s*$ allows for leading/trailing whitespace on the closing delimiter + // + // NOTE: if a new heredoc suffix is needed, add a corresponding heredocPattern entry here + // AND update the suffixes slice in the test in template_injection_utils_test.go so that + // the exhaustiveness check catches any future drift between the two lists. + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*EOF['"].*?\n\s*\w*EOF\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*EOF.*?\n\s*\w*EOF\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*EOL['"].*?\n\s*\w*EOL\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*EOL.*?\n\s*\w*EOL\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*END['"].*?\n\s*\w*END\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*END.*?\n\s*\w*END\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*HEREDOC['"].*?\n\s*\w*HEREDOC\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*HEREDOC.*?\n\s*\w*HEREDOC\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*JSON['"].*?\n\s*\w*JSON\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*JSON.*?\n\s*\w*JSON\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*YAML['"].*?\n\s*\w*YAML\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*YAML.*?\n\s*\w*YAML\s*$`), + }, + { + quoted: regexp.MustCompile(`(?ms)<<\s*['"]\w*SQL['"].*?\n\s*\w*SQL\s*$`), + unquoted: regexp.MustCompile(`(?ms)<<\s*\w*SQL.*?\n\s*\w*SQL\s*$`), + }, +} // removeHeredocContent removes heredoc sections from shell commands. // Heredocs (e.g., cat > file << 'EOF' ... EOF) are safe for template expressions diff --git a/pkg/workflow/yaml.go b/pkg/workflow/yaml.go index 1e9212c37e3..d4e7f6ae272 100644 --- a/pkg/workflow/yaml.go +++ b/pkg/workflow/yaml.go @@ -166,6 +166,8 @@ func UnquoteYAMLKey(yamlStr string, key string) string { // Create a regex pattern that matches the quoted key at the start of a line // Pattern: (start of line or newline) + (optional whitespace) + quoted key + colon + // key is escaped via regexp.QuoteMeta above, so the compiled pattern only ever + // matches key literally; it cannot introduce ReDoS or invalid syntax. pattern := `(^|\n)([ \t]*)"` + regexp.QuoteMeta(key) + `":` // Use cached compiled regex to avoid recompiling on every call @@ -175,11 +177,11 @@ func UnquoteYAMLKey(yamlStr string, key string) string { re, typeOK = cached.(*regexp.Regexp) if !typeOK { unquoteYAMLKeyCache.Delete(key) - re = regexp.MustCompile(pattern) + re = regexp.MustCompile(pattern) //nolint:regexpdynamicpattern unquoteYAMLKeyCache.Store(key, re) } } else { - re = regexp.MustCompile(pattern) + re = regexp.MustCompile(pattern) //nolint:regexpdynamicpattern unquoteYAMLKeyCache.Store(key, re) } // Use ReplaceAllString with capture group references for a single-pass replacement.