corazawaf · jptosso · Apr 17, 2026
@@ -0,0 +1,55 @@
+# ADR-NNNN: <short decision title>
+
+- **Status:** accepted
+- **Date:** YYYY-MM-DD (PR merge date)
+- **Version:** vX.Y.Z
+- **PR:** [#NNNN](https://github.com/corazawaf/coraza/pull/NNNN)
+- **Issue(s):** [#MMMM](https://github.com/corazawaf/coraza/issues/MMMM) — or "No linked issue"
+- **Deciders:** @author, @reviewer1, @reviewer2
+- **Category:** Feature | Parity | Perf | Refactor
+
+## Context and Problem
+
+Why the change was needed. Quote from the issue or PR body where possible.
+
+## Decision Drivers
+
+- driver 1
+- driver 2
+
+## Considered Options
+
+- option A
+- option B
+
+## Decision Outcome
+
+Chosen: **<option>**, because <reason grounded in PR discussion>.
+
+## Technical Discussion
+
+Summary of substantive thread. Every ADR must contain **either** at least one quoted
+comment with a direct permalink **or** the sentence "No substantive technical
+discussion recorded on the PR or issue thread." Do not fabricate discussion.
+
+Representative quote(s):
+
+> "<quote>" — @user ([comment](https://github.com/corazawaf/coraza/pull/NNNN#issuecomment-XXXXX))
+
+## Participants
+
+- @author — author
+- @reviewer1 — review
+- @reviewer2 — review comments
+- @commenter3 — issue thread
+
+## Consequences
+
+- **Positive:** …
+- **Negative / follow-up:** …
+
+## References
+
+- PR: https://github.com/corazawaf/coraza/pull/NNNN
+- Issue: https://github.com/corazawaf/coraza/issues/MMMM
+- Related ADRs: ADR-XXXX
@@ -0,0 +1,69 @@
+# ADR-0001: Enable RESPONSE_ARGS collection
+
+- **Status:** accepted
+- **Date:** 2023-06-12
+- **Version:** v3.0.1
+- **PR:** [#811](https://github.com/corazawaf/coraza/pull/811)
+- **Issue(s):** No linked issue
+- **Deciders:** @jptosso, @M4tteoP, @fzipi
+- **Category:** Parity (ModSecurity parity)
+
+## Context and Problem
+
+ModSecurity exposes a `RESPONSE_ARGS` collection so rules can inspect arguments
+parsed from the response body (for example JSON response fields). Coraza v3.0.0
+had the scaffolding but the collection was not wired up end-to-end — there was
+no way to write response-phase argument matching rules.
+
+## Decision Drivers
+
+- Parity with ModSecurity behaviour for rules that inspect response bodies.
+- Symmetry with the existing `ARGS_GET`/`ARGS_POST` request-phase infrastructure.
+
+## Considered Options
+
+- Build a brand-new response-argument pipeline.
+- Reuse the existing request-side body processors and simply enable the
+  response-arg collection on the transaction.
+
+## Decision Outcome
+
+Chosen: **reuse the existing body processors** to populate a response-arg
+collection. The PR is minimal, surfacing the feature rather than introducing a
+new subsystem.
+
+## Technical Discussion
+
+The PR itself ships without a written description. Review feedback focused on
+housekeeping rather than design, and follow-up work was explicitly deferred.
+
+> "Can we move this TODO to a proper issue so someone might implement it?"
+> — @fzipi ([review comment](https://github.com/corazawaf/coraza/pull/811#discussion_r1226569095))
+
+> "There is a work in progress around it: …" — @M4tteoP, pointing at the
+> in-progress response-arg discussion on OWASP Slack
+> ([review comment](https://github.com/corazawaf/coraza/pull/811#discussion_r1227294607))
+
+No substantive architectural debate took place on the PR thread itself; the
+design was discussed out-of-band (OWASP Slack) and the code change was merged
+with two approvals.
+
+## Participants
+
+- @jptosso — author
+- @M4tteoP — review, Slack discussion pointer
+- @fzipi — review
+
+## Consequences
+
+- **Positive:** Response-phase rules can now match on parsed body arguments,
+  closing a compatibility gap with ModSecurity.
+- **Negative / follow-up:** Response-body argument parsing uses the same
+  processors as requests, so any parser limitations (JSON depth, etc.) apply
+  symmetrically. Later limits such as `SecArgumentsLimit` (ADR-0002) were added
+  to keep this surface bounded.
+
+## References
+
+- PR: https://github.com/corazawaf/coraza/pull/811
+- Related ADRs: ADR-0002 (SecArgumentsLimit directive)
@@ -0,0 +1,90 @@
+# ADR-0002: `SecArgumentsLimit` directive
+
+- **Status:** accepted
+- **Date:** 2023-06-14
+- **Version:** v3.0.2
+- **PR:** [#812](https://github.com/corazawaf/coraza/pull/812)
+- **Issue(s):** No linked issue
+- **Deciders:** @potats0, @jptosso, @jcchavezs, @fzipi, @M4tteoP, @anuraaga, @airween, @theseion
+- **Category:** Parity (ModSecurity parity)
+
+## Context and Problem
+
+Coraza accepted unbounded numbers of parsed arguments, leaving it open to
+parameter-pollution and resource-exhaustion attacks. ModSecurity documents a
+`SecArgumentsLimit` directive (default 1000) for this; Coraza had none.
+
+## Decision Drivers
+
+- ModSecurity parity — the directive already exists in the reference
+  implementation.
+- Hard bound on argument count to protect against DoS.
+- Keep the cost of enforcement close to the public entrypoint rather than
+  duplicating it in every body processor.
+
+## Considered Options
+
+- Enforce the limit inside every body processor (JSON, urlencoded, XML, …).
+- Enforce the limit only on the public `addArguments` helper that feeds the
+  `ARGS_GET`/`ARGS_POST` collections, and leave body-processor-internal writes
+  for a follow-up.
+- Implement a new bounded collection type that self-enforces the cap.
+
+## Decision Outcome
+
+Chosen: **enforce on the public helper that wraps `ARGS_GET` / `ARGS_POST`**,
+default 1000, documented as a known gap for body-processor writes. A helper
+function was introduced so the check is not duplicated across the three writer
+call sites.
+
+## Technical Discussion
+
+Extensive review (30+ inline comments). Three themes drove the final shape.
+
+**1. Scope of enforcement.** @jptosso flagged early that body processors bypass
+the helper and therefore the limit:
+
+> "Internally coraza doesn't consume this functions, as we directly write into
+> de collections, so even with this implementation we would still get unlimited
+> arguments from body processors. It will only apply for ArgsGet and ArgsPath.
+> My idea would be to create a new collection that implements Map, and add a
+> validation." — @jptosso ([comment](https://github.com/corazawaf/coraza/pull/812#issuecomment-1588401916))
+
+@jcchavezs accepted the narrower scope to unblock the merge:
+
+> "I believe we can accept this PR as it is now and tackle the lack of internal
+> validation in a next PR."
+> — @jcchavezs ([comment](https://github.com/corazawaf/coraza/pull/812#issuecomment-1589536399))
+
+**2. Default value.** @jptosso asked for the missing default of 1000 per
+ModSecurity docs; the PR was updated to match.
+
+**3. API shape.** A `Len()` method was added to the collection. @jptosso noted
+idiomatic Go:
+
+> "`Len() int` is the standard for Length in golang"
+> — @jptosso ([review comment](https://github.com/corazawaf/coraza/pull/812#discussion_r1227428689))
+
+`Length` was renamed to `Len` accordingly, and a helper was extracted so the
+check is not duplicated across the three writer call sites.
+
+## Participants
+
+- @potats0 — author
+- @jptosso — review, drove scope clarification and API naming
+- @jcchavezs — review, approved narrower scope, asked for `0` validation
+- @fzipi — review, suggestion edits on `Len()` call sites
+- @M4tteoP, @anuraaga, @airween, @theseion — review
+
+## Consequences
+
+- **Positive:** Public argument intake is bounded by default; `ARGS_GET` and
+  `ARGS_POST` cannot grow unboundedly from attacker-supplied inputs.
+- **Negative / follow-up:** Body-processor-internal writes are still unbounded
+  until a bounded `Map` collection lands. This was explicitly deferred.
+
+## References
+
+- PR: https://github.com/corazawaf/coraza/pull/812
+- ModSecurity reference: https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual-(v2.x)#secargumentslimit
+- Related ADRs: ADR-0001 (RESPONSE_ARGS collection)
@@ -0,0 +1,103 @@
+# ADR-0003: HTTPS audit log writer
+
+- **Status:** accepted
+- **Date:** 2023-07-11
+- **Version:** v3.0.3
+- **PR:** [#826](https://github.com/corazawaf/coraza/pull/826)
+- **Issue(s):** Discussion [#813](https://github.com/corazawaf/coraza/issues/813)
+- **Deciders:** @jptosso, @jcchavezs, @anuraaga
+- **Category:** Feature
+
+## Context and Problem
+
+Prior to v3.0.3 Coraza could write audit logs to disk but had no pluggable way
+to ship them over HTTP(S) to a remote collector. ModSecurity had a similar
+feature via MLOGC; modern deployments want TLS-capable remote audit streaming
+without MLOGC-era constraints.
+
+## Decision Drivers
+
+- Enable cloud-native audit shipping (TLS, HTTP-based SIEM ingestion).
+- Intentionally drop MLOGC compatibility — it is a v2-era protocol.
+- Honour the formatter's MIME type so remote collectors can decode correctly.
+
+## Considered Options
+
+- Batched remote writes (accumulate N records, flush on timer).
+- Per-record HTTP writes, no batching, accept the loss of efficiency for v3.
+- Ship a full pluggable exporter subsystem now.
+
+## Decision Outcome
+
+Chosen: **per-record HTTP writes now; defer batching to v4** because Coraza
+v3 has no `WAF.Close()` method, so buffered records cannot be safely flushed
+on shutdown.
+
+> "Unfortunatelly we can't do bathing as we can't close the exporter (WAF does
+> not support close method and hence when terminating the APP the batched
+> requests will be lost). I think it is a good idea to add a Close method now
+> but not to force people to use it and maybe the exporter can listen to
+> termination signal"
+> — @jcchavezs ([comment](https://github.com/corazawaf/coraza/pull/826#issuecomment-1607190436))
+
+MIME-type awareness was introduced alongside this PR via the formatter
+interface refactor in [ADR-0005](0005-auditlogformatter-interface.md).
+
+## Technical Discussion
+
+Substantive review focused on lifecycle, transport hygiene, and future-proofing.
+
+**Lifecycle / Close:** the missing `WAF.Close()` blocked batching and shaped
+the whole design. The v3 compromise was "send one request per audit record,
+revisit in v4". That revisit eventually landed in v3.5.0 as
+[ADR-0043](0043-waf-close-per-owner-memoize.md).
+
+**Transport hygiene.** @jcchavezs flagged two concrete HTTP-client issues:
+
+> "I think any 2xx should be accepted. Restricting to OK feels mistaken as 201
+> and 204 are also valid status codes."
+> — @jcchavezs ([review](https://github.com/corazawaf/coraza/pull/826#discussion_r1241901929))
+
+> "Please drain the body before closing it using something like
+> `io.Copy(io.Discard, res.Body)` to be able to reuse the connection."
+> — @jcchavezs ([review](https://github.com/corazawaf/coraza/pull/826#discussion_r1241902417))
+
+Both were applied.
+
+**Content type.** @jcchavezs noted the missing Content-Type plumbing, which
+led directly to the formatter-interface refactor:
+
+> "One thing missing here is the content type support. We could add it to
+> formatters (yet not to the interface) and do interface assertion ok init and
+> record the value to be added to the requests."
+> — @jcchavezs ([comment](https://github.com/corazawaf/coraza/pull/826#issuecomment-1630021852))
+
+Out-of-band decision-making is called out explicitly on the PR:
+
+> "Decisions we made during meeting
+> https://owasp.slack.com/archives/C02BXH135AT/p1687955362171029"
+> — @jcchavezs ([comment](https://github.com/corazawaf/coraza/pull/826#issuecomment-1612492704))
+
+The Slack thread itself is not visible to outside readers.
+
+## Participants
+
+- @jptosso — author
+- @jcchavezs — review (lifecycle, transport, content-type)
+- @anuraaga — review
+
+## Consequences
+
+- **Positive:** First-party HTTPS audit forwarding; MIME-aware so JSON,
+  native, and OCSF formatters can share the writer.
+- **Negative / follow-up:** No batching, no retry, no backoff — a record per
+  HTTP request. Higher-level reliability is left to the operator (SIEM-side
+  buffering). Batching awaits `WAF.Close()` (shipped in v3.5.0 —
+  [ADR-0043](0043-waf-close-per-owner-memoize.md)).
+- Drops MLOGC compatibility by design.
+
+## References
+
+- PR: https://github.com/corazawaf/coraza/pull/826
+- Related ADRs: ADR-0005 (AuditLogFormatter interface), ADR-0028 (syslog
+  audit writer), ADR-0043 (`WAF.Close()`)