Don't log params in exception messages #6295

New Issue

admin · 2026-01-22T15:30:16+01:00

admin commented

2026-01-22 15:30:16 +01:00

Originally created by @Danack on GitHub (Sep 5, 2019).

Originally assigned to: @Ocramius on GitHub.

Feature Request

Q	A
New Feature	yes?
RFC	No idea what that means, but probably
BC Break	probably, depending on how you look at it.

Summary

Currently it seems that parameter values are used as part of the message that is generated when at least some exceptions are thrown.

For example, due to a programming error where a user was being inserted, instead of being updated, this found it's way into my log files:

Exception type:  Doctrine\DBAL\Exception\UniqueConstraintViolationException
Exception message:   An exception occurred while executing 'INSERT INTO adminuser (username, password_hash, google_2fa_secret, created_at, updated_at) VALUES (?, ?, ?, ?, ?)' with params ["admin@example.com", "abcdef", "abdefgh", "2019-09-05 00:40:07", "2019-09-05 00:40:07"]:

The problem is that this message contains secrets, that for technical, legal, and ethical reasons shouldn't be put into log files.

The request is to either by default, or as an optional setting, not include parameter values in the message, and possibly to add the capability to retrieve them via a getParams method on each exception.

And because someone might find it interesting, a philosophical reasoning: https://en.wikipedia.org/wiki/Principle_of_least_astonishment

Originally created by @Danack on GitHub (Sep 5, 2019). Originally assigned to: @Ocramius on GitHub. ### Feature Request | Q | A |------------ | ------ | New Feature | yes? | RFC | No idea what that means, but probably | BC Break | probably, depending on how you look at it. #### Summary Currently it seems that parameter values are used as part of the message that is generated when at least some exceptions are thrown. For example, due to a programming error where a user was being inserted, instead of being updated, this found it's way into my log files: ``` Exception type: Doctrine\DBAL\Exception\UniqueConstraintViolationException Exception message: An exception occurred while executing 'INSERT INTO adminuser (username, password_hash, google_2fa_secret, created_at, updated_at) VALUES (?, ?, ?, ?, ?)' with params ["admin@example.com", "abcdef", "abdefgh", "2019-09-05 00:40:07", "2019-09-05 00:40:07"]: ``` The problem is that this message contains secrets, that for technical, legal, and ethical reasons shouldn't be put into log files. The request is to either by default, or as an optional setting, not include parameter values in the message, and possibly to add the capability to retrieve them via a `getParams` method on each exception. And because someone might find it interesting, a philosophical reasoning: https://en.wikipedia.org/wiki/Principle_of_least_astonishment

admin added the Won't Fix Question labels 2026-01-22 15:30:16 +01:00

admin closed this issue

2026-01-22 15:30:17 +01:00

admin commented

2026-01-22 15:30:17 +01:00

@Ocramius commented on GitHub (Sep 5, 2019):

Disagree on this: this can happen with any kind of exception produced by any technical layer.

It is generally normal to throw exceptions and provide some context (trace, message, identifiers, values) to facilitate interactions with debugging and logging.

If these exceptions happen to 100% contain sensitive information (f.e. "exception: password XYZ is too short!") then you should most certainly not include the sensitive information in the exception message, but here we are talking about an infrastructure layer that just pushes information from consumer to SQL domain.

This also applies to any kind of transport layer that may contain part of a message in a log (f.e. "could not decode message XYZ", "malformed header: XYZ" - will it contain a password? nobody knows!). Some sort of exfiltration into logs will always occur.

Therefore, closing here based on following rationale:

the data exfiltration impact is limited to well-known error logging scope.
error logging scope is not user-facing
the usefulness of the data in these messages massively outweighs the issue, where applicable
the data being exposed is of technical nature, and it is not possible to discern whether it contains sensitive or not

If we were to change this approach, we'd have to re-evaluate exception design in the whole ecosystem, and not just here. Maybe worth bringing up in internals, to allow having a separate $context on exceptions (much like it happens in c4421fcac1/Psr/Log/LoggerInterface.php (L30) ? )

Similarly related to https://github.com/doctrine/dbal/issues/2899#issuecomment-339675090

@Ocramius commented on GitHub (Sep 5, 2019): Disagree on this: this can happen with any kind of exception produced by any technical layer. It is generally normal to throw exceptions and provide some context (trace, message, identifiers, values) to facilitate interactions with debugging and logging. If these exceptions happen to 100% contain sensitive information (f.e. "exception: password XYZ is too short!") then you should most certainly not include the sensitive information in the exception message, but here we are talking about an infrastructure layer that just pushes information from consumer to SQL domain. This also applies to any kind of transport layer that may contain part of a message in a log (f.e. "could not decode message XYZ", "malformed header: XYZ" - will it contain a password? nobody knows!). Some sort of exfiltration into logs will always occur. Therefore, closing here based on following rationale: 1. the data exfiltration impact is limited to well-known error logging scope. 2. error logging scope is not user-facing 3. the usefulness of the data in these messages massively outweighs the issue, where applicable 4. the data being exposed is of technical nature, and it is not possible to discern whether it contains sensitive or not If we were to change this approach, we'd have to re-evaluate exception design in the whole ecosystem, and not just here. Maybe worth bringing up in internals, to allow having a separate `$context` on exceptions (much like it happens in https://github.com/php-fig/log/blob/c4421fcac1edd5a324fda73e589a5cf96e52ffd0/Psr/Log/LoggerInterface.php#L30 ? ) Similarly related to https://github.com/doctrine/dbal/issues/2899#issuecomment-339675090

admin commented

2026-01-22 15:30:18 +01:00

@Ocramius commented on GitHub (Sep 5, 2019):

Note: I realise that PCI compliance and similar systems can make you fail an audit if a user enters a credit card in a comment field, and you end up saving it by accident (and good luck with finding out!).

It is not up to technical layers to adhere to these requirements.

@Ocramius commented on GitHub (Sep 5, 2019): Note: I realise that PCI compliance and similar systems can make you fail an audit if a user enters a credit card in a comment field, and you end up saving it by accident (and good luck with finding out!). It is not up to technical layers to adhere to these requirements.

admin commented

2026-01-22 15:30:18 +01:00

@Danack commented on GitHub (Sep 5, 2019):

"the data being exposed is of technical nature, and it is not possible to discern whether it contains sensitive or not"

So it should default to being treated as sensitive, not as insensitive.

"because the parameters are somewhere in the trace."

Which is why I don't log parameters in stack traces either.

"This also applies to any kind of transport layer that may contain part of a message in a log."

That's why I don't put messages from transport layers into my general logging system.

Some sort of exfiltration into logs will always occur..... It is not up to technical layers to adhere to these requirements.

I think I disagree with both of those statements. Or at the very least, it should be possible to use software in a way that user data has to be deliberately exported, rather than it being an accidental disclosure.

Obviously I can work around this pretty easily by just snipping all exception messages after the phrase 'with params', so can't justify getting involved in Doctrine internals to pursue this, however this seems like a design flaw to me.

@Danack commented on GitHub (Sep 5, 2019): > "the data being exposed is of technical nature, and it is not possible to discern whether it contains sensitive or not" So it should default to being treated as sensitive, not as insensitive. > "because the parameters are somewhere in the trace." Which is why I don't log parameters in stack traces either. > "This also applies to any kind of transport layer that may contain part of a message in a log." That's why I don't put messages from transport layers into my general logging system. > Some sort of exfiltration into logs will always occur..... It is not up to technical layers to adhere to these requirements. I think I disagree with both of those statements. Or at the very least, it should be possible to use software in a way that user data has to be deliberately exported, rather than it being an accidental disclosure. Obviously I can work around this pretty easily by just snipping all exception messages after the phrase 'with params', so can't justify getting involved in Doctrine internals to pursue this, however this seems like a design flaw to me.

admin commented

2026-01-22 15:30:18 +01:00

@Ocramius commented on GitHub (Sep 5, 2019):

@Danack I'd say that if this needs to be fixed, it needs to be fixed by removing any sprintf() in the entire design of all libraries involved here.

I don't think that's a sensible choice: I understand the frustration, but I think this goes more deep into the separation of contextual information (see Sentry and their scrubbing approach) passed to a logger, and the lack of this construct in the design of the Throwable API.

@Ocramius commented on GitHub (Sep 5, 2019): @Danack I'd say that if this needs to be fixed, it needs to be fixed by removing any `sprintf()` in the entire design of all libraries involved here. I don't think that's a sensible choice: I understand the frustration, but I think this goes more deep into the separation of contextual information (see Sentry and their scrubbing approach) passed to a logger, and the lack of this construct in the design of the `Throwable` API.

admin commented

2026-01-22 15:30:19 +01:00

@martijn80 commented on GitHub (May 22, 2022):

Php8.2 upcoming SensitiveParameterValue feature could be of help here.

See https://wiki.php.net/rfc/redact_parameters_in_back_traces

@martijn80 commented on GitHub (May 22, 2022): Php8.2 upcoming SensitiveParameterValue feature could be of help here. See https://wiki.php.net/rfc/redact_parameters_in_back_traces

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6295