From 5753da7be3aec46e4c757abf93a9bb534dbb584f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 23:55:40 +0000 Subject: [PATCH] fix: address Marmot group-icon audit findings (media_type, multi-server fetch, recomposition, placeholder) - Resolve the group icon across the viewer's default Blossom server AND the group admins' configured servers via BlossomServerResolver (BUD-03), so members other than the uploader can actually load it; optimistically load the default server while the async probe runs. Reuses existing resolver + HEAD caches. - Compress/downscale the picked avatar up front so the stored media_type matches the actual (post-compression) bytes rather than the original picked MIME; clean up the intermediate temp file. - Register the decryption cipher only when the URL/cipher changes instead of on every feed-row recomposition. - Give the create-screen placeholder avatar a stable random seed instead of "". Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JL3GXW1fmHa3xWfQjLLqfp --- .../chats/marmotGroup/CreateGroupScreen.kt | 6 +- .../marmotGroup/MarmotGroupIconDisplay.kt | 45 ++++++++-- .../send/MarmotGroupIconUploader.kt | 90 +++++++++++-------- .../chats/rooms/ChatroomHeaderCompose.kt | 3 +- 4 files changed, 101 insertions(+), 43 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt index 0fb1201224..a1346bdf51 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt @@ -64,6 +64,10 @@ fun CreateGroupScreen( var groupName by remember { mutableStateOf("") } var groupDescription by remember { mutableStateOf("") } var pickedIcon by remember { mutableStateOf(null) } + // Stable seed for the placeholder avatar shown before an icon is picked. The real + // group id is generated per creation attempt (so retries don't collide), so this is + // a separate cosmetic seed rather than "". + val avatarSeed = remember { RandomInstance.bytes(32).toHexKey() } var isCreating by remember { mutableStateOf(false) } var showKeyPackageRelayDialog by remember { mutableStateOf(false) } val scope = rememberCoroutineScope() @@ -142,7 +146,7 @@ fun CreateGroupScreen( ) MarmotGroupIconEditor( - groupId = "", + groupId = avatarSeed, existingImage = null, pickedMedia = pickedIcon, removeRequested = false, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupIconDisplay.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupIconDisplay.kt index 069235c468..fa59b898cf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupIconDisplay.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupIconDisplay.kt @@ -22,24 +22,31 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.commons.model.marmotGroups.MarmotGroupImage import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.quartz.marmot.mip01Groups.MarmotGroupImageCipher +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nipB7Blossom.BlossomServerUrl +import com.vitorpamplona.quartz.nipB7Blossom.BlossomUri /** * Resolve the URL from which a Marmot group's encrypted avatar can be loaded, and * register its decryption cipher in the encrypted-blob HTTP cache so any Coil load * of that URL transparently yields the decrypted image (via `EncryptedBlobInterceptor`). * - * The blob is content-addressed on Blossom by [MarmotGroupImage.hash]; because the - * canonical scheme stores only the hash (not a URL), we reconstruct the URL against - * the viewer's default Blossom server. When the blob does not live there, the load - * simply fails and callers fall back to the relay icon. + * The blob is content-addressed on Blossom by [MarmotGroupImage.hash]; the canonical + * scheme stores only the hash, and the icon usually lives on the *uploader's* Blossom + * server rather than the viewer's. So we resolve it through [Amethyst.blossomResolver], + * which probes the viewer's default server (passed as a first-try `xs` hint) and the + * group admins' configured servers (via their pubkeys as `as` authors, BUD-03). While + * that async probe runs, we optimistically load from the viewer's default server so the + * common case (shared server) shows instantly; if nothing resolves, callers fall back to + * the relay icon. * * Returns null when there is no image to show. */ @@ -47,13 +54,39 @@ import com.vitorpamplona.quartz.nipB7Blossom.BlossomServerUrl fun rememberMarmotGroupIconUrl( image: MarmotGroupImage?, accountViewModel: AccountViewModel, + adminPubkeys: List = emptyList(), ): String? { if (image == null) return null val serverBaseUrl = accountViewModel.account.settings.defaultFileServer.baseUrl - val url = remember(image.hash, serverBaseUrl) { BlossomServerUrl.blob(serverBaseUrl, image.hash) } + val fallbackUrl = remember(image.hash, serverBaseUrl) { BlossomServerUrl.blob(serverBaseUrl, image.hash) } + + // A blossom: URI carrying the default server as a first-try hint and the admins as + // authors. Extension "bin" keeps the resolver's HEAD check type-agnostic, matching the + // application/octet-stream encrypted blob. + val blossomUri = + remember(image.hash, serverBaseUrl, adminPubkeys) { + BlossomUri( + sha256 = image.hash, + extension = "bin", + servers = listOf(serverBaseUrl), + authors = adminPubkeys, + size = null, + ).toUriString() + } + + val resolver = Amethyst.instance.blossomResolver + val url by produceState(resolver.cachedFindServer(blossomUri)?.serverUrl ?: fallbackUrl, blossomUri) { + value = resolver.findServers(blossomUri)?.serverUrl ?: fallbackUrl + } + val cipher = remember(image) { MarmotGroupImageCipher(image.key, image.nonce, image.mediaType) } - Amethyst.instance.keyCache.add(url, cipher, image.mediaType) + // Register the cipher only when the URL or cipher changes (not on every recomposition), + // and synchronously during composition so the interceptor can decrypt before Coil fetches. + remember(url, cipher) { + Amethyst.instance.keyCache.add(url, cipher, image.mediaType) + url + } return url } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotGroupIconUploader.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotGroupIconUploader.kt index ba64f21ffa..a1c433d09b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotGroupIconUploader.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotGroupIconUploader.kt @@ -24,6 +24,7 @@ import android.content.Context import android.net.Uri import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.service.uploads.CompressorQuality +import com.vitorpamplona.amethyst.service.uploads.MediaCompressor import com.vitorpamplona.amethyst.service.uploads.UploadOrchestrator import com.vitorpamplona.amethyst.service.uploads.UploadingState import com.vitorpamplona.amethyst.ui.actions.mediaServers.ServerName @@ -34,6 +35,8 @@ import com.vitorpamplona.quartz.marmot.mip04EncryptedMedia.Mip04MediaEncryption import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.utils.Log +import java.io.File /** * The parameters produced by encrypting + uploading a new Marmot group avatar, @@ -86,47 +89,64 @@ class MarmotGroupIconUploader( server: ServerName, context: Context, ): MarmotGroupIconUpload { - val mediaType = Mip04MediaEncryption.canonicalizeMimeType(mimeType ?: DEFAULT_MIME) + // Compress/downscale up front (avatars don't need full resolution) so the stored + // media_type reflects the ACTUAL post-compression bytes. The AEAD binds media_type, + // and MediaCompressor transcodes images to JPEG — so we must know the final type + // before building the cipher, then upload the already-compressed bytes uncompressed. + val compressed = MediaCompressor().compress(uri, mimeType, CompressorQuality.MEDIUM, context.applicationContext) + val mediaType = Mip04MediaEncryption.canonicalizeMimeType(compressed.contentType ?: mimeType ?: DEFAULT_MIME) val cipher = MarmotGroupImageCipher.forNewImage(mediaType) val uploadKey = MarmotGroupImageEncryption.generateUploadKey() val uploadSigner = NostrSignerInternal(KeyPair(privKey = uploadKey)) - val state = - UploadOrchestrator().uploadEncrypted( - uri = uri, - mimeType = mediaType, - alt = null, - contentWarningReason = null, - compressionQuality = CompressorQuality.MEDIUM, - encrypt = cipher, - server = server, - account = account, - context = context, - stripMetadata = true, - forcedSigner = uploadSigner, - ) + try { + val state = + UploadOrchestrator().uploadEncrypted( + uri = compressed.uri, + mimeType = mediaType, + alt = null, + contentWarningReason = null, + compressionQuality = CompressorQuality.UNCOMPRESSED, + encrypt = cipher, + server = server, + account = account, + context = context, + stripMetadata = true, + forcedSigner = uploadSigner, + ) - if (state is UploadingState.Finished && state.result is UploadOrchestrator.OrchestratorResult.ServerResult) { - val serverResult = state.result - val hash = - serverResult.uploadedHash - ?: throw IllegalStateException("Blossom server did not return a content hash for the group icon") - return MarmotGroupIconUpload( - imageHash = hash, - imageKey = cipher.imageKey, - imageNonce = cipher.imageNonce, - imageUploadKey = uploadKey, - mediaType = mediaType, - ) - } - - val message = - if (state is UploadingState.Error) { - stringRes(context, state.errorResource, *state.params) - } else { - "Group icon upload failed" + if (state is UploadingState.Finished && state.result is UploadOrchestrator.OrchestratorResult.ServerResult) { + val serverResult = state.result + val hash = + serverResult.uploadedHash + ?: throw IllegalStateException("Blossom server did not return a content hash for the group icon") + return MarmotGroupIconUpload( + imageHash = hash, + imageKey = cipher.imageKey, + imageNonce = cipher.imageNonce, + imageUploadKey = uploadKey, + mediaType = mediaType, + ) } - throw IllegalStateException(message) + + val message = + if (state is UploadingState.Error) { + stringRes(context, state.errorResource, *state.params) + } else { + "Group icon upload failed" + } + throw IllegalStateException(message) + } finally { + // Delete the intermediate compressed temp file (compress returns the original + // URI unchanged when it skips compression, so only delete a distinct temp). + if (compressed.uri != uri) { + try { + compressed.uri.path?.let { path -> File(path).takeIf { it.exists() }?.delete() } + } catch (e: Exception) { + Log.w("MarmotGroupIconUploader", "Failed to delete temp icon file", e) + } + } + } } companion object { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt index 3405f35e5f..36871ecf68 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt @@ -322,6 +322,7 @@ private fun MarmotGroupRoomCompose( val displayName by chatroom.displayName.collectAsStateWithLifecycle() val image by chatroom.image.collectAsStateWithLifecycle() val relays by chatroom.relays.collectAsStateWithLifecycle() + val adminPubkeys by chatroom.adminPubkeys.collectAsStateWithLifecycle() val author = lastMessage.author val noteEvent = lastMessage.event @@ -331,7 +332,7 @@ private fun MarmotGroupRoomCompose( // NIP-11 icon of one of the group's relays (fetched on a cache miss). val channelPicture = if (image != null) { - rememberMarmotGroupIconUrl(image, accountViewModel) + rememberMarmotGroupIconUrl(image, accountViewModel, adminPubkeys) } else { loadMarmotRelayIcon(relays) }