Filter/block reports sent to ACRA

.idea/dictionaries/usernames.xml

@@ -83,6 +83,7 @@
       <w>quiro91</w>
       <w>tarekkma</w>
       <w>vianey</w>
+      <w>voczi</w>
       <w>zanki</w>
     </words>
   </dictionary>

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

@@ -124,7 +124,7 @@ open class AnkiDroidApp : Application(), Configuration.Provider, ChangeManager.S
         // Ensures any change is propagated to widgets
         ChangeManager.subscribe(this)
 
-        CrashReportService.initialize(this)
+        CrashReportService.initialize(this, preferences)
         val logType = LogType.value
         when (logType) {
             LogType.DEBUG -> Timber.plant(DebugTree())

AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt

@@ -18,6 +18,7 @@ package com.ichi2.anki
 import android.app.Application
 import android.content.Context
 import android.content.SharedPreferences
+import android.util.Patterns
 import androidx.annotation.StringRes
 import androidx.annotation.VisibleForTesting
 import androidx.core.content.edit
@@ -28,9 +29,11 @@ import com.ichi2.anki.analytics.UsageAnalytics
 import com.ichi2.anki.analytics.UsageAnalytics.sendAnalyticsException
 import com.ichi2.anki.exception.ManuallyReportedException
 import com.ichi2.anki.exception.UserSubmittedException
+import com.ichi2.anki.preferences.get
 import com.ichi2.anki.preferences.sharedPrefs
 import com.ichi2.libanki.utils.TimeManager
 import com.ichi2.utils.WebViewDebugging.setDataDirectorySuffix
+import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException
 import org.acra.ACRA
 import org.acra.ReportField
 import org.acra.config.CoreConfigurationBuilder
@@ -73,9 +76,13 @@ object CrashReportService {
     lateinit var acraCoreConfigBuilder: CoreConfigurationBuilder
         private set
     private lateinit var mApplication: Application
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    var mPreferences: SharedPreferences? = null
     private const val WEBVIEW_VER_NAME = "WEBVIEW_VER_NAME"
     private const val MIN_INTERVAL_MS = 60000
-    private const val EXCEPTION_MESSAGE = "Exception report sent by user manually. See: 'Comment/USER_COMMENT'"
+    private const val EXCEPTION_MESSAGE =
+        "Exception report sent by user manually. See: 'Comment/USER_COMMENT'"
 
     private enum class ToastType(@StringRes private val toastMessageRes: Int) {
         AUTO_TOAST(R.string.feedback_auto_toast_text),
@@ -160,8 +167,9 @@ object CrashReportService {
      * data directory. Analytics falls back to a sensible default if this is not set.
      */
     @JvmStatic
-    fun initialize(application: Application) {
+    fun initialize(application: Application, preferences: SharedPreferences) {
         mApplication = application
+        mPreferences = preferences
         // FIXME ACRA needs to reinitialize after language is changed, but with the new language
         //   this is difficult because the Application (AnkiDroidApp) does not change it's baseContext
         //   perhaps a solution could be to change AnkiDroidApp to have a context wrapper that it sets
@@ -257,34 +265,93 @@ object CrashReportService {
     }
 
     /** Used when we don't have an exception to throw, but we know something is wrong and want to diagnose it  */
-    fun sendExceptionReport(message: String?, origin: String?) {
+    fun sendExceptionReport(message: String?, origin: String?): Boolean =
         sendExceptionReport(ManuallyReportedException(message), origin, null)
-    }
 
-    fun sendExceptionReport(e: Throwable, origin: String?) {
+    fun sendExceptionReport(e: Throwable, origin: String?): Boolean =
         sendExceptionReport(e, origin, null)
-    }
 
-    fun sendExceptionReport(e: Throwable, origin: String?, additionalInfo: String?) {
+    fun sendExceptionReport(e: Throwable, origin: String?, additionalInfo: String?): Boolean =
         sendExceptionReport(e, origin, additionalInfo, false)
-    }
 
-    fun sendExceptionReport(e: Throwable, origin: String?, additionalInfo: String?, onlyIfSilent: Boolean) {
+    fun sendExceptionReport(
+        e: Throwable,
+        origin: String?,
+        additionalInfo: String?,
+        onlyIfSilent: Boolean
+    ): Boolean {
         sendAnalyticsException(e, false)
         AnkiDroidApp.sentExceptionReportHack = true
         val reportMode = mApplication.applicationContext.sharedPrefs()
             .getString(FEEDBACK_REPORT_KEY, FEEDBACK_REPORT_ASK)
+
         if (onlyIfSilent) {
             if (FEEDBACK_REPORT_ALWAYS != reportMode) {
                 Timber.i("sendExceptionReport - onlyIfSilent true, but ACRA is not 'always accept'. Skipping report send.")
-                return
+                return false
             }
         }
+
         if (FEEDBACK_REPORT_NEVER != reportMode) {
+            if (processException(e) == null) {
+                return false
+            }
+
             ACRA.errorReporter.putCustomData("origin", origin ?: "")
             ACRA.errorReporter.putCustomData("additionalInfo", additionalInfo ?: "")
             ACRA.errorReporter.handleException(e)
+            return true
         }
+
+        return false
+    }
+
+    fun Throwable.replaceMessage(newMessage: String?): Throwable =
+        Throwable(newMessage, this.cause)
+
+    private val exceptionRules: List<(Throwable) -> Throwable?> = listOf(
+        { e ->
+            // BackendSyncServerMessage may contain PII and we do not want this leaked to ACRA.
+            // If the exception is of a cause deemed to be associated with a message containing PII,
+            // then do not send it at all to retain user privacy.
+            // Related: https://github.com/ankidroid/Anki-Android/issues/17392
+            // and also https://github.com/ankitects/anki/commit/ba1f5f4
+            if (e.cause is BackendSyncServerMessageException) null else e
+        },
+        { e ->
+
+            // Make sure we filter out any email addresses which may be leaked by the sync server.
+            e.replaceMessage(
+                Patterns.EMAIL_ADDRESS.toRegex().replace(e.message.orEmpty(), "-EMAIL-")
+            )
+        },
+        { e ->
+            // Filter out the current username in the message (if it exists) as it is considered PII.
+            e.replaceMessage(
+                e.message.orEmpty()
+                    .replace(mPreferences?.get(SyncPreferences.USERNAME).toString(), "-USERNAME-")
+            )
+        }
+    )
+
+    // Apply each and every rule in exceptionRules on the original exception.
+    // This means that after this method call, we can get an exception that has the username and
+    // email of the user filtered out so it doesn't get sent to ACRA as is.
+    // If the method returns null, that means we do not send a report to ACRA at all as we have
+    // decided to block this based on some arbitrary rule(s) enforced on the exception's contents.
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    fun processException(e: Throwable): Throwable? {
+        var newE: Throwable? = e
+
+        for (rule in exceptionRules) {
+            newE = rule.invoke(newE!!)
+
+            if (newE == null) {
+                return null
+            }
+        }
+
+        return newE
     }
 
     fun isProperServiceProcess(): Boolean {

AnkiDroid/src/test/java/com/ichi2/anki/CrashReportServiceTest.kt

@@ -0,0 +1,66 @@
+/*
+ *  Copyright (c) 2024 voczi <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it under
+ *  the terms of the GNU General Public License as published by the Free Software
+ *  Foundation; either version 3 of the License, or (at your option) any later
+ *  version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ *  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ *  PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with
+ *  this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.ichi2.anki
+
+import androidx.core.content.edit
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.CrashReportService.processException
+import com.ichi2.anki.preferences.sharedPrefs
+import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException
+import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException
+import org.hamcrest.MatcherAssert.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mockito.mock
+import kotlin.Throwable
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+
+@RunWith(AndroidJUnit4::class)
+class CrashReportServiceTest : RobolectricTest() {
+    @Test
+    fun `Normal exceptions are untouched`() {
+        val message = "This is a test message which should not be filtered nor blocked"
+        val e = Throwable(message, mock(BackendDeckIsFilteredException::class.java))
+        val processed = processException(e)
+
+        assertNotNull(processed, "Returned exception should not be null")
+        assertEquals(processed.message!!, message, "Returned exception's message should stay the same")
+    }
+
+    @Test
+    fun `Backend sync server messages are blocked`() {
+        assertNull(processException(Throwable("test", mock(BackendSyncServerMessageException::class.java))), "Returned exception is null")
+    }
+
+    @Test
+    fun `Emails are filtered out`() {
+        val msg = processException(Throwable("Lorem ipsum 123 [email protected] Lorem ipsum."))!!.message!!
+        assertThat("Exception message does not contain the email", !msg.contains("[email protected]"))
+    }
+
+    @Test
+    fun `Username is filtered out`() {
+        val preferences = targetContext.sharedPrefs()
+        preferences.edit() { putString(SyncPreferences.USERNAME, "MyUser") }
+        CrashReportService.mPreferences = preferences
+
+        val msg = processException(Throwable("Lorem ipsum 123 MyUser Lorem ipsum."))!!.message!!
+        assertThat("Exception message not contain the username", !msg.contains("MyUser"))
+    }
+}