From dd9ee0b5c68010ccc929f005a7fe3eeb5603bfd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 21:29:23 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20close=20audit=20findings=20=E2=80=94=20r?= =?UTF-8?q?eport=20leak,=20model-level=20rumor=20guards,=20hide=20share/bo?= =?UTF-8?q?okmarks=20on=20private=20notes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge-readiness audit follow-ups: - Account.report(note): reporting a private rumor now reports the AUTHOR (p-tag only) instead of publishing a kind-1984 that e-tags the private rumor id onto public relays (the one confirmed leak) - Defense-in-depth guards at the model layer so the invariant no longer relies on UI gating alone: RepostAction.repost returns null / throws for empty-sig targets (covers Account.boost, createBoostEvent, and the desktop call path), ReactionAction.reactTo (simple overload) throws, and Account.broadcast no-ops for unsigned non-wrapped events — without the guard it would disclose the rumor JSON to relays even though they reject the signature - Hide remaining actions that can't work on private rumors, per review: share buttons (action row, both note menus) and all bookmark/playlist/ emoji-list rows (their lists reference an id other devices can't resolve; public lists would also leak it) - ZapCustomDialog: remember(accountViewModel, baseNote) so the preselected zap type can't go stale on lazy-list slot reuse Broadcast of the gift wrap itself (like DMs do via WrappedEvent.host) needs host tracking for non-WrappedEvent rumor kinds in quartz — left as a follow-up; the broadcast row stays hidden for kind-1 rumors. https://claude.ai/code/session_01B39MQmrT3dz137nfpXABvo --- .../vitorpamplona/amethyst/model/Account.kt | 16 ++++- .../amethyst/ui/note/NoteQuickActionMenu.kt | 3 + .../amethyst/ui/note/ReactionsRow.kt | 10 +-- .../amethyst/ui/note/ZapCustomDialog.kt | 2 +- .../amethyst/ui/note/elements/DropDownMenu.kt | 68 ++++++++++--------- .../model/nip18Reposts/RepostAction.kt | 6 ++ .../model/nip25Reactions/ReactionAction.kt | 6 ++ 7 files changed, 72 insertions(+), 39 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index c3c576ecbe..643c05ea67 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -945,7 +945,15 @@ class Account( note: Note, type: ReportType, content: String = "", - ) = sendMyPublicAndPrivateOutbox(ReportAction.report(note, type, content, userProfile(), signer)) + ) { + if (note.isPrivateRumor()) { + // A kind-1984 e-tagging the rumor would leak the private id onto + // public relays. Report the author instead (p-tag only). + note.author?.let { report(it, type, content) } + } else { + sendMyPublicAndPrivateOutbox(ReportAction.report(note, type, content, userProfile(), signer)) + } + } suspend fun report( user: User, @@ -1320,6 +1328,12 @@ class Account( suspend fun broadcast(note: Note) { note.event?.let { noteEvent -> + if (noteEvent !is WrappedEvent && noteEvent.sig.isEmpty()) { + // Unsealed rumor without a host wrap (e.g. a kind-1 private + // reply): publishing it would disclose the private content to + // relays even though they reject the missing signature. + return + } if (noteEvent is WrappedEvent && noteEvent.host != null) { // download the event and send it. noteEvent.host?.let { host -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt index 5dda5bfab2..48327b6790 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt @@ -406,6 +406,9 @@ fun CardBody( ) { onWantsToEditDraft() } + } else if (note.isPrivateRumor()) { + // No external share link for private rumors: nobody can + // resolve the id from relays and sharing it leaks the id. } else { NoteQuickActionItem( icon = MaterialSymbols.Share, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt index e80244fdb6..9573f040ac 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ReactionsRow.kt @@ -316,10 +316,12 @@ private fun InnerReactionRow( } ReactionRowAction.Share -> { - ShareReaction( - note = baseNote, - grayTint = MaterialTheme.colorScheme.placeholderText, - ) + if (!isPrivateRumor) { + ShareReaction( + note = baseNote, + grayTint = MaterialTheme.colorScheme.placeholderText, + ) + } } ReactionRowAction.Pay -> { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ZapCustomDialog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ZapCustomDialog.kt index c037859a59..8c29e8e01c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ZapCustomDialog.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/ZapCustomDialog.kt @@ -162,7 +162,7 @@ fun ZapCustomDialog( } var selectedZapType by - remember(accountViewModel) { + remember(accountViewModel, baseNote) { val default = accountViewModel.defaultZapType() mutableStateOf( if (isPrivateTarget && default != LnZapEvent.ZapType.NONZAP) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/DropDownMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/DropDownMenu.kt index 43184744fc..e43869f47d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/DropDownMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/DropDownMenu.kt @@ -226,17 +226,19 @@ fun NoteDropDownMenu( onDismiss() } } - M3ActionRow(icon = MaterialSymbols.Share, text = stringRes(R.string.quick_action_share)) { - val sendIntent = - Intent().apply { - action = Intent.ACTION_SEND - type = "text/plain" - putExtra(Intent.EXTRA_TEXT, externalLinkForNote(note)) - putExtra(Intent.EXTRA_TITLE, stringRes(actContext, R.string.quick_action_share_browser_link)) - } - val shareIntent = Intent.createChooser(sendIntent, stringRes(actContext, R.string.quick_action_share)) - actContext.startActivity(shareIntent) - onDismiss() + if (!isPrivateRumor) { + M3ActionRow(icon = MaterialSymbols.Share, text = stringRes(R.string.quick_action_share)) { + val sendIntent = + Intent().apply { + action = Intent.ACTION_SEND + type = "text/plain" + putExtra(Intent.EXTRA_TEXT, externalLinkForNote(note)) + putExtra(Intent.EXTRA_TITLE, stringRes(actContext, R.string.quick_action_share_browser_link)) + } + val shareIntent = Intent.createChooser(sendIntent, stringRes(actContext, R.string.quick_action_share)) + actContext.startActivity(shareIntent) + onDismiss() + } } } @@ -307,6 +309,12 @@ fun NoteDropDownMenu( // Showing both at once is noisy and makes "bookmark" feel like the catch-all when // it really isn't for these kinds. when { + isPrivateRumor -> { + // No bookmark/playlist/emoji-list rows for private rumors: + // those lists reference the note by id, which other devices + // can't resolve from relays and public lists would leak. + } + note.event is MusicTrackEvent && note is AddressableNote -> { // Music tracks (kind 36787) belong in playlists (kind 34139). The // sheet behind this nav lets the user toggle membership across all of @@ -335,19 +343,15 @@ fun NoteDropDownMenu( } else -> { - if (!isPrivateRumor) { - val noteBookmarkType = if (note.event is LongTextNoteEvent) stringRes(R.string.article) else stringRes(R.string.post) - M3ActionRow(icon = MaterialSymbols.BookmarkAdd, text = stringRes(R.string.manage_bookmark_label, noteBookmarkType)) { - if (note.event is LongTextNoteEvent) { - nav.nav(Route.ArticleBookmarkManagement((note as AddressableNote).address)) - } else { - nav.nav(Route.PostBookmarkManagement(note.idHex)) - } - onDismiss() + val noteBookmarkType = if (note.event is LongTextNoteEvent) stringRes(R.string.article) else stringRes(R.string.post) + M3ActionRow(icon = MaterialSymbols.BookmarkAdd, text = stringRes(R.string.manage_bookmark_label, noteBookmarkType)) { + if (note.event is LongTextNoteEvent) { + nav.nav(Route.ArticleBookmarkManagement((note as AddressableNote).address)) + } else { + nav.nav(Route.PostBookmarkManagement(note.idHex)) } + onDismiss() } - // Private bookmarks are stored inside the list's encrypted - // content, so a private rumor's id stays off public relays. if (state.isPrivateBookmarkNote) { M3ActionRow(icon = MaterialSymbols.LockOpen, text = stringRes(R.string.remove_from_private_bookmarks)) { accountViewModel.removePrivateBookmark(note) @@ -359,17 +363,15 @@ fun NoteDropDownMenu( onDismiss() } } - if (!isPrivateRumor) { - if (state.isPublicBookmarkNote) { - M3ActionRow(icon = MaterialSymbols.BookmarkRemove, text = stringRes(R.string.remove_from_public_bookmarks)) { - accountViewModel.removePublicBookmark(note) - onDismiss() - } - } else { - M3ActionRow(icon = MaterialSymbols.Bookmark, text = stringRes(R.string.add_to_public_bookmarks)) { - accountViewModel.addPublicBookmark(note) - onDismiss() - } + if (state.isPublicBookmarkNote) { + M3ActionRow(icon = MaterialSymbols.BookmarkRemove, text = stringRes(R.string.remove_from_public_bookmarks)) { + accountViewModel.removePublicBookmark(note) + onDismiss() + } + } else { + M3ActionRow(icon = MaterialSymbols.Bookmark, text = stringRes(R.string.add_to_public_bookmarks)) { + accountViewModel.addPublicBookmark(note) + onDismiss() } } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip18Reposts/RepostAction.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip18Reposts/RepostAction.kt index 0da9932423..9034277c0a 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip18Reposts/RepostAction.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip18Reposts/RepostAction.kt @@ -47,6 +47,11 @@ object RepostAction { if (!signer.isWriteable()) { throw IllegalStateException("Cannot repost: signer is not writeable") } + if (eventHint.event.sig.isEmpty()) { + // Unsealed private rumor: a public kind-6/16 would e-tag the + // private rumor id onto public relays. + throw IllegalStateException("Cannot repost a private rumor") + } // Use NIP-18 RepostEvent (kind 6) for text notes (kind 1) // Use GenericRepostEvent (kind 16) for all other kinds @@ -76,6 +81,7 @@ object RepostAction { ): Event? { // All validation in commons if (!signer.isWriteable()) return null + if (note.isPrivateRumor()) return null if (note.hasBoostedInTheLast5Minutes(signer.pubKey)) return null val hint = note.toEventHint() ?: return null diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip25Reactions/ReactionAction.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip25Reactions/ReactionAction.kt index fe4b41d552..ffeeaff9d3 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip25Reactions/ReactionAction.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip25Reactions/ReactionAction.kt @@ -57,6 +57,12 @@ object ReactionAction { if (!signer.isWriteable()) { throw IllegalStateException("Cannot react: signer is not writeable") } + if (eventHint.event.sig.isEmpty()) { + // Unsealed private rumor: a public kind-7 would e-tag the private + // rumor id onto public relays. Use reactToWithGroupSupport, which + // gift-wraps reactions for empty-sig targets. + throw IllegalStateException("Cannot react publicly to a private rumor") + } // Handle custom emoji reactions (format: ":emoji_name:") val template =