Repository navigation
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Ai Contribution Disclosure | The PR body omits the required lowercase ## what, ## why, and ## refs sections. It also omits ## ai disclosure. The reviewed commit message contains the prohibited Co-authored-by: attributio… |
Add lowercase ## what, ## why, and ## refs sections. Add ## ai disclosure with concrete **tools used** including model and version, **assisted with** describing the generated content, and **review performed** describing specif… |
✅ Passed checks (17 passed)
-
Check name Status Explanation Title check ✅ Passed The title clearly and concisely describes the main documentation change: engine-specific rule-range behavior for ById exclusions. Description check ✅ Passed The description includes the required Proposed changes and Further comments sections. It explains the documentation updates, affected directives, engine differences, scope, and linked issue. Linked Issues check ✅ Passed Issue #301requires clear documentation of rule-range support across engines. The PR updates2-3-false-positives-and-tuning.md, removes the obsolete ModSecurity v3 limitation for `ctl:ruleRemoveById…Out of Scope Changes check ✅ Passed The supplied summary identifies changes only in content/2-how-crs-works/2-3-false-positives-and-tuning.md. The changes directly support issue#301. No unrelated runtime code, rule IDs, paranoia leve…Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… Regex Assembly Is The Source Of Truth ✅ Passed Passed: not applicable. The pull-request diff changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It does not modify an@rxpattern inrules/*.confand does not add or modify…Rule Change Requires Go-Ftw Test Coverage ✅ Passed Not applicable: the authoritative diff changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It adds noSecRulechange inrules/*.conforplugins/*.conf, and it changes no pa…Redos Risk & Re2 Compatibility ✅ Passed Passed — not applicable. The pull request changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It does not add or modify an@rxpattern inrules/*.confor `regex-assembly/*.ra…False Positive Risk & Existing Coverage ✅ Passed Passed — not applicable. The pull request changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It does not add or widen a detection pattern, or add a rule, inrules/*.conf, `plu…Crs Rule Metadata & Id Conventions ✅ Passed Passed: not applicable. The pull request changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It does not add or modify aSecRuleinrules/*.conf,plugins/*.conf, or `crs-se…Rule & Config Breaking Changes ✅ Passed PASS — The authoritative PR diff contains only one modified Markdown documentation file. It changes a footnote and adds a rule-range support table; it does not remove or renumber rule IDs, change defa… Owasp Security (Web, Api & Llm) ✅ Passed PASS — The authoritative diff changes only one Markdown documentation file. It removes an outdated footnote and adds a rule-range support matrix; it does not change runtime code, dependencies, authent… Unpinned Dependencies & Actions ✅ Passed Passed; not applicable. The reviewed diff changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It does not change a dependency manifest, lockfile, Dockerfile, workflow, pipeline, …Secrets, Payloads & Pii In Logs ✅ Passed PASS — The authoritative diff changes only one Markdown documentation file. The added and modified lines describe rule-range support and contain no logging, telemetry, errors, stack traces, request/re… New Dependency Scrutiny ✅ Passed PASS — The authoritative PR diff changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. It adds no package or dependency manifest entry, GitHub Actionuses:step, or Buildkite plu…Install & Build-Time Code Execution ✅ Passed PASS: The authoritative PR diff changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md. The changes are documentation text and an ApacheSecRuleRemoveByIdexample. They add no inst…Renovate: Config Present And Valid ✅ Passed PASS — the custom check is not triggered. The reviewed PR changes only content/2-how-crs-works/2-3-false-positives-and-tuning.md; it does not touch any Renovate configuration path. The PR head conta…
Full details: Ai Contribution Disclosure
-
Explanation
The PR body omits the required lowercase
## what,## why, and## refssections. It also omits## ai disclosure. The reviewed commit message contains the prohibitedCo-authored-by:attribution trailer, and the PR body contains theCOPILOT CODING AGENT SUFFIXmarker. The diff is limited to one documentation file, so no large-test-batch signal is present, but these explicit policy violations are sufficient.Resolution
Add lowercase
## what,## why, and## refssections. Add## ai disclosurewith concrete**tools used**including model and version,**assisted with**describing the generated content, and**review performed**describing specific verification. Remove theCo-authored-by:trailer and the AI-tool signature marker from the commit message and PR body.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Deploying crs-documentation with
|
| Latest commit: |
8e21407
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://383f38fa.documentation-km5.pages.dev |
| Branch Preview URL: | https://copilot-document-rule-range.documentation-km5.pages.dev |
fzipi
changed the title
fzipi
left a comment
fzipi
left a comment
There was a problem hiding this comment.
The v2 column is wrong for the two target-by-ID rows: v2 expands ranges there, only v3 doesn't (suggestion inline). The rest looks good. The same mistake was in the CRS example file prose and is being fixed in coreruleset/coreruleset#4866.
ai disclosure
- tools used: Claude Code (Opus 5.5)
- assisted with: drafting this review
- review performed: read the ModSecurity v2 source for both the directive and the ctl action (
apache2_config.c,re_actions.c,re.c)
| | `SecRuleUpdateTargetById` | ⚠️ First ID only | ⚠️ First ID only | ✅ Full range | | ||
| | `ctl:ruleRemoveTargetById` | ⚠️ First ID only | ⚠️ First ID only | ✅ Full range | |
There was a problem hiding this comment.
ModSecurity v2 expands ranges for both of these. SecRuleUpdateTargetById and ctl:ruleRemoveTargetById both match rules through msre_ruleset_rule_matches_exception() → rule_id_in_range() (apache2/re.c), the same path SecRuleRemoveById uses. Only v3 is first-ID-only.
| | `SecRuleUpdateTargetById` | ⚠️ First ID only | ⚠️ First ID only | ✅ Full range | | |
| | `ctl:ruleRemoveTargetById` | ⚠️ First ID only | ⚠️ First ID only | ✅ Full range | | |
| | `SecRuleUpdateTargetById` | ✅ Full range | ⚠️ First ID only | ✅ Full range | | |
| | `ctl:ruleRemoveTargetById` | ✅ Full range | ⚠️ First ID only | ✅ Full range | |
There was a problem hiding this comment.
Updated in 8e21407: both rows now show ModSecurity v2 as ✅ Full range, with only ModSecurity v3 marked ⚠️ First ID only.
Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Fixed in 8e21407. The v2 column for |
Proposed changes
This updates the false-positive tuning docs to explicitly describe rule-range support differences across engines, aligned with recent corrections in core CRS docs. The goal is to prevent incorrect exclusion guidance when using
ByIddirectives/actions with ranges.ctl:ruleRemoveByIdrange syntax is unsupported in ModSecurity v3.SecRuleRemoveByIdctl:ruleRemoveByIdSecRuleUpdateTargetByIdctl:ruleRemoveTargetByIdFurther comments
2-3-false-positives-and-tuning.md; no behavioral/runtime code changes.Summary by CodeRabbit
ByIddirectives and actions across ModSecurity v2, ModSecurity v3, and Coraza.