fix(buzz): audit fixes for the Buzz interop PR

- Remove leftover diagnostic statements in NotificationFeedFilter (one ran a content check on every notification candidate). - Join on a Buzz open channel joins directly: every Buzz channel carries the `closed` tag, so the button opened the invite-code dialog instead. - Profile push: a signer failure no longer crashes the channel screen, and a failed attempt is retried later instead of being marked done. - `OK false "duplicate: …"` counts as accepted, not as a refusal. - Auto-join on post happens once per channel instead of on every post until the relay's roster catches up (each re-sent 9021 and re-published 10009). - The thread screen's Buzz check is reactive (cold-start trap, as in the top bar), so uploads and typing target the workspace once it is recognized. - Only domain-label/glued NIP-19 entities are skipped as citations; path links such as njump.me/npub1… keep being cited as before. - JPEG sanitizer walks entropy-coded scans: drops metadata after a scan and ends at the real EOI, so appended payloads containing FF D9 are cut. - observeChatMessageEdit resolves its initial value once per note instead of on every recomposition; forum-comment reply links validate their ids. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 19:28:25 +00:00 · 2026-10-01 09:11:49 -04:00
parent 864fcae134
commit e63488f664
12 changed files with 126 additions and 55 deletions
@@ -161,12 +161,11 @@ fun MinichatScreen(
    val canPost by remember { derivedStateOf { composer.text.isNotBlank() } }
    // A thread in a Buzz workspace channel: its media must live on the workspace's own server
    // (the relay refuses any other image host) and its typing signal is scoped to the thread.
-    val buzzGroup =
-        remember(rootNote) {
-            rootNote.inGatherers
-                ?.firstNotNullOfOrNull { it as? RelayGroupChannel }
-                ?.takeIf { BuzzRelayDialect.isBuzz(it.groupId.relayUrl) }
-        }
+    // Reactive on the dialect: a relay is marked Buzz only once its first verified Buzz event lands,
+    // which a cold start can reach after this screen opens.
+    val buzzRelays by BuzzRelayDialect.flow.collectAsStateWithLifecycle()
+    val relayGroup = remember(rootNote) { rootNote.inGatherers?.firstNotNullOfOrNull { it as? RelayGroupChannel } }
+    val buzzGroup = relayGroup?.takeIf { it.groupId.relayUrl in buzzRelays }
    val uploadState =
        remember(buzzGroup) {
            ChatFileUploadState(
@@ -173,7 +173,15 @@ fun RelayGroupTopBar(
        if (!isBuzzRelay) return@LaunchedEffect
        accountViewModel.account.userMetadata
            .getUserMetadataFlow()
-            .collect { accountViewModel.account.relayGroups.shareProfileWithBuzzWorkspace(channel.groupId.relayUrl) }
+            .collect {
+                try {
+                    accountViewModel.account.relayGroups.shareProfileWithBuzzWorkspace(channel.groupId.relayUrl)
+                } catch (e: Exception) {
+                    if (e is CancellationException) throw e
+                    // A refused/failed signature must not take down the screen; it retries next time.
+                    Log.w("RelayGroupTopBar", "Could not share the profile with ${channel.groupId.relayUrl.url}", e)
+                }
+            }
    }

    TopBarExtensibleWithBackButton(
@@ -286,8 +294,10 @@ fun RelayGroupTopBar(
                // is still what Buzz lists, counts and offers in its mention picker — its own client
                // shows Join here too, so a reader can become a member.
                FilledTonalButton(onClick = {
-                    // Closed groups need an invite code; open groups join directly.
-                    if (channel.isClosed()) {
+                    // Closed groups need an invite code; open groups join directly. Every Buzz channel
+                    // carries the `closed` tag, so a Buzz OPEN channel (one that doesn't require
+                    // membership to post) joins directly too.
+                    if (channel.isClosed() && channel.requiresMembershipToPost()) {
                        showJoinCode = true
                    } else {
                        requested = true
@@ -115,6 +115,7 @@ import com.vitorpamplona.quartz.nip22Comments.CommentEvent
 import com.vitorpamplona.quartz.nip22Comments.notify
 import com.vitorpamplona.quartz.nip28PublicChat.base.notify
 import com.vitorpamplona.quartz.nip28PublicChat.message.ChannelMessageEvent
+import com.vitorpamplona.quartz.nip29RelayGroups.GroupId
 import com.vitorpamplona.quartz.nip29RelayGroups.hTag
 import com.vitorpamplona.quartz.nip29RelayGroups.moderation.previous
 import com.vitorpamplona.quartz.nip30CustomEmoji.EmojiUrlTag
@@ -139,6 +140,7 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import java.util.concurrent.ConcurrentHashMap
 import kotlin.coroutines.cancellation.CancellationException

@Stable
@@ -630,6 +632,9 @@ open class ChannelNewMessageViewModel :
        }
    }

+    // Buzz channels this composer has already tried to join (see [joinBuzzChannelBeforePosting]).
+    private val buzzJoinAttempted: MutableSet<GroupId> = ConcurrentHashMap.newKeySet()
+
    /**
     * Posting to a Buzz open channel joins it first. The relay takes a non-member's message there,
     * but channel membership is what Buzz lists, counts and offers in its mention picker, and its
@@ -640,6 +645,9 @@ open class ChannelNewMessageViewModel :
        if (channel !is RelayGroupChannel || !BuzzRelayDialect.isBuzz(channel.groupId.relayUrl)) return
        if (channel.event?.isBuzzDm() == true) return
        if (channel.membershipOf(accountViewModel.account.userProfile().pubkeyHex).isMember()) return
+        // Once per channel: the relay's roster (39002) lags the join, and every post before it
+        // lands would otherwise re-send the kind-9021 and re-publish the kind-10009 list.
+        if (!buzzJoinAttempted.add(channel.groupId)) return
        try {
            accountViewModel.account.relayGroups.joinRelayGroup(channel)
        } catch (e: Exception) {
@@ -84,6 +84,7 @@ import com.vitorpamplona.quartz.nip29RelayGroups.tags.GroupPin
 import com.vitorpamplona.quartz.nip7DThreads.ThreadEvent
 import com.vitorpamplona.quartz.utils.TimeUtils
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.update
 import kotlinx.serialization.encodeToString
 import kotlinx.serialization.json.Json
 import kotlin.uuid.ExperimentalUuidApi
@@ -137,6 +138,7 @@ class AccountRelayGroupActions(
            if (key in current) return
            if (profileSharedWith.compareAndSet(current, current + key)) break
        }
+        try {
            val fresh =
                if (TimeUtils.now() - profile.createdAt < BUZZ_FRESH_PROFILE_SECS) {
                    profile
@@ -144,6 +146,11 @@ class AccountRelayGroupActions(
                    account.signer.sign<MetadataEvent>(TimeUtils.now(), MetadataEvent.KIND, profile.tags, profile.content)
                }
            account.client.publish(fresh, setOf(relay))
+        } catch (e: Throwable) {
+            // Not shared (signer refused, timed out, …): forget the attempt so a later visit retries.
+            profileSharedWith.update { it - key }
+            throw e
+        }
    }

    /**
@@ -1386,7 +1386,7 @@ open class EventCache :
                // A Buzz forum comment counts as a reply of its thread's root post (and of the comment
                // it answers, when nested), so the forum list's reply count sees it. A direct reply
                // carries only the `reply` marker, which then IS the root.
-                listOfNotNull(event.threadRoot(), event.replyTo()).distinct().map { getOrCreateNote(it) }
+                listOfNotNull(event.threadRoot(), event.replyTo()).distinct().mapNotNull { checkGetOrCreateNote(it) }
            }

            is GitStatusEvent -> {
@@ -407,7 +407,6 @@ class NotificationFeedFilter(

    override fun feed(): List<Note> {
        val filterParams = buildFilterParams(account)
-        com.vitorpamplona.quartz.utils.Log

        val notifications =
            LocalCache.notes.filterIntoSet { _, note ->
@@ -437,7 +436,6 @@ class NotificationFeedFilter(

    private fun innerApplyFilter(collection: Collection<Note>): Set<Note> {
        val filterParams = buildFilterParams(account)
-        com.vitorpamplona.quartz.utils.Log

        return collection.filterTo(HashSet()) { acceptableEvent(it, filterParams) }
    }
@@ -490,9 +488,7 @@ class NotificationFeedFilter(
        val event = note.event
        if (event !is ChatEvent && event !is StreamMessageV2Event) return false
        if (!event.isTaggedUser(me)) return false
-        val group = LocalCache.getRelayGroupChannelForContent(note)
-        com.vitorpamplona.quartz.utils.Log
-        if (group == null) return false
+        val group = LocalCache.getRelayGroupChannelForContent(note) ?: return false
        return BuzzRelayDialect.isBuzz(group.groupId.relayUrl)
    }

@@ -531,9 +527,6 @@ class NotificationFeedFilter(
        filterParams: FilterByListParams,
    ): Boolean {
        val loggedInUserHex = account.userProfile().pubkeyHex
-        if (it.event?.content?.startsWith("BZ-6") == true) {
-            com.vitorpamplona.quartz.utils.Log
-        }

        // When the user opts out of seeing Messages on the Notification tab, drop
        // direct/group message events (DMs and Marmot group chats) entirely.
@@ -237,8 +237,11 @@ fun RenderTextEvent(
 /** The newest in-place edit of a chat message (Concord, Buzz or Marmot), recomposing as edits land. */
@Composable
 fun observeChatMessageEdit(note: Note): Note? {
+    // Resolved once per note, not on every recomposition: produceState's initialValue argument is
+    // re-evaluated each time this runs, and these feeds recompose a lot.
+    val initial = remember(note) { note.latestChatEdit() }
    val latest by
-        produceState(initialValue = note.latestChatEdit(), note.idHex) {
+        produceState(initialValue = initial, note.idHex) {
            note
                .flow()
                .edits.stateFlow
@@ -45,6 +45,12 @@ object BuzzMediaSanitizer {

    private fun ByteArray.u8(i: Int) = this[i].toInt() and 0xff

+    /**
+     * Walks the JPEG marker by marker, entropy-coded scans included, so a metadata segment after a
+     * scan (progressive files interleave tables and scans) is dropped like one before it, and the
+     * file ends at the end-of-image marker that closes the last scan — not at whatever `FFD9` some
+     * appended payload (a motion-photo video, an extra JPEG) happens to contain.
+     */
    fun stripJpeg(b: ByteArray): ByteArray? {
        if (b.size < 4 || b.u8(0) != 0xff || b.u8(1) != 0xd8) return null
        val out = ByteBuilder(b.size)
@@ -58,17 +64,7 @@ object BuzzMediaSanitizer {
            val marker = b.u8(j)
            when {
                marker == 0xd9 -> {
-                    out.write(byteArrayOf(0xff.toByte(), 0xd9.toByte()), 0, 2)
-                    return out.toByteArray()
-                }
-
-                marker == 0xda -> {
-                    // Start of scan: entropy-coded data (and any later tables/scans) runs to the
-                    // last end-of-image marker; whatever trails it is dropped.
-                    val eoi = lastEoi(b)
-                    if (eoi < j) return null
-                    out.write(byteArrayOf(0xff.toByte()), 0, 1)
-                    out.write(b, j, eoi + 2 - j)
+                    out.write(EOI, 0, 2)
                    return out.toByteArray()
                }

@@ -77,6 +73,8 @@ object BuzzMediaSanitizer {
                    i = j + 1
                }

+                marker == 0xd8 -> return null
+
                else -> {
                    if (j + 3 > b.size) return null
                    val len = (b.u8(j + 1) shl 8) or b.u8(j + 2)
@@ -84,13 +82,35 @@ object BuzzMediaSanitizer {
                    val end = j + 1 + len
                    if (end > b.size) return null
                    if (keepJpegSegment(marker, b, j + 3, end)) {
-                        out.write(byteArrayOf(0xff.toByte()), 0, 1)
+                        out.write(FF, 0, 1)
                        out.write(b, j, end - j)
                    }
                    i = end
+                    if (marker == 0xda) {
+                        // Entropy-coded data runs to the next marker that isn't a stuffed byte
+                        // (FF 00) or a restart (FF D0-D7).
+                        val scanEnd = endOfScan(b, end) ?: return null
+                        out.write(b, end, scanEnd - end)
+                        i = scanEnd
                    }
                }
            }
+        }
+        return null
+    }
+
+    private fun endOfScan(
+        b: ByteArray,
+        from: Int,
+    ): Int? {
+        var k = from
+        while (k + 1 < b.size) {
+            if (b.u8(k) == 0xff) {
+                val next = b.u8(k + 1)
+                if (next != 0x00 && next != 0xff && next !in 0xd0..0xd7) return k
+            }
+            k++
+        }
        return null
    }

@@ -121,14 +141,9 @@ object BuzzMediaSanitizer {
        }
    }

-    private fun lastEoi(b: ByteArray): Int {
-        var k = b.size - 2
-        while (k >= 0) {
-            if (b.u8(k) == 0xff && b.u8(k + 1) == 0xd9) return k
-            k--
-        }
-        return -1
-    }
+    private val FF = byteArrayOf(0xff.toByte())
+
+    private val EOI = byteArrayOf(0xff.toByte(), 0xd9.toByte())

    private val PNG_SIGNATURE = byteArrayOf(0x89.toByte(), 'P'.code.toByte(), 'N'.code.toByte(), 'G'.code.toByte(), 0x0d, 0x0a, 0x1a, 0x0a)

@@ -31,7 +31,8 @@ import com.vitorpamplona.quartz.utils.Log
 /**
 * Listens to INostrClient's relay messages and reports `OK`s for published events: [onRelayReceived] for an acceptance and, when given,
 * [onRelayRejected] for a definitive refusal with the relay's reason. An `auth-required:` refusal
- * is NOT reported — the client authenticates and resends, so it is not the relay's final answer.
+ * is NOT reported — the client authenticates and resends, so it is not the relay's final answer —
+ * and a `duplicate:` refusal counts as an acceptance.
 */
 class RelayInsertConfirmationCollector(
    val client: INostrClient,
@@ -46,7 +47,9 @@ class RelayInsertConfirmationCollector(
                msg: Message,
            ) {
                if (msg !is OkMessage) return
-                if (msg.success) {
+                // NIP-01: "duplicate:" means the relay already has the event. Most relays send it
+                // with `true`, some with `false`; either way the event is there.
+                if (msg.success || msg.message.startsWith(DUPLICATE_PREFIX)) {
                    onRelayReceived(msg.eventId, relay)
                } else if (!msg.message.startsWith(AUTH_REQUIRED_PREFIX)) {
                    onRelayRejected?.invoke(msg.eventId, relay, msg.message)
@@ -67,3 +70,5 @@ class RelayInsertConfirmationCollector(
 }

 private const val AUTH_REQUIRED_PREFIX = "auth-required:"
+
+private const val DUPLICATE_PREFIX = "duplicate:"
@@ -314,9 +314,9 @@ object Nip19Parser {
    }

    /**
-     * True when the entity starting at [at] is glued to the token before it — an `npub1…` that is a
-     * URL's subdomain (`https://npub1….blossom.band/…`) or part of a path or word — rather than a
-     * reference on its own. `nostr:` and `@` prefixes are part of a reference, so they are skipped.
+     * True when the entity starting at [at] is glued to the word before it (`xnpub1…`, `a.npub1…`)
+     * rather than standing on its own. `nostr:` and `@` prefixes are part of a reference, so they are
+     * skipped; a `/` is not glue, so a path link like `njump.me/npub1…` still counts as cited.
     */
    private fun isEmbeddedAt(
        content: String,
@@ -327,9 +327,12 @@ object Nip19Parser {
        if (j >= 6 && content.regionMatches(j - 6, "nostr:", 0, 6, ignoreCase = true)) j -= 6
        if (j == 0) return false
        val c = content[j - 1]
-        return c.isLetterOrDigit() || c == '/' || c == '.' || c == '-' || c == '_' || c == '=' || c == '?' || c == '&' || c == '#'
+        return c.isLetterOrDigit() || c == '.' || c == '-' || c == '_'
    }

+    /** The entity is a domain label — `npub1….blossom.band` — so it names a host, not a person. */
+    private fun isDomainLabel(additionalChars: String): Boolean = additionalChars.length > 1 && additionalChars[0] == '.' && additionalChars[1].isLetterOrDigit()
+
    private fun allBech32(
        content: String,
        from: Int,
@@ -371,8 +374,10 @@ object Nip19Parser {
    fun parseAllStandalone(content: String): List<Entity> {
        val returningList = mutableListOf<Entity>()
        forEachNip19Match(content, FIXED_58_PREFIXES, VARIABLE_PREFIXES, standaloneOnly = true) { type, key, additionalChars ->
+            if (!isDomainLabel(additionalChars)) {
                parseComponents(type, key, additionalChars)?.entity?.let { returningList.add(it) }
            }
+        }
        return returningList
    }

@@ -91,4 +91,23 @@ class BuzzMediaSanitizerTest {
        assertContentEquals(notJpeg, BuzzMediaSanitizer.sanitize(notJpeg, "image/jpeg"))
        assertEquals(true, BuzzMediaSanitizer.handles("IMAGE/JPEG"))
    }
+
+    @Test
+    fun metadataAfterAScanIsDroppedToo() {
+        val secondScan = seg(0xda, bytes(1, 1, 0, 0, 63, 0)) + bytes(0x77, 0x01)
+        val input = bytes(0xff, 0xd8) + jfif + dqt + sosAndData + exif + seg(0xc4, ByteArray(5) { 2 }) + secondScan + eoi
+
+        val clean = BuzzMediaSanitizer.sanitize(input, "image/jpeg")
+
+        assertContentEquals(bytes(0xff, 0xd8) + jfif + dqt + sosAndData + seg(0xc4, ByteArray(5) { 2 }) + secondScan + eoi, clean)
+    }
+
+    @Test
+    fun appendedPayloadEndsAtTheRealEndOfImage() {
+        // A motion photo appends a video (or another JPEG) that can itself contain FF D9.
+        val appended = bytes(0x00, 0x11, 0xff, 0xd9, 0x22)
+        val input = bytes(0xff, 0xd8) + jfif + dqt + sosAndData + eoi + appended
+
+        assertContentEquals(bytes(0xff, 0xd8) + jfif + dqt + sosAndData + eoi, BuzzMediaSanitizer.sanitize(input, "image/jpeg"))
+    }
 }
@@ -35,8 +35,15 @@ class FindNostrUrisTest {
    }

    @Test
-    fun npubInAPathIsNotCited() {
-        assertTrue(findNostrUris("see https://example.com/p/$npub").isEmpty())
+    fun npubGluedToAWordIsNotCited() {
+        assertTrue(findNostrUris("x$npub").isEmpty())
+    }
+
+    @Test
+    fun aPathLinkToAProfileIsStillCited() {
+        // njump-style links have always been cited; only domain labels are excluded.
+        assertEquals(1, findNostrUris("see https://njump.me/$npub").filterIsInstance<NPub>().size)
+        assertEquals(1, findNostrUris("end of sentence nostr:$npub.").filterIsInstance<NPub>().size)
    }

    @Test