mantra/mantra-kmp
1
0
Fork 0
Code Issues 2 Pull Requests Actions 1 Packages Projects Releases 1 Wiki Activity

test: pin where a commit's bytes land when the row recording it is written

Browse Source 
The mis-routed `framedCommitBytes` fixed in the previous commit was invisible for
one reason: nothing anywhere covered the persisted row. The bytes that reach a
relay come off the in-memory `CommitResult`, so the wire path stayed correct and
the stored path was wrong, and no test looked at the stored path.

## Why the mapping moved before it could be tested

A test that built `MarmotCommitResult` itself would have been writing its own copy
of the mapping and asserting against that. It would have passed against the buggy
code, because the bug was at the call site the test was not using.

So the mapping is now `MarmotCommitResult.from`, called by
`MarmotOutboundDao.inviteMember` and exercised directly by the test. That also
removes the shape that produced the bug rather than just the instance of it: the
old call site listed its named arguments in an order different from the
declaration, which is what put `preCommitExporterSecret` and `framedCommitBytes`
two lines apart. `from` lists the payload in declaration order, in one place, so
there is no second site to get wrong.

## What is covered

Four tests, each payload given a distinct self-identifying value so that a field
arriving in the wrong column names both halves of the mistake instead of comparing
equal by accident:

  - every payload field lands in its own column.
  - the framed commit column never holds the exporter secret -- the regression,
    stated as an invariant rather than an equality so it keeps holding for a
    `CommitResult` this test did not anticipate.
  - a `CommitResult` that never framed its commit still stores a commit. quartz
    defaults `framedCommitBytes` to `commitBytes` and the entity repeats that
    default; the fallback must not quietly become the secret either.
  - the bookkeeping `DatabaseNostrRepository` reads back on acknowledgement is
    carried through. `id`, `chatRoomId`, `userPublicKey` and
    `peerKeyPackageEventId` are all 64-char hex, so two of them swapped in `from`
    would typecheck exactly as silently as the original bug.

Checked by reintroducing `framedCommitBytes = commitResult.preCommitExporterSecret`
into `from`: three of the four fail. A green suite that would stay green against
the bug it names is not coverage.

## What is not covered, and why

That the bytes published equal the bytes stored -- the property one level above
this one -- still is not. It needs the DAO, and the DAO needs Room: `commonTest`
carries only `kotlin.test`, the room3 KSP processor is registered for the android
and ios targets alone with `kspJvm` commented out, and `getInMemoryDatabaseBuilder`
wants a `PlatformContext` no unit test has. That is a Robolectric or instrumented
target, which is a larger change than this fix earns and is better decided on its
own merits than smuggled in here.

The ack-triggered rebroadcast that would have turned the bug into a live fault does
not exist yet, so there is nothing to test there either. When it is written, the
invariant it needs is already asserted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-05 23:24:21 +02:00
parent 248a527267
commit fb21678813
3 changed files with 173 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt
View File

@@ -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)
)
)

35
composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/MarmotCommitResult.kt
View File

@@ -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 {

134
composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/MarmotCommitResultMappingTest.kt Normal file
View File

@@ -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)
}
}