Merge branch 'claude/team-key-share-rotate-4b2619'

Two conflicts, and the second is worth recording. main's M4 bullet gained the
Android signing decision while this branch rewrote the M5 line either side of it;
both are kept.

The other is an ADR number collision: two sessions each took 0010, one for vault
key rotation and one for Android distribution, and both are now on main. ADR
numbers are identifiers — "see ADR 0010" appears in code comments as well as in
prose — so leaving two would make every such reference ambiguous. The rotation
ADR landed first and is referenced from crypto.md, the gaps document, ADR 0009
and the sync code; the Android one is referenced from README and android-port.md.
So the later and cheaper one moves: 0010-android-distribution.md is now ADR 0011,
with its title and both references updated. Nothing about either decision changes.

tests/DodoSSH.Client.App.Tests/TeamSharingTests.cs

@@ -169,6 +169,11 @@ public sealed class TeamSharingTests : IAsyncLifetime
         teams.Status.ShouldContain("Rotated", customMessage: teams.Status);
         teams.Status.ShouldContain("Platform secrets");
 
+        // The last act of a rotation is moving what is already stored onto the new key. Proven by the
+        // bytes in DodoSSH.Client.Sync.Tests; what this asserts is that the shell asks for it at all,
+        // and says which of the two guarantees the user has ended up with.
+        teams.Status.ShouldContain("re-sealed under the new key", customMessage: teams.Status);
+
         // Gone entirely, at every generation. A revocation that left the history behind would leave them
         // able to read everything written before they went, from a copy of the ciphertext.
         server.GenerationsGranted(vaultId, leaving).ShouldBeEmpty();

tests/DodoSSH.Client.Sync.Tests/SyncHarness.cs

@@ -1,3 +1,4 @@
+using System.Security.Cryptography;
 using DodoSSH.Client.Domain;
 using DodoSSH.Client.Storage;
 using DodoSSH.Crypto;
@@ -16,6 +17,9 @@ internal sealed class SyncDevice : IDisposable
     private readonly ClientCacheFactory factory;
     private readonly LocalCacheProtector protector;
 
+    private readonly FakeVaultServer server;
+    private readonly SyncOptions options;
+
     private SyncDevice(
         string name,
         ClientCacheFactory factory,
@@ -27,6 +31,8 @@ internal sealed class SyncDevice : IDisposable
         Name = name;
         this.factory = factory;
         this.protector = protector;
+        this.server = server;
+        this.options = options;
         Keyring = keyring;
 
         Items = new ItemStore(factory, protector);
@@ -96,6 +102,15 @@ internal sealed class SyncDevice : IDisposable
     internal Task<SyncReport> SyncAsync() =>
         Engine.SyncAsync(SyncHarness.VaultId, TestContext.Current.CancellationToken);
 
+    /// <summary>Moves everything this machine can see onto the vault's current key.</summary>
+    /// <remarks>
+    /// Built per call rather than held, as the engine is: it carries no state between passes, and one
+    /// per call is what the session does.
+    /// </remarks>
+    internal Task<ResealReport> ResealAsync() =>
+        new VaultResealer(server, Items, Outbox, Keyring, TimeProvider.System, options)
+            .ResealAsync(SyncHarness.VaultId, TestContext.Current.CancellationToken);
+
     internal Task<ItemListing<HostSecret>> ListAsync() =>
         Hosts.ListAsync(SyncHarness.VaultId, TestContext.Current.CancellationToken);
 
@@ -274,6 +289,41 @@ internal sealed class SyncHarness : IDisposable
         }
     }
 
+    /// <summary>
+    /// Rotates the vault: a new key, taken by both machines, and a server that says so.
+    /// </summary>
+    /// <returns>The key the vault has just moved off, so a test can prove it no longer opens anything.</returns>
+    /// <remarks>
+    /// <para>
+    /// Stands in for the server call the real rotation makes. What matters here is the state it leaves —
+    /// a vault whose current generation is one past everything stored in it — and the grant round trip
+    /// that produces that state is <c>DodoSSH.Api.Tests</c>'s subject, not this suite's.
+    /// </para>
+    /// <para>
+    /// Each keyring gets its own copy of the bytes, because a keyring owns what it is handed and zeroes
+    /// it on disposal; sharing one array would leave the second machine holding a zeroed key at the end
+    /// of a test and produce failures that look like a decryption bug.
+    /// </para>
+    /// </remarks>
+    internal byte[] Rotate()
+    {
+        var generation = Server.KeyGeneration + 1;
+
+        First.Keyring.TryGetAt(VaultId, Server.KeyGeneration, out var previous).ShouldBeTrue();
+
+        var superseded = previous.ToArray();
+        var key = VaultKeys.Create();
+
+        First.Keyring.Adopt(VaultId, [.. key], generation);
+        Second.Keyring.Adopt(VaultId, [.. key], generation);
+
+        CryptographicOperations.ZeroMemory(key);
+
+        Server.KeyGeneration = generation;
+
+        return superseded;
+    }
+
     /// <summary>Brings both devices up to date, twice, so the result is a settled state.</summary>
     /// <remarks>
     /// Twice because one pass per device is not enough for a change made on one to be merged on the

tests/DodoSSH.Client.Sync.Tests/VaultResealTests.cs

@@ -0,0 +1,224 @@
+namespace DodoSSH.Client.Sync.Tests;
+
+/// <summary>
+/// Moving a rotated vault's stored items onto its current key.
+/// </summary>
+/// <remarks>
+/// <para>
+/// A rotation re-keys the vault and not its contents, which is what makes it cheap and safe (ADR 0010)
+/// and what leaves this pass to be run. The claim it has to earn is narrow and testable: after it, the
+/// key the vault has moved off opens nothing. Every test here that says "resealed" also checks that,
+/// because a pass that re-wrapped everything under the same key would report exactly the same numbers.
+/// </para>
+/// <para>
+/// The other half is the push path. A change queued before a rotation is sealed under the old key, and
+/// sending it as it stands would put a brand-new item into the vault under the key the person who was
+/// just removed still holds — the one hole a pass over stored items cannot see.
+/// </para>
+/// </remarks>
+public sealed class VaultResealTests
+{
+    [Fact]
+    public async Task ARotatedVault_MovesItsStoredItemsOntoTheNewKey()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        var web = await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+        var db = await harness.First.CreateAsync(SyncHarness.Host("db-01"));
+
+        await harness.SettleAsync();
+
+        var superseded = harness.Rotate();
+
+        var report = await harness.First.ResealAsync();
+
+        report.Resealed.ShouldBe(2);
+        report.Complete.ShouldBeTrue();
+        report.KeyGeneration.ShouldBe(2u);
+
+        foreach (var entityId in (Guid[])[web, db])
+        {
+            var row = harness.Server.Find(entityId).ShouldNotBeNull();
+
+            row.Payload.KeyGeneration.ShouldBe(2u);
+
+            // The point of the whole pass: the key somebody left with opens nothing here any more.
+            HostCipher.TryOpen(row.Payload, superseded, entityId, row.Version).ShouldBeNull();
+        }
+
+        // And the vault still reads as itself — the plaintext was carried across, not re-encoded.
+        var hosts = await harness.First.HostsSortedAsync();
+
+        hosts.Select(host => host.Label).ShouldBe(["db-01", "web-01"]);
+    }
+
+    /// <remarks>
+    /// The pass is run after every rotation and can be run again at any time, so "nothing left to do"
+    /// has to be cheap and silent rather than a second round of writes. A pass that re-sealed on every
+    /// call would churn the vault's version numbers and hand every other client a pull per item.
+    /// </remarks>
+    [Fact]
+    public async Task ASecondPass_FindsNothingLeftToDo()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+        await harness.SettleAsync();
+
+        harness.Rotate();
+
+        (await harness.First.ResealAsync()).Resealed.ShouldBe(1);
+
+        var again = await harness.First.ResealAsync();
+
+        again.Resealed.ShouldBe(0);
+        again.Complete.ShouldBeTrue();
+        harness.Server.PushCount.ShouldBe(2, "an empty pass must not send a batch at all");
+    }
+
+    /// <remarks>
+    /// An item with an edit waiting to go is left alone by the pass and re-sealed by the push instead.
+    /// Doing it here as well would overwrite the user's queued work with the version the server holds,
+    /// which is the one thing a re-keying pass must never do.
+    /// </remarks>
+    [Fact]
+    public async Task AQueuedEdit_IsLeftToThePushPathAndStillLandsUnderTheNewKey()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        var entityId = await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+
+        await harness.SettleAsync();
+
+        // Queued while the old key was current, and not yet pushed.
+        await harness.First.UpdateAsync(entityId, SyncHarness.Host("web-01", notes: "moved rack"));
+
+        var superseded = harness.Rotate();
+
+        var report = await harness.First.ResealAsync();
+
+        report.Deferred.ShouldBe(1);
+        report.Resealed.ShouldBe(0);
+        report.Complete.ShouldBeTrue("a queued change is not something this pass has left undone");
+
+        await harness.First.SyncAsync();
+
+        var row = harness.Server.Find(entityId).ShouldNotBeNull();
+
+        row.Payload.KeyGeneration.ShouldBe(2u);
+        HostCipher.TryOpen(row.Payload, superseded, entityId, row.Version).ShouldBeNull();
+
+        // The edit itself survived the re-sealing, which is the half that would be easy to lose.
+        var seen = await harness.Second.SyncAsync();
+
+        seen.Pulled.ShouldBeGreaterThan(0);
+        (await harness.Second.FindAsync(entityId)).Secret.Notes.ShouldBe("moved rack");
+    }
+
+    /// <remarks>
+    /// Somebody else writing an item mid-pass is not a failure and not a merge — there is nothing to
+    /// merge, since this pass changes no content. It is counted, left where it is, and picked up by the
+    /// next pass against the version they left behind. That is the whole of the resumability claim.
+    /// </remarks>
+    [Fact]
+    public async Task AnItemWrittenElsewhereMeanwhile_IsCountedAndPickedUpNextTime()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        var entityId = await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+
+        await harness.SettleAsync();
+
+        var superseded = harness.Rotate();
+
+        // A third machine that has not heard about the rotation yet: it writes version 2 under the key
+        // it still believes is current. That is the item this pass has to find and move, and sealing it
+        // by hand is the only way to produce one — every client in this harness now holds the new key.
+        var held = harness.Server.Find(entityId).ShouldNotBeNull();
+
+        harness.Server.ExternalUpsert(
+            entityId,
+            HostCipher.Seal(
+                SyncHarness.Host("web-01", notes: "renamed elsewhere"),
+                superseded,
+                entityId,
+                keyGeneration: 1,
+                itemVersion: held.Version + 1),
+            held.Fields);
+
+        var contested = await harness.First.ResealAsync();
+
+        contested.Contested.ShouldBe(1);
+        contested.Resealed.ShouldBe(0);
+        contested.Complete.ShouldBeFalse();
+
+        // Read what they wrote, then run the pass again: nothing to recover, nothing to decide.
+        await harness.First.SyncAsync();
+
+        var second = await harness.First.ResealAsync();
+
+        second.Resealed.ShouldBe(1);
+        second.Complete.ShouldBeTrue();
+
+        var row = harness.Server.Find(entityId).ShouldNotBeNull();
+
+        row.Payload.KeyGeneration.ShouldBe(2u);
+        HostCipher.TryOpen(row.Payload, superseded, entityId, row.Version).ShouldBeNull();
+    }
+
+    /// <remarks>
+    /// Every synced type, not the one the tests happen to use most. The pass is written over the item
+    /// store rather than over the repositories precisely so that a type added later is covered without
+    /// anybody remembering to add it — and this is the test that would notice if it stopped being true.
+    /// </remarks>
+    [Fact]
+    public async Task EveryKindOfItem_MovesOntoTheNewKey()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+        await harness.First.CreateKeyAsync(SyncHarness.Key("deploy"));
+        await harness.First.CreateCredentialAsync(SyncHarness.Credential("registry"));
+        await harness.First.CreateKnownHostAsync(SyncHarness.KnownHost("db.internal"));
+
+        await harness.SettleAsync();
+
+        harness.Rotate();
+
+        var report = await harness.First.ResealAsync();
+
+        report.Resealed.ShouldBe(4);
+        report.Complete.ShouldBeTrue();
+
+        // Read back through the repositories, so this asserts the items are usable and not merely
+        // rewritten: a pass that produced ciphertext nobody could open would pass every count above.
+        (await harness.First.ListAsync()).Unreadable.ShouldBe(0);
+        (await harness.First.ListKeysAsync()).Unreadable.ShouldBe(0);
+        (await harness.First.ListCredentialsAsync()).Unreadable.ShouldBe(0);
+        (await harness.First.ListKnownHostsAsync()).Unreadable.ShouldBe(0);
+    }
+
+    /// <remarks>
+    /// A member who has been rotated past and not yet re-wrapped holds the history and no current key.
+    /// They must not attempt this: there is nothing to seal under, and the honest answer is a report of
+    /// zero rather than an exception on a background pass nobody asked for.
+    /// </remarks>
+    [Fact]
+    public async Task AMachineWithNoCurrentKey_DoesNothingRatherThanFailing()
+    {
+        using var harness = await SyncHarness.CreateAsync();
+
+        await harness.First.CreateAsync(SyncHarness.Host("web-01"));
+        await harness.SettleAsync();
+
+        // What RefreshVaultsAsync does when the server reports a generation this machine has no grant
+        // for: the vault is marked unreadable and the write target goes with it.
+        harness.First.Keyring.MarkUnreadable(SyncHarness.VaultId);
+
+        var report = await harness.First.ResealAsync();
+
+        report.KeyGeneration.ShouldBe(0u);
+        report.Resealed.ShouldBe(0);
+        harness.Server.PushCount.ShouldBe(1, "nothing was sent");
+    }
+}