Charter transfers are retry-safe: idempotent no-op + self-validated links

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

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

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');
@@ -198,6 +203,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 +237,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();
       }

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:

test/group_service_test.dart

@@ -544,6 +544,99 @@ 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<StateError>()),
+      );
+    });
+
+    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);