Skip to content

Commit 1b4c40f

Browse files
authored
Add regexpdynamicpattern linter: flag non-constant regexp compile patterns (#50674)
1 parent 55f52a8 commit 1b4c40f

9 files changed

Lines changed: 285 additions & 2 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# ADR-50674: Add regexpdynamicpattern Static Analysis Linter
2+
3+
**Date**: 2026-08-05
4+
**Status**: Draft
5+
**Deciders**: Unknown
6+
7+
---
8+
9+
### Context
10+
11+
The `pkg/linters` package provides custom Go static analysis passes that enforce codebase-wide safety
12+
and style invariants. `regexp.Compile` and `regexp.MustCompile` are safe when called with compile-time
13+
constant patterns, but when the pattern is built from dynamic input (e.g. `fmt.Sprintf`, string
14+
concatenation with variables, or function parameters) two distinct risks arise: a malformed pattern
15+
causes a runtime panic (or a returned error that callers often ignore), and if the dynamic portion is
16+
influenced by untrusted input the pattern can trigger catastrophic backtracking (ReDoS). The existing
17+
`regexpcompileinfunction` linter only enforces *where* compilation occurs (package-level vs.
18+
function-level), not *what* is being compiled, leaving the dynamic-pattern risk unaddressed.
19+
20+
### Decision
21+
22+
We will introduce a new `regexpdynamicpattern` analyzer in `pkg/linters/regexpdynamicpattern/` that
23+
reports any `regexp.Compile` or `regexp.MustCompile` call whose first argument is not a compile-time
24+
constant string (literal, `const` identifier, or constant-only expression). Package identity is
25+
resolved via the type checker to handle aliased imports without false positives. The analyzer is
26+
registered in `pkg/linters/registry.go` alongside the existing linters and respects
27+
`//nolint:regexpdynamicpattern` suppressions for intentional dynamic patterns.
28+
29+
### Alternatives Considered
30+
31+
#### Alternative 1: Rely solely on the existing `regexpcompileinfunction` linter
32+
33+
`regexpcompileinfunction` enforces that regexp compilation occurs at package level. A package-level
34+
`var re = regexp.MustCompile(buildPattern())` would still pass that linter while introducing a
35+
dynamic-pattern risk. This alternative was rejected because it does not address the class of risk
36+
identified: the *content* of the pattern, not its *location*, determines the safety hazard.
37+
38+
#### Alternative 2: Runtime validation wrapper
39+
40+
Wrap `regexp.Compile`/`MustCompile` with a project-internal helper that validates or sanitizes the
41+
pattern at runtime. This would catch panics but cannot prevent ReDoS (the unsafe pattern still
42+
executes) and introduces runtime overhead on every compilation call. It also requires migrating all
43+
call sites, whereas a static linter operates without any code changes to call sites that already use
44+
constant patterns.
45+
46+
### Consequences
47+
48+
#### Positive
49+
- Eliminates an entire class of runtime panics caused by malformed dynamically-constructed regexp patterns.
50+
- Reduces the ReDoS attack surface by flagging call sites where untrusted input could flow into regexp compilation.
51+
- Consistent with the project's existing philosophy of enforcing safety invariants at analysis time rather than at runtime.
52+
53+
#### Negative
54+
- Intentional dynamic regexp patterns (e.g. test helpers that build patterns from parameters) require `//nolint:regexpdynamicpattern` suppressions, adding annotation noise.
55+
- 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).
56+
57+
#### Neutral
58+
- 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).
59+
- The new analyzer composes with the existing `nolint` and `filecheck` infrastructure (generated-file skipping, suppression directives) without requiring any changes to those internal packages.
60+
61+
---
62+
63+
*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

linters

9.05 KB
Binary file not shown.

pkg/linters/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ This package currently provides custom Go analyzers in the following subpackages
4343
- `panic-in-library-code` — reports `panic()` calls in library packages (`pkg/*`) where errors should be returned instead.
4444
- `rawloginlib` — reports direct usage of the standard `log` package in library packages, where `pkg/logger` should be used.
4545
- `regexpcompileinfunction` — reports `regexp.MustCompile` / `regexp.Compile` calls inside functions that should be package-level.
46+
- `regexpdynamicpattern` — reports `regexp.MustCompile` / `regexp.Compile` calls whose pattern is not a compile-time constant string.
4647
- `seenmapbool` — reports `map[string]bool` used as a set (values always `true`) that should use `map[string]struct{}` instead.
4748
- `sortslice` — reports `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc`.
4849
- `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
112113
| `panic-in-library-code` | Custom `go/analysis` analyzer that flags `panic()` usage in library packages |
113114
| `rawloginlib` | Custom `go/analysis` analyzer that flags standard-library `log` package calls in library packages |
114115
| `regexpcompileinfunction` | Custom `go/analysis` analyzer that flags regexp compilation inside function bodies |
116+
| `regexpdynamicpattern` | Custom `go/analysis` analyzer that flags regexp.MustCompile/Compile calls with non-constant patterns |
115117
| `seenmapbool` | Custom `go/analysis` analyzer that flags `map[string]bool` used as a set that should use `map[string]struct{}` |
116118
| `sortslice` | Custom `go/analysis` analyzer that flags `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc` |
117119
| `sprintferrdot` | Custom `go/analysis` analyzer that flags redundant `.Error()` calls on error values passed to `fmt` format functions |
@@ -170,6 +172,7 @@ import (
170172
panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
171173
"github.com/github/gh-aw/pkg/linters/rawloginlib"
172174
"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
175+
"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
173176
"github.com/github/gh-aw/pkg/linters/sortslice"
174177
"github.com/github/gh-aw/pkg/linters/sprintfbool"
175178
"github.com/github/gh-aw/pkg/linters/sprintfint"
@@ -199,6 +202,7 @@ _ = osexitinlibrary.Analyzer
199202
_ = panicinlibrarycode.Analyzer
200203
_ = rawloginlib.Analyzer
201204
_ = regexpcompileinfunction.Analyzer
205+
_ = regexpdynamicpattern.Analyzer
202206
_ = sortslice.Analyzer
203207
_ = sprintfbool.Analyzer
204208
_ = sprintfint.Analyzer
@@ -245,6 +249,7 @@ _ = trimleftright.Analyzer
245249
- `github.com/github/gh-aw/pkg/linters/panic-in-library-code` — panic-in-library-code analyzer subpackage
246250
- `github.com/github/gh-aw/pkg/linters/rawloginlib` — raw-log-in-lib analyzer subpackage
247251
- `github.com/github/gh-aw/pkg/linters/regexpcompileinfunction` — regexp-compile-in-function analyzer subpackage
252+
- `github.com/github/gh-aw/pkg/linters/regexpdynamicpattern` — regexp-dynamic-pattern analyzer subpackage
248253
- `github.com/github/gh-aw/pkg/linters/seenmapbool` — seen-map-bool analyzer subpackage
249254
- `github.com/github/gh-aw/pkg/linters/sortslice` — sort-slice analyzer subpackage
250255
- `github.com/github/gh-aw/pkg/linters/sprintferrdot` — sprintf-err-dot analyzer subpackage

pkg/linters/doc.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Package linters is a namespace for gh-aw's custom Go analysis linters.
22
//
3-
// All 62 active analyzers:
3+
// All 63 active analyzers:
44
//
55
// - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)
66
// - 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 @@
3939
// - panic-in-library-code — flags panic() calls in library packages
4040
// - rawloginlib — flags direct usage of the standard log package in library packages
4141
// - regexpcompileinfunction — flags regexp.MustCompile/Compile calls inside functions
42+
// - regexpdynamicpattern — flags regexp.MustCompile/Compile calls whose pattern is not a compile-time constant
4243
// - seenmapbool — flags map[string]bool used as a set that should use map[string]struct{}
4344
// - sortslice — flags sort.Slice / sort.SliceStable calls that should use slices.SortFunc / slices.SortStableFunc
4445
// - sprintferrdot — flags redundant .Error() calls on error values passed to fmt format functions
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Package regexpdynamicpattern implements a Go analysis linter that flags
2+
// calls to regexp.MustCompile() and regexp.Compile() whose pattern argument
3+
// is not a compile-time constant string. Dynamically constructed patterns can
4+
// panic at runtime on malformed input and, when influenced by untrusted
5+
// input, can enable catastrophic-backtracking (ReDoS) denial-of-service
6+
// attacks.
7+
package regexpdynamicpattern
8+
9+
import (
10+
"go/ast"
11+
"go/token"
12+
"go/types"
13+
14+
"golang.org/x/tools/go/analysis"
15+
"golang.org/x/tools/go/analysis/passes/inspect"
16+
17+
"github.com/github/gh-aw/pkg/linters/internal/astutil"
18+
"github.com/github/gh-aw/pkg/linters/internal/filecheck"
19+
"github.com/github/gh-aw/pkg/linters/internal/nolint"
20+
"github.com/github/gh-aw/pkg/logger"
21+
)
22+
23+
var pkgLog = logger.New("linters:regexpdynamicpattern")
24+
25+
// Analyzer is the regexp-dynamic-pattern analysis pass.
26+
var Analyzer = &analysis.Analyzer{
27+
Name: "regexpdynamicpattern",
28+
Doc: "reports regexp.MustCompile and regexp.Compile calls whose pattern is not a compile-time constant string",
29+
URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/regexpdynamicpattern",
30+
Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer},
31+
Run: run,
32+
}
33+
34+
func run(pass *analysis.Pass) (any, error) {
35+
pkgLog.Printf("analyzing package %s", pass.Pkg.Path())
36+
insp, err := astutil.Inspector(pass)
37+
if err != nil {
38+
return nil, err
39+
}
40+
noLintIndex, err := nolint.Index(pass)
41+
if err != nil {
42+
return nil, err
43+
}
44+
generatedFiles, err := filecheck.Index(pass)
45+
if err != nil {
46+
return nil, err
47+
}
48+
49+
for cur := range insp.Root().Preorder((*ast.CallExpr)(nil)) {
50+
call, ok := cur.Node().(*ast.CallExpr)
51+
if !ok || !isRegexpCompileCall(pass, call) {
52+
continue
53+
}
54+
if hasConstantStringPattern(pass, call) {
55+
continue
56+
}
57+
58+
pos := pass.Fset.PositionFor(call.Pos(), false)
59+
if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) {
60+
continue
61+
}
62+
if nolint.HasDirectiveForLinter(pos, noLintIndex, "regexpdynamicpattern") {
63+
continue
64+
}
65+
pkgLog.Printf("flagging dynamic regexp pattern at %s", pos)
66+
pass.Report(analysis.Diagnostic{
67+
Pos: call.Pos(),
68+
End: call.End(),
69+
Message: "regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input",
70+
})
71+
}
72+
73+
return nil, nil
74+
}
75+
76+
// isRegexpCompileCall checks if the call is to regexp.MustCompile or regexp.Compile,
77+
// resolving the package identity via the type checker to handle aliased imports
78+
// and avoid false positives from local identifiers named "regexp".
79+
func isRegexpCompileCall(pass *analysis.Pass, call *ast.CallExpr) bool {
80+
sel, ok := call.Fun.(*ast.SelectorExpr)
81+
if !ok {
82+
return false
83+
}
84+
if sel.Sel.Name != "MustCompile" && sel.Sel.Name != "Compile" {
85+
return false
86+
}
87+
ident, ok := sel.X.(*ast.Ident)
88+
if !ok || pass.TypesInfo == nil {
89+
return false
90+
}
91+
obj := pass.TypesInfo.ObjectOf(ident)
92+
if obj == nil {
93+
return false
94+
}
95+
pkgName, ok := obj.(*types.PkgName)
96+
if !ok || pkgName.Imported() == nil {
97+
return false
98+
}
99+
return pkgName.Imported().Path() == "regexp"
100+
}
101+
102+
// hasConstantStringPattern checks whether the regexp pattern is a compile-time constant string,
103+
// such as a string literal, const identifier, or an expression built entirely from constants
104+
// (e.g. concatenation of string literals/consts). Non-constant expressions such as
105+
// fmt.Sprintf results, concatenation involving variables, or function parameters return false.
106+
func hasConstantStringPattern(pass *analysis.Pass, call *ast.CallExpr) bool {
107+
if len(call.Args) == 0 {
108+
return false
109+
}
110+
111+
patternArg := call.Args[0]
112+
if lit, ok := patternArg.(*ast.BasicLit); ok && lit.Kind == token.STRING {
113+
return true
114+
}
115+
116+
tv, ok := pass.TypesInfo.Types[patternArg]
117+
if !ok || tv.Value == nil || tv.Type == nil {
118+
return false
119+
}
120+
121+
basic, ok := tv.Type.Underlying().(*types.Basic)
122+
return ok && basic.Kind() == types.String
123+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//go:build !integration
2+
3+
package regexpdynamicpattern_test
4+
5+
import (
6+
"testing"
7+
8+
"golang.org/x/tools/go/analysis/analysistest"
9+
10+
"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
11+
)
12+
13+
func TestAnalyzer(t *testing.T) {
14+
testdata := analysistest.TestData()
15+
analysistest.Run(t, testdata, regexpdynamicpattern.Analyzer, "regexpdynamicpattern")
16+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package regexpdynamicpattern
2+
3+
import (
4+
"fmt"
5+
"regexp"
6+
)
7+
8+
// not flagged: literal pattern at package level.
9+
var packageLevelRegexp = regexp.MustCompile(`^[a-z]+$`)
10+
11+
const constPattern = `^const$`
12+
const constSuffix = `$`
13+
14+
// not flagged: literal pattern.
15+
func ValidateLiteral(input string) bool {
16+
re := regexp.MustCompile(`^[a-z]+$`)
17+
return re.MatchString(input)
18+
}
19+
20+
// not flagged: const identifier pattern.
21+
func ValidateConst(input string) bool {
22+
re := regexp.MustCompile(constPattern)
23+
return re.MatchString(input)
24+
}
25+
26+
// not flagged: concatenation of constant-only expressions.
27+
func ValidateConstConcat(input string) bool {
28+
re := regexp.MustCompile(`^const` + constSuffix)
29+
return re.MatchString(input)
30+
}
31+
32+
// flagged: pattern built with fmt.Sprintf.
33+
func ValidateSprintf(prefix, input string) (bool, error) {
34+
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`
35+
if err != nil {
36+
return false, err
37+
}
38+
return re.MatchString(input), nil
39+
}
40+
41+
// flagged: string concatenation with a variable.
42+
func ValidateConcatVariable(suffix, input string) bool {
43+
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`
44+
return re.MatchString(input)
45+
}
46+
47+
// flagged: pattern passed through from a function parameter.
48+
func ValidateDynamic(pattern, input string) (bool, error) {
49+
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`
50+
if err != nil {
51+
return false, err
52+
}
53+
return re.MatchString(input), nil
54+
}
55+
56+
func suppressedPreviousLine(pattern, input string) (bool, error) {
57+
//nolint:regexpdynamicpattern
58+
re, err := regexp.Compile(pattern)
59+
if err != nil {
60+
return false, err
61+
}
62+
return re.MatchString(input), nil
63+
}
64+
65+
func suppressedSameLine(pattern, input string) (bool, error) {
66+
re, err := regexp.Compile(pattern) //nolint:regexpdynamicpattern
67+
if err != nil {
68+
return false, err
69+
}
70+
return re.MatchString(input), nil
71+
}

pkg/linters/registry.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
4141
"github.com/github/gh-aw/pkg/linters/rawloginlib"
4242
"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
43+
"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
4344
"github.com/github/gh-aw/pkg/linters/seenmapbool"
4445
"github.com/github/gh-aw/pkg/linters/sortslice"
4546
"github.com/github/gh-aw/pkg/linters/sprintfbool"
@@ -111,6 +112,7 @@ var allAnalyzers = []*analysis.Analyzer{
111112
panicinlibrarycode.Analyzer,
112113
rawloginlib.Analyzer,
113114
regexpcompileinfunction.Analyzer,
115+
regexpdynamicpattern.Analyzer,
114116
ssljson.Analyzer,
115117
seenmapbool.Analyzer,
116118
sortslice.Analyzer,

pkg/linters/spec_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
panicinlibrarycode "github.com/github/gh-aw/pkg/linters/panic-in-library-code"
4949
"github.com/github/gh-aw/pkg/linters/rawloginlib"
5050
"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
51+
"github.com/github/gh-aw/pkg/linters/regexpdynamicpattern"
5152
"github.com/github/gh-aw/pkg/linters/seenmapbool"
5253
"github.com/github/gh-aw/pkg/linters/sortslice"
5354
"github.com/github/gh-aw/pkg/linters/sprintfbool"
@@ -98,7 +99,7 @@ type docAnalyzer struct {
9899
// errortypeassertion, errstringmatch, execcommandwithoutcontext, fileclosenotdeferred, fmterrorfnoverbs, fprintlnsprintf,
99100
// goroutinemissingrecover, hardcodedfilepath, httpnoctx, httprespbodyclose, httpstatuscode, ioutildeprecated, jsonmarshalignoredeerror, largefunc, lenstringsplit, lenstringzero,
100101
// logfatallibrary, manualmutexunlock, mapclearloop, mapdeletecheck, nilctxpassed, osexitinlibrary, osgetenvlibrary, ossetenvlibrary, panic-in-library-code, rawloginlib,
101-
// regexpcompileinfunction, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
102+
// regexpcompileinfunction, regexpdynamicpattern, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
102103
// strconvparseignorederror, stringbytesroundtrip, stringreplaceminusone, stringsconcatloop, stringscountcontains, stringsindexcontains, stringsindexhasprefix, stringsjoinone, timeafterleak, timesleepnocontext, timenowsub,
103104
// tolowerequalfold, trimleftright, uncheckedflushreturn, uncheckedtypeassertion, walkfuncerrshadow, wgdonenotdeferred, writebytestring
104105
func documentedAnalyzers() []docAnalyzer {
@@ -140,6 +141,7 @@ func documentedAnalyzers() []docAnalyzer {
140141
{"panic-in-library-code", panicinlibrarycode.Analyzer},
141142
{"rawloginlib", rawloginlib.Analyzer},
142143
{"regexpcompileinfunction", regexpcompileinfunction.Analyzer},
144+
{"regexpdynamicpattern", regexpdynamicpattern.Analyzer},
143145
{"seenmapbool", seenmapbool.Analyzer},
144146
{"sortslice", sortslice.Analyzer},
145147
{"sprintferrdot", sprintferrdot.Analyzer},

0 commit comments

Comments
 (0)