Skip to content

Commit

Permalink
Filter/block reports sent to ACRA
Browse files Browse the repository at this point in the history
  • Loading branch information
voczi committed Nov 9, 2024
1 parent 542649b commit 9f80bfc
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 0 deletions.
1 change: 1 addition & 0 deletions .idea/dictionaries/usernames.xml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
<w>quiro91</w>
<w>tarekkma</w>
<w>vianey</w>
<w>voczi</w>
<w>zanki</w>
</words>
</dictionary>
Expand Down
42 changes: 42 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package com.ichi2.anki
import android.app.Application
import android.content.Context
import android.content.SharedPreferences
import android.util.Patterns
import androidx.annotation.CheckResult
import androidx.annotation.StringRes
import androidx.annotation.VisibleForTesting
import androidx.core.content.edit
Expand All @@ -28,9 +30,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
Expand Down Expand Up @@ -281,12 +285,50 @@ object CrashReportService {
}
}
if (FEEDBACK_REPORT_NEVER != reportMode) {
if (e.removePII() == null) {
return
}
ACRA.errorReporter.putCustomData("origin", origin ?: "")
ACRA.errorReporter.putCustomData("additionalInfo", additionalInfo ?: "")
ACRA.errorReporter.handleException(e)
}
}

private fun Throwable.replaceMessage(newMessage: String?): Throwable =
Exception(newMessage, this.cause)

private val emailRegex = Patterns.EMAIL_ADDRESS.toRegex()
private val throwableRules: 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(emailRegex.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(
mApplication.sharedPrefs().get(SyncPreferences.USERNAME).toString(),
"-USERNAME-"
)
)
}
)

// Strip Personally Identifiable Information (PII) from Exception.message
// or return null if we should discard completely.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@CheckResult
fun Throwable.removePII(): Throwable? =
throwableRules.fold(this) { nextThrowable, rule -> rule.invoke(nextThrowable) ?: return null }

fun isProperServiceProcess(): Boolean {
return ACRA.isACRASenderServiceProcess()
}
Expand Down
65 changes: 65 additions & 0 deletions AnkiDroid/src/test/java/com/ichi2/anki/CrashReportServiceTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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 anki.backend.BackendError
import com.ichi2.anki.CrashReportService.removePII
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 processed = BackendDeckIsFilteredException(BackendError.newBuilder().setMessage(message).build()).removePII()

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(mock(BackendSyncServerMessageException::class.java).removePII(), "Returned exception is null")
}

@Test
fun `Emails are filtered out`() {
val msg = Throwable("Lorem ipsum 123 [email protected] Lorem ipsum.").removePII()!!.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") }

val msg = Throwable("Lorem ipsum 123 MyUser Lorem ipsum.").removePII()!!.message!!
assertThat("Exception message not contain the username", !msg.contains("MyUser"))
}
}

0 comments on commit 9f80bfc

Please sign in to comment.