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

Merge pull request #333 from JMR-dev/feat-325-applog-seam

Browse Source 
feat(logging): AppLog seam — record scrubbed throwables + accountLogRef (#325)
This commit was merged in pull request #333.
This commit is contained in:
Jason Ross
2026-07-04 21:01:39 -05:00
committed by GitHub
parent b352e3838c 9ea339436d
commit 41015a5c40
5 changed files with 203 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/org/libremail/reporting/AccountLogRef.kt
+39
View File
@@ -0,0 +1,39 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.reporting
import java.security.MessageDigest
/**
* A short, stable, non-reversible reference for an account — safe to write to logs and a [DebugReport].
*
* [Account.id][org.libremail.domain.model.Account.id] embeds the raw email address (e.g.
* `"outlook:user@domain.com"` or `"imap:user@domain.com"`), so it is PII and must **never** be logged
* directly. This returns the id's scheme prefix followed by a truncated SHA-256 of the whole id — e.g.
* `"outlook:a1b2c3"` — which is:
*
* - **stable**: the same id always maps to the same reference (a hash, not a random token), so lines
* for one account correlate across a session and across reports;
* - **non-reversible**: a truncated one-way hash cannot be turned back into the email;
* - **non-PII**: the scheme (`outlook`/`imap`) names the auth kind, not the user, and the hex hash
* contains no `@`, domain, or local part. If the part before the first `:` is not a bare token
* (e.g. an id that is itself an address), it is replaced with [GENERIC_SCHEME] so no address can
* leak through the prefix.
*/
fun accountLogRef(accountId: String): String {
val candidate = accountId.substringBefore(':', missingDelimiterValue = "")
val scheme = candidate.takeIf(::isBareScheme) ?: GENERIC_SCHEME
return "$scheme:${sha256Hex(accountId).take(REF_HASH_LENGTH)}"
}
/** Prefix used when an id has no scheme, or one that could itself carry PII (an `@`, a dot, …). */
private const val GENERIC_SCHEME = "acct"
/** Hex characters of the SHA-256 kept in the reference; short but collision-safe for a device's few accounts. */
private const val REF_HASH_LENGTH = 6
/** A bare scheme token is letters/digits only, so an address (with `@`/`.`) can never pass as a scheme. */
private fun isBareScheme(text: String): Boolean = text.isNotEmpty() && text.all(Char::isLetterOrDigit)
private fun sha256Hex(input: String): String = MessageDigest.getInstance("SHA-256")
.digest(input.toByteArray(Charsets.UTF_8))
.joinToString("") { "%02x".format(it) }
app/src/main/kotlin/org/libremail/reporting/AppLog.kt
+27 -1
View File
@@ -8,6 +8,12 @@ import android.util.Log
* be attached to a user-reviewed [DebugReport]. Call [install] once at startup. Never pass PII
* (email addresses, message content, credentials) to these methods — the buffer can end up in a
* report the user reviews and may submit.
*
* The throwable-carrying [d]/[w]/[e] overloads forward the throwable to Logcat as usual, but record a
* **PII-scrubbed** rendering of its stack trace into the buffer via [StackTraceScrubber]: exception
* class names and stack frames are kept, while exception *messages* — which can embed a server host or
* an account email (e.g. a `ConnectException` or an auth failure) — are stripped. To refer to an
* account in a log line, log [accountLogRef]`(account.id)` rather than the raw id, which is PII.
*/
object AppLog {
@Volatile
@@ -22,6 +28,11 @@ object AppLog {
buffer?.record('D', tag, message)
}
fun d(tag: String, message: String, throwable: Throwable?) {
Log.d(tag, message, throwable)
buffer?.record('D', tag, bufferLine(message, throwable))
}
fun i(tag: String, message: String) {
Log.i(tag, message)
buffer?.record('I', tag, message)
@@ -32,8 +43,23 @@ object AppLog {
buffer?.record('W', tag, message)
}
fun w(tag: String, message: String, throwable: Throwable?) {
Log.w(tag, message, throwable)
buffer?.record('W', tag, bufferLine(message, throwable))
}
fun e(tag: String, message: String, throwable: Throwable? = null) {
Log.e(tag, message, throwable)
buffer?.record('E', tag, message)
buffer?.record('E', tag, bufferLine(message, throwable))
}
/**
* The buffer text for a log call: the caller [message] alone, or — when a [throwable] is present —
* the message followed by the throwable's **scrubbed** stack trace (exception class names and stack
* frames kept; host/email-bearing exception messages stripped by [StackTraceScrubber]).
*/
private fun bufferLine(message: String, throwable: Throwable?): String {
if (throwable == null) return message
return "$message\n" + StackTraceScrubber.scrub(throwable.stackTraceToString())
}
}
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) }
}
}