feat: support custom regex for rule id lookup#628
feat: support custom regex for rule id lookup#628blinxen wants to merge 9 commits intocoreruleset:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configurability for the regexes used to extract WAF rule IDs from log lines, addressing the limitation described in #614.
Changes:
- Introduces default regex constants and new config fields for standard and JSON log rule-id extraction.
- Stores compiled regexes on
FTWLogLinesand updates log parsing to use those instance regexes. - Adds/updates tests to validate default regex initialization and a custom regex override path.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
config/types.go |
Adds default regex constants and exposes regex fields on global config. |
config/config.go |
Sets default values for the new config fields. |
config/runner_config.go |
Propagates the new config fields into RunnerConfig. |
waflog/types.go |
Adds regex fields to FTWLogLines and initializes them during reset. |
waflog/waflog.go |
Compiles regexes from runner config into FTWLogLines and adds a setter. |
waflog/read.go |
Switches rule-id parsing from package-level regexes to instance regex fields. |
waflog/read_test.go |
Adds a test for custom std-log rule-id regex behavior. |
waflog/waflog_test.go |
Extends reset test to assert regex initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Please close conversations as you address the review 😄 |
|
I am pretty new to AI reviews. I thought the AI would trigger a re-review after all conversations are closed, I guess that was wrong. Should I open them again and comment on the reviews? Thanks for reviewing this PR! |
|
No worries, I've triggered a new review. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ll.stdLogIdRegex = compiledRegex | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
WithStdLogIdRegex introduces a public override hook for only the std log regex, but there’s no equivalent for the JSON log regex. Either add a matching WithJsonLogIdRegex (and consider a combined setter), or keep these overrides internal to tests via config to avoid an asymmetric public API.
| func (ll *FTWLogLines) WithJsonLogIdRegex(regex string) error { | |
| compiledRegex, err := regexp.Compile(regex) | |
| if err != nil { | |
| return err | |
| } | |
| ll.jsonLogIdRegex = compiledRegex | |
| return nil | |
| } |
There was a problem hiding this comment.
@Copilot
What do you mean with
keep these overrides internal to tests via config
| s.Require().NoError(err) | ||
| ll.WithStartMarker(bytes.ToLower([]byte(startMarkerLine))) | ||
| ll.WithEndMarker(bytes.ToLower([]byte(endMarkerLine))) | ||
| err = ll.WithStdLogIdRegex(`\[id="(\d+)"\]`) |
There was a problem hiding this comment.
This line is indented with spaces and doesn’t match gofmt formatting used throughout the file; running gofmt will fix it and avoid style/tooling failures in CI/pre-commit hooks.
| err = ll.WithStdLogIdRegex(`\[id="(\d+)"\]`) | |
| err = ll.WithStdLogIdRegex(`\[id="(\d+)"\]`) |
There was a problem hiding this comment.
@Copilot This seems to be an issue with .editorconfig. I changed it in my latest commit.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
It does not looks like the AI does anything if I comment on the review or address the AI directly. Do I have to address and then close the conversation? |
I have the same issue with #614 and created this PR in response :D. The PR adds both regexes used for rule id lookup into the global configuration.