Skunk sends all arguments to tracing/logging

[zcox]

We observed, in production, Skunk sending all query/command arguments to our tracing backend (Honeycomb). We also observed Skunk sending arguments to our logging backend (Datadog) when the query/command failed with a Postgres exception (e.g. unique constraint violation). This leaked sensitive data to those backends, requiring manual deletion.

As a basic example, when executing UPDATE users SET sensitive_data = $text WHERE user_id = $uuid Skunk would send the actual values for the sensitive_data and user_id columns to the tracing backend, in the bind trace's arguments field.

We put together a small example demonstrating both argument leaks. The readme and the code contain more details.

While we were able to mitigate the leaks in our case by essentially disabling tracing/logging for Skunk, we were unable to find a systematic way to disable sending arguments to tracing/logging in Skunk. Skunk provides much useful tracing/logging data, and it would be great to retain that, while preventing arguments from leaking.

[tpolecat]

It's hard to know what to do here since it's often very useful to know the exact arguments when an error occurs. They are also reported in exceptions. An option would be to add something to Encoder to indicate that the arguments shouldn't be logged, like WHERE data = ${text.redacted} and then have the logging mechanisms use an encoder method that respects the redaction request.

[mpilquist]

Adding a redacted method to Encoder seems like a good solution.

[zcox]

One thing that isn't quite clear from my description above, but which is included in the linked example repo, is that something inside Skunk (or a dep?) is printing exceptions that occur inside transactions directly to stderr/stdout. This doesn't happen when the exception occurs outside of an explicit transaction. We read through much Skunk code, and could not identify where this printing originates.

That said, exceptions (hopefully) occur rarely in production. Arguments are exposed to tracing on every database operation, successful or failed (modulo sampling). It would be great to be able to disable or configure that.

[mpilquist]

@zcox That's likely due to an uncaught exception in a fiber, which cats-effect then logs. If we can find the offending fiber, we could fix via .voidError or something similar.

[mpilquist]

Exploring opt-in redaction in #900.

Note the primary key exception example comes from Postgres proper. Not sure best approach there -- we could opt-in regex based parsing of exception messages but not sure if that's a good idea.

[mpilquist]

Fixed in #900 which is available in Skunk 1.0 release candidates.