Add modsecurity_response_body directive to skip response body buffering (closes #206)

[tomsommer]

what

• New directive modsecurity_response_body on | off (http, server, location; default on, inherited per location).
• With off:
  • the header filter no longer sets r->filter_need_in_memory, so nginx no longer reads file-backed response buffers (static files, proxied responses buffered to disk) into memory and copies them through the output chain before sending them (with sendfile enabled they are transmitted straight from the file, as they would be without the module);
  • the body filter no longer calls msc_append_response_body() or the per-buffer intervention check;
  • the RESPONSE_BODY phase is still invoked on the last buffer, so libmodsecurity's own SecResponseBodyAccess / SecResponseBodyMimeType logic still decides whether phase 4 rules run; RESPONSE_BODY is empty and RESPONSE_CONTENT_LENGTH is 0.
• README section for the directive; the outdated "adds four new directives" sentence becomes count-free.
• New test tests/modsecurity-response-body-directive.t (13 assertions): audit-log evidence that a RESPONSE_BODY rule matches by default and not with off while a status-based phase 4 rule still runs, the body is delivered intact, inheritance into a nested location works, and, on --with-debug builds, the copy filter's "write new buf t:1 f:1" record appears only for the default location.

why

• Closes [Perf] Don't read full response into mem if not needed NGINX #206. The module unconditionally forces every response body into memory, in every location where modsecurity is on, regardless of SecResponseBodyAccess. The FIXME next to that line has said so since 2017.
• libmodsecurity's appendResponseBody() only filters on the MIME-type list, not on SecResponseBodyAccess (that is checked in processResponseBody()), so turning body access off in the rules does not avoid the copy, and the connector cannot query that setting through the C API. An nginx-level directive is the only lever.
• Response buffers are handed to the client as soon as they have been fed to ModSecurity, so phase 4 can only ever affect the last chunk of a streamed response; sites that do not rely on it pay the full copy for nothing.

references

• Closes [Perf] Don't read full response into mem if not needed NGINX #206.
• Overlaps Add an Option to Skip Body Inspections ( Closes #343 ) #356's modsecurity_skip_resp_body_filter: that variant skips the body filter but leaves filter_need_in_memory set (so the copy into memory remains) and never runs the RESPONSE_BODY phase; this PR does both and adds tests. Naming follows Add modsecurity_request_body directive to skip request body buffering (supersedes #356) #391 (modsecurity_request_body).
• Leaves the return ret; on the last-buffer intervention path in the body filter untouched because fix: fail closed on swallowed WAF return values #384 owns that block.

---

Origin: this change comes from a performance review of the connector done with Claude Fable 5.1 (Anthropic). The patch and its test were verified by building the module against nginx master with libmodsecurity 3.0.14 (PCRE2) and, with upstream CI's flags (--without-pcre2 --with-http_v2_module --with-http_auth_request_module), against libmodsecurity 3.0.9 (PCRE1), then running the full tests/modsecurity*.t suite in both builds (16 files, 264 tests, all passing).

Summary by CodeRabbit

• New Features

  • Added the modsecurity_response_body directive to control whether response bodies are inspected by ModSecurity.
  • Response-body inspection is enabled by default and can be disabled at the location, server, or main configuration level.
  • Disabled inspection avoids buffering response data while preserving subsequent request processing.

• Documentation

  • Added usage guidance for configuring response-body inspection.

• Tests

  • Added coverage for default, disabled, and inherited configuration behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bbe27974-6fd3-4b4a-a75b-ec39ca40b370

📥 Commits

Reviewing files that changed from the base of the PR and between 9eb44fd and 2256e6e.

📒 Files selected for processing (6)

• README.md
• src/ngx_http_modsecurity_body_filter.c
• src/ngx_http_modsecurity_common.h
• src/ngx_http_modsecurity_header_filter.c
• src/ngx_http_modsecurity_module.c
• tests/modsecurity-response-body-directive.t

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

The module adds the modsecurity_response_body directive. It defaults to enabled and inherits across configuration levels. When disabled, response bodies are not buffered or inspected, while later request phases continue to run.

Response body inspection control

Layer / File(s)	Summary
Configuration contract src/ngx_http_modsecurity_common.h, src/ngx_http_modsecurity_module.c	Adds the response_body configuration field and registers modsecurity_response_body for main, server, and location contexts. The value defaults to enabled and inherits from parent configurations.
Conditional filter execution src/ngx_http_modsecurity_header_filter.c, src/ngx_http_modsecurity_body_filter.c	The header filter requests in-memory buffering only when response body inspection is enabled. The body filter skips response-body processing when it is disabled while preserving positive intervention handling.
Validation and documentation tests/modsecurity-response-body-directive.t, README.md	Tests enabled, disabled, and inherited configurations, audit-log matches, phase-4 execution, error logs, and file-versus-memory handling. Documents the directive and an example for static files.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Nginx
  participant ModSecurity
  participant AuditLog
  Client->>Nginx: request configured location
  Nginx->>Nginx: apply modsecurity_response_body
  Nginx->>ModSecurity: inspect response body when enabled
  ModSecurity->>AuditLog: record response-body and phase-4 matches
  Nginx-->>Client: return response body

Loading

Merge Risk: ⚪ Minimal · up to 2256e

The new directive behaves as documented for enabled, disabled, and inherited configurations, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The directive provides a runtime control and defaults to enabled. The enabled path preserves response-body inspection. The disabled path avoids forced in-memory buffering and body copies, and the test…	Update disabled-mode handling so it bypasses the module's post-proxy_pass header and body filter processing while preserving the log phase and the required non-body-dependent behavior. Add automated coverage that verifies the filters are …
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding the modsecurity_response_body directive to disable response-body buffering. It is concise and specific.
Out of Scope Changes check	✅ Passed	The directive, configuration field and merge logic, header and body filter changes, documentation, and 13 tests all directly support issue #206. The tests cover default inspection, disabled inspection…

Full details: Linked Issues check

Explanation

The directive provides a runtime control and defaults to enabled. The enabled path preserves response-body inspection. The disabled path avoids forced in-memory buffering and body copies, and the tests cover inheritance and body integrity. However, issue #206 requires disabled mode to eliminate post-proxy_pass header and body filter processing while retaining the log phase. The implementation still runs the module header filter and runs RESPONSE_BODY on the final buffer when disabled. The available diff inspection failed because required repository objects were unavailable, so this conclusion relies on the PR summary and linked requirements.

Resolution

Update disabled-mode handling so it bypasses the module's post-proxy_pass header and body filter processing while preserving the log phase and the required non-body-dependent behavior. Add automated coverage that verifies the filters are bypassed, not only that body buffering is avoided.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[airween]

Hi @tomsommer,

thanks for this too.

So it's similar to #391 but for the response body, right? And this bypasses the engine's SecResponseBodyAccess directive?

Thank you for this, I have same opinion here like in case of the other PR. I ask for your patience here too.

[tomsommer]

Hi @airween, thanks for looking at this one too.

Yes, it is the response-side counterpart of #391, but I would not describe it as bypassing SecResponseBodyAccess, because it cannot make the engine inspect anything the admin turned off. The two settings act at different points:

• SecResponseBodyAccess decides whether libmodsecurity evaluates the response body (checked in Transaction::processResponseBody()).
• modsecurity_response_body decides whether the connector collects and hands over the body at all.

With the directive at its default on, behaviour is exactly as it is today: every buffer is passed to the library and SecResponseBodyAccess together with SecResponseBodyMimeType decides what happens to it. With off, the connector hands nothing over, so RESPONSE_BODY is empty and RESPONSE_CONTENT_LENGTH is 0 no matter how the engine is configured. The connector still calls the response-body phase on the last buffer, so phase 4 rules that do not look at the body (for example on RESPONSE_STATUS) keep working, subject as always to the engine's own decision whether to run phase 4 at all.

The reason a connector-side switch is needed even when an admin has already set SecResponseBodyAccess Off is that the cost is paid before the engine's setting is consulted, in two places:

1. Transaction::appendResponseBody() filters only on SecResponseBodyMimeType; it does not look at SecResponseBodyAccess, which is checked later in processResponseBody(). So the copy into the library's buffer happens either way, and the connector cannot query that setting through the C API.
2. More importantly, the header filter sets r->filter_need_in_memory = 1 unconditionally. That makes nginx read file-backed responses (static files, or proxied responses buffered to disk) into memory and copy them through the output chain before sending them, even when no rule will ever look at them. sendfile still performs the transmission, so the cost is the extra read and copy rather than the loss of sendfile, but it is paid on every response in a location where modsecurity is on.

That second point is what #206 has been asking for since 2020, and it is the part that is not reachable from the rules language at all. It also makes this PR slightly more than a mirror image of #391: the request side is only about not reading and buffering the body, the response side additionally stops forcing the body into memory.

On naming, the same offer as in #391 applies: if modsecurity_response_body reads too close to SecResponseBodyAccess, I am happy to rename both directives to something like modsecurity_request_body_buffering and modsecurity_response_body_buffering, which describes what they actually control. I would keep whatever is chosen symmetric across the two PRs.

No rush from my side, and it makes sense to land the earlier PRs first. Happy to rebase whenever that happens.