diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt index c03944bd..caad421b 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt @@ -435,17 +435,13 @@ abstract class MarmotOutboundDao( // Save commitResult... in case we need to broadcast welcomeEvent after relay acknowledgement... database.marmotCommitResultDao().upsert( - MarmotCommitResult( - id = commitEvent.id, - isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation, + MarmotCommitResult.from( + commitEventId = commitEvent.id, + commitResult = commitResult, chatRoomId = nostrGroupId, - commitBytes = commitResult.commitBytes, - preCommitExporterSecret = commitResult.preCommitExporterSecret, - welcomeBytes = commitResult.welcomeBytes, - framedCommitBytes = commitResult.framedCommitBytes, - groupInfoBytes = commitResult.groupInfoBytes, userPublicKey = userPublicKey, peerKeyPackageEventId = peerKeyPackage.id, + isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation, createdAt = Instant.fromEpochSeconds(commitEvent.createdAt) ) ) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MarmotCommitResult.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MarmotCommitResult.kt index 160a7a5c..ef749688 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MarmotCommitResult.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MarmotCommitResult.kt @@ -8,6 +8,7 @@ import press.mantra.compose.database.model.traits.SoftDeletableEntity import press.mantra.compose.database.model.traits.TimestampedEntity import press.mantra.compose.database.model.traits.UserViewableEntity import co.touchlab.kermit.Logger +import com.vitorpamplona.quartz.marmot.mls.messages.CommitResult import com.vitorpamplona.quartz.nip01Core.core.HexKey import kotlin.time.Clock import kotlin.time.Instant @@ -72,6 +73,40 @@ data class MarmotCommitResult( // TODO: Rename this to GiftWrapPayload... companion object { const val TAG = "MarmotCommitResult" + /** + * The persisted record of a commit, built from the [CommitResult] that produced it. + * + * The five payload fields are carried over from quartz verbatim -- same names, same + * order, same `ByteArray` type on both sides of the copy -- so a value taken from the + * wrong field of the right object typechecks and reaches the database unnoticed. + * `framedCommitBytes = commitResult.preCommitExporterSecret` survived exactly that way, + * storing the group's pre-commit exporter secret in the column documented to hold a + * broadcastable MLS envelope. + * + * Mapping here rather than at the call site means it is written once, in declaration + * order, and pinned by MarmotCommitResultMappingTest. + */ + fun from( + commitEventId: HexKey, + commitResult: CommitResult, + chatRoomId: HexKey, + userPublicKey: HexKey, + peerKeyPackageEventId: HexKey, + isOneMemberInitialGroupCreation: Boolean, + createdAt: Instant, + ): MarmotCommitResult = MarmotCommitResult( + id = commitEventId, + userPublicKey = userPublicKey, + peerKeyPackageEventId = peerKeyPackageEventId, + chatRoomId = chatRoomId, + isOneMemberInitialGroupCreation = isOneMemberInitialGroupCreation, + commitBytes = commitResult.commitBytes, + welcomeBytes = commitResult.welcomeBytes, + groupInfoBytes = commitResult.groupInfoBytes, + framedCommitBytes = commitResult.framedCommitBytes, + preCommitExporterSecret = commitResult.preCommitExporterSecret, + createdAt = createdAt, + ) } override fun equals(other: Any?): Boolean { diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/MarmotCommitResultMappingTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/MarmotCommitResultMappingTest.kt new file mode 100644 index 00000000..74cd329e --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/MarmotCommitResultMappingTest.kt @@ -0,0 +1,134 @@ +package press.mantra.compose.database.model + +import com.vitorpamplona.quartz.marmot.mls.messages.CommitResult +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.time.Instant + +/** + * Where a commit's bytes land when the row that records it is written. + * + * `MarmotCommitResult` carries quartz's `CommitResult` payload fields verbatim -- + * `commitBytes`, `welcomeBytes`, `groupInfoBytes`, `framedCommitBytes`, + * `preCommitExporterSecret`, the same names and all of them `ByteArray`. A value + * taken from the wrong field of the right object therefore typechecks, and + * `framedCommitBytes = commitResult.preCommitExporterSecret` reached the database + * that way and sat there unnoticed: the column documented to hold a broadcastable + * `MlsMessage(PublicMessage(FramedContent(commit)))` envelope held 32 bytes of the + * group's pre-commit exporter secret instead. + * + * Nothing caught it because nothing read the column. The bytes that reached the + * relay come off the in-memory `CommitResult`, so the wire stayed correct while the + * record of it did not, and the row is written precisely so that the + * acknowledgement path in `DatabaseNostrRepository` can pick work back up later. A + * rebroadcast reading `framedCommitBytes` would have published noise the group + * decrypts, fails to parse, and drops -- silent, which is this subsystem's + * characteristic failure. + * + * So the routing is pinned here. Every payload gets a distinct, self-identifying + * value: a field that ends up in the wrong column names both halves of the mistake + * when it fails, rather than comparing equal by accident. + */ +class MarmotCommitResultMappingTest { + private val commitBytes = "raw-commit".encodeToByteArray() + private val framedCommitBytes = "framed-commit-envelope".encodeToByteArray() + private val welcomeBytes = "welcome".encodeToByteArray() + private val groupInfoBytes = "group-info".encodeToByteArray() + + /** Stands in for `MLS-Exporter("marmot", "group-event", 32)` at the pre-commit epoch. */ + private val preCommitExporterSecret = ByteArray(32) { 0x5E } + + private val commitEventId = "a".repeat(64) + private val chatRoomId = "b".repeat(64) + private val userPublicKey = "c".repeat(64) + private val peerKeyPackageEventId = "d".repeat(64) + private val createdAt = Instant.fromEpochSeconds(1_700_000_000) + + private fun commitResult( + framedCommitBytes: ByteArray = this.framedCommitBytes, + preCommitExporterSecret: ByteArray = this.preCommitExporterSecret, + ) = CommitResult( + commitBytes = commitBytes, + welcomeBytes = welcomeBytes, + groupInfoBytes = groupInfoBytes, + framedCommitBytes = framedCommitBytes, + preCommitExporterSecret = preCommitExporterSecret, + ) + + private fun map(commitResult: CommitResult) = MarmotCommitResult.from( + commitEventId = commitEventId, + commitResult = commitResult, + chatRoomId = chatRoomId, + userPublicKey = userPublicKey, + peerKeyPackageEventId = peerKeyPackageEventId, + isOneMemberInitialGroupCreation = false, + createdAt = createdAt, + ) + + @Test + fun `every payload field lands in its own column`() { + val row = map(commitResult()) + + assertContentEquals(commitBytes, row.commitBytes, "commitBytes") + assertContentEquals(welcomeBytes, row.welcomeBytes, "welcomeBytes") + assertContentEquals(groupInfoBytes, row.groupInfoBytes, "groupInfoBytes") + assertContentEquals(framedCommitBytes, row.framedCommitBytes, "framedCommitBytes") + assertContentEquals( + preCommitExporterSecret, + row.preCommitExporterSecret, + "preCommitExporterSecret" + ) + } + + @Test + fun `the framed commit column never holds the exporter secret`() { + // The regression. Stated as the invariant rather than as an equality check, + // so it keeps holding for a CommitResult this test did not anticipate. + val row = map(commitResult()) + + assertFalse( + row.framedCommitBytes.contentEquals(row.preCommitExporterSecret), + "the group's exporter secret was stored as the framed commit" + ) + } + + @Test + fun `a CommitResult that never framed its commit still stores a commit`() { + // quartz defaults framedCommitBytes to commitBytes, and the entity repeats that + // default. Whichever of the two a row ends up with, it must be a commit -- the + // fallback must not quietly become the secret either. + val unframed = CommitResult( + commitBytes = commitBytes, + welcomeBytes = welcomeBytes, + groupInfoBytes = groupInfoBytes, + preCommitExporterSecret = preCommitExporterSecret, + ) + + val row = map(unframed) + + assertContentEquals(commitBytes, row.framedCommitBytes) + assertFalse( + row.framedCommitBytes.contentEquals(row.preCommitExporterSecret), + "the group's exporter secret was stored as the framed commit" + ) + } + + @Test + fun `the bookkeeping the acknowledgement path reads is carried through`() { + // DatabaseNostrRepository finds this row by the commit event's id and delivers the + // welcome using chatRoomId, userPublicKey and peerKeyPackageEventId. All four are + // supplied by the caller rather than the CommitResult, so they are checked here to + // keep the argument order of `from` honest -- every one of them is a 64-char hex + // string, and swapping two would otherwise typecheck as silently as the bug did. + val row = map(commitResult()) + + assertEquals(commitEventId, row.id) + assertEquals(chatRoomId, row.chatRoomId) + assertEquals(userPublicKey, row.userPublicKey) + assertEquals(peerKeyPackageEventId, row.peerKeyPackageEventId) + assertEquals(createdAt, row.createdAt) + assertFalse(row.isOneMemberInitialGroupCreation) + } +}