Skip to content

feat: chaitin waf response logging - #13763

Open
blaisewang wants to merge 3 commits into
apache:masterfrom
blaisewang:feat/chaitin-waf-response-logging
Open

feat: chaitin waf response logging#13763
blaisewang wants to merge 3 commits into
apache:masterfrom
blaisewang:feat/chaitin-waf-response-logging

Conversation

@blaisewang

Copy link
Copy Markdown
Contributor

Description

lua-resty-t1k 1.2.0 can report the response to the SafeLine WAF detection service in addition to the request. This PR bumps the dependency and exposes the capability through the chaitin-waf plugin.

Until now the plugin only reported requests, so anything the WAF could have detected on the way out — data leaks in a response body, a successful exploit's output, an unexpected status code — was invisible to SafeLine. Enabling response reporting closes that gap without changing how requests are handled.

Three new config options, available both at plugin level and in the plugin metadata:

Option Type Default Description
log_resp boolean false Report the response to the WAF service.
resp_body_size integer (>= 0) 4 How much of the response body to report, in KB. 0 reports only the status line and headers.
extra_ignored_content_types string Comma separated response Content-Type values to skip, on top of the built-in list.

How it works:

  • The plugin now implements the body_filter and log phases, delegating to t1k.do_body_filter() and t1k.do_log().
  • body_filter buffers up to resp_body_size KB of the response body; the report itself is sent from an ngx.timer during the log phase, after the response has been handed back to the client. It therefore adds no latency to the response.
  • The report is advisory only: detection results appear in the SafeLine console, and APISIX never blocks or modifies a response based on them. Request handling is unchanged.
  • Responses are skipped when the request was already blocked, or when the response Content-Type is in the ignored list (audio, video, font, image and other binary media types out of the box).

One refactor was needed along the way: get_conf applied the metadata and plugin level config blocks with two verbatim copies of the same field-by-field assignment. Adding three more options would have meant keeping three copies in sync, so both are folded into a local helper. Precedence is unchanged — plugin level config still overrides metadata.

Docs updated in both en and zh, including a new "Response Logging" section that covers the async reporting model, the skip conditions, and the memory cost of buffering response bodies on routes serving large responses.

Which issue(s) this PR fixes:

N/A — no existing issue; this adds a new capability to the plugin.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. dependencies Pull requests that update a dependency file doc Documentation things enhancement New feature or request plugin labels Jul 30, 2026
Comment thread apisix/plugins/chaitin-waf.lua

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two P2 issues need to be addressed before merge:

  1. Response reporting can fail silently, and the tests do not verify the core behavior. ngx.timer.at does not consume callback return values, so connect/send/receive failures returned by socket.do_socket are neither logged nor measured. Tests 18-21 only verify configuration acceptance and the client response; they do not prove that the mock WAF receives the response status, headers, and truncated body, or that ignored content types are skipped. Please add observable error handling and an end-to-end assertion with bounded polling for the asynchronous report.

  2. The required exact-head CI is failing. The t/plugin/[a-k]*.t job is red in kafka-logger.t TEST 18 because the expected partition_id: 1 and partition_id: 2 log entries are missing. This appears outside the files changed by this PR, but the required check still needs to pass before merge. Please rerun it and investigate if the failure is reproducible: failed job.

nic-6443
nic-6443 previously approved these changes Aug 5, 2026
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Aug 5, 2026
blaisewang and others added 3 commits August 5, 2026 19:10
lua-resty-t1k 1.2.0 can report the response back to the SafeLine WAF
detection service in addition to the request. Bump the dependency and
expose it through three new `config` options:

- `log_resp` enables response reporting, off by default so existing
  configurations keep their current behavior
- `resp_body_size` caps how much of the response body is buffered and
  reported, in KB, defaulting to 4
- `extra_ignored_content_types` skips further response content types on
  top of the built-in ignored list

Wire up the `body_filter` and `log` phases to the corresponding library
entry points. The report is sent from an ngx.timer during the log phase,
so it does not delay the response, and the detection result is only
visible in the SafeLine console: a response is never blocked or modified
based on it.

`get_conf` applied the metadata and route level config blocks with two
copies of the same field-by-field assignment, which three more options
would have made harder to keep in sync. Fold both into a local helper,
keeping the existing precedence of route level config over metadata.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add schema validation cases for the `resp_body_size` minimum and the
`extra_ignored_content_types` type, plus round-trip cases asserting that a
route configured with response logging still serves its response unchanged,
and that the default configuration behaves the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous tests only checked that the plugin accepted the new options
and that the response reached the client unchanged. They never showed
that anything was reported, so response logging could have been broken
outright and they would still have passed.

Assert instead on what lua-resty-t1k logs while it reports: that a report
was sent, how much of the response body it buffered, and that an ignored
content type or a disabled log_resp skips the report entirely. The
truncation case is the sharpest one, since the client receives 4096 bytes
while only 1024 are buffered for the report.

Reporting happens in a timer after the response has been handed back, so
a failure to deliver it cannot surface in the response. Cover that too,
now that the library logs it.

pass_keepalive() is needed because the library pools the connection and
sends the response report over the same one, which pass() closes as soon
as it has answered the request report.

Bump lua-resty-t1k to 1.2.1-0 for the log lines these tests match.
@blaisewang
blaisewang force-pushed the feat/chaitin-waf-response-logging branch from dea2e30 to 73b9035 Compare August 5, 2026 11:10
@blaisewang

Copy link
Copy Markdown
Contributor Author

Two P2 issues need to be addressed before merge:

  1. Response reporting can fail silently, and the tests do not verify the core behavior. ngx.timer.at does not consume callback return values, so connect/send/receive failures returned by socket.do_socket are neither logged nor measured. Tests 18-21 only verify configuration acceptance and the client response; they do not prove that the mock WAF receives the response status, headers, and truncated body, or that ignored content types are skipped. Please add observable error handling and an end-to-end assertion with bounded polling for the asynchronous report.
  2. The required exact-head CI is failing. The t/plugin/[a-k]*.t job is red in kafka-logger.t TEST 18 because the expected partition_id: 1 and partition_id: 2 log entries are missing. This appears outside the files changed by this PR, but the required check still needs to pass before merge. Please rerun it and investigate if the failure is reproducible: failed job.
  1. Fixed in lua-resty-t1k 1.2.1 — ngx.timer.at discarded the callback's return values, so do_socket failures are now logged at error level, and the dependency is bumped here. Added t/plugin/chaitin-waf-log-resp.t, which asserts the report is actually sent, that a 4096-byte response is truncated to 1024 for reporting while the client still gets all 4096, that ignored content types and log_resp: false skip it, and that delivery failures are logged.

  2. Unrelated to this PR. CI should pass on rerun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file doc Documentation things enhancement New feature or request plugin size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants