Lettuce instrumentation does not sanitize password in exception message

[ericmm]

Describe the bug The value of db.statement gets sanitized properly thanks to RedisCommandSanitizer. However when there's a RedisCommandExecutionException thrown, the exception message, stacktrace & status description may contain sensitive data (e.g. password), and they get logged in plain text.

code here: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/lettuce/lettuce-5.1/library/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java#LL235C31-L235C31

and here: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/lettuce/lettuce-5.1/library/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java#L242

pic1:

pic2:

pic3:

Steps to reproduce N.A.

What did you expect to see? the sensitive data should be redacted

What did you see instead? the sensitive data is in plain text.

What version are you using? latest version

Environment Compiler: Jdk17 OS: Linux

Additional context Checked the Lettuce core source code, it does not have special handling on the error output. The error message ("ERR unknown command HELLO, with args beginning with: 3, AUTH, xxxx, xxxxxxx, ") seems directly from Redis server response.

[mateuszrzeszutek]

Also relevant: #3039

[trask]

hi @ericmm, it may also be worth asking in the lettuce repo in case there's anything they can do without limiting the usefulness of the exception messages, since this would affect other folks who are logging these exceptions

[ericmm]

Thanks @trask & @mateuszrzeszutek , the error message seems directly from Redis server (https://github.com/redis/redis/blob/07d54dc5baa1b2e7adb5ce8e7bee398d376fe13b/src/server.c#L3729). I think the problem here is users have no control over this logging since it's done at instrumentation level. If it's a lettuce (or other redis library) exception thrown in users' code, they can choose to throw a custom exception (not exposing the original stacktrace).

[trask]

@maryliag did you end up handling a similar issue in Node.js?

[maryliag]

Yes, on Node.js this is no sanitization on the error message, so sensitive information could leak. Since there was not easy/performant way to do this, I was making the change to remove to not save the error message itself, and use just the code for the error type