You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
regexpdynamicpattern (pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go) flags regexp.Compile/MustCompile/CompilePOSIX/MustCompilePOSIX calls whose pattern argument isn't a constant-foldable string (i.e. a dynamically built regex, which can't be validated at compile time and is a recurring source of runtime panics/injection-shaped bugs). It is currently not part of the CI-enforced linter set in .github/workflows/cgo.yml, despite being clean, type-resolved, and having zero unsuppressed violations across every CI-enforced package today.
Correctness verified
isRegexpCompileCall resolves the receiver via pass.TypesInfo.ObjectOf(ident) and checks pkgName.Imported().Path() == "regexp" — this is proper type-resolved package identity (not fragile ident.Name == "regexp" string matching), and correctly covers all 4 compile-style entry points (Compile, MustCompile, CompilePOSIX, MustCompilePOSIX).
hasConstantStringPattern correctly accepts literal and constant-foldable string expressions and rejects everything else (variables, concatenation of non-constants, fmt.Sprintf-built patterns, etc.).
The linter is wired into internal/filecheck (test files are skipped via ShouldSkipFilename/IsTestFile) and supports internal/nolint line-level suppression.
Zero violations across CI-enforced packages
cgo.yml's LINTER_PACKAGES for the enforced run is ./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow. Every dynamically-constructed regexp.Compile/MustCompile call site in those 5 packages already carries a justified (nolint/redacted):regexpdynamicpattern comment, e.g.:
pkg/workflow/template_injection_utils.go (dynamic pattern for template placeholder matching)
pkg/workflow/gh_cli_permissions.go
pkg/workflow/yaml.go
pkg/parser/schema_suggestions.go
This mirrors the pattern from the already-closed remediation issue #50995 (which nolint-suppressed the known dynamic-pattern sites) and confirms there are no unsuppressed true positives left to fix before enforcing.
Recommendation
Add -regexpdynamicpattern to both LINTER_FLAGS definitions in .github/workflows/cgo.yml (the default run, and the GOOS=js GOARCH=wasm variant) so the linter graduates from advisory-only to CI-enforced, consistent with how #50995/#50999 already closed out the correctness work needed to make this safe.
Caveats
go build/go vet/go test are unavailable in this analysis environment (blocked pending approval), so "zero violations" was established by code-reading + targeted grep across the 5 enforced packages rather than a live golint-custom run. Recommend a maintainer run make golint-custom LINTER_FLAGS="-regexpdynamicpattern -test=false" LINTER_PACKAGES="./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow" to confirm a clean pass before merging the CI flag change.
Summary
regexpdynamicpattern(pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go) flagsregexp.Compile/MustCompile/CompilePOSIX/MustCompilePOSIXcalls whose pattern argument isn't a constant-foldable string (i.e. a dynamically built regex, which can't be validated at compile time and is a recurring source of runtime panics/injection-shaped bugs). It is currently not part of the CI-enforced linter set in.github/workflows/cgo.yml, despite being clean, type-resolved, and having zero unsuppressed violations across every CI-enforced package today.Correctness verified
isRegexpCompileCallresolves the receiver viapass.TypesInfo.ObjectOf(ident)and checkspkgName.Imported().Path() == "regexp"— this is proper type-resolved package identity (not fragileident.Name == "regexp"string matching), and correctly covers all 4 compile-style entry points (Compile,MustCompile,CompilePOSIX,MustCompilePOSIX).hasConstantStringPatterncorrectly accepts literal and constant-foldable string expressions and rejects everything else (variables, concatenation of non-constants,fmt.Sprintf-built patterns, etc.).internal/filecheck(test files are skipped viaShouldSkipFilename/IsTestFile) and supportsinternal/nolintline-level suppression.Zero violations across CI-enforced packages
cgo.yml'sLINTER_PACKAGESfor the enforced run is./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow. Every dynamically-constructedregexp.Compile/MustCompilecall site in those 5 packages already carries a justified(nolint/redacted):regexpdynamicpatterncomment, e.g.:pkg/workflow/template_injection_utils.go(dynamic pattern for template placeholder matching)pkg/workflow/gh_cli_permissions.gopkg/workflow/yaml.gopkg/parser/schema_suggestions.goThis mirrors the pattern from the already-closed remediation issue #50995 (which nolint-suppressed the known dynamic-pattern sites) and confirms there are no unsuppressed true positives left to fix before enforcing.
Recommendation
Add
-regexpdynamicpatternto bothLINTER_FLAGSdefinitions in.github/workflows/cgo.yml(the default run, and theGOOS=js GOARCH=wasmvariant) so the linter graduates from advisory-only to CI-enforced, consistent with how#50995/#50999already closed out the correctness work needed to make this safe.Caveats
go build/go vet/go testare unavailable in this analysis environment (blocked pending approval), so "zero violations" was established by code-reading + targeted grep across the 5 enforced packages rather than a livegolint-customrun. Recommend a maintainer runmake golint-custom LINTER_FLAGS="-regexpdynamicpattern -test=false" LINTER_PACKAGES="./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow"to confirm a clean pass before merging the CI flag change.