fix: seal the Welcome, the one gift wrap an MLS room must publish

No invite to a Marmot room has been delivered since 1700e6d. The Welcome
was built, hashed, queued and logged exactly as before -- and then refused
one step short of the relay, by a guard that had no idea it was looking at
one.

**The guard.** 1700e6d ("send a direct message into the group, wrapped for
one member") added a backstop at the top of sealGiftWrapPayload:

    val chatRoom = database.chatRoomDao().findChatRoomById(giftWrapPayload.chatRoomId)
    if (chatRoom?.chatRoom?.mlsGroupState != null) { log; return }

Its reasoning is sound and still is: a Marmot direct message is a genuine,
correctly signed NIP-59 gift wrap, indistinguishable from one this path
would be right to publish, and the only thing keeping it off a relay is
that it never becomes a GiftWrapPayload row. Refusing at the seal as well
means a future caller cannot walk one onto a relay by accident.

**Why it caught the Welcome.** MarmotOutboundDao.deliveryWelcome writes a
GiftWrapPayload with chatRoomId = nostrGroupId -- the MLS room's own id,
which by construction has mlsGroupState set. Every Welcome therefore
matched a refusal keyed on mlsGroupState alone. There is no Welcome that
does not: the tag identifying the room is the whole point of the event.

The kind:444 arm further down -- the one that wraps only for the
participant who published the referenced key package -- became
unreachable, which is why nothing in the logs said "welcome" at all.

**Why it went unnoticed.** The room reaches the correct state on the
inviter's side whether or not the Welcome goes out: addMember advances the
epoch, the group state is persisted, the Participant row exists, and
"Invited X to chat" is written to the transcript. From the coordinator's
side an invitee who never heard anything is indistinguishable from one who
joined -- recorded as a known gap in docs/shared-key-ceremony.md, and this
is what was behind it.

**The second-order damage.** observeUnsealedGiftWrapPayloads is

    SELECT * FROM GiftWrapPayload WHERE publicKey = :p AND giftWrapSealId IS NULL

collected as a Flow<GiftWrapPayload?> -- one row at a time. giftWrapSealId
is only ever set inside persistAndBroadcastGiftWrap, which the guard
returns before reaching, so a refused payload stays unsealed forever and
sits at the head of that queue. The first Welcome a user queued blocked
every gift wrap behind it, in every room, for the life of the install.

**The fix.** Kind first, room second. MIP-02 addresses kind:444 to someone
who is not yet in the group and holds no key to read a kind:445 -- a
relay-borne gift wrap is the only way to reach them, and deliveryWelcome
queues one on purpose. Every other kind is refused exactly as before.
sendChatMessage already branches on mlsGroupState before writing a
payload, so an MLS room's messages never arrive here anyway; the guard
stays as the backstop it was meant to be.

docs/marmot-direct-messages.md claimed "An MLS room should never produce a
NIP-17 gift wrap for any reason". That premise is what made the guard look
complete, so it is corrected rather than merely amended, with the test to
apply when adding a kind to the exemption: can its recipient read a
kind:445? If so, it does not belong on this path.

**Not fixed, deliberately.** A refusal still leaves the payload unsealed
and head-blocking. That is now unreachable -- nothing else can queue a
payload against an MLS room -- but it remains a trap for whatever gets
refused next. Marking a payload refused needs a state the schema does not
have, so it is left for its own change rather than smuggled in here.

Verified: :composeApp:compileDebugKotlinAndroid succeeds. No test covers
this -- DatabaseChatRepository is Room-backed, and Room-backed code has no
unit test harness in this project.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-06 00:40:31 +02:00
parent 39eac61838
commit 65e4a3acc0
2 changed files with 39 additions and 17 deletions

View File

@@ -324,19 +324,30 @@ class DatabaseChatRepository(
giftWrapPayload: GiftWrapPayload,
nostrSignerSync: NostrSignerSync
) {
// An MLS room must never produce a NIP-17 gift wrap. Its messages already travel
// inside kind:445, and its direct messages are real, correctly signed NIP-59 wraps
// -- indistinguishable from something this path would be right to publish. The
// only thing keeping one off a relay is that it never becomes a GiftWrapPayload,
// so refuse here too rather than trusting every future caller to know that.
// See docs/marmot-direct-messages.md.
val chatRoom = database.chatRoomDao().findChatRoomById(giftWrapPayload.chatRoomId)
if (chatRoom?.chatRoom?.mlsGroupState != null) {
logger.e(
"Refusing to seal payload ${giftWrapPayload.id}: ${giftWrapPayload.chatRoomId} is an MLS room, " +
"and sealing would broadcast it to relays"
)
return
// An MLS room must never produce a NIP-17 gift wrap of its *contents*. Its messages
// already travel inside kind:445, and its direct messages are real, correctly signed
// NIP-59 wraps -- indistinguishable from something this path would be right to
// publish. The only thing keeping one off a relay is that it never becomes a
// GiftWrapPayload, so refuse here too rather than trusting every future caller to
// know that. See docs/marmot-direct-messages.md.
//
// The Welcome is the one thing an MLS room is *supposed* to publish this way, and
// the reason it reaches this path at all. MIP-02 addresses kind:444 to a joiner who
// holds no group state yet: they cannot read a kind:445, so a relay-borne gift wrap
// is the only way to reach them, and `MarmotOutboundDao.deliveryWelcome` queues one
// here deliberately. Every Welcome carries its room's nostrGroupId, so a refusal
// keyed on `mlsGroupState` alone catches all of them and no invite is ever
// delivered -- and because the unsealed queue is a single-row flow, the refused
// Welcome sits at its head and blocks every payload behind it too.
if (giftWrapPayload.kind != WelcomeEvent.KIND) {
val chatRoom = database.chatRoomDao().findChatRoomById(giftWrapPayload.chatRoomId)
if (chatRoom?.chatRoom?.mlsGroupState != null) {
logger.e(
"Refusing to seal payload ${giftWrapPayload.id}: ${giftWrapPayload.chatRoomId} is an MLS room, " +
"and sealing would broadcast it to relays"
)
return
}
}
database.participantDao().findParticipantsByChatRoomId(giftWrapPayload.chatRoomId).forEach { participant ->

View File

@@ -159,9 +159,20 @@ but the tables it is kept out of stops it going to a relay.
`GiftWrapMessage` could not be written anyway without a `NostrEvent` row — its
foreign key — and `NostrEvent` is the broadcast join target. The rule is also
enforced at the other end: `sealGiftWrapPayload` refuses any payload whose room has
a non-null `mlsGroupState`, and logs. An MLS room should never produce a NIP-17 gift
wrap for any reason, and an invariant in code is what stops a later refactor from
walking a direct message onto a relay without reading this page first.
a non-null `mlsGroupState`, and logs. An invariant in code is what stops a later
refactor from walking a direct message onto a relay without reading this page first.
**The Welcome is exempt, and the exemption is not optional.** MIP-02 addresses
kind:444 to someone who is not yet in the group and holds no key to read a
kind:445 — a relay-borne gift wrap is the only way to reach them, and
`MarmotOutboundDao.deliveryWelcome` writes exactly such a `GiftWrapPayload` row on
purpose. Every Welcome carries its own room's `nostrGroupId`, so a refusal keyed on
`mlsGroupState` alone matches all of them: while it did, no invite to any Marmot
room was ever delivered, and — since the unsealed queue is a single-row flow — the
first refused Welcome sat at its head and blocked every payload queued behind it.
The check is therefore on kind first, room second. When adding a kind to that
exemption, the question to answer is whether its recipient can read a kind:445; if
they can, it does not belong on this path.
## Attribution comes from MLS, not from the payload
@@ -301,7 +312,7 @@ sender, `senderIdentity` is the only source of attribution there is.
| `managers/MarmotInboundManager.kt` | the kind:1059 carve-out; requires the sender identity |
| `database/model/ChatMessage.kt` | `TYPE_DIRECT_MESSAGE`; the kind:1059 arm and its three outcomes |
| `database/dao/NostrDao.kt` | resolves `senderIdentity` and passes it in |
| `database/repository/DatabaseChatRepository.kt` | queues the rumor; refuses to seal an MLS room's payload |
| `database/repository/DatabaseChatRepository.kt` | queues the rumor; refuses to seal an MLS room's payload, except a Welcome |
| `database/dao/MarmotOutboundDao.kt` | wraps on the way out; scrubs the plaintext |
| `database/model/MarmotInnerEvent.kt` | `directMessageRecipientPublicKey`, the outbound signal |
| `ui/view/model/ChatMessageListViewModel.kt` | armed state; the two renderings |