The Pull Request That Logged an Email Address at Debug Level

Share
The Pull Request That Logged an Email Address at Debug Level. Abstract code review illustration in orange and dark grey on debugly.dev

The diff added a helpful debug line: log the request body when the payment call fails, so the next incident is easier. The reviewer approved it, because it is exactly the line you want during an incident. It is also a line that writes customers' personal data into a log aggregator that retention keeps for a year, that a dozen tools can read, and that nobody audits, and the two facts are the same line.

Log statements are the least reviewed data flow in most codebases, because they read as diagnostics rather than as exports. But a log line that contains request data is a data export to every system downstream of the log stream, and it should be reviewed with the same suspicion as any other place data leaves the boundary.

This is the checklist I run on any diff that touches logging, written after one too many incidents where the leak was a helpful debug line.

The request body is the dangerous default

The most common defect is logging the whole request or response object, because the whole object is the convenient thing to print and the most useful during debugging. It is also the thing most likely to contain the sensitive fields, because request bodies are where applications receive names, emails, addresses, tokens and payment references.

The review question is what the object contains at its widest, not what the happy test case contains. A body that is innocuous in the test may carry the sensitive variant in production, and the log statement does not distinguish. Logging the whole object is therefore a bet that no sensitive field will ever pass through it, and that bet is made once and paid on every request.

Debug level is not a protection

The defence "it is only debug level" is the one I hear most, and it is wrong in three specific ways.

Debug level in production is one configuration change away, and configuration changes happen during incidents, exactly when the sensitive requests are flowing, which is the worst possible correlation. Many teams have flipped debug on during an outage and discovered afterwards that the outage traffic, full of real customer data, was logged at debug for the duration.

Second, many pipelines ship all levels to the same aggregator, and the level is just a field. The data arrives wherever the stream goes regardless of the level, so the level controls display, not distribution.

Third, debug statements rot into being called in production paths that are not errors, a verbose mode, a feature flag, a support tool, and the level that was theoretical becomes routine.

The specific shapes to flag

Interpolated bodies and headers. Any log that stringifies the request, the headers, or the raw body. Headers are worse than people expect, because they carry authorisation tokens and API keys, and a helpful "log the headers" line exports credentials on every request.

Error objects with attached context. Modern error wrapping attaches the failing request or user to the error for debugging, and a log of the error then transitively serialises the attachment. The review must follow what the error carries, not just the message.

Query strings. URLs with parameters are logged constantly and treated as safe, but query strings are where tokens and email addresses ride in less careful systems, and the access log is a long retained, widely readable store. The review should know what your query strings can contain and treat the access log accordingly.

Fallback serialisation. A log helper that, when given an unknown object, JSON stringifies it wholesale is a standing trap, because any future caller that passes a rich object gets a rich export. The helper's default is the policy, and the default should be allowlist, not stringify.

The shapes that are fine

Logging is not the enemy and I want to name the healthy shapes so the review is not a blanket suspicion.

Log identifiers, not contents: a request id, a user id that is an internal reference rather than an email, a count, a duration. The id lets you correlate to the authoritative store where the sensitive data lives with its own access control, which is the whole point of structured logging in structured logging and what to log.

Log the shape, not the substance: the presence of a field, its length, a hash of the value for correlation, none of which are personal data, all of which are debuggable.

And log the decision, at the boundary: a single, reviewed serialiser that knows the schema and drops the sensitive fields by policy, so that the allowlist lives in one place and every log statement inherits it.

The review habit that catches it

The question I ask on any logging diff is one sentence: if this line ran on every production request for a month, where would the data end up, and who could read it there. The answer names the aggregator, the retention and the audience, and very often the author hears their own line's consequence for the first time in that answer, because the mental model was "a line in a file", and the reality is "a field in a widely readable, long retained store".

That question converts the review from style to data flow, and data flow is what the regulation and the incident both care about.

The rule

A log statement that contains request data is an export, and debug level is a display setting, not a boundary. Review the line by following the object to its widest production content and the stream to its widest reader, and prefer ids, shapes and hashes over contents, with the sensitive field policy living in one reviewed serialiser.

The helpful debug line is how the well intentioned leak ships, and it is the same "the convenience is the boundary" defect as the cache key in the cache that returned another customer's data, where the convenient construction quietly decided who sees what.