Skip to content

fix: remove credential-leaking debug logs in logger plugins#13502

Open
AlinsRan wants to merge 6 commits into
apache:masterfrom
AlinsRan:fix/logger-remove-leaking-debug-logs
Open

fix: remove credential-leaking debug logs in logger plugins#13502
AlinsRan wants to merge 6 commits into
apache:masterfrom
AlinsRan:fix/logger-remove-leaking-debug-logs

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

Several logger plugins print the full serialized log payload at info/debug level. The payload is the serialized log entries, which by default include request/response headers and bodies, so it can leak credentials (e.g. Authorization/Cookie headers) into the error log:

  • elasticsearch-logger.lua — logged uri and the full bulk body.
  • syslog/init.lua — logged the full RFC5424 payload and every buffered entry.
  • sls-logger.lua — logged the full RFC5424 payload before sending it (debug level).
  • kafka-logger.lua / rocketmq-logger.lua — logged the full serialized batch payload right before sending it.

This PR removes those statements.

For elasticsearch-logger, syslog and sls-logger, no test relies on the removed lines. For kafka-logger and rocketmq-logger, the existing tests used the removed log line to observe what was queued for delivery, so the same observability is reproduced with a test-only hook that logs each batch entry from batch-processor-manager (the entry is the exact object that gets sent, so content-sensitive assertions — including the no_error_log body-filter cases — keep working). Production code no longer logs the payload.

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 (N/A)
  • I have verified that the changes pass the existing tests

… syslog

The elasticsearch-logger plugin logged the full bulk request body and the
syslog plugin logged the full RFC5424 payload (plus every buffered entry)
at info/debug level. These messages contain the serialized request and
response data (headers, bodies) and can leak credentials into the error log.

Follow-up to apache#13205, which removed the same pattern from sls-logger,
tcp-logger and udp-logger but did not cover these two plugins.

Signed-off-by: AlinsRan <alinsran@apache.org>
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Jun 10, 2026
…-logger

Both plugins logged the full serialized batch payload via
core.log.info("send data to kafka/rocketmq: ", data) right before sending
it. That payload is the serialized log entries, which by default include
request and response headers and bodies, so it can leak credentials into
the error log when the log level is set to info.

Remove the production log statements. The tests relied on this line to
observe what was queued for delivery, so the same observability is
reproduced with a test-only hook that logs each batch entry from
batch-processor-manager.

Signed-off-by: AlinsRan <alinsran@apache.org>
@AlinsRan AlinsRan changed the title fix: remove credential-leaking debug logs in elasticsearch-logger and syslog fix: remove credential-leaking debug logs in logger plugins Jun 10, 2026
AlinsRan added 2 commits June 11, 2026 02:54
syslog.t TEST 20/21 asserted the request/response body that appeared in
the removed core.log.info("collect_data:"..rfc5424_data) line. Log each
batch entry (the rfc5424 payload) from a test-only batch-processor hook so
those assertions keep working; production code no longer logs the payload.
…-only http hook

elasticsearch-logger.t (TEST 14/19/20) and elasticsearch-logger2.t
(TEST 5-9) asserted the endpoint uri and bulk body that appeared in the
removed core.log.info("uri: ..., body: ...") line. Wrap http.request_uri
in a block preprocessor so the same uri/body is logged from the test only,
composing with the existing per-block mocks.
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jun 10, 2026
AlinsRan added 2 commits June 11, 2026 09:20
Address review feedback: the add_entry_to_new_processor wrapper logged the
entry unconditionally, while add_entry only logged on success. Make both
wrappers log only when the entry was actually queued (the manager returns
true only after pushing), so the test hooks never report an entry that was
discarded.
sls-logger logged the full rfc5424 payload via
core.log.debug("sls logger send data ", log_message) right before sending
it. The payload is the serialized log data (request/response headers and
bodies), so it can leak credentials into the error log at debug level.
No test relies on this line.

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

Approved.

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

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants