Files
DodoSSH/src/DodoSSH.Client.Sync/VaultItemRepository.cs
T
jaap-jan bee6202949 Let a host be moved to another vault
The one thing the host editor's vault picker has always been unable to offer,
and the comment beside it said so: an existing host's vault was not a field
because the two vaults are encrypted under different keys. That is still true.
What changed is that it is no longer a reason to have nothing.

**A move is a copy and a tombstone, and it cannot be anything else.** A payload
is sealed under its vault's key and its AAD binds the vault, the entity id and
the item version, so no edit moves one and no server call could — the server
holds ciphertext it cannot read. What crosses is the plaintext, in this process,
between an unwrap under one key and a seal under another. VaultItemRepository
gained MoveAsync for it, so the three decisions below live in one place with
their reasons rather than being re-derived at each call site.

The item takes a new id. Keeping it would put one entity id in two vaults, and
the item table is keyed on the type and the id rather than on the vault — so the
destination's row and the source's tombstone would be the same row, and the move
would delete what it had just written.

The write comes first and the tombstone second, which decides what an
interruption leaves: a copy in both vaults, visible and deletable, rather than a
tombstone with nothing on the other side. Both are queued rather than sent, so
the window is a crash between two local writes; it is still worth being on the
survivable side of.

Two activity lines rather than one, because that is what the two vaults actually
record. A single "moved" line would have to be written to one of them and would
be missing from the other's history.

**The group and the tags stay behind, and that is the half that makes this
honest.** Both are items of the vault the host is leaving: the editor's group
picker offers one vault's groups and the chips are drawn from one vault's tags.
A host carrying either across would resolve it on the machine that moved it —
groups and tags are resolved over every readable vault — and dangle for everybody
else in the destination. The mover and their colleagues would be looking at two
different hosts. Cleared and reported beats carried and invisible.

The key or password binding is kept, and the difference is not inconsistency.
Those genuinely resolve across vaults — one key on twenty hosts in three vaults
is the arrangement they exist for — so clearing them would take a working host
and make one that cannot connect. What the message does instead is name a
binding that is now outside the destination, because that is precisely what the
other members of it will not be able to resolve.

**It is not in the editor**, on either head: the desktop puts it in the detail
pane's ⋯ menu above the separator Delete sits below, and the phone beside EDIT.
A picker inside the form would move a machine as a side effect of correcting a
port, which is the bug the editor's own vault picker was fenced off to prevent in
the first place. The panel takes the footer as the deletion question does, and
says what will be left behind before the tap rather than after it — on a phone,
where the status line afterwards is one line on a screen somebody has already
navigated away from, that is the only place it reliably gets read.

The phone hides the button where there is nowhere to go rather than offering one
that answers with a refusal; the desktop keeps its menu entry either way, because
a menu that grew and shrank would be a menu whose items move.

One thing found while writing the test and deliberately not changed. The pass
that follows every write on this screen reports what it moved and supersedes the
confirmation — for a save and a delete as much as for a move — so the move's own
sentence is what somebody sees offline. The test asserts it in that state and
says why. Making confirmations survive their own sync pass is a question about
the whole screen rather than about this.

Four places said an item could never be moved, two of them sentences on screen in
both heads. All four now say what is true, including the design gaps document,
where the chevron beside the vault name stays undrawn for a different reason: a
chevron on a subtitle implies an edit, and this is a re-seal, a new id and two
references left behind.
2026-08-04 16:04:15 +02:00

569 lines
22 KiB
C#

using DodoSSH.Client.Domain;
using DodoSSH.Client.Storage;
using DodoSSH.Contracts;
namespace DodoSSH.Client.Sync;
/// <summary>One vault item as the interface should show it.</summary>
/// <param name="EntityId">The item id.</param>
/// <param name="Secret">The decrypted item.</param>
/// <param name="Version">
/// The server version this is based on. Zero for an item that has never been accepted.
/// </param>
/// <param name="HasUnsyncedChanges">
/// Whether this reflects a local edit the server has not accepted yet. Worth showing: it is the
/// difference between "saved" and "saved here".
/// </param>
/// <param name="IsBlocked">
/// Whether the pending change was refused and is waiting on a person, so it will not retry on its own.
/// </param>
/// <param name="IsReadOnly">
/// Whether this item was written by a newer client and so must not be edited here, because re-encoding
/// it would drop fields this build cannot represent.
/// </param>
public sealed record VaultItem<TSecret>(
Guid EntityId,
TSecret Secret,
int Version,
bool HasUnsyncedChanges,
bool IsBlocked,
bool IsReadOnly)
where TSecret : class, IVaultSecret;
/// <summary>The items of one kind in a vault, and what could not be read.</summary>
/// <param name="Items">The readable items.</param>
/// <param name="Unreadable">
/// How many items would not decrypt. Surfaced rather than swallowed: a non-zero count here after a
/// rekey is the signal that new grants are needed.
/// </param>
public sealed record ItemListing<TSecret>(IReadOnlyList<VaultItem<TSecret>> Items, int Unreadable)
where TSecret : class, IVaultSecret;
/// <summary>
/// Reading and writing one kind of vault item, as the interface sees them.
/// </summary>
/// <remarks>
/// <para>
/// The view is the mirror of the server's state with the outbox laid over it, which is what makes the
/// application feel local: an edit appears immediately and a delete disappears immediately, whether or
/// not the network is there. Nothing here talks to the server; the sync engine reconciles later.
/// </para>
/// <para>
/// Writes never touch the mirror. That separation is load-bearing — the mirror is the common ancestor a
/// three-way merge needs, and a repository that updated it on save would destroy the very state that
/// lets a conflict be merged instead of arbitrated.
/// </para>
/// <para>
/// Every read and write is scoped to <see cref="IItemKind{TSecret}.EntityType"/>, which is also what
/// keeps two kinds apart in storage: the item table is keyed on the type as well as the id, so a host and
/// a key could share an id and never see each other's rows.
/// </para>
/// </remarks>
internal sealed class VaultItemRepository<TSecret>(
IItemKind<TSecret> kind,
ItemStore items,
OutboxStore outbox,
VaultKeyring keyring,
IActivityLogSink? activity = null)
where TSecret : class, IVaultSecret
{
/// <summary>
/// Whether writes through this repository are worth recording.
/// </summary>
/// <remarks>
/// Asked once rather than at each call site, and false for the log kinds themselves — which is the guard
/// that stops the activity log producing an entry for every entry it writes, without end. See
/// <see cref="IItemKind{TSecret}.IsAudited"/>.
/// </remarks>
private bool IsAudited => activity is not null && kind.IsAudited;
/// <summary>Reads every item of this kind the user should see in a vault.</summary>
internal async Task<ItemListing<TSecret>> ListAsync(
Guid vaultId,
CancellationToken cancellationToken)
{
// TryGet rather than CanRead, which answers false for a disposed keyring where this has to
// throw: a locked session being read from is a caller holding something it should have let go
// of, and the exception is what says so.
if (!keyring.TryGet(vaultId, out _, out _))
{
throw new VaultUnreadableException(vaultId);
}
var mirrored = await items
.ListAsync(vaultId, kind.EntityType, includeDeleted: true, cancellationToken)
.ConfigureAwait(false);
var pending = await outbox.ListAllAsync(vaultId, cancellationToken).ConfigureAwait(false);
var pendingByEntity = pending
.Where(operation => operation.EntityType == kind.EntityType)
.ToDictionary(operation => operation.EntityId);
var listed = new List<VaultItem<TSecret>>();
var unreadable = 0;
foreach (var item in mirrored)
{
if (pendingByEntity.Remove(item.EntityId, out var local))
{
AddPending(listed, ref unreadable, vaultId, local);
continue;
}
AddMirrored(listed, ref unreadable, vaultId, item);
}
// Whatever is left has no mirror row yet: items created here and not yet accepted.
foreach (var local in pendingByEntity.Values)
{
AddPending(listed, ref unreadable, vaultId, local);
}
return new ItemListing<TSecret>(listed, unreadable);
}
/// <summary>
/// Adds an item, returning the id it was given.
/// </summary>
/// <remarks>
/// The id is generated here, not by the server, which is what lets an item be created with no network
/// at all — the point of the whole outbox. UUIDv7 so that ids sort by creation time, which keeps
/// index locality reasonable on the server side.
/// </remarks>
internal async Task<Guid> CreateAsync(
Guid vaultId,
TSecret secret,
CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(secret);
Validate(secret);
var (vaultKey, generation) = Key(vaultId);
var entityId = Guid.CreateVersion7();
await outbox.QueueAsync(
new QueuedChange(
vaultId,
kind.EntityType,
entityId,
SyncOperation.Upsert,
ExpectedVersion: null,
kind.Seal(secret, vaultKey.Span, entityId, generation, itemVersion: 1),
kind.Fields(secret),
Ancestor: null),
cancellationToken).ConfigureAwait(false);
// After the queue, deliberately. A crash between the two loses one advisory line; the reverse order
// records an item that was never created.
if (IsAudited)
{
activity!.Record(
vaultId, kind.EntityType, entityId, secret.Label, ActivityOperation.Created, []);
}
return entityId;
}
/// <summary>
/// Replaces an item's contents.
/// </summary>
/// <remarks>
/// The base is taken from the pending operation when there is one, and from the mirror otherwise.
/// Reading it the other way round would seal the payload at a version that does not match the
/// <c>expectedVersion</c> the coalesced row keeps — and because the AAD binds the item version, the
/// result would encrypt cleanly and never decrypt again.
/// </remarks>
internal async Task UpdateAsync(
Guid vaultId,
Guid entityId,
TSecret secret,
CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(secret);
Validate(secret);
var (vaultKey, generation) = Key(vaultId);
var pending = await outbox
.FindAsync(vaultId, kind.EntityType, entityId, cancellationToken)
.ConfigureAwait(false);
var expectedVersion = pending is not null
? pending.ExpectedVersion
: await MirrorVersionAsync(vaultId, entityId, cancellationToken).ConfigureAwait(false);
var ancestor = pending?.Ancestor
?? await MirrorAncestorAsync(vaultId, entityId, cancellationToken).ConfigureAwait(false);
// Read before the queue overwrites it, and compared after. The version this decrypts at is the one
// the payload was sealed at, which is why the pending and mirror cases differ: a pending payload
// holds the version the server will assign, and a mirror row holds the one it has.
var before = IsAudited
? Open(vaultId, entityId, pending, ancestor)
: null;
await outbox.QueueAsync(
new QueuedChange(
vaultId,
kind.EntityType,
entityId,
SyncOperation.Upsert,
expectedVersion,
kind.Seal(
secret,
vaultKey.Span,
entityId,
generation,
SyncVersions.NextVersion(expectedVersion)),
kind.Fields(secret),
ancestor),
cancellationToken).ConfigureAwait(false);
if (IsAudited)
{
// An empty list when the previous version could not be read, which is why nothing may take empty
// to mean "nothing changed" — it also means "we could not tell".
activity!.Record(
vaultId,
kind.EntityType,
entityId,
secret.Label,
ActivityOperation.Updated,
before is null ? [] : kind.Changes(before, secret));
}
}
/// <summary>
/// Moves an item into another vault.
/// </summary>
/// <param name="fromVaultId">The vault it is in.</param>
/// <param name="toVaultId">The vault it should be in.</param>
/// <param name="entityId">The item.</param>
/// <param name="secret">
/// What to write into the destination. The caller's, rather than read from here, because moving is the
/// one operation where the item does not arrive unchanged: references to things that live in the vault
/// it is leaving are the mover's to resolve, and this layer has no way to know which those are.
/// </param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>The id the item has in its new vault.</returns>
/// <remarks>
/// <para>
/// <b>A copy and a tombstone, and it cannot be anything else.</b> An item's payload is sealed under
/// its vault's key and its AAD binds the vault, the entity id and the item version — so there is no
/// edit that moves one, and no server call that could: the server holds ciphertext it cannot read.
/// What crosses is the plaintext, in this process, between an unwrap under one key and a seal under
/// another.
/// </para>
/// <para>
/// <b>A new id, deliberately.</b> Keeping it would put one entity id in two vaults, and the item table
/// is keyed on the type and the id rather than on the vault — so the destination's row and the
/// source's tombstone would be the same row, and the move would delete what it had just written.
/// Callers holding the old id have to take the new one back.
/// </para>
/// <para>
/// <b>The write comes first and the tombstone second</b>, which decides what an interruption leaves
/// behind: a copy in both vaults, which is visible and can be deleted, rather than a tombstone with
/// nothing on the other side, which is the host gone. Both are queued rather than sent, so the window
/// is a crash between two local writes — narrow, and worth choosing the survivable side of anyway.
/// </para>
/// <para>
/// Two activity lines, not one: a create in the destination and a delete in the source, which is what
/// the vaults actually record. A single "moved" line would have to be written to one of them and would
/// be missing from the other's history.
/// </para>
/// </remarks>
internal async Task<Guid> MoveAsync(
Guid fromVaultId,
Guid toVaultId,
Guid entityId,
TSecret secret,
CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(secret);
if (fromVaultId == toVaultId)
{
throw new ArgumentException(
"That item is already in that vault.", nameof(toVaultId));
}
// Both keys before either write, so a destination this session cannot write to is refused with
// nothing having happened rather than after the source item has gone.
_ = Key(fromVaultId);
_ = Key(toVaultId);
var moved = await CreateAsync(toVaultId, secret, cancellationToken).ConfigureAwait(false);
await DeleteAsync(fromVaultId, entityId, cancellationToken).ConfigureAwait(false);
return moved;
}
/// <summary>
/// Deletes an item.
/// </summary>
/// <remarks>
/// <para>
/// Queued as a tombstone, never a local removal. An offline client that simply forgot the row would
/// be unable to tell the server anything, and the item would come back on the next pull.
/// </para>
/// <para>
/// Unless the server has never heard of the item, which is the one case where a tombstone is not only
/// unnecessary but wrong — see <see cref="NeverReachedTheServer" />.
/// </para>
/// </remarks>
internal async Task DeleteAsync(Guid vaultId, Guid entityId, CancellationToken cancellationToken)
{
var pending = await outbox
.FindAsync(vaultId, kind.EntityType, entityId, cancellationToken)
.ConfigureAwait(false);
// Read before either branch, because both of them destroy it — and a delete's line is the one that
// most needs a name, since the item it refers to is about to stop existing.
var label = IsAudited
? await LabelAsync(vaultId, entityId, pending, cancellationToken).ConfigureAwait(false)
: null;
if (pending is not null && NeverReachedTheServer(pending))
{
await outbox.CompleteAsync(pending.Sequence, cancellationToken).ConfigureAwait(false);
// Recorded even though nothing goes to the server. Somebody created an item and then removed it,
// which is two things they did — and a log that showed only the create would describe a keychain
// that does not exist.
Audit(vaultId, entityId, label);
return;
}
var expectedVersion = pending is not null
? pending.ExpectedVersion
: await MirrorVersionAsync(vaultId, entityId, cancellationToken).ConfigureAwait(false);
var ancestor = pending?.Ancestor
?? await MirrorAncestorAsync(vaultId, entityId, cancellationToken).ConfigureAwait(false);
await outbox.QueueAsync(
new QueuedChange(
vaultId,
kind.EntityType,
entityId,
SyncOperation.Delete,
expectedVersion,
Payload: null,
Fields: null,
ancestor),
cancellationToken).ConfigureAwait(false);
Audit(vaultId, entityId, label);
}
/// <summary>Records a delete, if this kind is audited at all.</summary>
private void Audit(Guid vaultId, Guid entityId, string? label)
{
if (IsAudited)
{
// The label may be null when the item could not be decrypted, which is a state worth recording
// rather than skipping: an item nobody can read is still one somebody deleted.
activity!.Record(
vaultId,
kind.EntityType,
entityId,
label ?? "(an item that could not be read)",
ActivityOperation.Deleted,
[]);
}
}
/// <summary>What an item is currently called, for a log line written as it goes away.</summary>
private async Task<string?> LabelAsync(
Guid vaultId,
Guid entityId,
PendingOperation? pending,
CancellationToken cancellationToken)
{
var ancestor = await MirrorAncestorAsync(vaultId, entityId, cancellationToken)
.ConfigureAwait(false);
return Open(vaultId, entityId, pending, ancestor)?.Label;
}
/// <summary>
/// Decrypts whichever version of an item this machine currently shows.
/// </summary>
/// <remarks>
/// The pending payload first, because that is what the user is looking at — an item edited offline twice
/// should report the second edit against the first, not against what the server last accepted. The
/// version each is opened at differs for the reason the sealing side differs: a queued payload is sealed
/// at the version the server <em>will</em> assign, and a mirror row holds the one it has.
/// </remarks>
private TSecret? Open(
Guid vaultId,
Guid entityId,
PendingOperation? pending,
StoredAncestor? ancestor)
{
if (pending is { Operation: SyncOperation.Upsert, Payload: { } queued })
{
return keyring.TryGetAt(vaultId, queued.KeyGeneration, out var queuedKey)
? kind.TryOpen(
queued,
queuedKey.Span,
entityId,
SyncVersions.NextVersion(pending.ExpectedVersion))?.Secret
: null;
}
if (ancestor is null
|| !keyring.TryGetAt(vaultId, ancestor.Payload.KeyGeneration, out var vaultKey))
{
return null;
}
return kind.TryOpen(ancestor.Payload, vaultKey.Span, entityId, ancestor.Version)?.Secret;
}
/// <summary>
/// Whether a queued change describes an item the server cannot be holding.
/// </summary>
/// <remarks>
/// <para>
/// A null <c>ExpectedVersion</c> means the row is a create — including a create that has since been
/// edited, because coalescing keeps the original expected version. So there is no server row and no
/// mirror row, and dropping the queued change makes the item genuinely gone. Queueing a tombstone
/// instead asks the server to delete something it has never seen, which it answers <c>Invalid</c>; the
/// change is parked, and the user is left with a rejected item they already deleted and a pending count
/// that never reaches zero. Add a host on a laptop with no network, change your mind, and that is the
/// state — it applies to all four item types.
/// </para>
/// <para>
/// The attempt count is what makes this safe rather than merely convenient. Nothing sent cannot have
/// landed. A parked row cannot have landed either — parking is what the pusher does when the server has
/// refused, so the refusal is the evidence. What is left is a create that went out and whose answer was
/// never seen: in flight, or failed in a way that might yet have been applied. That one still gets a
/// tombstone, because the server may be holding the item and a local drop would strand it there for
/// ever. A refused tombstone is recoverable; an orphan on the server is not.
/// </para>
/// </remarks>
private static bool NeverReachedTheServer(PendingOperation pending) =>
pending is { Operation: SyncOperation.Upsert, ExpectedVersion: null }
&& (pending.Attempts == 0 || pending.IsParked);
private static void Validate(TSecret secret)
{
if (!secret.TryValidate(out var error))
{
throw new ArgumentException(error, nameof(secret));
}
}
/// <summary>Adds one row of the server's mirror to a listing, or counts it as unreadable.</summary>
private void AddMirrored(
List<VaultItem<TSecret>> listed,
ref int unreadable,
Guid vaultId,
StoredItem item)
{
if (item.IsDeleted || item.Payload is null)
{
return;
}
// The generation the item names, not the vault's current one. A rotated vault holds items
// written under two or three keys at once, and a list that assumed the newest would report
// everything older as unreadable.
if (!keyring.TryGetAt(vaultId, item.Payload.KeyGeneration, out var vaultKey))
{
unreadable++;
return;
}
var opened = kind.TryOpen(item.Payload, vaultKey.Span, item.EntityId, item.Version);
if (opened is null)
{
unreadable++;
return;
}
listed.Add(new VaultItem<TSecret>(
item.EntityId, opened.Secret, item.Version, false, false, opened.IsReadOnly));
}
private void AddPending(
List<VaultItem<TSecret>> listed,
ref int unreadable,
Guid vaultId,
PendingOperation local)
{
if (local.Operation == SyncOperation.Delete)
{
// Gone as far as this machine is concerned, even before the server agrees.
return;
}
if (local.Payload is null)
{
unreadable++;
return;
}
// A queued change is sealed under whatever generation was current when it was queued, which is
// not necessarily the current one: a rotation can land between an offline edit and its push.
if (!keyring.TryGetAt(vaultId, local.Payload.KeyGeneration, out var vaultKey))
{
unreadable++;
return;
}
var version = SyncVersions.NextVersion(local.ExpectedVersion);
var opened = kind.TryOpen(local.Payload, vaultKey.Span, local.EntityId, version);
if (opened is null)
{
unreadable++;
return;
}
listed.Add(new VaultItem<TSecret>(
local.EntityId,
opened.Secret,
local.ExpectedVersion ?? 0,
HasUnsyncedChanges: true,
local.IsParked,
opened.IsReadOnly));
}
private (ReadOnlyMemory<byte> VaultKey, uint Generation) Key(Guid vaultId) =>
keyring.TryGet(vaultId, out var vaultKey, out var generation)
? (vaultKey, generation)
: throw new VaultUnreadableException(vaultId);
private async Task<int?> MirrorVersionAsync(
Guid vaultId,
Guid entityId,
CancellationToken cancellationToken)
{
var item = await items
.FindAsync(vaultId, kind.EntityType, entityId, cancellationToken)
.ConfigureAwait(false);
// A null means the server has never seen this item, which is exactly what "create" is.
return item?.Version;
}
private async Task<StoredAncestor?> MirrorAncestorAsync(
Guid vaultId,
Guid entityId,
CancellationToken cancellationToken)
{
var item = await items
.FindAsync(vaultId, kind.EntityType, entityId, cancellationToken)
.ConfigureAwait(false);
return item?.Payload is null
? null
: new StoredAncestor(item.Version, item.Payload, item.Fields);
}
}