From 4260a304bda8f5ad13e85fcfc0721b0663378d7d Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Fri, 3 Jul 2026 12:53:58 -0500 Subject: [PATCH 1/2] feat(reporting): add PII-free account summary to debug reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DiagnosticsCollector now includes one " ()" entry per account (the count is the list size) in DebugReport.accounts, alongside the existing settings + recent-log capture. The provider is a coarse bucket derived from the IMAP host (Gmail/Yahoo/iCloud/Outlook/AOL/Other) — never the raw host or email — so no PII leaks; a custom domain buckets to "Other". Accounts are cached like settings so crash reports (built on the crashing thread) include the last-known snapshot; accounts live in the non-auth AccountDatabase, so reading them never blocks on the encrypted cache. Recent device/app logs were already captured (RingLogBuffer) and serialize as the report's "logs". Closes #235 Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/reporting/DebugReport.kt | 7 ++ .../reporting/DiagnosticsCollector.kt | 82 ++++++++++++++----- .../libremail/reporting/CrashReporterTest.kt | 6 +- .../reporting/DiagnosticsCollectorTest.kt | 40 ++++++++- 4 files changed, 112 insertions(+), 23 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt b/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt index 2c782f7..b59d821 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 = "", @@ -59,6 +61,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 @@ -77,6 +80,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"), @@ -90,6 +96,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", ""), ) 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/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), + ) } From 664d79a42724a42d42d7f77d5d9ab2a9a33b6889 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Fri, 3 Jul 2026 18:48:25 -0500 Subject: [PATCH 2/2] fix(test): pass accountRepository to DiagnosticsCollector in CrashReporterInstallTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DiagnosticsCollector gained a 4th constructor param (accountRepository) for the PII-free account summary, but CrashReporterInstallTest still constructed it with 3 args — a compile error that broke the Unit tests and Static analysis gates. Add the AccountRepository mock with observeAccounts() stubbed to an empty flow (matching DiagnosticsCollectorTest) and pass it to both call sites. Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/reporting/CrashReporterInstallTest.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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())