diff --git a/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt b/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt index 8f12a52..ea2c5b6 100644 --- a/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt +++ b/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt @@ -28,6 +28,8 @@ data class DebugReport( val stackTrace: String?, val settings: Map, val logs: List, + /** One PII-free " ()" entry per account (issue #235); size is the account count. */ + val accounts: List = 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(logsJson.length()) for (i in 0 until logsJson.length()) logs.add(logsJson.getString(i)) + val accountsJson = json.optJSONArray("accounts") + val accounts = ArrayList(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), diff --git a/app/src/main/kotlin/org/libremail/reporting/DiagnosticsCollector.kt b/app/src/main/kotlin/org/libremail/reporting/DiagnosticsCollector.kt index 6b09218..6bd00fb 100644 --- a/app/src/main/kotlin/org/libremail/reporting/DiagnosticsCollector.kt +++ b/app/src/main/kotlin/org/libremail/reporting/DiagnosticsCollector.kt @@ -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 = 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 = 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) = 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, accounts: List) = + 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 = 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 " ()" 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): List = + 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" + } } diff --git a/app/src/test/kotlin/org/libremail/reporting/CrashReporterInstallTest.kt b/app/src/test/kotlin/org/libremail/reporting/CrashReporterInstallTest.kt index d72c745..a06194c 100644 --- a/app/src/test/kotlin/org/libremail/reporting/CrashReporterInstallTest.kt +++ b/app/src/test/kotlin/org/libremail/reporting/CrashReporterInstallTest.kt @@ -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() + private val accountRepository = mockk { + 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()) diff --git a/app/src/test/kotlin/org/libremail/reporting/CrashReporterTest.kt b/app/src/test/kotlin/org/libremail/reporting/CrashReporterTest.kt index b09dd3a..8c4426c 100644 --- a/app/src/test/kotlin/org/libremail/reporting/CrashReporterTest.kt +++ b/app/src/test/kotlin/org/libremail/reporting/CrashReporterTest.kt @@ -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() + private val accountRepository = mockk() @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")) diff --git a/app/src/test/kotlin/org/libremail/reporting/DiagnosticsCollectorTest.kt b/app/src/test/kotlin/org/libremail/reporting/DiagnosticsCollectorTest.kt index d478790..43ad6d8 100644 --- a/app/src/test/kotlin/org/libremail/reporting/DiagnosticsCollectorTest.kt +++ b/app/src/test/kotlin/org/libremail/reporting/DiagnosticsCollectorTest.kt @@ -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() + private val accountRepository = mockk { + 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), + ) }