Merge pull request #245 from JMR-dev/feat-235-debug-report-accounts
feat(reporting): add PII-free account summary to debug reports
This commit was merged in pull request #245.
This commit is contained in:
@@ -28,6 +28,8 @@ data class DebugReport(
|
||||
val stackTrace: String?,
|
||||
val settings: Map<String, String>,
|
||||
val logs: List<String>,
|
||||
/** One PII-free "<provider> (<authType>)" entry per account (issue #235); size is the account count. */
|
||||
val accounts: List<String> = emptyList(),
|
||||
val userComment: String = "",
|
||||
/** Reply-to address the user supplied when submitting (see #159); required for online submit. */
|
||||
val userEmail: String = "",
|
||||
@@ -66,6 +68,7 @@ data class DebugReport(
|
||||
.put("userComment", userComment)
|
||||
.put("userEmail", userEmail)
|
||||
.put("settings", settingsJson)
|
||||
.put("accounts", JSONArray(accounts))
|
||||
.put("logs", JSONArray(logs))
|
||||
if (stackTrace != null) json.put("stackTrace", stackTrace)
|
||||
return json
|
||||
@@ -84,6 +87,9 @@ data class DebugReport(
|
||||
val logsJson = json.getJSONArray("logs")
|
||||
val logs = ArrayList<String>(logsJson.length())
|
||||
for (i in 0 until logsJson.length()) logs.add(logsJson.getString(i))
|
||||
val accountsJson = json.optJSONArray("accounts")
|
||||
val accounts = ArrayList<String>(accountsJson?.length() ?: 0)
|
||||
if (accountsJson != null) for (i in 0 until accountsJson.length()) accounts.add(accountsJson.getString(i))
|
||||
return DebugReport(
|
||||
id = json.getString("id"),
|
||||
createdAtMillis = json.getLong("createdAtMillis"),
|
||||
@@ -97,6 +103,7 @@ data class DebugReport(
|
||||
stackTrace = if (json.has("stackTrace")) json.getString("stackTrace") else null,
|
||||
settings = settings,
|
||||
logs = logs,
|
||||
accounts = accounts,
|
||||
userComment = json.optString("userComment", ""),
|
||||
userEmail = json.optString("userEmail", ""),
|
||||
surfaced = json.optBoolean("surfaced", false),
|
||||
|
||||
@@ -5,19 +5,24 @@ import android.os.Build
|
||||
import kotlinx.coroutines.flow.first
|
||||
import org.libremail.data.settings.AppSettings
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.AuthType
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import java.util.UUID
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
/**
|
||||
* Assembles a [DebugReport] from app/device metadata, a stack trace (for crashes), a minimal
|
||||
* non-PII settings summary, and the recent in-app log buffer. Only the fields listed in [summarize]
|
||||
* are captured — deliberately no account emails, server names, or message content.
|
||||
* Assembles a [DebugReport] from app/device metadata, a stack trace (for crashes), a minimal non-PII
|
||||
* settings summary, a PII-free account summary, and the recent in-app log buffer. Only the fields
|
||||
* listed in [summarize] / [summarizeAccounts] are captured — deliberately no account emails, server
|
||||
* names, or message content.
|
||||
*/
|
||||
@Singleton
|
||||
class DiagnosticsCollector @Inject constructor(
|
||||
private val appVersion: AppVersionProvider,
|
||||
private val settingsRepository: SettingsRepository,
|
||||
private val accountRepository: AccountRepository,
|
||||
private val logBuffer: RingLogBuffer,
|
||||
) {
|
||||
// Cached so a crash report (built synchronously on the crashing thread) can still include
|
||||
@@ -26,36 +31,47 @@ class DiagnosticsCollector @Inject constructor(
|
||||
@Volatile
|
||||
private var cachedSettings: Map<String, String> = emptyMap()
|
||||
|
||||
/** Pre-reads settings so a later crash report can include them. Safe to call and ignore. */
|
||||
// Same rationale as [cachedSettings]: a crash report can't read the DB-backed account list on the
|
||||
// crashing thread, so a warmed snapshot is used. Accounts live in the non-auth AccountDatabase, so
|
||||
// reading them never blocks on the encrypted cache.
|
||||
@Volatile
|
||||
private var cachedAccounts: List<String> = emptyList()
|
||||
|
||||
/** Pre-reads settings + accounts so a later crash report can include them. Safe to call and ignore. */
|
||||
suspend fun warmSettingsCache() {
|
||||
cachedSettings = summarize(settingsRepository.settings.first())
|
||||
cachedAccounts = summarizeAccounts(accountRepository.observeAccounts().first())
|
||||
}
|
||||
|
||||
/** Builds a report for a user-initiated ("Report a problem") request; includes live settings. */
|
||||
suspend fun collectManual(): DebugReport {
|
||||
val settings = summarize(settingsRepository.settings.first())
|
||||
val accounts = summarizeAccounts(accountRepository.observeAccounts().first())
|
||||
cachedSettings = settings
|
||||
return build(ReportKind.MANUAL, throwable = null, settings = settings)
|
||||
cachedAccounts = accounts
|
||||
return build(ReportKind.MANUAL, throwable = null, settings = settings, accounts = accounts)
|
||||
}
|
||||
|
||||
/** Builds a crash report synchronously; it must not block or throw on the crashing thread. */
|
||||
fun collectCrash(throwable: Throwable): DebugReport =
|
||||
build(ReportKind.CRASH, throwable = throwable, settings = cachedSettings)
|
||||
build(ReportKind.CRASH, throwable = throwable, settings = cachedSettings, accounts = cachedAccounts)
|
||||
|
||||
private fun build(kind: ReportKind, throwable: Throwable?, settings: Map<String, String>) = DebugReport(
|
||||
id = UUID.randomUUID().toString(),
|
||||
createdAtMillis = System.currentTimeMillis(),
|
||||
kind = kind,
|
||||
appVersionName = appVersion.versionName,
|
||||
appVersionCode = appVersion.versionCode,
|
||||
androidRelease = Build.VERSION.RELEASE ?: "",
|
||||
androidSdkInt = Build.VERSION.SDK_INT,
|
||||
deviceManufacturer = Build.MANUFACTURER ?: "",
|
||||
deviceModel = Build.MODEL ?: "",
|
||||
stackTrace = throwable?.stackTraceToString(),
|
||||
settings = settings,
|
||||
logs = logBuffer.snapshot().map { it.formatted() },
|
||||
)
|
||||
private fun build(kind: ReportKind, throwable: Throwable?, settings: Map<String, String>, accounts: List<String>) =
|
||||
DebugReport(
|
||||
id = UUID.randomUUID().toString(),
|
||||
createdAtMillis = System.currentTimeMillis(),
|
||||
kind = kind,
|
||||
appVersionName = appVersion.versionName,
|
||||
appVersionCode = appVersion.versionCode,
|
||||
androidRelease = Build.VERSION.RELEASE ?: "",
|
||||
androidSdkInt = Build.VERSION.SDK_INT,
|
||||
deviceManufacturer = Build.MANUFACTURER ?: "",
|
||||
deviceModel = Build.MODEL ?: "",
|
||||
stackTrace = throwable?.stackTraceToString(),
|
||||
settings = settings,
|
||||
accounts = accounts,
|
||||
logs = logBuffer.snapshot().map { it.formatted() },
|
||||
)
|
||||
|
||||
private fun summarize(settings: AppSettings): Map<String, String> = linkedMapOf(
|
||||
"dynamicColor" to settings.dynamicColor.toString(),
|
||||
@@ -66,4 +82,30 @@ class DiagnosticsCollector @Inject constructor(
|
||||
"encryptCache" to settings.encryptCache.toString(),
|
||||
"fetchPolicy" to settings.fetchPolicy.name,
|
||||
)
|
||||
|
||||
/**
|
||||
* One PII-free "<provider> (<authType>)" entry per account (issue #235); the account count is the
|
||||
* list size. Deliberately NO email address or server hostname: [providerLabel] buckets the IMAP host
|
||||
* to a coarse known-provider name (or "Other" for custom domains), so a custom mail host never leaks.
|
||||
*/
|
||||
private fun summarizeAccounts(accounts: List<Account>): List<String> =
|
||||
accounts.map { "${providerLabel(it)} (${it.authType.name})" }
|
||||
}
|
||||
|
||||
/** Coarse, non-PII provider bucket for an account — never the raw host or email (issue #235). */
|
||||
private fun providerLabel(account: Account): String = when (account.authType) {
|
||||
AuthType.OAUTH_OUTLOOK -> "Outlook"
|
||||
AuthType.PASSWORD_IMAP -> imapProviderLabel(account.imap.host)
|
||||
}
|
||||
|
||||
private fun imapProviderLabel(host: String): String {
|
||||
val h = host.lowercase()
|
||||
return when {
|
||||
"gmail" in h || "googlemail" in h -> "Gmail"
|
||||
"yahoo" in h -> "Yahoo"
|
||||
"icloud" in h || "me.com" in h || "mac.com" in h -> "iCloud"
|
||||
"outlook" in h || "office365" in h || "hotmail" in h || "live.com" in h -> "Outlook"
|
||||
"aol" in h -> "AOL"
|
||||
else -> "Other"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,12 +4,14 @@ package org.libremail.reporting
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.rules.TemporaryFolder
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
@@ -30,6 +32,9 @@ class CrashReporterInstallTest {
|
||||
every { versionCode } returns 1L
|
||||
}
|
||||
private val settingsRepository = mockk<SettingsRepository>()
|
||||
private val accountRepository = mockk<AccountRepository> {
|
||||
every { observeAccounts() } returns flowOf(emptyList())
|
||||
}
|
||||
|
||||
private var original: Thread.UncaughtExceptionHandler? = null
|
||||
|
||||
@@ -49,7 +54,7 @@ class CrashReporterInstallTest {
|
||||
Thread.setDefaultUncaughtExceptionHandler(previous)
|
||||
val store = ReportStore(tempFolder.root)
|
||||
val buffer = RingLogBuffer()
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, buffer)
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, accountRepository, buffer)
|
||||
val reporter = CrashReporter(collector, store, buffer)
|
||||
|
||||
reporter.install()
|
||||
@@ -70,7 +75,7 @@ class CrashReporterInstallTest {
|
||||
Thread.setDefaultUncaughtExceptionHandler(mockk(relaxed = true))
|
||||
val store = ReportStore(tempFolder.root)
|
||||
val buffer = RingLogBuffer()
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, buffer)
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, accountRepository, buffer)
|
||||
val reporter = CrashReporter(collector, store, buffer)
|
||||
reporter.install()
|
||||
val installed = requireNotNull(Thread.getDefaultUncaughtExceptionHandler())
|
||||
|
||||
@@ -7,6 +7,7 @@ import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.rules.TemporaryFolder
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
@@ -20,12 +21,13 @@ class CrashReporterTest {
|
||||
every { versionCode } returns 1L
|
||||
}
|
||||
private val settingsRepository = mockk<SettingsRepository>()
|
||||
private val accountRepository = mockk<AccountRepository>()
|
||||
|
||||
@Test
|
||||
fun `persisting a forced crash saves a report offered on next launch`() {
|
||||
val store = ReportStore(tempFolder.root)
|
||||
val buffer = RingLogBuffer()
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, buffer)
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, accountRepository, buffer)
|
||||
val reporter = CrashReporter(collector, store, buffer)
|
||||
|
||||
reporter.persist(IllegalStateException("forced crash"))
|
||||
@@ -48,7 +50,7 @@ class CrashReporterTest {
|
||||
// written to the local store. Nothing here can send data off the device.
|
||||
val store = ReportStore(tempFolder.root)
|
||||
val buffer = RingLogBuffer()
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, buffer)
|
||||
val collector = DiagnosticsCollector(appVersion, settingsRepository, accountRepository, buffer)
|
||||
val reporter = CrashReporter(collector, store, buffer)
|
||||
|
||||
reporter.persist(RuntimeException("boom"))
|
||||
|
||||
@@ -9,6 +9,11 @@ import org.junit.Test
|
||||
import org.libremail.data.settings.AppSettings
|
||||
import org.libremail.data.settings.FetchPolicy
|
||||
import org.libremail.data.settings.SettingsRepository
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.AuthType
|
||||
import org.libremail.domain.model.MailSecurity
|
||||
import org.libremail.domain.model.ServerConfig
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
@@ -20,8 +25,11 @@ class DiagnosticsCollectorTest {
|
||||
every { versionCode } returns 42L
|
||||
}
|
||||
private val settingsRepository = mockk<SettingsRepository>()
|
||||
private val accountRepository = mockk<AccountRepository> {
|
||||
every { observeAccounts() } returns flowOf(emptyList())
|
||||
}
|
||||
private val logBuffer = RingLogBuffer()
|
||||
private val collector = DiagnosticsCollector(appVersion, settingsRepository, logBuffer)
|
||||
private val collector = DiagnosticsCollector(appVersion, settingsRepository, accountRepository, logBuffer)
|
||||
|
||||
@Test
|
||||
fun `crash report includes stack trace and app version`() = runTest {
|
||||
@@ -78,4 +86,34 @@ class DiagnosticsCollectorTest {
|
||||
|
||||
assertTrue(report.logs.any { it.contains("hello-breadcrumb") })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `manual report summarizes accounts as PII-free provider labels`() = runTest {
|
||||
every { settingsRepository.settings } returns flowOf(AppSettings())
|
||||
every { accountRepository.observeAccounts() } returns flowOf(
|
||||
listOf(
|
||||
account("a@example.com", AuthType.OAUTH_OUTLOOK, "outlook.office365.com"),
|
||||
account("b@gmail.com", AuthType.PASSWORD_IMAP, "imap.gmail.com"),
|
||||
account("c@corp.example", AuthType.PASSWORD_IMAP, "mail.corp.example"),
|
||||
),
|
||||
)
|
||||
|
||||
val report = collector.collectManual()
|
||||
|
||||
assertEquals(
|
||||
listOf("Outlook (OAUTH_OUTLOOK)", "Gmail (PASSWORD_IMAP)", "Other (PASSWORD_IMAP)"),
|
||||
report.accounts,
|
||||
)
|
||||
// No email address or server hostname leaks — a custom host buckets to "Other".
|
||||
assertTrue(report.accounts.none { it.contains("@") || it.contains("example") || it.contains(".com") })
|
||||
}
|
||||
|
||||
private fun account(email: String, authType: AuthType, imapHost: String) = Account(
|
||||
id = "id:$email",
|
||||
email = email,
|
||||
displayName = email,
|
||||
authType = authType,
|
||||
imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS),
|
||||
smtp = ServerConfig(imapHost, 587, MailSecurity.STARTTLS),
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user