pgx
pgx copied to clipboard
newEncodeError adds potentially sensitive data to error messages.
func newEncodeError(value any, m *Map, oid uint32, formatCode int16, err error) error {
var format string
switch formatCode {
case TextFormatCode:
format = "text"
case BinaryFormatCode:
format = "binary"
default:
format = fmt.Sprintf("unknown (%d)", formatCode)
}
var dataTypeName string
if t, ok := m.TypeForOID(oid); ok {
dataTypeName = t.Name
} else {
dataTypeName = "unknown type"
}
return fmt.Errorf("unable to encode %#v into %s format for %s (OID %d): %w", value, format, dataTypeName, oid, err)
}
The last line is what I'm referencing. I moved away from sqlx because it does this and a bug printed email signup codes to some error logs. I know this is trying to be helpful but the error has no type so you can't check for it to filter it out, and normal error wrapping means it will almost certainly end up in the logs -- why else have an error if not to print it. Most people wouldn't expect a library to print your data like this.
https://github.com/jackc/pgx/blob/61d3c965ad442cc14d6b0e39e0ab3821f3684c03/pgtype/pgtype.go#L1936
I can submit an update for this, but I'm not sure what people would prefer instead. Should we print the type information or value?
Thanks everyone!
I'm not sure the best approach here either. There have been similar concerns with error messages in the connection process including the database url.
AFAIK, it is idiomatic Go for errors to include relevant arguments. Especially in this case, the value may help explain why there was an error. For example, a Go int64 may be encoded into a PostgreSQL integer, but it will fail if the value is too large.
It's burdensome for every library to have to account that its error messages may logged. But it's also burdensome for an application developer to try to safely log errors... 🤷
If something is to be done, I'd like to establish a pattern that could be done in every case like this. In no particular order, here are a few possibilities.
- Anything that could leak sensitive information is not included in the error message. This is simple and straight forward, but does sacrifice some of the usefulness of error messages.
- Anything that could leak sensitive information would be given a specific error type. The
Error()method would be sanitized. The potentially sensitive information could be retrieved from a new unsanitized error message method, or by methods or fields specific to the type (for example,Valuein the encode error case) - A
SanitizedErrorwrapper error could be used. It would be given a sanitized error message, but the original error message could be accessed by unwrapping the error.