Guard raw logback message and parameters with sendDefaultPii if an …

…`encoder` has been configured (#2976)
getsentry · Oct 11, 2023 · e0b84aa · e0b84aa
1 parent 1513e7e
commit e0b84aa
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Cleanup `startTransaction` overloads ([#2964](https://github.com/getsentry/sentry-java/pull/2964))
   - We have reduce the number of overloads by allowing to pass in `TransactionOptions` instead of having separate parameters for certain options.
   - `TransactionOptions` has defaults set and can be customized
+- Raw logback message and parameters are now guarded by `sendDefaultPii` if an `encoder` has been configured ([#2976](https://github.com/getsentry/sentry-java/pull/2976))
 
 ## 7.0.0-beta.1
 

diff --git a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java
@@ -99,9 +99,14 @@ protected void append(@NotNull ILoggingEvent eventObject) {
   protected @NotNull SentryEvent createEvent(@NotNull ILoggingEvent loggingEvent) {
     final SentryEvent event = new SentryEvent(DateUtils.getDateTime(loggingEvent.getTimeStamp()));
     final Message message = new Message();
-    message.setMessage(loggingEvent.getMessage());
+
+    // if encoder is set we treat message+params as PII as encoders may be used to mask/strip PII
+    if (encoder == null || options.isSendDefaultPii()) {
+      message.setMessage(loggingEvent.getMessage());
+      message.setParams(toParams(loggingEvent.getArgumentArray()));
+    }
+
     message.setFormatted(formatted(loggingEvent));
-    message.setParams(toParams(loggingEvent.getArgumentArray()));
     event.setMessage(message);
     event.setLogger(loggingEvent.getLoggerName());
     event.setLevel(formatLevel(loggingEvent.getLevel()));

diff --git a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt
@@ -35,7 +35,7 @@ import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 class SentryAppenderTest {
-    private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, contextTags: List<String>? = null, encoder: Encoder<ILoggingEvent>? = null) {
+    private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, contextTags: List<String>? = null, encoder: Encoder<ILoggingEvent>? = null, sendDefaultPii: Boolean = false) {
         val logger: Logger = LoggerFactory.getLogger(SentryAppenderTest::class.java)
         val loggerContext = LoggerFactory.getILoggerFactory() as LoggerContext
         val transportFactory = mock<ITransportFactory>()
@@ -47,6 +47,7 @@ class SentryAppenderTest {
             val appender = SentryAppender()
             val options = SentryOptions()
             options.dsn = dsn
+            options.isSendDefaultPii = sendDefaultPii
             contextTags?.forEach { options.addContextTag(it) }
             appender.setOptions(options)
             appender.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel)
@@ -118,14 +119,35 @@ class SentryAppenderTest {
     fun `encodes message`() {
         var encoder = PatternLayoutEncoder()
         encoder.pattern = "encoderadded %msg"
-        fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder)
-        fixture.logger.info("testing encoding")
+        fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder, sendDefaultPii = true)
+        fixture.logger.info("testing encoding {}", "param1")
 
         verify(fixture.transport).send(
             checkEvent { event ->
                 assertNotNull(event.message) { message ->
-                    assertEquals("encoderadded testing encoding", message.formatted)
-                    assertEquals("testing encoding", message.message)
+                    assertEquals("encoderadded testing encoding param1", message.formatted)
+                    assertEquals("testing encoding {}", message.message)
+                    assertEquals(listOf("param1"), message.params)
+                }
+                assertEquals("io.sentry.logback.SentryAppenderTest", event.logger)
+            },
+            anyOrNull()
+        )
+    }
+
+    @Test
+    fun `if encoder is set treats raw message and params as PII`() {
+        var encoder = PatternLayoutEncoder()
+        encoder.pattern = "encoderadded %msg"
+        fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder, sendDefaultPii = false)
+        fixture.logger.info("testing encoding {}", "param1")
+
+        verify(fixture.transport).send(
+            checkEvent { event ->
+                assertNotNull(event.message) { message ->
+                    assertEquals("encoderadded testing encoding param1", message.formatted)
+                    assertNull(message.message)
+                    assertNull(message.params)
                 }
                 assertEquals("io.sentry.logback.SentryAppenderTest", event.logger)
             },
@@ -151,7 +173,7 @@ class SentryAppenderTest {
     @Test
     fun `fallsback when encoder throws`() {
         var encoder = ThrowingEncoder()
-        fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder)
+        fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder, sendDefaultPii = true)
         fixture.logger.info("testing when encoder throws")
 
         verify(fixture.transport).send(