npgsql: PostgreSQL returns inserted data in exception if insert fails
Postgres returns row data in the Detail section of its errors which is in turn captured in the PostgresException Data dictionary using the key Detail key. At first glance this makes sense but it is also leaking database data into exceptions which often get logged.
For the relatively common case of violating a not-null constraint this is the data dictionary that is serialised into logs:
...
"Data": {
"Severity": "ERROR",
"SqlState": "23502",
"Code": "23502",
"MessageText": "null value in column \"email\" violates not-null constraint",
"Detail": "Failing row contains (123, william, denton, null, 2019-06-19 01:55:53.6688).",
"SchemaName": "service",
"TableName": "person",
"ColumnName": "email",
"File": "execMain.c",
"Line": "2008",
"Routine": "ExecConstraints"
},
"ClassName": "Npgsql.PostgresException",
"Message": "23502: null value in column \"email\" violates not-null constraint",
"StackTrace": ...
...
In this case it’s just some PII (Personally identifiable information) of the user I inserted into my person table. But this could be much, much more serious; passwords, social security numbers, even credit card numbers and other secret tokens. Inserted data lives in the DB not in a logging system.
Proposal
I realise this is an issue with postgres itself and npgsql is just passing the message on, but I propose Npgsql should suppress the Detail message in exceptions, and instead provide an opt-in way of populating potentially unsafe properties if the user desires them.
This could be viewed as a breaking change, but leaving the detail message there is not safe by default and many people will be unwittingly logging things they didn’t realise.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 20 (20 by maintainers)
Commits related to this issue
- Conditionaly suppress exception detail (#2501) Based on a new ConnectionString property SuppressDetailInPostgressError drop the DETAILS field sent by postgres as this often contains user data which m... — committed to dwat001/npgsql by dwat001 4 years ago
- Conditionaly suppress logging of parameters (#2501) Based on a new connection string property, suppress the logging of Command parameters. — committed to dwat001/npgsql by dwat001 4 years ago
- Tidy ExcludeParametersFromLogs and SuppressDetailedExceptions (#2501) Address review comments: - Fix wording in Documentation - Rename SuppressDetailInPostgressError to SuppressDetailedExceptions - P... — committed to dwat001/npgsql by dwat001 4 years ago
- Conditionaly suppress exception detail (#2501) Based on a new ConnectionString property SuppressDetailInPostgressError drop the DETAILS field sent by postgres as this often contains user data which m... — committed to dwat001/npgsql by dwat001 4 years ago
- Conditionaly suppress logging of parameters (#2501) Based on a new connection string property, suppress the logging of Command parameters. — committed to dwat001/npgsql by dwat001 4 years ago
- Test surpression of exception detail (#2501) — committed to dwat001/npgsql by dwat001 4 years ago
- Code Tidy (#2501) As per review use not operator instead of comparison to false. — committed to dwat001/npgsql by dwat001 4 years ago
- Fixes to parameter logging and error detail Closes #2501 — committed to roji/Npgsql by roji 4 years ago
- Fixes to parameter logging and error detail Closes #2501 — committed to npgsql/npgsql by roji 4 years ago
- Fixes to parameter logging and error detail Closes #2501 (cherry picked from commit f5666ed2294bb2fdb8c67a92e143466724f8e326) — committed to npgsql/npgsql by roji 4 years ago
- Fixes to parameter logging and error detail Closes #2501 (cherry picked from commit f5666ed2294bb2fdb8c67a92e143466724f8e326) — committed to npgsql/npgsql by roji 4 years ago
Static properties are evil and affect all users in an application. So I think we should add another parameter to the connection string, and it should be generic to handle security issues (parameter logging, details and other cases), maybe an
enumfor extensibility.