Add regexpdynamicpattern linter: flag non-constant regexp compile patterns

docs/adr/50674-add-regexpdynamicpattern-linter.md

@@ -0,0 +1,63 @@
+# ADR-50674: Add regexpdynamicpattern Static Analysis Linter
+
+**Date**: 2026-08-05
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The `pkg/linters` package provides custom Go static analysis passes that enforce codebase-wide safety
+and style invariants. `regexp.Compile` and `regexp.MustCompile` are safe when called with compile-time
+constant patterns, but when the pattern is built from dynamic input (e.g. `fmt.Sprintf`, string
+concatenation with variables, or function parameters) two distinct risks arise: a malformed pattern
+causes a runtime panic (or a returned error that callers often ignore), and if the dynamic portion is
+influenced by untrusted input the pattern can trigger catastrophic backtracking (ReDoS). The existing
+`regexpcompileinfunction` linter only enforces *where* compilation occurs (package-level vs.
+function-level), not *what* is being compiled, leaving the dynamic-pattern risk unaddressed.
+
+### Decision
+
+We will introduce a new `regexpdynamicpattern` analyzer in `pkg/linters/regexpdynamicpattern/` that
+reports any `regexp.Compile` or `regexp.MustCompile` call whose first argument is not a compile-time
+constant string (literal, `const` identifier, or constant-only expression). Package identity is
+resolved via the type checker to handle aliased imports without false positives. The analyzer is
+registered in `pkg/linters/registry.go` alongside the existing linters and respects
+`//nolint:regexpdynamicpattern` suppressions for intentional dynamic patterns.
+
+### Alternatives Considered
+
+#### Alternative 1: Rely solely on the existing `regexpcompileinfunction` linter
+
+`regexpcompileinfunction` enforces that regexp compilation occurs at package level. A package-level
+`var re = regexp.MustCompile(buildPattern())` would still pass that linter while introducing a
+dynamic-pattern risk. This alternative was rejected because it does not address the class of risk
+identified: the *content* of the pattern, not its *location*, determines the safety hazard.
+
+#### Alternative 2: Runtime validation wrapper
+
+Wrap `regexp.Compile`/`MustCompile` with a project-internal helper that validates or sanitizes the
+pattern at runtime. This would catch panics but cannot prevent ReDoS (the unsafe pattern still
+executes) and introduces runtime overhead on every compilation call. It also requires migrating all
+call sites, whereas a static linter operates without any code changes to call sites that already use
+constant patterns.
+
+### Consequences
+
+#### Positive
+- Eliminates an entire class of runtime panics caused by malformed dynamically-constructed regexp patterns.
+- Reduces the ReDoS attack surface by flagging call sites where untrusted input could flow into regexp compilation.
+- Consistent with the project's existing philosophy of enforcing safety invariants at analysis time rather than at runtime.
+
+#### Negative
+- Intentional dynamic regexp patterns (e.g. test helpers that build patterns from parameters) require `//nolint:regexpdynamicpattern` suppressions, adding annotation noise.
+- The linter operates only on packages compiled with full type-checker information; packages analyzed without `TypesInfo` populated will silently skip pattern-constant checks (the analyzer returns `false` for unknown patterns rather than reporting a finding).
+
+#### Neutral
+- The `pkg/linters/doc.go` active-analyzer count increments from 62 to 63; documentation and `spec_test.go` must be updated whenever the analyzer list changes (this is already the project convention).
+- The new analyzer composes with the existing `nolint` and `filecheck` infrastructure (generated-file skipping, suppression directives) without requiring any changes to those internal packages.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/README.md

@@ -43,6 +43,7 @@ This package currently provides custom Go analyzers in the following subpackages
 - `panic-in-library-code` — reports `panic()` calls in library packages (`pkg/*`) where errors should be returned instead.
 - `rawloginlib` — reports direct usage of the standard `log` package in library packages, where `pkg/logger` should be used.
 - `regexpcompileinfunction` — reports `regexp.MustCompile` / `regexp.Compile` calls inside functions that should be package-level.
+- `regexpdynamicpattern` — reports `regexp.MustCompile` / `regexp.Compile` calls whose pattern is not a compile-time constant string.
 - `seenmapbool` — reports `map[string]bool` used as a set (values always `true`) that should use `map[string]struct{}` instead.
 - `sortslice` — reports `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc`.
 - `sprintferrdot` — reports redundant `.Error()` calls on error values passed to `fmt` format functions where the fmt package calls `.Error()` automatically.
@@ -112,6 +113,7 @@ This package currently provides custom Go analyzers in the following subpackages
 | `panic-in-library-code` | Custom `go/analysis` analyzer that flags `panic()` usage in library packages |
 | `rawloginlib` | Custom `go/analysis` analyzer that flags standard-library `log` package calls in library packages |
 | `regexpcompileinfunction` | Custom `go/analysis` analyzer that flags regexp compilation inside function bodies |
+| `regexpdynamicpattern` | Custom `go/analysis` analyzer that flags regexp.MustCompile/Compile calls with non-constant patterns |
 | `seenmapbool` | Custom `go/analysis` analyzer that flags `map[string]bool` used as a set that should use `map[string]struct{}` |
 | `sortslice` | Custom `go/analysis` analyzer that flags `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc` |
 | `sprintferrdot` | Custom `go/analysis` analyzer that flags redundant `.Error()` calls on error values passed to `fmt` format functions |
@@ -170,6 +172,7 @@ import (
 	panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
 	"github.com/github/gh-aw/pkg/linters/rawloginlib"
 	"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
+	"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
 	"github.com/github/gh-aw/pkg/linters/sortslice"
 	"github.com/github/gh-aw/pkg/linters/sprintfbool"
 	"github.com/github/gh-aw/pkg/linters/sprintfint"
@@ -199,6 +202,7 @@ _ = osexitinlibrary.Analyzer
 _ = panicinlibrarycode.Analyzer
 _ = rawloginlib.Analyzer
 _ = regexpcompileinfunction.Analyzer
+_ = regexpdynamicpattern.Analyzer
 _ = sortslice.Analyzer
 _ = sprintfbool.Analyzer
 _ = sprintfint.Analyzer
@@ -245,6 +249,7 @@ _ = trimleftright.Analyzer
 - `github.com/github/gh-aw/pkg/linters/panic-in-library-code` — panic-in-library-code analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/rawloginlib` — raw-log-in-lib analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/regexpcompileinfunction` — regexp-compile-in-function analyzer subpackage
+- `github.com/github/gh-aw/pkg/linters/regexpdynamicpattern` — regexp-dynamic-pattern analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/seenmapbool` — seen-map-bool analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/sortslice` — sort-slice analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/sprintferrdot` — sprintf-err-dot analyzer subpackage

pkg/linters/doc.go

@@ -1,6 +1,6 @@
 // Package linters is a namespace for gh-aw's custom Go analysis linters.
 //
-// All 62 active analyzers:
+// All 63 active analyzers:
 //
 //   - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)
 //   - appendoneelement — flags append(s, []T{x}...) calls where a single-element slice literal is spread and can be simplified to append(s, x)
@@ -39,6 +39,7 @@
 //   - panic-in-library-code — flags panic() calls in library packages
 //   - rawloginlib — flags direct usage of the standard log package in library packages
 //   - regexpcompileinfunction — flags regexp.MustCompile/Compile calls inside functions
+//   - regexpdynamicpattern — flags regexp.MustCompile/Compile calls whose pattern is not a compile-time constant
 //   - seenmapbool — flags map[string]bool used as a set that should use map[string]struct{}
 //   - sortslice — flags sort.Slice / sort.SliceStable calls that should use slices.SortFunc / slices.SortStableFunc
 //   - sprintferrdot — flags redundant .Error() calls on error values passed to fmt format functions

pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go

@@ -0,0 +1,123 @@
+// Package regexpdynamicpattern implements a Go analysis linter that flags
+// calls to regexp.MustCompile() and regexp.Compile() whose pattern argument
+// is not a compile-time constant string. Dynamically constructed patterns can
+// panic at runtime on malformed input and, when influenced by untrusted
+// input, can enable catastrophic-backtracking (ReDoS) denial-of-service
+// attacks.
+package regexpdynamicpattern
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+
+	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+var pkgLog = logger.New("linters:regexpdynamicpattern")
+
+// Analyzer is the regexp-dynamic-pattern analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "regexpdynamicpattern",
+	Doc:      "reports regexp.MustCompile and regexp.Compile calls whose pattern is not a compile-time constant string",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/regexpdynamicpattern",
+	Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	pkgLog.Printf("analyzing package %s", pass.Pkg.Path())
+	insp, err := astutil.Inspector(pass)
+	if err != nil {
+		return nil, err
+	}
+	noLintIndex, err := nolint.Index(pass)
+	if err != nil {
+		return nil, err
+	}
+	generatedFiles, err := filecheck.Index(pass)
+	if err != nil {
+		return nil, err
+	}
+
+	for cur := range insp.Root().Preorder((*ast.CallExpr)(nil)) {
+		call, ok := cur.Node().(*ast.CallExpr)
+		if !ok || !isRegexpCompileCall(pass, call) {
+			continue
+		}
+		if hasConstantStringPattern(pass, call) {
+			continue
+		}
+
+		pos := pass.Fset.PositionFor(call.Pos(), false)
+		if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) {
+			continue
+		}
+		if nolint.HasDirectiveForLinter(pos, noLintIndex, "regexpdynamicpattern") {
+			continue
+		}
+		pkgLog.Printf("flagging dynamic regexp pattern at %s", pos)
+		pass.Report(analysis.Diagnostic{
+			Pos:     call.Pos(),
+			End:     call.End(),
+			Message: "regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input",
+		})
+	}
+
+	return nil, nil
+}
+
+// isRegexpCompileCall checks if the call is to regexp.MustCompile or regexp.Compile,
+// resolving the package identity via the type checker to handle aliased imports
+// and avoid false positives from local identifiers named "regexp".
+func isRegexpCompileCall(pass *analysis.Pass, call *ast.CallExpr) bool {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+	if sel.Sel.Name != "MustCompile" && sel.Sel.Name != "Compile" {
+		return false
+	}
+	ident, ok := sel.X.(*ast.Ident)
+	if !ok || pass.TypesInfo == nil {
+		return false
+	}
+	obj := pass.TypesInfo.ObjectOf(ident)
+	if obj == nil {
+		return false
+	}
+	pkgName, ok := obj.(*types.PkgName)
+	if !ok || pkgName.Imported() == nil {
+		return false
+	}
+	return pkgName.Imported().Path() == "regexp"
+}
+
+// hasConstantStringPattern checks whether the regexp pattern is a compile-time constant string,
+// such as a string literal, const identifier, or an expression built entirely from constants
+// (e.g. concatenation of string literals/consts). Non-constant expressions such as
+// fmt.Sprintf results, concatenation involving variables, or function parameters return false.
+func hasConstantStringPattern(pass *analysis.Pass, call *ast.CallExpr) bool {
+	if len(call.Args) == 0 {
+		return false
+	}
+
+	patternArg := call.Args[0]
+	if lit, ok := patternArg.(*ast.BasicLit); ok && lit.Kind == token.STRING {
+		return true
+	}
+
+	tv, ok := pass.TypesInfo.Types[patternArg]
+	if !ok || tv.Value == nil || tv.Type == nil {
+		return false
+	}
+
+	basic, ok := tv.Type.Underlying().(*types.Basic)
+	return ok && basic.Kind() == types.String
+}

pkg/linters/regexpdynamicpattern/regexpdynamicpattern_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package regexpdynamicpattern_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
+)
+
+func TestAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, regexpdynamicpattern.Analyzer, "regexpdynamicpattern")
+}

pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go

@@ -0,0 +1,71 @@
+package regexpdynamicpattern
+
+import (
+	"fmt"
+	"regexp"
+)
+
+// not flagged: literal pattern at package level.
+var packageLevelRegexp = regexp.MustCompile(`^[a-z]+$`)
+
+const constPattern = `^const$`
+const constSuffix = `$`
+
+// not flagged: literal pattern.
+func ValidateLiteral(input string) bool {
+	re := regexp.MustCompile(`^[a-z]+$`)
+	return re.MatchString(input)
+}
+
+// not flagged: const identifier pattern.
+func ValidateConst(input string) bool {
+	re := regexp.MustCompile(constPattern)
+	return re.MatchString(input)
+}
+
+// not flagged: concatenation of constant-only expressions.
+func ValidateConstConcat(input string) bool {
+	re := regexp.MustCompile(`^const` + constSuffix)
+	return re.MatchString(input)
+}
+
+// flagged: pattern built with fmt.Sprintf.
+func ValidateSprintf(prefix, input string) (bool, error) {
+	re, err := regexp.Compile(fmt.Sprintf("^%s$", prefix)) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input`
+	if err != nil {
+		return false, err
+	}
+	return re.MatchString(input), nil
+}
+
+// flagged: string concatenation with a variable.
+func ValidateConcatVariable(suffix, input string) bool {
+	re := regexp.MustCompile(`^prefix` + suffix) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input`
+	return re.MatchString(input)
+}
+
+// flagged: pattern passed through from a function parameter.
+func ValidateDynamic(pattern, input string) (bool, error) {
+	re, err := regexp.Compile(pattern) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input`
+	if err != nil {
+		return false, err
+	}
+	return re.MatchString(input), nil
+}
+
+func suppressedPreviousLine(pattern, input string) (bool, error) {
+	//nolint:regexpdynamicpattern
+	re, err := regexp.Compile(pattern)
+	if err != nil {
+		return false, err
+	}
+	return re.MatchString(input), nil
+}
+
+func suppressedSameLine(pattern, input string) (bool, error) {
+	re, err := regexp.Compile(pattern) //nolint:regexpdynamicpattern
+	if err != nil {
+		return false, err
+	}
+	return re.MatchString(input), nil
+}

pkg/linters/registry.go

@@ -40,6 +40,7 @@ import (
 	panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
 	"github.com/github/gh-aw/pkg/linters/rawloginlib"
 	"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
+	"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
 	"github.com/github/gh-aw/pkg/linters/seenmapbool"
 	"github.com/github/gh-aw/pkg/linters/sortslice"
 	"github.com/github/gh-aw/pkg/linters/sprintfbool"
@@ -111,6 +112,7 @@ var allAnalyzers = []*analysis.Analyzer{
 	panicinlibrarycode.Analyzer,
 	rawloginlib.Analyzer,
 	regexpcompileinfunction.Analyzer,
+	regexpdynamicpattern.Analyzer,
 	ssljson.Analyzer,
 	seenmapbool.Analyzer,
 	sortslice.Analyzer,

pkg/linters/spec_test.go

@@ -48,6 +48,7 @@ import (
 	panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
 	"github.com/github/gh-aw/pkg/linters/rawloginlib"
 	"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
+	"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
 	"github.com/github/gh-aw/pkg/linters/seenmapbool"
 	"github.com/github/gh-aw/pkg/linters/sortslice"
 	"github.com/github/gh-aw/pkg/linters/sprintfbool"
@@ -98,7 +99,7 @@ type docAnalyzer struct {
 //	errortypeassertion, errstringmatch, execcommandwithoutcontext, fileclosenotdeferred, fmterrorfnoverbs, fprintlnsprintf,
 //	goroutinemissingrecover, hardcodedfilepath, httpnoctx, httprespbodyclose, httpstatuscode, ioutildeprecated, jsonmarshalignoredeerror, largefunc, lenstringsplit, lenstringzero,
 //	logfatallibrary, manualmutexunlock, mapclearloop, mapdeletecheck, nilctxpassed, osexitinlibrary, osgetenvlibrary, ossetenvlibrary, panic-in-library-code, rawloginlib,
-//	regexpcompileinfunction, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
+//	regexpcompileinfunction, regexpdynamicpattern, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
 //	strconvparseignorederror, stringbytesroundtrip, stringreplaceminusone, stringsconcatloop, stringscountcontains, stringsindexcontains, stringsindexhasprefix, stringsjoinone, timeafterleak, timesleepnocontext, timenowsub,
 //	tolowerequalfold, trimleftright, uncheckedflushreturn, uncheckedtypeassertion, walkfuncerrshadow, wgdonenotdeferred, writebytestring
 func documentedAnalyzers() []docAnalyzer {
@@ -140,6 +141,7 @@ func documentedAnalyzers() []docAnalyzer {
 		{"panic-in-library-code", panicinlibrarycode.Analyzer},
 		{"rawloginlib", rawloginlib.Analyzer},
 		{"regexpcompileinfunction", regexpcompileinfunction.Analyzer},
+		{"regexpdynamicpattern", regexpdynamicpattern.Analyzer},
 		{"seenmapbool", seenmapbool.Analyzer},
 		{"sortslice", sortslice.Analyzer},
 		{"sprintferrdot", sprintferrdot.Analyzer},