From 73c9c15d01a823ce1f43a3b061b734fe19af2473 Mon Sep 17 00:00:00 2001 From: nate Date: Thu, 20 Aug 2026 07:31:04 -0700 Subject: [PATCH 1/2] Charter transfers are retry-safe: idempotent no-op + self-validated links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit transferOwnershipWithCharter had a publish-retry trap: transfer A→B succeeds locally, the manifest publish fails, and the retry calls the builder again with the already-updated group — minting either a B→B self-link or a link not signed by the previous owner. validateCharter rejects both, permanently: under CharterPolicy.strict every member's decryptManifest then returns null and the group stops syncing, with the only recovery a deliberate charter-less republish (permanently dropping enforcement). The poisoning was silent at mint time. Two guards: - newOwnerUid == group.ownerUid returns the group unchanged, so publish-retry loops are safe by construction; - the builder validates its own extended chain before returning and throws a retryable StateError instead of handing back a link the validator would reject (e.g. stale local state whose tip the signing identity no longer matches). 74 tests, analyze clean. --- lib/src/group_service.dart | 25 +++++++++++- test/group_service_test.dart | 77 ++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/lib/src/group_service.dart b/lib/src/group_service.dart index 634f928..07ce62b 100644 --- a/lib/src/group_service.dart +++ b/lib/src/group_service.dart @@ -198,6 +198,15 @@ class GroupService { required String signingKeyDomain, bool allowUnchartedFallback = false, }) { + // Idempotent no-op when the target already owns the group. This is the + // RETRY trap: transfer A→B succeeds locally but the manifest publish + // fails; the retry calls this again with the already-updated group and + // would mint either a B→B self-link or a link not signed by the (now + // previous) owner — both permanently invalid under [validateCharter], + // which under a strict policy stops every member's manifest decrypting. + // A no-op makes publish-retry loops safe by construction. + if (newOwnerUid == group.ownerUid) return group; + final newMember = group.memberByUid(newOwnerUid); final charter = group.charter; final newEd = newMember?.edPubKeyB64; @@ -223,8 +232,20 @@ class GroupService { ts: nextCharterTimestamp( prevEntry, DateTime.now().millisecondsSinceEpoch), ); - return group - .copyWith(ownerUid: newOwnerUid, charter: [...charter, link]); + final extended = [...charter, link]; + // Never hand back a chain that can't validate: a poisoned charter is + // silent at mint time and permanent once published (e.g. the caller + // passed a group whose tip the [currentOwner] key no longer signs + // for). A throw here is retryable; a published bad link is not. + final check = validateCharter(sodium, extended, group.groupId); + if (!check.valid) { + throw StateError( + 'Refusing to mint an invalid charter link (${check.reason}): ' + 'the current owner/tip and the signer no longer agree — ' + 'reload the group state and retry.', + ); + } + return group.copyWith(ownerUid: newOwnerUid, charter: extended); } finally { signing.secretKey.dispose(); } diff --git a/test/group_service_test.dart b/test/group_service_test.dart index 1b8eb20..e1f2105 100644 --- a/test/group_service_test.dart +++ b/test/group_service_test.dart @@ -544,6 +544,83 @@ void main() { expect(r.height, 2); }); + test('a RETRY with the already-updated group is an idempotent no-op', () { + // The publish-retry trap: transfer A→B succeeds locally, the manifest + // publish fails, and the retry calls transfer again with the ALREADY + // updated group (owner=B, tip=B). Unguarded, that minted a B→B + // self-link or a link not signed by the previous owner — permanently + // invalid, which under a strict policy stops every member's manifest + // from decrypting. The retry must change nothing. + final owner = generateIdentity(sodium); + final next = generateIdentity(sodium); + var g = GroupService.createGroup( + sodium: sodium, + name: 'G', + identity: owner, + signingKeyDomain: signingDomain); + g = GroupService.addMember( + g, memberFor(next, edPubKeyB64: edKeyOf(next))); + final g2 = GroupService.transferOwnershipWithCharter( + sodium: sodium, + group: g, + currentOwner: owner, + newOwnerUid: next.uid, + signingKeyDomain: signingDomain, + ); + + // The retry, exactly as an app's publish-retry loop would issue it. + final retried = GroupService.transferOwnershipWithCharter( + sodium: sodium, + group: g2, + currentOwner: owner, // the retry still signs as the OLD owner + newOwnerUid: next.uid, + signingKeyDomain: signingDomain, + ); + expect(identical(retried, g2), isTrue, reason: 'no-op, not a new link'); + expect(retried.charter!.length, 2); + final r = validateCharter(sodium, retried.charter!, retried.groupId); + expect(r.valid, isTrue, reason: r.reason); + }); + + test('refuses to mint a link the validator would reject', () { + // A caller passes a group whose tip the signing identity no longer + // matches (stale local state after someone else's transfer): the + // built link would be signature-invalid. Minting it is silent and, + // once published, permanent — so the builder validates its own output + // and throws (retryable) instead. + final owner = generateIdentity(sodium); + final mid = generateIdentity(sodium); + final next = generateIdentity(sodium); + var g = GroupService.createGroup( + sodium: sodium, + name: 'G', + identity: owner, + signingKeyDomain: signingDomain); + g = GroupService.addMember(g, memberFor(mid, edPubKeyB64: edKeyOf(mid))); + g = GroupService.addMember( + g, memberFor(next, edPubKeyB64: edKeyOf(next))); + // Real transfer owner→mid: the tip now requires MID's signature. + final g2 = GroupService.transferOwnershipWithCharter( + sodium: sodium, + group: g, + currentOwner: owner, + newOwnerUid: mid.uid, + signingKeyDomain: signingDomain, + ); + // A further transfer signed by the ORIGINAL owner (stale state) must + // refuse rather than hand back a poisoned chain. + expect( + () => GroupService.transferOwnershipWithCharter( + sodium: sodium, + group: g2, + currentOwner: owner, // no longer the tip's signer + newOwnerUid: next.uid, + signingKeyDomain: signingDomain, + ), + throwsA(isA()), + ); + }); + test('refuses to silently downgrade when the ed key is unknown', () { final owner = generateIdentity(sodium); final next = generateIdentity(sodium); From 71ad63b97722cef08ac5d04a239a0f7b4165d431 Mon Sep 17 00:00:00 2001 From: nate Date: Thu, 20 Aug 2026 08:03:26 -0700 Subject: [PATCH 2/2] Review fixes: 0.5.0 housekeeping + uniform retry idempotence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG 0.5.0 entry, version bump, README test count (75 on this branch; the second transfer-safety/counter merge reconciles to 77). - Plain transferOwnership gains the same already-owner no-op as the charter variant — both transfer paths uniformly retry-safe, with an identical()-pinned test. --- CHANGELOG.md | 12 ++++++++++++ README.md | 2 +- lib/src/group_service.dart | 5 +++++ pubspec.yaml | 2 +- test/group_service_test.dart | 16 ++++++++++++++++ 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25942cb..6c68b38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## 0.5.0 + +- **Ownership transfers are retry-safe.** `transferOwnershipWithCharter` had + a publish-retry trap: a repeat call with the already-updated group minted a + self-link or a link not signed by the previous owner — permanently invalid + under `validateCharter`, which under a strict policy stopped every member's + manifest from decrypting. Both transfer variants are now idempotent no-ops + when the target already owns the group, and the charter builder validates + its own extended chain before returning (a retryable `StateError` instead + of a silently poisoned link). No wire/crypto change — the guards only + constrain what the builder emits. + ## 0.4.0 Adopter-portability release (the seams a consuming app needs to swap its own diff --git a/README.md b/README.md index a60e05f..6fcac98 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ flutter pub get flutter test ``` -Expect `All tests passed!` — 72 tests across three files: +Expect `All tests passed!` — 75 tests across three files: - **`group_model_test.dart`** (17) — `Group`/`GroupMember` JSON round-trips, the manifest-vs-local-storage field split, the unknown-field passthrough diff --git a/lib/src/group_service.dart b/lib/src/group_service.dart index 07ce62b..e906445 100644 --- a/lib/src/group_service.dart +++ b/lib/src/group_service.dart @@ -168,6 +168,11 @@ class GroupService { /// previously an `assert`, which is stripped in release builds and so let /// production set `ownerUid` to an arbitrary string. static Group transferOwnership(Group group, String newOwnerUid) { + // Same retry idempotence as the charter variant: a repeat with the + // already-updated group is a no-op, keeping both transfer paths + // uniformly safe in publish-retry loops. (The failure mode here is much + // milder — no chain to poison — but uniformity is free.) + if (newOwnerUid == group.ownerUid) return group; if (group.memberByUid(newOwnerUid) == null) { throw ArgumentError.value( newOwnerUid, 'newOwnerUid', 'New owner must be a member of the group'); diff --git a/pubspec.yaml b/pubspec.yaml index a8021ae..ec29033 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: groups description: "End-to-end-encrypted group membership, key rotation, manifests, and a signed ownership charter." -version: 0.4.0 +version: 0.5.0 publish_to: 'none' environment: diff --git a/test/group_service_test.dart b/test/group_service_test.dart index e1f2105..1b6146f 100644 --- a/test/group_service_test.dart +++ b/test/group_service_test.dart @@ -621,6 +621,22 @@ void main() { ); }); + test('plain transferOwnership is retry-idempotent too', () { + final owner = generateIdentity(sodium); + final next = generateIdentity(sodium); + var g = GroupService.createGroup( + sodium: sodium, + name: 'G', + identity: owner, + signingKeyDomain: signingDomain); + g = GroupService.addMember(g, memberFor(next)); + final g2 = GroupService.transferOwnership(g, next.uid); + // The retry: already the owner → the same group back, no throw even + // if roster state changed underneath (uniform with the charter path). + expect(identical(GroupService.transferOwnership(g2, next.uid), g2), + isTrue); + }); + test('refuses to silently downgrade when the ed key is unknown', () { final owner = generateIdentity(sodium); final next = generateIdentity(sodium);