Files
DodoSSH/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs
T
jaap-jan 70b3290a77 Bind an SSH key to a host instead of picking one per connection
A host now names the key it authenticates with, or none, as a field in its
encrypted payload — so the choice follows the host to every machine rather than
being made again each time somebody connects. The per-connection "Use key"
switch it replaces was a stopgap for not having this, and keeping both would
have left two mechanisms answering one question.

This is the first payload schema version bump, and it does not work the obvious
way. A host is written at the *lowest* schema version that can represent it: one
that binds a key is written at 2, one that does not is still written at 1, byte
for byte as it was before the field existed. The version is what makes an older
client refuse to edit an item, so stamping 2 unconditionally would mean
upgrading a single machine and renaming a single host made that host uneditable
on every machine that had not upgraded yet. Confining the cost to the hosts that
actually use the field is the difference between a team noticing a bump and a
team being blocked by one. HostSecretCodec states the rule so the next field
added follows it, and a test pins the version-1 bytes against a literal rather
than against the codec, because the claim is about history: every host already in
every vault has to re-encode to what it encoded before, or the first sync after
an upgrade would push the whole vault as changed.

A binding is an item id, not a copy of the key — a second copy of a private key
is one that goes stale — which means the reference can dangle when the key is
deleted on another machine. Both places that meets are handled the same way, by
refusing rather than falling back:

- Connecting to a host whose key is gone is refused outright. A host somebody
  deliberately set up for key-only access must not quietly start offering a
  password.
- Opening such a host in the editor keeps the binding, selected, labelled as
  missing. The quieter version of the same failure is someone editing the port
  and saving, silently converting the host to password authentication with
  nothing ever having said so.

Two things this found by being falsified:

- The merge was untested for the new field, and "just take the server's value"
  passed the entire suite — a local binding change would have been discarded with
  no conflict recorded. HostSecretMergeTests already had a test written for
  exactly this class of omission; it simply had not been extended.

- Adding a nullable field exposed a defect in HostSecretMerge.Field: it
  short-circuited when the discarded value was null, so the formatter never ran
  for the one case where null is a value rather than an absence, and a field
  whose absence has a name could not report it. Now the formatter always runs,
  and "no key" appears in the conflict log where an empty string used to.

Also fixes eight nullable warnings in SyncEndpointTests left by the server-side
SSH key commit, which had omitted the null-forgiving operator the rest of that
file uses. They were invisible until an unrelated change forced the project to
recompile.

The end-to-end slice now binds its host to its key, so a schema-version-2
payload goes through the real API, the real PostgreSQL and back out on a second
machine.

745 tests green. Zero warnings, dotnet format clean.
2026-07-29 20:42:51 +02:00

270 lines
10 KiB
C#

using static DodoSSH.Client.Domain.Tests.HostFactory;
namespace DodoSSH.Client.Domain.Tests;
/// <summary>
/// Merging a host field by field.
/// </summary>
/// <remarks>
/// The primitives are covered by <see cref="ThreeWayMergeTests"/>; this is about the wiring — that
/// every field is actually routed through a merge, that the collections use the right strategy, and
/// that a conflict names the field precisely enough for a user to act on it.
/// </remarks>
public sealed class HostSecretMergeTests
{
[Fact]
public void NeitherSideChanged_ProducesTheSameHostAndNoConflicts()
{
var host = Host();
var result = HostSecretMerge.Merge(host, host, host);
result.Merged.ShouldBe(host);
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void EachSideChangedADifferentField_BothSurvive()
{
// The reason a field-level merge is worth writing at all.
var ancestor = Host();
var local = ancestor with { Notes = "rotate quarterly" };
var remote = ancestor with { Username = "postgres" };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.Notes.ShouldBe("rotate quarterly");
result.Merged.Username.ShouldBe("postgres");
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void EveryScalarField_IsRoutedThroughAMerge()
{
// A field added to HostSecret but forgotten in the merge would silently revert to the remote
// value forever. Changing each one only locally proves each is actually consulted.
var ancestor = Host();
var local = ancestor with
{
Label = "prod-db-1",
Hostname = "db1.internal",
Port = 2222,
Username = "admin",
Notes = "primary",
JumpHostIds = JumpChain.Create([Bastion]),
Options = HostOptions.Create([new HostOption("Compression", "yes")]),
RelayEnabled = true,
SshKeyId = DeployKey,
};
var result = HostSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.ShouldBe(local);
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void ARemovedKeyBinding_IsNotResurrectedByTheOtherSide()
{
// The other direction, and the one a two-way diff gets wrong: null is a value here, not an absence.
// A host deliberately put back on a password must not silently regain its key because the server's
// copy still names one.
var ancestor = Host(sshKeyId: DeployKey);
var local = ancestor with { SshKeyId = null };
var result = HostSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.SshKeyId.ShouldBeNull();
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void TwoSidesBindingDifferentKeys_NamesBothIdsInTheConflict()
{
// An id is not a secret — it names a vault item rather than being the key — so both are shown. The
// user cannot tell which of two keys was dropped otherwise.
var other = Guid.Parse("0192f0c8-4444-7c3d-8e4f-5a6b7c8d9e04");
var ancestor = Host();
var local = ancestor with { SshKeyId = DeployKey };
var remote = ancestor with { SshKeyId = other };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.SshKeyId.ShouldBe(other);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.SshKeyId));
conflict.Kept.ShouldBe(other.ToString());
conflict.Discarded.ShouldBe(DeployKey.ToString());
}
[Fact]
public void ABindingClashingWithItsRemoval_SaysWhichSideHadNoKey()
{
// "no key" rather than a blank, for the same reason a clashing port is reported as a number: a
// conflict entry whose discarded value is empty reads as a bug in the conflict log.
var ancestor = Host(sshKeyId: DeployKey);
var local = ancestor with { SshKeyId = null };
var remote = ancestor with { SshKeyId = Relay };
var result = HostSecretMerge.Merge(ancestor, local, remote);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.SshKeyId));
conflict.Kept.ShouldBe(Relay.ToString());
conflict.Discarded.ShouldBe("no key");
}
[Fact]
public void AClashingScalar_TakesRemoteAndNamesTheFieldItDiscarded()
{
var ancestor = Host();
var local = ancestor with { Hostname = "db-mine.internal" };
var remote = ancestor with { Hostname = "db-theirs.internal" };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.Hostname.ShouldBe("db-theirs.internal");
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.Hostname));
conflict.Kept.ShouldBe("db-theirs.internal");
conflict.Discarded.ShouldBe("db-mine.internal");
conflict.DiscardedSide.ShouldBe(MergeSide.Local);
}
[Fact]
public void AClashingPort_IsReportedAsANumberNotAsBlank()
{
// Rendering the losing value is the entire point of the conflict record; a non-string field
// that formatted to nothing would leave the user unable to restore it.
var ancestor = Host(port: 22);
var result = HostSecretMerge.Merge(ancestor, ancestor with { Port = 2222 }, ancestor with { Port = 2200 });
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.Port));
conflict.Kept.ShouldBe("2200");
conflict.Discarded.ShouldBe("2222");
}
[Fact]
public void AJumpChain_MergesAsAWholeRouteRatherThanAsASet()
{
// Deliberate, and the opposite of how the directives merge. Unioning two chains would
// produce a route neither user configured and would silently change which machine is
// reached through which — so this conflicts instead, and reports the discarded route.
var ancestor = Host();
var local = ancestor with { JumpHostIds = JumpChain.Create([Bastion]) };
var remote = ancestor with { JumpHostIds = JumpChain.Create([Relay]) };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.JumpHostIds.Equals(JumpChain.Create([Relay])).ShouldBeTrue();
result.Merged.JumpHostIds.Count.ShouldBe(1);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.JumpHostIds));
conflict.Discarded.ShouldNotBeNull();
conflict.Discarded.ShouldContain(Bastion.ToString());
}
[Fact]
public void AReorderedJumpChain_IsAChange()
{
var ancestor = Host(jumps: [Bastion, Relay]);
var local = ancestor with { JumpHostIds = JumpChain.Create([Relay, Bastion]) };
var result = HostSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.JumpHostIds.Equals(JumpChain.Create([Relay, Bastion])).ShouldBeTrue();
}
[Fact]
public void Directives_MergePerNameSoBothAdditionsSurvive()
{
var ancestor = Host();
var local = ancestor with { Options = HostOptions.Create([new HostOption("Compression", "yes")]) };
var remote = ancestor with
{
Options = HostOptions.Create([new HostOption("ServerAliveInterval", "30")]),
};
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.Options.Count.ShouldBe(2);
result.Merged.Options.TryGetValue("Compression", out var compression).ShouldBeTrue();
compression.ShouldBe("yes");
result.Merged.Options.TryGetValue("ServerAliveInterval", out var keepAlive).ShouldBeTrue();
keepAlive.ShouldBe("30");
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void AClashingDirective_NamesTheDirectiveNotJustTheField()
{
// "Options changed" would be useless. The user needs to know which one.
var ancestor = Host(options: [("Compression", "yes")]);
var local = ancestor with { Options = HostOptions.Create([new HostOption("Compression", "no")]) };
var remote = ancestor with
{
Options = HostOptions.Create([new HostOption("Compression", "delayed")]),
};
var result = HostSecretMerge.Merge(ancestor, local, remote);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe("Options[Compression]");
conflict.Kept.ShouldBe("delayed");
conflict.Discarded.ShouldBe("no");
}
[Fact]
public void ARemovedDirectiveTheOtherSideEdited_KeepsTheValue()
{
var ancestor = Host(options: [("Compression", "yes")]);
var local = ancestor with { Options = HostOptions.Empty };
var remote = ancestor with { Options = HostOptions.Create([new HostOption("Compression", "no")]) };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.Options.TryGetValue("Compression", out var value).ShouldBeTrue();
value.ShouldBe("no");
result.Conflicts.ShouldHaveSingleItem().DiscardedWasRemoval.ShouldBeTrue();
}
[Fact]
public void TheMergedHost_IsAlwaysValidWhenBothInputsWere()
{
// A merge that produced an unstorable host would strand the item: it could never be pushed
// and the conflict could never clear.
var ancestor = Host();
var local = ancestor with { Label = "mine", Port = 2222 };
var remote = ancestor with { Label = "theirs", Hostname = "other.internal" };
var result = HostSecretMerge.Merge(ancestor, local, remote);
result.Merged.TryValidate(out var error).ShouldBeTrue(error);
}
[Fact]
public void ResolvingAConflictConverges()
{
// Two clients, both merging, must reach the same host and then stop. Re-merging the result
// against the remote produces no further conflict — which is what stops an endless
// push-conflict-merge-push loop between two machines.
var ancestor = Host();
var local = ancestor with { Notes = "mine", Username = "a" };
var remote = ancestor with { Notes = "theirs", Hostname = "other.internal" };
var first = HostSecretMerge.Merge(ancestor, local, remote);
first.HasConflicts.ShouldBeTrue();
var second = HostSecretMerge.Merge(remote, first.Merged, remote);
second.HasConflicts.ShouldBeFalse();
second.Merged.ShouldBe(first.Merged);
}
}