JMR-dev/LibreMail
1
0
Fork 0
Code Issues 40 Pull Requests 2 Actions Packages Projects Releases 2 Wiki Activity

feat(logging): AppLog seam — record scrubbed throwables + accountLogRef (#325)

Browse Source 
Add throwable-recording overloads to AppLog.d/w and make AppLog.e record the
throwable it is given: the throwable's stack trace is scrubbed via the existing
StackTraceScrubber (exception class names + frames kept; host/email-bearing
exception messages stripped) and appended to the buffered log line, so a
throwable can reach a user-reviewed DebugReport without leaking PII. The
existing no-throwable overloads are unchanged.

Add accountLogRef(accountId): a short, stable, non-reversible reference
(scheme prefix + truncated SHA-256 of the id) so downstream logging can
identify an account without logging the raw Account.id, which embeds the email.

Foundation for the #324 debug-logging strangler epic; consumed by #326–#330.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
JMR-dev
2026-07-04 20:23:38 -05:00
co-authored by Claude Opus 4.8
parent 47aaebccf5
commit 1a1fbf8d7f
5 changed files with 203 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/test/kotlin/org/libremail/reporting/AccountLogRefTest.kt
+72
View File
@@ -0,0 +1,72 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.reporting
import org.junit.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertTrue
/**
* [accountLogRef] turns a PII-bearing `Account.id` (which embeds the raw email) into a short, stable,
* non-reversible reference that is safe to write to logs and a [DebugReport].
*/
class AccountLogRefTest {
@Test
fun `is deterministic and stable for the same id`() {
val id = "outlook:user@example.com"
assertEquals(accountLogRef(id), accountLogRef(id))
}
@Test
fun `differs across different ids`() {
assertNotEquals(
accountLogRef("outlook:user@example.com"),
accountLogRef("outlook:other@example.com"),
)
// Same address under a different scheme is a different account, so it must map to a different ref.
assertNotEquals(
accountLogRef("outlook:user@example.com"),
accountLogRef("imap:user@example.com"),
)
}
@Test
fun `contains neither the email nor its domain and is not the raw id`() {
val id = "imap:Alice.Smith@example.com"
val ref = accountLogRef(id)
assertNotEquals(id, ref)
assertFalse(ref.contains("@"), ref)
assertFalse(ref.contains("Alice.Smith"), ref)
assertFalse(ref.contains("example.com"), ref)
assertFalse(ref.contains("example"), ref)
}
@Test
fun `keeps the non-PII scheme prefix so refs stay readable`() {
assertTrue(accountLogRef("outlook:user@example.com").startsWith("outlook:"), "outlook prefix")
assertTrue(accountLogRef("imap:user@example.com").startsWith("imap:"), "imap prefix")
}
@Test
fun `falls back to a generic prefix for an id with no scheme, never leaking the address`() {
val ref = accountLogRef("user@example.com")
assertTrue(ref.startsWith("acct:"), ref)
assertFalse(ref.contains("@"), ref)
assertFalse(ref.contains("example"), ref)
}
@Test
fun `never treats an address-shaped prefix as the scheme`() {
// If the part before the first ':' is itself an address, it must not surface as the prefix.
val ref = accountLogRef("user@example.com:143")
assertTrue(ref.startsWith("acct:"), ref)
assertFalse(ref.contains("@"), ref)
assertFalse(ref.contains("example"), ref)
}
}
app/src/test/kotlin/org/libremail/reporting/AppLogTest.kt
+56 -3
View File
@@ -9,7 +9,10 @@ import io.mockk.verify
import org.junit.After
import org.junit.Before
import org.junit.Test
import java.net.ConnectException
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
/**
* [AppLog] mirrors every Logcat call into the process [RingLogBuffer] so recent activity can be
@@ -24,8 +27,10 @@ class AppLogTest {
fun setUp() {
mockkStatic(Log::class)
every { Log.d(any(), any()) } returns 0
every { Log.d(any(), any(), any()) } returns 0
every { Log.i(any(), any()) } returns 0
every { Log.w(any<String>(), any<String>()) } returns 0
every { Log.w(any<String>(), any<String>(), any()) } returns 0
every { Log.e(any(), any(), any()) } returns 0
AppLog.install(buffer)
}
@@ -39,15 +44,14 @@ class AppLogTest {
AppLog.i("Tag", "info line")
AppLog.w("Tag", "warn line")
AppLog.e("Tag", "error line")
AppLog.e("Tag", "error with cause", IllegalStateException("boom"))
val snapshot = buffer.snapshot()
assertEquals(
listOf('D', 'I', 'W', 'E', 'E'),
listOf('D', 'I', 'W', 'E'),
snapshot.map { it.level },
)
assertEquals(
listOf("debug line", "info line", "warn line", "error line", "error with cause"),
listOf("debug line", "info line", "warn line", "error line"),
snapshot.map { it.message },
)
verify { Log.d("Tag", "debug line") }
@@ -55,4 +59,53 @@ class AppLogTest {
verify { Log.w("Tag", "warn line") }
verify { Log.e("Tag", "error line", null) }
}
@Test
fun `e with a throwable records the scrubbed trace - class and frames kept, host and email stripped`() {
val cause = ConnectException("Failed to connect to imap.example.com/93.184.216.34:993 for user@example.com")
AppLog.e("Net", "connect failed", cause)
val entry = buffer.snapshot().single()
assertEquals('E', entry.level)
// The caller message stays, and the throwable's class name + a frame are kept for usefulness.
assertTrue(entry.message.startsWith("connect failed\n"), entry.message)
assertTrue(entry.message.contains("java.net.ConnectException"), entry.message)
assertTrue(entry.message.contains("at org.libremail.reporting.AppLogTest"), entry.message)
// PII embedded in the exception message is gone: host, ip, port and email.
assertFalse(entry.message.contains("imap.example.com"), entry.message)
assertFalse(entry.message.contains("93.184.216.34"), entry.message)
assertFalse(entry.message.contains(":993"), entry.message)
assertFalse(entry.message.contains("user@example.com"), entry.message)
assertFalse(entry.message.contains("example"), entry.message)
verify { Log.e("Net", "connect failed", cause) }
}
@Test
fun `w and d throwable overloads record the scrubbed trace and forward the throwable to Logcat`() {
val cause = ConnectException("auth failed for user@example.org at imap.mail.example.org:143")
AppLog.w("Net", "warn cause", cause)
AppLog.d("Net", "debug cause", cause)
val snapshot = buffer.snapshot()
assertEquals(listOf('W', 'D'), snapshot.map { it.level })
assertEquals(listOf("warn cause", "debug cause"), snapshot.map { it.message.substringBefore('\n') })
snapshot.forEach { entry ->
assertTrue(entry.message.contains("java.net.ConnectException"), entry.message)
assertFalse(entry.message.contains("user@example.org"), entry.message)
assertFalse(entry.message.contains("imap.mail.example.org"), entry.message)
assertFalse(entry.message.contains(":143"), entry.message)
assertFalse(entry.message.contains("example"), entry.message)
}
verify { Log.w("Net", "warn cause", cause) }
verify { Log.d("Net", "debug cause", cause) }
}
@Test
fun `a null throwable records the message alone`() {
AppLog.w("Net", "just a warning", null)
assertEquals("just a warning", buffer.snapshot().single().message)
}
}
app/src/test/kotlin/org/libremail/reporting/AppLogUninstalledTest.kt
+9 -2
View File
@@ -23,8 +23,10 @@ class AppLogUninstalledTest {
fun setUp() {
mockkStatic(Log::class)
every { Log.d(any(), any()) } returns 0
every { Log.d(any(), any(), any()) } returns 0
every { Log.i(any(), any()) } returns 0
every { Log.w(any<String>(), any<String>()) } returns 0
every { Log.w(any<String>(), any<String>(), any()) } returns 0
every { Log.e(any(), any(), any()) } returns 0
AppLog::class.java.getDeclaredField("buffer").apply { isAccessible = true }.set(AppLog, null)
}
@@ -34,16 +36,21 @@ class AppLogUninstalledTest {
@Test
fun `logging before a buffer is installed only forwards to Logcat`() {
val cause = IllegalStateException("boom")
AppLog.d("Tag", "debug")
AppLog.d("Tag", "debug with cause", cause)
AppLog.i("Tag", "info")
AppLog.w("Tag", "warn")
AppLog.w("Tag", "warn with cause", cause)
AppLog.e("Tag", "error")
AppLog.e("Tag", "error with cause", IllegalStateException("boom"))
AppLog.e("Tag", "error with cause", cause)
verify { Log.d("Tag", "debug") }
verify { Log.d("Tag", "debug with cause", cause) }
verify { Log.i("Tag", "info") }
verify { Log.w("Tag", "warn") }
verify { Log.w("Tag", "warn with cause", cause) }
verify { Log.e("Tag", "error", null) }
verify { Log.e("Tag", "error with cause", any<IllegalStateException>()) }
verify { Log.e("Tag", "error with cause", cause) }
}
}