feat: add check for TX.N usage without capture #87

fzipi · 2025-11-25T15:39:13Z

what

add check for TX.N usage without capture

why

well, the above

refs

Fixes #69.

fzipi · 2026-01-02T13:21:26Z

Now that we fixed this upstream, can you take a look @airween ?

Copilot

Pull request overview

This PR adds a new linter check to detect usage of TX.N (captured transaction variables TX:0 through TX:9) without a corresponding capture action in ModSecurity rules. The check extends beyond the existing functionality that only checked TX.N as rule targets to also detect TX.N in action arguments (msg, logdata, setvar, tag) and operator arguments.

Key changes:

Enhanced the capture check to detect TX.N references in variable expansions like %{TX.1} in addition to target references like TX:1
Added comprehensive test coverage for different TX.N usage scenarios
Fixed a bug in variables_usage.py where re.I was incorrectly passed as a positional argument to re.sub()

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/crs_linter/rules/check_capture.py`	Enhanced capture check to detect TX.N in expansions (%{TX.N}), operator arguments, and action arguments (msg, logdata, setvar, tag)
`tests/test_check_capture.py`	Added comprehensive test suite with 13 test cases covering various TX.N usage scenarios including valid and invalid cases
`README.md`	Updated documentation to reflect the expanded scope of the capture check with examples
`src/crs_linter/rules/variables_usage.py`	Fixed incorrect regex flag usage by using `flags=` keyword argument
`.github/workflows/test.yml`	Updated CRS version from 4.18.0 to 4.21.0 for testing

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

src/crs_linter/rules/check_capture.py

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

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

src/crs_linter/rules/check_capture.py

tests/test_check_capture.py

.github/workflows/test.yml

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

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

src/crs_linter/rules/check_capture.py

Copilot · 2026-01-06T14:11:13Z

@fzipi I've opened a new pull request, #99, to work on those changes. Once the pull request is ready, I'll request review from you.

fzipi · 2026-01-06T14:27:51Z

Ugh, last change broke it.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

src/crs_linter/rules/check_capture.py

fzipi · 2026-01-14T14:04:46Z

@copilot Your previous suggestion broke the tests. Take a look at: https://github.com/coreruleset/crs-linter/actions/runs/20751085465/job/59580380141?pr=87

Copilot · 2026-01-14T14:04:56Z

@fzipi I've opened a new pull request, #108, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

src/crs_linter/rules/check_capture.py

Signed-off-by: Felipe Zipitria <[email protected]>

Co-authored-by: Copilot <[email protected]>

Add test case that covers scenario where TX.N appears both as a target variable and in an expansion (e.g., in msg or logdata) within same rule. This test would have caught the bug where use_captured_var_in_expansion was not set correctly when use_captured_var was already True. Also fix the bug by moving use_captured_var_in_expansion assignment outside the guard condition, following the same pattern as c598d31. Co-authored-by: fzipi <[email protected]>

Co-authored-by: fzipi <[email protected]>

Signed-off-by: Felipe Zipitria <[email protected]>

src/crs_linter/rules/check_capture.py

feat: add check for TX.N usage without capture Signed-off-by: Felipe Zipitria <[email protected]>

fzipi requested a review from airween November 25, 2025 15:39

fzipi force-pushed the feat/check-capture-tx branch from f37a12e to 4d27a6e Compare November 25, 2025 15:41

airween mentioned this pull request Nov 25, 2025

fix: add missing capture action to affected rules coreruleset/coreruleset#4361

Merged

fzipi force-pushed the feat/check-capture-tx branch from 4d27a6e to 12b7080 Compare January 2, 2026 13:19

fzipi requested a review from Copilot January 5, 2026 21:34

Copilot started reviewing on behalf of fzipi January 5, 2026 21:34 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

fzipi requested a review from Copilot January 6, 2026 13:31

Copilot started reviewing on behalf of fzipi January 6, 2026 13:31 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

tests/test_check_capture.py Show resolved Hide resolved

.github/workflows/test.yml Outdated Show resolved Hide resolved

fzipi requested a review from Copilot January 6, 2026 14:00

Copilot started reviewing on behalf of fzipi January 6, 2026 14:00 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

Copilot AI mentioned this pull request Jan 6, 2026

Add test coverage for TX.N in both target and expansion; fix bug #99

Merged

fzipi requested a review from Copilot January 14, 2026 13:56

Copilot started reviewing on behalf of fzipi January 14, 2026 13:57 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Outdated Show resolved Hide resolved

Copilot AI mentioned this pull request Jan 14, 2026

fix: initialize capture_level to 0 for consistency #108

Merged

Copilot AI reviewed Jan 14, 2026

View reviewed changes

fzipi requested a review from Copilot January 14, 2026 14:14

Copilot started reviewing on behalf of fzipi January 14, 2026 14:15 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

fzipi requested a review from Copilot January 23, 2026 12:13

Copilot started reviewing on behalf of fzipi January 23, 2026 12:13 View session

fzipi mentioned this pull request Jan 23, 2026

chore(941360,941370,941380): cleaning useless capture keyword coreruleset/coreruleset#4419

Merged

10 tasks

Copilot AI reviewed Jan 23, 2026

View reviewed changes

airween requested changes Jan 24, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

fzipi force-pushed the feat/check-capture-tx branch from ce38235 to 08deeec Compare January 24, 2026 17:47

fzipi and others added 10 commits January 25, 2026 22:34

feat: add check for TX.N usage without capture

9a8d62b

Signed-off-by: Felipe Zipitria <[email protected]>

fix: check argument or value for TX.N references

1285348

Co-authored-by: Copilot <[email protected]>

fix: remove guard condition and set flag when expansion pattern

14fafdc

Co-authored-by: Copilot <[email protected]>

fix: check capture_level is defined

23f1eb3

Co-authored-by: Copilot <[email protected]>

Initial plan

ef82e62

Initial plan

5b96339

fix: initialize capture_level to 0 for consistency

da48eb2

Co-authored-by: fzipi <[email protected]>

fix: rule chain has multiple capture actions at different chain levels

611f3f8

Signed-off-by: Felipe Zipitria <[email protected]>

fix: rebase conflict

03214f6

Signed-off-by: Felipe Zipitria <[email protected]>

fzipi force-pushed the feat/check-capture-tx branch from 08deeec to 03214f6 Compare January 26, 2026 01:34

fzipi requested a review from airween January 26, 2026 01:39

airween approved these changes Jan 26, 2026

View reviewed changes

src/crs_linter/rules/check_capture.py Show resolved Hide resolved

fzipi merged commit 8f9e541 into main Jan 26, 2026
4 checks passed

fzipi deleted the feat/check-capture-tx branch January 26, 2026 20:37

fzipi added a commit that referenced this pull request Jan 27, 2026

Merge pull request #87 from coreruleset/feat/check-capture-tx

7723aa4

feat: add check for TX.N usage without capture Signed-off-by: Felipe Zipitria <[email protected]>

feat: add check for TX.N usage without capture #87

feat: add check for TX.N usage without capture #87

Uh oh!

Conversation

fzipi commented Nov 25, 2025

what

why

refs

Uh oh!

fzipi commented Jan 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI commented Jan 6, 2026

Uh oh!

fzipi commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

fzipi commented Jan 14, 2026

Uh oh!

Copilot AI commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants