test: add nolog, auditlog test by jcchavezs · Pull Request #1307 · corazawaf/coraza

jcchavezs · 2025-02-14T07:20:58Z

Runs actions as they cannot run in #1304

codecov · 2025-02-14T07:22:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.74%. Comparing base (ed7f3b6) to head (2b2b989).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1307      +/-   ##
==========================================
- Coverage   86.74%   86.74%   -0.01%     
==========================================
  Files         179      179              
  Lines        8802     8801       -1     
==========================================
- Hits         7635     7634       -1     
  Misses        908      908              
  Partials      259      259

Flag	Coverage Δ
coraza.no_memoize	`86.81% <100.00%> (-0.01%)`	⬇️
coraza.rule.case_sensitive_args_keys	`86.70% <100.00%> (-0.01%)`	⬇️
coraza.rule.mandatory_rule_id_check	`86.72% <100.00%> (-0.01%)`	⬇️
coraza.rule.multiphase_evaluation	`86.46% <100.00%> (-0.01%)`	⬇️
coraza.rule.no_regex_multiline	`86.69% <100.00%> (-0.01%)`	⬇️
coraza.rule.rx_prefilter	`86.73% <100.00%> (-0.01%)`	⬇️
default	`86.74% <100.00%> (-0.01%)`	⬇️
examples+	`17.41% <100.00%> (-0.02%)`	⬇️
examples+coraza.no_memoize	`84.67% <100.00%> (-0.01%)`	⬇️
examples+coraza.rule.case_sensitive_args_keys	`84.66% <100.00%> (-0.01%)`	⬇️
examples+coraza.rule.mandatory_rule_id_check	`84.77% <100.00%> (-0.01%)`	⬇️
examples+coraza.rule.multiphase_evaluation	`86.46% <100.00%> (-0.01%)`	⬇️
examples+coraza.rule.no_regex_multiline	`84.59% <100.00%> (-0.01%)`	⬇️
examples+coraza.rule.rx_prefilter	`84.74% <100.00%> (-0.01%)`	⬇️
examples+no_fs_access	`84.09% <100.00%> (-0.01%)`	⬇️
ftw	`86.74% <100.00%> (-0.01%)`	⬇️
no_fs_access	`86.06% <100.00%> (-0.01%)`	⬇️
tinygo	`86.73% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jcchavezs · 2025-02-14T14:08:59Z

Are you happy with this @M4tteoP ?

M4tteoP · 2025-02-14T14:36:07Z

Happy to see a green CI :D, allow me some time to review and reason the whole reported issue 🙇🏻‍♂️

Copilot

Pull request overview

This PR implements proper handling of audit log parts flags, specifically separating the "log" and "audit" actions for rule matches. It ensures that messages are correctly included in audit logs based on the H flag (AuditLogPartAuditLogTrailer) and the rule's audit status.

Changes:

Introduced separate Audit flag tracking for matched rules, distinct from the Log flag
Fixed the audit log logic to check the Audit() method instead of Log() when determining which messages to include in audit logs
Moved message logging from the AuditLogPartRulesMatched case to AuditLogPartAuditLogTrailer case where it belongs (H flag)
Added comprehensive tests to verify the behavior of nolog/auditlog combinations and H flag handling
Updated dependency versions

Reviewed changes

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

Show a summary per file

File	Description
internal/corazarules/rule_match.go	Added `Audit_` field and `Audit()` method to track audit logging separately from error logging
internal/corazawaf/transaction.go	Fixed audit log message inclusion logic to use `Audit()` instead of `Log()`, moved message processing to correct case, and set `Audit_` field on matched rules
internal/corazawaf/transaction_test.go	Added `Audit` flag to test rule to match new behavior
testing/auditlog_test.go	Added comprehensive tests for nolog/auditlog combinations and H flag behavior
testing/coreruleset/go.sum	Updated dependency versions
go.sum	Updated dependency versions
examples/http-server/go.sum	Updated dependency versions
examples/http-server/go.mod	Updated dependency versions

Copilot · 2026-01-16T20:48:20Z

 				}
 			}
+		case types.AuditLogPartRulesMatched:
+			// implement matched rules


This TODO comment suggests that AuditLogPartRulesMatched (K flag) is not implemented. However, this appears to be an intentional placeholder since matched rules are now being handled in AuditLogPartAuditLogTrailer. Consider either implementing this part or documenting why it's left empty (e.g., 'K part is intentionally not implemented as matched rules are logged in H part').

Suggested change

// implement matched rules

// K part is intentionally not implemented.

// Matched rules are logged in the audit log trailer (H part) above.

fzipi · 2026-04-03T04:22:04Z

@jcchavezs Can we close this one?

fzipi · 2026-04-03T20:57:59Z

@copilot is this change needed?

…essage Agent-Logs-Url: https://github.com/corazawaf/coraza/sessions/409d9fbf-7395-48ef-a5aa-edabd0176121 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Copilot · 2026-04-03T21:01:41Z

@copilot is this change needed?

Yes, the changes are needed. The PR fixes the audit log parts flag handling — specifically, messages were being gated on the Log() flag instead of Audit(), and the H flag (AuditLogPartAuditLogTrailer) wasn't being respected correctly when determining which matched-rule messages to include in audit logs.

In commit bd50da1 I also fixed a bug from the review where tx.audit = r.Audit was unconditionally overwriting the accumulated audit state — it now uses tx.audit = tx.audit || r.Audit so that if any matching rule sets auditlog, the transaction remains audited.

…-7395-48ef-a5aa-edabd0176121 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

fzipi · 2026-04-03T21:07:27Z

@copilot Can you fix the conflicts?

Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Agent-Logs-Url: https://github.com/corazawaf/coraza/sessions/e42484b1-45d0-46cb-ad66-5a4fbdd9db79 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Copilot · 2026-04-03T21:14:05Z

@copilot Can you fix the conflicts?

Done — merged main into this branch in commit 2b2b989. All conflicts in internal/corazarules/rule_match.go, internal/corazawaf/transaction.go, testing/auditlog_test.go, and the dependency files are resolved. All tests pass.

fzipi

LGTM, adds a new test.

tty2 added 5 commits February 10, 2025 09:47

add tests for the expected behavior

403a311

make tests more precise

bc3d899

clean up deafult config

82d946d

Merge branch 'main' into audit-log-parts-flags

6e80d65

move messages to H part, separate error logs and audit logs

af9e194

jcchavezs requested a review from a team as a code owner February 14, 2025 07:20

Merge branch 'main' into audit-log-parts-flags

f1c70c3

fzipi requested a review from Copilot January 16, 2026 20:46

Copilot started reviewing on behalf of fzipi January 16, 2026 20:46 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Copilot started work on behalf of fzipi April 3, 2026 20:58 View session

fix: preserve audit flag across rules, fix K part comment, fix test m…

bd50da1

…essage Agent-Logs-Url: https://github.com/corazawaf/coraza/sessions/409d9fbf-7395-48ef-a5aa-edabd0176121 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Agent-Logs-Url: https://github.com/corazawaf/coraza/sessions/409d9fbf…

0366c0d

…-7395-48ef-a5aa-edabd0176121 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Copilot finished work on behalf of fzipi April 3, 2026 21:02

Copilot AI requested a review from fzipi April 3, 2026 21:02

Copilot started work on behalf of fzipi April 3, 2026 21:07 View session

Copilot AI and others added 2 commits April 3, 2026 21:13

Merge branch 'main' into audit-log-parts-flags

0c5074a

Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Merge branch 'main' into audit-log-parts-flags

2b2b989

Agent-Logs-Url: https://github.com/corazawaf/coraza/sessions/e42484b1-45d0-46cb-ad66-5a4fbdd9db79 Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Copilot finished work on behalf of fzipi April 3, 2026 21:14

fzipi changed the title ~~Audit log parts flags~~ test: add nolog, auditlog test Apr 3, 2026

fzipi approved these changes Apr 3, 2026

View reviewed changes

fzipi merged commit c431d85 into main Apr 3, 2026
24 checks passed

fzipi deleted the audit-log-parts-flags branch April 3, 2026 23:06

	// implement matched rules
	// K part is intentionally not implemented.
	// Matched rules are logged in the audit log trailer (H part) above.

Uh oh!

Conversation

jcchavezs commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jcchavezs commented Feb 14, 2025

Uh oh!

M4tteoP commented Feb 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fzipi commented Apr 3, 2026

Uh oh!

fzipi commented Apr 3, 2026

Uh oh!

Copilot AI commented Apr 3, 2026

Uh oh!

fzipi commented Apr 3, 2026

Uh oh!

Copilot AI commented Apr 3, 2026

Uh oh!

fzipi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jcchavezs commented Feb 14, 2025 •

edited

Loading

codecov Bot commented Feb 14, 2025 •

edited

Loading