app/src/main/kotlin/org/libremail/data/attachment/AttachmentUriGrants.kt

@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data.attachment
+
+import android.content.Context
+import android.content.Intent
+import android.net.Uri
+import dagger.hilt.android.qualifiers.ApplicationContext
+import org.libremail.data.local.dao.DraftDao
+import org.libremail.data.local.dao.OutboxDao
+import org.libremail.data.local.toOutgoingAttachments
+import javax.inject.Inject
+import javax.inject.Singleton
+
+/**
+ * Releases the persistable read grants the compose pickers take on attachment / inline-image URIs.
+ * `ComposeScreen` calls `takePersistableUriPermission` on every pick, but nothing released them, so
+ * the app accumulated indefinite read access to every file/photo ever attached and could hit the
+ * per-app persisted-grant cap — after which a silently-swallowed take fails a later draft's image
+ * reload (post-batch security review, Low).
+ *
+ * The picked bytes are copied into the app cache when a message is enqueued
+ * (`MailRepositoryImpl.copyAttachments`), so a grant is only truly needed to reload an image when a
+ * *draft* is reopened. Callers therefore invoke [releaseUnreferenced] once the referencing row is
+ * gone — a draft is deleted, or an outbox message is sent or cancelled — passing that row's URIs.
+ * A URI still referenced by another live draft or outbox row is kept; releasing a grant we do not
+ * actually hold is expected and swallowed.
+ */
+@Singleton
+class AttachmentUriGrants @Inject constructor(
+    @ApplicationContext private val context: Context,
+    private val draftDao: DraftDao,
+    private val outboxDao: OutboxDao,
+) {
+    /**
+     * Releases the persistable grant of each URI in [uris] that no *remaining* draft or outbox row
+     * still references. Call it after deleting the row that referenced them, so that row no longer
+     * counts toward the "still referenced" check.
+     */
+    suspend fun releaseUnreferenced(uris: Collection<String>) {
+        if (uris.isEmpty()) return
+        unreferencedUris(uris, referencedUris()).forEach(::release)
+    }
+
+    /** Every attachment / inline-image URI still referenced by a draft or a queued outbox message. */
+    private suspend fun referencedUris(): Set<String> {
+        val fromDrafts = draftDao.getAll().flatMap { it.attachments.toOutgoingAttachments() }
+        val fromOutbox = outboxDao.getAll().flatMap { it.attachments.toOutgoingAttachments() }
+        return (fromDrafts + fromOutbox).mapTo(HashSet()) { it.uri }
+    }
+
+    private fun release(uri: String) {
+        // Only a grant we actually hold can be released; a URI never persisted (or already released)
+        // throws SecurityException, which is expected here and deliberately ignored.
+        runCatching {
+            context.contentResolver.releasePersistableUriPermission(
+                Uri.parse(uri),
+                Intent.FLAG_GRANT_READ_URI_PERMISSION,
+            )
+        }
+    }
+}
+
+/**
+ * The distinct URIs in [candidates] not present in [referenced] — i.e. the grants safe to release.
+ * Kept as a pure top-level function so the release decision is unit-testable without Android types.
+ */
+internal fun unreferencedUris(candidates: Collection<String>, referenced: Set<String>): List<String> =
+    candidates.distinct().filterNot { it in referenced }

app/src/main/kotlin/org/libremail/data/local/dao/DraftDao.kt

@@ -19,6 +19,10 @@ interface DraftDao {
     @Query("SELECT * FROM drafts WHERE id = :id LIMIT 1")
     suspend fun getById(id: String): DraftEntity?
 
+    /** All drafts, used to check whether an attachment URI is still referenced before releasing its grant. */
+    @Query("SELECT * FROM drafts")
+    suspend fun getAll(): List<DraftEntity>
+
     @Insert(onConflict = OnConflictStrategy.REPLACE)
     suspend fun upsert(draft: DraftEntity)
 

app/src/main/kotlin/org/libremail/data/local/dao/OutboxDao.kt

@@ -15,6 +15,9 @@ interface OutboxDao {
     @Query("SELECT * FROM outbox ORDER BY createdAt")
     suspend fun getAll(): List<OutboxEntity>
 
+    @Query("SELECT * FROM outbox WHERE id = :id LIMIT 1")
+    suspend fun getById(id: String): OutboxEntity?
+
     @Query("SELECT * FROM outbox ORDER BY createdAt")
     fun observeAll(): Flow<List<OutboxEntity>>
 

app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt

@@ -20,6 +20,7 @@ import kotlinx.coroutines.withContext
 import org.libremail.data.ReplyBuilder
 import org.libremail.data.SignatureBlock
 import org.libremail.data.Snippet
+import org.libremail.data.attachment.AttachmentUriGrants
 import org.libremail.data.attachmentCacheDir
 import org.libremail.data.local.dao.AccountDao
 import org.libremail.data.local.dao.AttachmentDao
@@ -32,6 +33,7 @@ import org.libremail.data.local.entity.MessageRouting
 import org.libremail.data.local.entity.OutboxEntity
 import org.libremail.data.local.toDomain
 import org.libremail.data.local.toEntity
+import org.libremail.data.local.toOutgoingAttachments
 import org.libremail.data.local.toOutgoingAttachmentsJson
 import org.libremail.data.settings.AccountSettingsRepository
 import org.libremail.data.settings.SignatureRepository
@@ -49,6 +51,7 @@ import org.libremail.domain.model.OutgoingAttachment
 import org.libremail.domain.model.OutgoingMessage
 import org.libremail.domain.model.ReplyMode
 import org.libremail.domain.model.UnreadCount
+import org.libremail.domain.model.sanitizeAttachmentName
 import org.libremail.domain.repository.MailRepository
 import org.libremail.mail.ImapClient
 import java.io.File
@@ -70,6 +73,7 @@ class MailRepositoryImpl @Inject constructor(
     private val sendScheduler: SendScheduler,
     private val accountSettingsRepository: AccountSettingsRepository,
     private val signatureRepository: SignatureRepository,
+    private val attachmentUriGrants: AttachmentUriGrants,
 ) : MailRepository {
 
     // Application-lifetime scope for fire-and-forget server pushes that must outlive the caller — e.g.
@@ -421,7 +425,7 @@ class MailRepositoryImpl @Inject constructor(
     private fun copyAttachments(outboxId: String, attachments: List<OutgoingAttachment>) {
         if (attachments.isEmpty()) return
         attachments.forEachIndexed { index, attachment ->
-            val safeName = attachment.name.substringAfterLast('/').substringAfterLast('\\').ifBlank { "attachment" }
+            val safeName = sanitizeAttachmentName(attachment.name)
             val dir = File(context.cacheDir, "outbox/$outboxId/$index").apply { mkdirs() }
             runCatching {
                 context.contentResolver.openInputStream(Uri.parse(attachment.uri))?.use { input ->
@@ -437,15 +441,24 @@ class MailRepositoryImpl @Inject constructor(
 
     override suspend fun saveDraft(draft: Draft) = draftDao.upsert(draft.toEntity())
 
-    override suspend fun deleteDraft(id: String) = draftDao.delete(id)
+    override suspend fun deleteDraft(id: String) {
+        // Capture the draft's attachment URIs before the row is gone, then release any persistable grant
+        // no other live draft/outbox row still needs (security review): a deleted draft can never reopen.
+        val uris = draftDao.getById(id)?.attachments?.toOutgoingAttachments()?.map { it.uri }.orEmpty()
+        draftDao.delete(id)
+        attachmentUriGrants.releaseUnreferenced(uris)
+    }
 
     override fun observeOutbox(): Flow<List<OutboxMessage>> = outboxDao.observeAll().map { rows ->
         rows.map { it.toDomain() }
     }
 
     override suspend fun cancelOutboxMessage(id: String) {
+        val uris = outboxDao.getById(id)?.attachments?.toOutgoingAttachments()?.map { it.uri }.orEmpty()
         outboxDao.delete(id)
         File(context.cacheDir, "outbox/$id").deleteRecursively()
+        // The queued copy is gone; release any picked-URI grant no other live draft/outbox row needs.
+        attachmentUriGrants.releaseUnreferenced(uris)
     }
 
     override suspend fun retryOutbox() = sendScheduler.sendNow()
@@ -484,7 +497,7 @@ class MailRepositoryImpl @Inject constructor(
      * and avoids filename collisions between messages.
      */
     private fun attachmentFile(messageId: String, partIndex: Int, filename: String): File {
-        val safeName = filename.substringAfterLast('/').substringAfterLast('\\').ifBlank { "attachment" }
+        val safeName = sanitizeAttachmentName(filename)
         return File(attachmentCacheDir(context.cacheDir, messageId), "$partIndex/$safeName")
     }
 }

app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt

@@ -9,8 +9,10 @@ import androidx.work.WorkerParameters
 import dagger.Lazy
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
+import org.libremail.data.attachment.AttachmentUriGrants
 import org.libremail.data.local.dao.AccountDao
 import org.libremail.data.local.dao.OutboxDao
+import org.libremail.data.local.entity.OutboxEntity
 import org.libremail.data.local.toDomain
 import org.libremail.data.local.toOutgoingAttachments
 import org.libremail.data.security.EncryptedCacheGuard
@@ -39,6 +41,8 @@ class SendWorker @AssistedInject constructor(
     private val graphSender: GraphSender,
     private val connectionFactory: Lazy<MailConnectionFactory>,
     private val cacheGuard: EncryptedCacheGuard,
+    // Lazy for the same reason as the DAOs above: resolving it touches the Room DB.
+    private val attachmentUriGrants: Lazy<AttachmentUriGrants>,
 ) : CoroutineWorker(appContext, workerParams) {
 
     private companion object {
@@ -50,6 +54,7 @@ class SendWorker @AssistedInject constructor(
         val outboxDao = this.outboxDao.get()
         val accountDao = this.accountDao.get()
         val connectionFactory = this.connectionFactory.get()
+        val attachmentUriGrants = this.attachmentUriGrants.get()
         val pending = outboxDao.getAll()
         if (pending.isEmpty()) return Result.success()
 
@@ -60,6 +65,7 @@ class SendWorker @AssistedInject constructor(
             if (account == null) {
                 outboxDao.delete(entity.id) // account removed — drop the queued message
                 attachmentDir.deleteRecursively()
+                attachmentUriGrants.releaseUnreferenced(entity.attachmentUris())
                 continue
             }
             runCatching {
@@ -87,6 +93,9 @@ class SendWorker @AssistedInject constructor(
                 onSuccess = {
                     outboxDao.delete(entity.id)
                     attachmentDir.deleteRecursively()
+                    // The picked bytes were staged at enqueue; with the row sent, drop the persistable
+                    // grant unless a live draft/outbox row still references the same URI (security review).
+                    attachmentUriGrants.releaseUnreferenced(entity.attachmentUris())
                 },
                 onFailure = { e ->
                     if (e is GraphSendException && e.mayHaveSent) {
@@ -167,4 +176,7 @@ class SendWorker @AssistedInject constructor(
         ?.sortedBy { it.name.toIntOrNull() ?: Int.MAX_VALUE }
         ?.mapNotNull { it.listFiles()?.firstOrNull() }
         .orEmpty()
+
+    /** The picked content-URIs this queued message was built from, for releasing their persistable grants. */
+    private fun OutboxEntity.attachmentUris(): List<String> = attachments.toOutgoingAttachments().map { it.uri }
 }

app/src/main/kotlin/org/libremail/domain/model/OutgoingMessage.kt

@@ -33,3 +33,12 @@ data class OutgoingAttachment(
     val contentId: String? = null,
     val isInline: Boolean = false,
 )
+
+/**
+ * Sanitizes a picked or received attachment display name before it becomes an on-disk filename or a
+ * MIME `Content-Disposition` filename: drops any path prefix and strips ISO control characters
+ * (including CR/LF), so a crafted name can neither traverse directories nor inject header lines.
+ * Falls back to `"attachment"` when nothing usable remains.
+ */
+fun sanitizeAttachmentName(raw: String): String =
+    raw.substringAfterLast('/').substringAfterLast('\\').filterNot { it.isISOControl() }.ifBlank { "attachment" }

app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt

@@ -74,6 +74,7 @@ import kotlinx.coroutines.flow.collect
 import org.libremail.R
 import org.libremail.domain.model.Account
 import org.libremail.domain.model.OutgoingAttachment
+import org.libremail.domain.model.sanitizeAttachmentName
 import org.libremail.ui.compose.format.FontRegistry
 
 @OptIn(ExperimentalMaterial3Api::class)
@@ -410,7 +411,9 @@ private fun queryFileName(context: Context, uri: Uri): String {
     val name = context.contentResolver
         .query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)
         ?.use { cursor -> if (cursor.moveToFirst()) cursor.getString(0) else null }
-    return name ?: uri.lastPathSegment?.substringAfterLast('/') ?: "attachment"
+    // Strip path separators / control chars so a crafted display name can't traverse dirs or inject
+    // into the on-disk name or the MIME Content-Disposition filename (security review).
+    return sanitizeAttachmentName(name ?: uri.lastPathSegment.orEmpty())
 }
 
 @Composable

app/src/test/kotlin/org/libremail/data/attachment/AttachmentUriGrantsTest.kt

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data.attachment
+
+import org.junit.Test
+import kotlin.test.assertEquals
+
+/**
+ * The release decision that [AttachmentUriGrants] runs after a draft/outbox row is deleted: a picked
+ * URI's persistable grant is released only when no *remaining* draft or outbox row still references
+ * it. The Android release call itself (`releasePersistableUriPermission`) is a thin wrapper; the
+ * decision here is the part worth pinning.
+ */
+class AttachmentUriGrantsTest {
+
+    @Test
+    fun `releases a uri referenced by no remaining row`() {
+        assertEquals(
+            listOf("content://a"),
+            unreferencedUris(candidates = listOf("content://a"), referenced = emptySet()),
+        )
+    }
+
+    @Test
+    fun `keeps a uri still referenced by another live draft or outbox row`() {
+        // content://shared is attached to a second draft that is still around, so its grant must stay.
+        assertEquals(
+            listOf("content://gone"),
+            unreferencedUris(
+                candidates = listOf("content://gone", "content://shared"),
+                referenced = setOf("content://shared"),
+            ),
+        )
+    }
+
+    @Test
+    fun `deduplicates candidate uris`() {
+        assertEquals(
+            listOf("content://a"),
+            unreferencedUris(candidates = listOf("content://a", "content://a"), referenced = emptySet()),
+        )
+    }
+
+    @Test
+    fun `releases nothing when every candidate is still referenced`() {
+        assertEquals(
+            emptyList(),
+            unreferencedUris(candidates = listOf("content://a"), referenced = setOf("content://a")),
+        )
+    }
+}

app/src/test/kotlin/org/libremail/data/repository/MailRepositoryGrantsTest.kt

@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.data.repository
+
+import android.content.Context
+import io.mockk.Runs
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.coVerifyOrder
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+import org.libremail.data.attachment.AttachmentUriGrants
+import org.libremail.data.local.dao.DraftDao
+import org.libremail.data.local.dao.OutboxDao
+import org.libremail.data.local.entity.DraftEntity
+import org.libremail.data.local.entity.OutboxEntity
+import java.nio.file.Files
+
+/**
+ * Deleting a draft, or cancelling a queued outbox message, must hand the removed row's picked-URI
+ * grants to [AttachmentUriGrants] so they can be released once nothing references them (post-batch
+ * security review). The release itself is decided/executed in [AttachmentUriGrants]; here we pin that
+ * the repository captures the right URIs and calls it *after* the row is gone.
+ */
+class MailRepositoryGrantsTest {
+
+    private val draftDao = mockk<DraftDao>()
+    private val outboxDao = mockk<OutboxDao>()
+    private val attachmentUriGrants = mockk<AttachmentUriGrants>(relaxed = true)
+    private val context = mockk<Context>(relaxed = true)
+
+    private val repository = MailRepositoryImpl(
+        context = context,
+        messageDao = mockk(relaxed = true),
+        accountDao = mockk(relaxed = true),
+        attachmentDao = mockk(relaxed = true),
+        outboxDao = outboxDao,
+        draftDao = draftDao,
+        folderDao = mockk(relaxed = true),
+        imapClient = mockk(relaxed = true),
+        connectionFactory = mockk(relaxed = true),
+        sendScheduler = mockk(relaxed = true),
+        accountSettingsRepository = mockk(relaxed = true),
+        signatureRepository = mockk(relaxed = true),
+        attachmentUriGrants = attachmentUriGrants,
+    )
+
+    @Test
+    fun `deleteDraft releases the deleted draft's URI grants after removing the row`() = runTest {
+        coEvery { draftDao.getById("d1") } returns draftEntity(
+            "d1",
+            """[{"uri":"content://pick/1","name":"a.png"}]""",
+        )
+        coEvery { draftDao.delete("d1") } just Runs
+
+        repository.deleteDraft("d1")
+
+        // Order matters: the row must be gone before the "still referenced?" check runs, so a draft
+        // doesn't keep its own grant alive on the way out.
+        coVerifyOrder {
+            draftDao.delete("d1")
+            attachmentUriGrants.releaseUnreferenced(listOf("content://pick/1"))
+        }
+    }
+
+    @Test
+    fun `deleteDraft of an attachment-less draft releases nothing`() = runTest {
+        coEvery { draftDao.getById("d2") } returns null // already gone: no attachments to release
+        coEvery { draftDao.delete("d2") } just Runs
+
+        repository.deleteDraft("d2")
+
+        coVerify { attachmentUriGrants.releaseUnreferenced(emptyList()) }
+    }
+
+    @Test
+    fun `cancelOutboxMessage releases the cancelled message's URI grants`() = runTest {
+        every { context.cacheDir } returns Files.createTempDirectory("outbox").toFile()
+        coEvery { outboxDao.getById("o1") } returns outboxEntity(
+            "o1",
+            """[{"uri":"content://pick/2","name":"b.png"}]""",
+        )
+        coEvery { outboxDao.delete("o1") } just Runs
+
+        repository.cancelOutboxMessage("o1")
+
+        coVerify { attachmentUriGrants.releaseUnreferenced(listOf("content://pick/2")) }
+    }
+
+    private fun draftEntity(id: String, attachmentsJson: String) = DraftEntity(
+        id = id,
+        accountId = "acct",
+        toAddresses = "",
+        ccAddresses = "",
+        bccAddresses = "",
+        subject = "",
+        body = "",
+        updatedAt = 0L,
+        attachments = attachmentsJson,
+    )
+
+    private fun outboxEntity(id: String, attachmentsJson: String) = OutboxEntity(
+        id = id,
+        accountId = "acct",
+        toAddresses = "bob@example.org",
+        ccAddresses = "",
+        subject = "Hi",
+        body = "body",
+        createdAt = 0L,
+        attachments = attachmentsJson,
+    )
+}

app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt

@@ -79,6 +79,8 @@ class MailRepositoryImplTest {
         sendScheduler = mockk(),
         accountSettingsRepository = accountSettingsRepository,
         signatureRepository = signatureRepository,
+        // Grant-release wiring (deleteDraft / cancelOutboxMessage) is covered by MailRepositoryGrantsTest.
+        attachmentUriGrants = mockk(relaxed = true),
     )
 
     @Test

app/src/test/kotlin/org/libremail/domain/model/SanitizeAttachmentNameTest.kt

@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail.domain.model
+
+import org.junit.Test
+import kotlin.test.assertEquals
+
+/**
+ * A picked/received attachment name becomes an on-disk filename and a MIME `Content-Disposition`
+ * filename, so it must not carry path separators (directory traversal) or CR/LF/control characters
+ * (header injection). [sanitizeAttachmentName] is the single chokepoint the compose picker and the
+ * outbox staging both run names through.
+ */
+class SanitizeAttachmentNameTest {
+
+    @Test
+    fun `keeps an ordinary name unchanged`() {
+        assertEquals("report.pdf", sanitizeAttachmentName("report.pdf"))
+    }
+
+    @Test
+    fun `strips CR and LF so a name cannot inject a header line`() {
+        assertEquals(
+            "invoice.pdfX-Evil: 1",
+            sanitizeAttachmentName("invoice.pdf\r\nX-Evil: 1"),
+        )
+    }
+
+    @Test
+    fun `strips other control characters`() {
+        // Char(9) = TAB and Char(0) = NUL are ISO control chars — removed, leaving the printable name.
+        val tab = Char(9)
+        val nul = Char(0)
+        assertEquals("ab.png", sanitizeAttachmentName("a" + tab + "b" + nul + ".png"))
+    }
+
+    @Test
+    fun `drops any path prefix using either separator`() {
+        assertEquals("passwd", sanitizeAttachmentName("../../etc/passwd"))
+        assertEquals("evil.exe", sanitizeAttachmentName("C:\\Windows\\evil.exe"))
+    }
+
+    @Test
+    fun `falls back to a default when nothing usable remains`() {
+        assertEquals("attachment", sanitizeAttachmentName(""))
+        assertEquals("attachment", sanitizeAttachmentName("\r\n"))
+        assertEquals("attachment", sanitizeAttachmentName("some/dir/"))
+    }
+}