Skip to content

Adjust OWASP docs for version 0.5 - #20

Merged
sascha-egerer merged 10 commits into
mainfrom
docs/preset-owasp-crs-0.5
Sep 8, 2026
Merged

sascha-egerer merged 10 commits into
mainfrom
docs/preset-owasp-crs-0.5

Conversation

@sascha-egerer

Copy link
Copy Markdown
Contributor

No description provided.

…ReDoS notes

Replaces the stale '8 KiB truncate-and-head-match' description (that behavior let
a payload evade past the head and left the 8 KiB head as the peak backtracking
cost) with the new fail-closed per-value length cap. Adds a Per-Value Length Cap
section covering setMaxInspectableValueLength() and the lower-vs-higher trade-off
(CPU bound vs false-positive blocks on large legitimate values), and fixes the
operator table and ReDoS warning box to match.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated docs contain a few correctness/clarity issues (notably inconsistent operator token casing and minor grammar) that can mislead users copying configuration examples.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the documentation to reflect OWASP CRS preset package 0.5 behavior (notably anomaly scoring and expanded tuning/diagnostics guidance) and adds a dedicated performance guide to help users reason about runtime cost under load.

Changes:

  • Update OWASP CRS docs to describe anomaly-scoring evaluation, fail-closed behavior, tuning/exclusions, and new diagnostics headers.
  • Expand FAQ guidance for debugging blocked requests with the new rule/score diagnostics headers.
  • Add an “Advanced > Performance” page and link it in the VitePress sidebar.
File summaries
File Description
docs/features/owasp-crs.md Major doc update for CRS preset v0.5: anomaly scoring model, tuning/exclusions, diagnostics headers, and related guidance.
docs/faq.md Updates diagnostics-header FAQ to include rule-id list + anomaly score header.
docs/advanced/performance.md New performance guide documenting per-request costs, store overhead, and CRS cost considerations.
docs/.vitepress/config.ts Adds the new Performance page to the Advanced navigation.
Review details

Suppressed comments (1)

docs/features/owasp-crs.md:443

  • Operator names in this sentence use camelCase (@beginsWith, @endsWith) but the documented operators are lowercase (@beginswith, @endswith). This inconsistency makes it unclear which token users should write.
The string operators (`@streq`, `@contains`, `@beginsWith`, `@endsWith`) match
  • Files reviewed: 4/4 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/faq.md
Comment thread docs/features/owasp-crs.md Outdated
Comment thread docs/features/owasp-crs.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

Changes are documentation-focused and coherent; the only findings are minor wording/consistency nits with straightforward fixes.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

docs/advanced/performance.md:12

  • This page cites “PHP 8.5” for the benchmark environment, while other docs state the minimum requirement is PHP 8.2+. Adding that note here would prevent confusion for readers on older supported versions.
    docs/features/owasp-crs.md:76
  • Spelling is inconsistent with the rest of the docs, which use American English (“behavior”). Consider changing “behaviour” to “behavior” for consistency.

This issue also appears on line 99 of the same file.

docs/features/owasp-crs.md:99

  • owasp_log_data is singular, so “provides them” should be “provides it” for grammatical correctness.
provides them, and `owasp_fail_closed` on a fail-closed block
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The updated OWASP CRS docs include at least one example/signature mismatch and a couple of correctness/clarity issues that could cause copy/paste failures or confusion for users.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

docs/features/owasp-crs.md:99

  • Minor grammar: owasp_log_data is singular, so the pronoun should be singular too.
    docs/features/owasp-crs.md:166
  • The when: callback example only accepts $value, but the text below says the condition is invoked with (string $value, ?string $name, string $variable). In PHP 8+, calling a closure with more arguments than it declares throws an ArgumentCountError, so this snippet would break if the matcher passes all three parameters.
    docs/features/owasp-crs.md:698
  • This code block isn’t self-contained (missing use imports for Presets, ParanoiaLevel, and CoreRuleSetMatcher), while earlier examples in this page include the needed imports; adding them makes the snippet copy/pasteable.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@sascha-egerer
sascha-egerer merged commit 5ed2f83 into main Sep 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants