fix: log resolved client_ip in WAF violation log (supersedes #306, adds tests)

[boecks]

This carries the one-line fix from #306 unchanged — original commit by @thelogh, authorship preserved via cherry-pick — rebased onto current main, plus the end-to-end regression test requested in the #306 review: a request arriving through a trusted proxy with X-Forwarded-For triggers a deny rule, and the test asserts the WAF rule violation detected log entry reports the forwarded client IP rather than the proxy's RemoteAddr. The test fails without the fix and passes with it.

Context: @thelogh did write tests in response to the review, but they were opened as thelogh#1 against his own fork's main rather than the patch-1 branch backing #306, so they never surfaced there and #306 has been stalled since early June. Not trying to take anything over — if @thelogh updates patch-1 instead, happy to close this one.

Field report: we've been running this exact fix in production since mid-June (fork pin in an xcaddy build) on a public-facing Caddy edge with Coraza + OWASP CRS in front of ~14 vhosts, alongside CrowdSec log ingestion that consumes these violation logs — the corrected client_ip is what makes those logs actionable for banning. No regressions observed; we'd love to retire the fork.

Summary by CodeRabbit

• Bug Fixes

  • Corrected WAF violation logs to record the originating client IP from forwarded request headers when trusted proxies are configured.
  • Improved accuracy of logged client and request details for denied requests.

• Tests

  • Added regression coverage verifying client IP and request URI values in violation logs.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Client IP violation logging

Layer / File(s)	Summary
Violation logging and regression test coraza.go, coraza_test.go	Violation logs use getClientAddress(r) for client_ip; an integration test verifies the X-Forwarded-For address and /denyme URI in the JSON log.
Estimated code review effort: 2 (Simple)	~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: logging the resolved client_ip in WAF violation logs and adding tests.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}