Skip to content

Fix regexpdynamicpattern POSIX coverage and rationale - #51002

Merged
pelikhan merged 3 commits into
mainfrom
copilot/fix-regexpdynamicpattern-issue
Aug 7, 2026
Merged

Fix regexpdynamicpattern POSIX coverage and rationale#51002
pelikhan merged 3 commits into
mainfrom
copilot/fix-regexpdynamicpattern-issue

Conversation

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

regexpdynamicpattern framed dynamic Go regexp patterns as catastrophic-backtracking/ReDoS risks, which is inaccurate for Go’s RE2-based regexp. It also missed regexp.CompilePOSIX and regexp.MustCompilePOSIX, which accept the same pattern argument and have the same dynamic-pattern concerns.

  • Analyzer coverage

    • Treats all regexp compile entry points as candidates:
      regexp.Compile(...)
      regexp.MustCompile(...)
      regexp.CompilePOSIX(...)
      regexp.MustCompilePOSIX(...)
  • Diagnostic accuracy

    • Removes ReDoS/catastrophic-backtracking wording.
    • Describes the actual risks: malformed dynamic patterns panic in MustCompile variants, return errors in Compile variants, or allow untrusted input to control pattern complexity/size.
  • Tests and docs

    • Adds POSIX testdata for dynamic patterns that should be reported.
    • Adds POSIX constant/literal testdata that should remain allowed.
    • Updates the analyzer package doc, linter docs, and ADR text to match the corrected threat model.

Copilot AI and others added 2 commits August 7, 2026 05:22
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix rationale in regexpdynamicpattern for ReDoS Fix regexpdynamicpattern POSIX coverage and rationale Aug 7, 2026
Copilot AI requested a review from pelikhan August 7, 2026 05:27
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Great work, @Copilot! 🎉 This PR looks excellent and ready for review.

Summary

You've successfully refined the regexpdynamicpattern linter by:

  • Updating the ADR rationale to clarify the actual risk (pattern complexity/size under untrusted input) rather than catastrophic backtracking claims
  • Extending detection to POSIX variants (regexp.CompilePOSIX and regexp.MustCompilePOSIX)
  • Adding comprehensive test coverage for both POSIX and non-POSIX patterns

The changes are focused, well-tested, and clearly documented. The diff size is reasonable and all modifications align directly with fixing issue #50999.

Ready for maintainer review and merge once the final validation checklist is complete.

Generated by ✅ Contribution Check · auto · 46 AIC · ⊞ 8.7K ·

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR Triage

Category: docs (linter rationale/coverage doc update) · Risk: low · Score: 25/100 (impact 8, urgency 5, quality 12)

Recommendation: defer — grouped with #51017, #51016, #50995 (lint-quality-cleanup batch)

Improves POSIX coverage docs/rationale for the regexpdynamicpattern linter ADR. Draft, CI pending. One human comment already on the thread — recommend addressing that first before promoting out of draft.

Generated by 🔧 PR Triage Agent · auto · 55.6 AIC · ⌖ 2.45 AIC · ⊞ 7.9K ·

@pelikhan
pelikhan marked this pull request as ready for review August 7, 2026 14:09
Copilot AI balanced review requested due to automatic review settings August 7, 2026 14:09
@pelikhan
pelikhan merged commit d7d6cab into main Aug 7, 2026
1 check passed
@pelikhan
pelikhan deleted the copilot/fix-regexpdynamicpattern-issue branch August 7, 2026 14:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Corrects the linter’s Go regexp threat model and adds POSIX compile-function coverage.

Changes:

  • Detects dynamic patterns in CompilePOSIX and MustCompilePOSIX.
  • Replaces inaccurate ReDoS wording with panic, error, and pattern-size risks.
  • Adds POSIX test cases and synchronizes documentation.
Show a summary per file
File Description
pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go Expands detection and revises diagnostics.
pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go Adds positive and negative POSIX cases.
pkg/linters/README.md Updates the linter summary.
pkg/linters/doc.go Synchronizes package documentation.
docs/adr/50674-add-regexpdynamicpattern-linter.md Corrects rationale and records POSIX scope.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.86.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regexpdynamicpattern: ReDoS rationale is factually wrong, misses CompilePOSIX/MustCompilePOSIX

3 participants