DodoTech-Public/DodoSSH
Public Access
2
0
Fork 0
Code Issues Pull Requests 2 Actions Packages Projects Releases 2 Wiki Activity

Build the three things the phone's + needs, before the + exists

Browse Source 
Steps 1 to 3 of docs/adding-hosts-on-the-phone.md: the domain half. Nothing
on either head has changed, which is deliberate — the plan orders these first
because everything the editors will bind to has to exist and be merge-safe
before a screen can offer it.

HostGroupSecret gains a parent and four defaults, and the codec gains the
version rule it never had. It stamped CurrentSchemaVersion unconditionally,
which was harmless with one field and one version and stops being harmless
here: upgrading one machine and renaming any group would have made that group
uneditable on every machine still on the old build. It now emits the lowest
version that loses nothing, so a flat group with no defaults still encodes at
version 1, byte for byte, pinned against a literal.

Tags become a real item over the reserved slot. Secret, codec, merge, cipher,
repository, both registries, the EF entity and a generated AddTagItem
migration. TagCipher names AadResourceType.Tag as a constant rather than
casting the wire type, because Tag is 5 on the wire and 8 in the crypto enum
and 5 there is Credential — a cast would seal every tag under the resource
type for a password, encrypt and decrypt perfectly on the machine that wrote
it, and only fail when another implementation refused the item, by which time
the AAD is frozen into stored ciphertext. HostTag stays reserved and unused:
the one thing the join buys over a set on the host is bought instead by
merging TagIds per id.

HostSecret grows TagIds and Port goes nullable, which is the change with the
widest blast radius and the only one that loses an item rather than locking
one. A host with no port of its own omits the property, an older build reads
int Port as 0, and TryValidate refuses it — unreadable rather than read-only.
That cost is confined to hosts which actually inherit, because the version is
a maximum over the fields present; the alternative, writing 22 into every
host, is the lie inheritance exists to stop telling.

One decision the plan did not specify. "Three states where there were two" is
four — key, credential, typed password, or the group's answer — and two
nullable ids carry three. Naming neither id now means inherit, so
AsksForPassword says "a typed password even under a group that lends a key"
out loud. Only true is ever written and a decoded false folds back to null, so
a host that never touched it encodes as it always did. Nothing already stored
changed meaning: no group could lend a binding before this build, so every
existing host resolves exactly as it did.

HostInheritance is the resolver, and its visited set is load-bearing rather
than defensive. Two clients can each re-parent A under B and B under A while
offline; the merge sees one item against one item and the server sees
ciphertext, so nothing upstream can refuse the pair. With inheritance the
chain is walked at connect time, so an unguarded cycle is not an undrawable
sidebar — it is a shell that never opens. Stopping at the first repeat
degrades it to a group that reads as a root, and clearing the parent is the
repair.

A tag set turns out to be the one field on a host that can never ask the user
anything. TagSet.ToIdMap keys by the value, so no key can hold two values, so
the both-sides-moved-differently branch of the keyed merge is unreachable —
asserted over the whole eight-row matrix. The conflict loop is kept anyway,
because that proof is one edit from ceasing to hold and what it would cause is
a discarded tag nothing records.

Three guard tests failed by design and were fixed rather than relaxed: the
ordered pull filter, the AAD pinning table, and the server's refusal of a
plaintext parent — that last one survives with its reason rewritten, because
the refusal now means "the parent is not the server's to hold" rather than
"there is no such thing as a parent". The prose that said groups are flat is
rewritten in all four places it appeared, not deleted.

The five view-model sites that read Port directly now go through the resolver,
which is a down payment on step 4 rather than the whole of it. HostFields.From
still emits the stored port, and that is the one remaining place where an
unresolved read would be a wrong wire rather than a wrong label.

Verified by the whole suite: 1382 tests over nineteen projects, none failing.
Both heads build. Nothing seen on a display, because nothing on a display has
changed yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
jaap-jan
2026-08-03 10:21:02 +02:00
co-authored by Claude Opus 5
parent c9eca96ce7
commit 8c04ba60b0
35 changed files with 4921 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/DodoSSH.Client.Domain.Tests/HostFactory.cs
+45 -3
View File
@@ -1,6 +1,8 @@
namespace DodoSSH.Client.Domain.Tests;
/// <summary>Builds hosts for the suites, so each test varies only what it is about.</summary>
/// <summary>
/// Builds hosts, and the groups they are filed under, so each test varies only what it is about.
/// </summary>
internal static class HostFactory
{
internal static Guid Bastion { get; } = Guid.Parse("0192f0c8-1111-7c3d-8e4f-5a6b7c8d9e01");
@@ -13,10 +15,21 @@ internal static class HostFactory
/// <summary>A group id, for the hosts that are filed under one.</summary>
internal static Guid Production { get; } = Guid.Parse("0192f0c8-4444-7c3d-8e4f-5a6b7c8d9e04");
/// <summary>A tag id, for the hosts that wear one.</summary>
internal static Guid Pci { get; } = Guid.Parse("0192f0c8-8888-7c3d-8e4f-5a6b7c8d9e08");
/// <summary>A second tag id, for the tests about two people tagging one host.</summary>
internal static Guid EuWest { get; } = Guid.Parse("0192f0c8-9999-7c3d-8e4f-5a6b7c8d9e09");
/// <remarks>
/// <paramref name="port"/> defaults to an explicit 22 rather than to null, so a test that says nothing
/// about ports gets the host shape that existed before inheritance did — which is what nearly every
/// suite here is still about. Passing <see langword="null"/> is how a test asks for the new one.
/// </remarks>
internal static HostSecret Host(
string label = "prod-db",
string hostname = "db.internal",
int port = 22,
int? port = 22,
string? username = "deploy",
string? notes = null,
Guid[]? jumps = null,
@@ -24,7 +37,9 @@ internal static class HostFactory
bool relayEnabled = false,
Guid? sshKeyId = null,
Guid? credentialId = null,
Guid? groupId = null) =>
bool? asksForPassword = null,
Guid? groupId = null,
Guid[]? tags = null) =>
new()
{
Label = label,
@@ -39,6 +54,33 @@ internal static class HostFactory
RelayEnabled = relayEnabled,
SshKeyId = sshKeyId,
CredentialId = credentialId,
AsksForPassword = asksForPassword,
GroupId = groupId,
TagIds = tags is null ? TagSet.Empty : TagSet.Create(tags),
};
/// <summary>
/// A group, flat and defaulting nothing unless the test says otherwise.
/// </summary>
/// <remarks>
/// The defaults of this builder are the shape a group had before it could nest or default anything,
/// which is what most suites still want: a heading with a name. A test that passes nothing here is
/// asserting about the old shape on purpose.
/// </remarks>
internal static HostGroupSecret Group(
string label = "production",
Guid? parentId = null,
int? defaultPort = null,
string? defaultUsername = null,
Guid? defaultSshKeyId = null,
Guid? defaultCredentialId = null) =>
new()
{
Label = label,
ParentId = parentId,
DefaultPort = defaultPort,
DefaultUsername = defaultUsername,
DefaultSshKeyId = defaultSshKeyId,
DefaultCredentialId = defaultCredentialId,
};
}
tests/DodoSSH.Client.Domain.Tests/HostGroupSecretTests.cs
+217 -11
View File
@@ -1,42 +1,141 @@
using System.Text;
using static DodoSSH.Client.Domain.Tests.HostFactory;
namespace DodoSSH.Client.Domain.Tests;
/// <summary>
/// A group: one name, and the reasons it is only that.
/// A group: a name, a parent, and the four things hosts under it fall back to.
/// </summary>
/// <remarks>
/// There is very little behaviour here to test, which is itself the design — every field that was considered
/// and left out (a parent, a member list) was left out because of what it would do to the merge. What these
/// tests pin is that the envelope round-trips, that a nameless group cannot be stored, and that renaming the
/// same group on two machines is reported rather than silently resolved.
/// <para>
/// A member list is still absent and still for the reason it always was — membership is a pointer on each
/// host, so two people filing two machines into one group is two writes to two items. A parent is present,
/// and it was not: see <see cref="HostGroupSecret"/> for why nesting stopped being worth refusing once the
/// defaults made the chain something the connect path had to walk anyway.
/// </para>
/// <para>
/// What these pin is that the envelope round-trips, that a nameless or unstorable group cannot be stored,
/// that a group carrying none of the new fields still encodes at version 1 byte for byte, and that every
/// field is actually consulted by the merge rather than quietly deferring to the server forever.
/// </para>
/// </remarks>
public sealed class HostGroupSecretTests
{
private static Guid Parent { get; } = Guid.Parse("0192f0c8-5555-7c3d-8e4f-5a6b7c8d9e05");
private static Guid TeamCredential { get; } = Guid.Parse("0192f0c8-6666-7c3d-8e4f-5a6b7c8d9e06");
[Fact]
public void AGroup_RoundTrips()
public void AFlatGroupWithNoDefaults_RoundTripsAtTheVersionItAlwaysHad()
{
var group = new HostGroupSecret { Label = "production" };
var group = Group();
HostGroupSecretCodec.TryDecode(HostGroupSecretCodec.Encode(group), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.Group.ShouldBe(group);
document.SchemaVersion.ShouldBe(HostGroupSecretCodec.CurrentSchemaVersion);
document.SchemaVersion.ShouldBe(HostGroupSecretCodec.BaseSchemaVersion);
document.IsReadOnly.ShouldBeFalse();
}
[Fact]
public void ANestedGroupCarryingEveryDefault_RoundTrips()
{
var group = Group(
parentId: Parent,
defaultPort: 2222,
defaultUsername: "deploy",
defaultCredentialId: TeamCredential);
HostGroupSecretCodec.TryDecode(HostGroupSecretCodec.Encode(group), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.Group.ShouldBe(group);
document.SchemaVersion.ShouldBe(HostGroupSecretCodec.ParentAndDefaultsSchemaVersion);
document.IsReadOnly.ShouldBeFalse();
}
[Fact]
public void AddingTheParentAndDefaults_DidNotChangeTheBytesOfAGroupWithoutThem()
{
// Pinned against a literal rather than against the codec, because the claim is about history: every
// group already in every vault must re-encode to what it encoded before any of these fields existed,
// or the first sync after an upgrade would push every group as changed. Byte-for-byte, so a new
// field that serialised ahead of the name — or a null that serialised as null — would fail here.
//
// The version in this literal is the other half of the claim. This codec used to stamp
// CurrentSchemaVersion unconditionally, and had that survived, a flat group would now be written at
// 2 and read as uneditable on every machine that had not upgraded.
var bytes = HostGroupSecretCodec.Encode(Group());
Encoding.UTF8.GetString(bytes).ShouldBe("""{"schemaVersion":1,"label":"production"}""");
}
[Theory]
[MemberData(nameof(GroupsCarryingOneNewField))]
public void AGroupCarryingAnyNewField_IsWrittenAtTheVersionThatIntroducedThem(HostGroupSecret group)
{
HostGroupSecretCodec.TryDecode(HostGroupSecretCodec.Encode(group), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.SchemaVersion.ShouldBe(
HostGroupSecretCodec.ParentAndDefaultsSchemaVersion,
"a version that cannot represent the field just written makes an older client decode the "
+ "group as editable and drop that field on the next save");
}
public static TheoryData<HostGroupSecret> GroupsCarryingOneNewField() =>
[
Group(parentId: Parent),
Group(defaultPort: 2222),
Group(defaultUsername: "deploy"),
Group(defaultSshKeyId: DeployKey),
Group(defaultCredentialId: TeamCredential),
];
[Theory]
[InlineData("")]
[InlineData(" ")]
public void AGroupWithNoName_IsRefused(string label)
{
new HostGroupSecret { Label = label }.TryValidate(out var reason).ShouldBeFalse();
Group(label: label).TryValidate(out var reason).ShouldBeFalse();
reason.ShouldNotBeNull();
}
[Fact]
public void TryValidate_RejectsWhatCannotBeStored()
{
Group(parentId: Guid.Empty).TryValidate(out _).ShouldBeFalse();
Group(defaultPort: 0).TryValidate(out _).ShouldBeFalse();
Group(defaultPort: 65536).TryValidate(out _).ShouldBeFalse();
Group(defaultSshKeyId: Guid.Empty).TryValidate(out _).ShouldBeFalse();
Group(defaultCredentialId: Guid.Empty).TryValidate(out _).ShouldBeFalse();
// Null is the absence of each of these, and the absence is always storable — it is what every group
// in every vault written before this build carries.
Group().TryValidate(out _).ShouldBeTrue();
}
[Fact]
public void AGroupDefaultsOneWay_NotTwo()
{
// The same exclusion a host is held to, for the same reason: a group naming both leaves "what do
// hosts under this authenticate with?" without a single answer. Necessary but not sufficient — a
// host naming a credential under a group naming a key is two valid records, so the resolver enforces
// it again across the chain.
var both = Group(defaultSshKeyId: DeployKey, defaultCredentialId: TeamCredential);
both.TryValidate(out var reason).ShouldBeFalse();
reason.ShouldNotBeNull().ShouldContain("not both");
Group(defaultSshKeyId: DeployKey).TryValidate(out _).ShouldBeTrue();
Group(defaultCredentialId: TeamCredential).TryValidate(out _).ShouldBeTrue();
}
[Fact]
public void AGroupWrittenByANewerClient_IsReadableButNotWritableHere()
{
@@ -64,10 +163,59 @@ public sealed class HostGroupSecretTests
document.ShouldBeNull();
}
[Fact]
public void AnUnstorablePayload_FailsToDecodeRatherThanProducingAGroupThatCannotBeSaved()
{
// A default port outside the range cannot have been written by this build, so it is either a bug in
// some client or a corrupted write. Decoding it would produce a group the editor could open and
// never save, with nothing on screen to say which field was the problem.
var payload = Encoding.UTF8.GetBytes("""{"schemaVersion":2,"label":"production","defaultPort":0}""");
HostGroupSecretCodec.TryDecode(payload, out var document).ShouldBeFalse();
document.ShouldBeNull();
}
[Fact]
public void EveryScalarField_IsRoutedThroughAMerge()
{
// A field added to HostGroupSecret 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 = Group();
var local = ancestor with
{
Label = "prod",
ParentId = Parent,
DefaultPort = 2222,
DefaultUsername = "deploy",
DefaultCredentialId = TeamCredential,
};
var result = HostGroupSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.ShouldBe(local);
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void TheKeyFieldTheExclusionKeepsOutOfTheOtherTest_IsAlsoRoutedThroughAMerge()
{
// DefaultSshKeyId cannot appear beside DefaultCredentialId in one valid group, so it gets its own
// pass rather than being the one field the guard above silently skips.
var ancestor = Group();
var local = ancestor with { DefaultSshKeyId = DeployKey };
var result = HostGroupSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.ShouldBe(local);
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void TwoDifferentRenames_AreReportedWithBothNames()
{
var ancestor = new HostGroupSecret { Label = "production" };
var ancestor = Group();
var result = HostGroupSecretMerge.Merge(
ancestor,
@@ -83,6 +231,64 @@ public sealed class HostGroupSecretTests
conflict.Discarded.ShouldBe("prod");
}
[Fact]
public void TwoDifferentReparentings_AreReportedWithBothParents()
{
// The clash that admits a cycle. This merge sees one group against one group, so it cannot know the
// pair it is half of — it resolves, reports, and leaves the containment to the resolver's visited
// set. What it must not do is resolve silently.
var other = Guid.Parse("0192f0c8-7777-7c3d-8e4f-5a6b7c8d9e07");
var ancestor = Group();
var result = HostGroupSecretMerge.Merge(
ancestor,
ancestor with { ParentId = Parent },
ancestor with { ParentId = other });
result.Merged.ParentId.ShouldBe(other);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostGroupSecret.ParentId));
conflict.Kept.ShouldBe(other.ToString());
conflict.Discarded.ShouldBe(Parent.ToString());
}
[Fact]
public void ADefaultClearedLocally_IsNotResurrectedByTheOtherSide()
{
// Null is a value here, not an absence: a group deliberately put back to no default user must not
// silently regain one because the server's copy still names it.
var ancestor = Group(defaultUsername: "deploy");
var local = ancestor with { DefaultUsername = null };
var result = HostGroupSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.DefaultUsername.ShouldBeNull();
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void ADefaultClearedOnOneSideAndChangedOnTheOther_NamesTheAbsenceInTheConflict()
{
// The formatter has to run for the null side too. Short-circuiting on null would print an empty
// string where the conflict log needs to say that what lost was the removal of the default.
var ancestor = Group(defaultPort: 22);
var result = HostGroupSecretMerge.Merge(
ancestor,
ancestor with { DefaultPort = null },
ancestor with { DefaultPort = 2222 });
result.Merged.DefaultPort.ShouldBe(2222);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostGroupSecret.DefaultPort));
conflict.Kept.ShouldBe("2222");
conflict.Discarded.ShouldBe("no default port");
}
/// <remarks>
/// The case that would collide if membership were held on the group instead of on each host: two people
/// filing two different machines into one group at the same time. It cannot reach the merge at all,
@@ -91,7 +297,7 @@ public sealed class HostGroupSecretTests
[Fact]
public void FilingHostsIntoAGroup_DoesNotTouchTheGroup()
{
var ancestor = new HostGroupSecret { Label = "production" };
var ancestor = Group();
var result = HostGroupSecretMerge.Merge(ancestor, ancestor, ancestor);
tests/DodoSSH.Client.Domain.Tests/HostInheritanceTests.cs
+257
View File
@@ -0,0 +1,257 @@
using static DodoSSH.Client.Domain.Tests.HostFactory;
namespace DodoSSH.Client.Domain.Tests;
/// <summary>
/// Walking a host's group chain for the values it did not state itself.
/// </summary>
/// <remarks>
/// <para>
/// This is the code the connect path runs, so its failures are not cosmetic. A wrong port dials the wrong
/// machine or nothing at all; a wrong binding sends a password to a host set up for key-only access; and a
/// walk that does not terminate is a shell that never opens, which is the one failure a user cannot even
/// describe.
/// </para>
/// <para>
/// The cycle tests are the reason the walk carries a visited set at all. A cycle cannot be created through
/// the editor and cannot be seen by the merge, which resolves one group against one group, or by the
/// server, which cannot read the payload — so it arrives assembled from two offline re-parents or not at
/// all. What these pin is that it degrades to a group reading as a root rather than hanging.
/// </para>
/// </remarks>
public sealed class HostInheritanceTests
{
private static Guid Estate { get; } = Guid.Parse("0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a");
private static Guid Region { get; } = Guid.Parse("0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b");
private static Guid TeamCredential { get; } = Guid.Parse("0192f0c8-cccc-7c3d-8e4f-5a6b7c8d9e0c");
[Fact]
public void AHostThatStatesEverything_InheritsNothing()
{
var host = Host(port: 2222, username: "deploy", sshKeyId: DeployKey, groupId: Production);
var resolved = HostInheritance.Resolve(
host,
Groups((Production, Group(defaultPort: 9999, defaultUsername: "root"))));
resolved.Port.Value.ShouldBe(2222);
resolved.Port.IsInherited.ShouldBeFalse();
resolved.Username.Value.ShouldBe("deploy");
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.SshKey);
resolved.Binding.EntityId.ShouldBe(DeployKey);
resolved.Binding.IsInherited.ShouldBeFalse();
}
[Fact]
public void AHostThatStatesNothing_TakesItsGroupsValuesAndSaysWhichGroup()
{
// The second half matters as much as the first: the editor draws an inherited value as a
// placeholder behind an empty box, so it has to know the value came from somewhere else.
var host = Host(port: null, username: null, groupId: Production);
var resolved = HostInheritance.Resolve(
host,
Groups((Production, Group(
defaultPort: 2222,
defaultUsername: "deploy",
defaultCredentialId: TeamCredential))));
resolved.Port.Value.ShouldBe(2222);
resolved.Port.FromGroupId.ShouldBe(Production);
resolved.Username.Value.ShouldBe("deploy");
resolved.Username.FromGroupId.ShouldBe(Production);
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.Credential);
resolved.Binding.EntityId.ShouldBe(TeamCredential);
resolved.Binding.FromGroupId.ShouldBe(Production);
}
[Fact]
public void AHostWithNoGroupAndNothingStated_FallsBackToTwentyTwoAndATypedPassword()
{
// Every host stored before any of this existed. Nothing changed underneath them: naming neither a
// key nor a credential resolved to a typed password then, and resolves to one now.
var resolved = HostInheritance.Resolve(Host(port: null, username: null), Groups());
resolved.Port.Value.ShouldBe(HostSecret.DefaultPort);
resolved.Port.IsInherited.ShouldBeFalse();
resolved.Username.Value.ShouldBeNull();
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.TypedPassword);
resolved.Binding.EntityId.ShouldBeNull();
}
[Fact]
public void TheNearestGroupThatStatesAValue_Wins()
{
var host = Host(port: null, username: null, groupId: Production);
var resolved = HostInheritance.Resolve(
host,
Groups(
(Production, Group(parentId: Region, defaultPort: 2222)),
(Region, Group(parentId: Estate, defaultPort: 9999, defaultUsername: "deploy")),
(Estate, Group(defaultUsername: "root"))));
resolved.Port.Value.ShouldBe(2222);
resolved.Port.FromGroupId.ShouldBe(Production);
// Each field is resolved on its own, so a group that answers one question does not stop the walk
// for the others.
resolved.Username.Value.ShouldBe("deploy");
resolved.Username.FromGroupId.ShouldBe(Region);
}
[Fact]
public void AHostWithAnEmptyUsername_HasNoUsernameRatherThanTheGroups()
{
// The three states in one nullable string. Null used to mean "no username", which the connect path
// refuses; it now means "ask the group", so the refusal has to stay reachable or a host under a
// group could never opt out of the group's user.
var resolved = HostInheritance.Resolve(
Host(username: string.Empty, groupId: Production),
Groups((Production, Group(defaultUsername: "deploy"))));
resolved.Username.Value.ShouldBe(string.Empty);
resolved.Username.IsInherited.ShouldBeFalse();
}
[Fact]
public void AHostPinnedToATypedPassword_DoesNotPickUpItsGroupsKey()
{
// The failure worth ruling out above all others in this file. A host deliberately put back on a
// typed password must not silently start authenticating with the fleet's key because somebody set a
// default on the group above it.
var resolved = HostInheritance.Resolve(
Host(asksForPassword: true, groupId: Production),
Groups((Production, Group(defaultSshKeyId: DeployKey))));
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.TypedPassword);
resolved.Binding.EntityId.ShouldBeNull();
resolved.Binding.IsInherited.ShouldBeFalse();
}
[Fact]
public void AHostNamingACredentialUnderAGroupNamingAKey_ResolvesToOneBindingAndItIsTheHosts()
{
// Two individually valid records that per-record validation cannot catch, because neither one is
// wrong on its own. The exclusion is enforced again here, and the nearer statement wins.
var resolved = HostInheritance.Resolve(
Host(credentialId: TeamCredential, groupId: Production),
Groups((Production, Group(defaultSshKeyId: DeployKey))));
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.Credential);
resolved.Binding.EntityId.ShouldBe(TeamCredential);
}
[Fact]
public void ANearerGroupsBinding_BeatsAFurtherOnesEvenWhenTheyDiffer()
{
var resolved = HostInheritance.Resolve(
Host(port: null, groupId: Production),
Groups(
(Production, Group(parentId: Estate, defaultCredentialId: TeamCredential)),
(Estate, Group(defaultSshKeyId: DeployKey))));
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.Credential);
resolved.Binding.FromGroupId.ShouldBe(Production);
}
[Fact]
public void AGroupDeletedOnAnotherMachine_LeavesTheHostUngroupedRatherThanFailing()
{
// A dangling id is the ordinary outcome of a delete, not a corruption: deleting a group does not
// rewrite the hosts that named it, deliberately. See HostGroupRepository.
var resolved = HostInheritance.Resolve(Host(port: null, username: null, groupId: Production), Groups());
resolved.Port.Value.ShouldBe(HostSecret.DefaultPort);
resolved.Username.Value.ShouldBeNull();
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.TypedPassword);
}
[Fact]
public void AGroupWhoseParentWasDeleted_StopsAtThatGroupRatherThanFailing()
{
var resolved = HostInheritance.Resolve(
Host(port: null, username: null, groupId: Production),
Groups((Production, Group(parentId: Estate, defaultPort: 2222))));
resolved.Port.Value.ShouldBe(2222);
resolved.Username.Value.ShouldBeNull();
}
[Fact]
public void ATwoGroupCycle_ResolvesRatherThanHanging()
{
// The state the old "groups are flat" decision existed to prevent, arriving anyway: two clients each
// re-parenting A under B and B under A while offline. The merge sees one group against one group and
// the server sees ciphertext, so nothing upstream can refuse the pair — which is why the answer is a
// walk that terminates rather than a state that cannot occur.
var resolved = HostInheritance.Resolve(
Host(port: null, username: null, groupId: Production),
Groups(
(Production, Group(parentId: Region, defaultPort: 2222)),
(Region, Group(parentId: Production, defaultUsername: "deploy"))));
// Both groups are still read once, because the repeat is what stops the walk rather than the loop
// being detected up front. What must not happen is a third visit.
resolved.Port.Value.ShouldBe(2222);
resolved.Username.Value.ShouldBe("deploy");
}
[Fact]
public void AGroupThatIsItsOwnParent_ReadsAsARoot()
{
// Not creatable through the editor and not knowable to HostGroupSecret.TryValidate, which sees the
// payload and not the item id. Contained here instead, in the same visited set that has to contain
// a longer cycle anyway.
var chain = HostInheritance
.Chain(Production, Groups((Production, Group(parentId: Production))))
.ToList();
chain.Count.ShouldBe(1);
chain[0].Id.ShouldBe(Production);
}
[Fact]
public void ACycleAboveAHost_StillResolvesEveryFieldItCanReach()
{
// The degradation the design accepts: defaults past the repeat are unresolved, everything before it
// is not, and clearing the parent in the editor is the repair. What it must never be is a connect
// that never returns.
var resolved = HostInheritance.Resolve(
Host(port: null, username: null, groupId: Production),
Groups(
(Production, Group(parentId: Region)),
(Region, Group(parentId: Production))));
resolved.Port.Value.ShouldBe(HostSecret.DefaultPort);
resolved.Username.Value.ShouldBeNull();
resolved.Binding.Kind.ShouldBe(ResolvedBindingKind.TypedPassword);
}
[Fact]
public void TheChain_YieldsNearestFirstAndStopsAtTheRoot()
{
var chain = HostInheritance
.Chain(
Production,
Groups(
(Production, Group(label: "production", parentId: Region)),
(Region, Group(label: "eu-west", parentId: Estate)),
(Estate, Group(label: "estate"))))
.ToList();
chain.Select(entry => entry.Group.Label).ShouldBe(["production", "eu-west", "estate"]);
}
[Fact]
public void TheChainOfAnUngroupedHost_IsEmpty()
{
HostInheritance.Chain(null, Groups()).ShouldBeEmpty();
}
private static Dictionary<Guid, HostGroupSecret> Groups(
params (Guid Id, HostGroupSecret Group)[] groups) =>
groups.ToDictionary(entry => entry.Id, entry => entry.Group);
}
tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs
+149 -4
View File
@@ -16,9 +16,10 @@ public sealed class HostSecretCodecTests
{
/// <remarks>
/// "Full" cannot mean every field: the two authentication bindings are mutually exclusive, so a host may
/// carry a key or a credential and never both. This one carries the credential, because that is the newer
/// of the two, plus a group — which is orthogonal to both and is what makes this host reach the highest
/// schema version a valid host can.
/// carry a key or a credential and never both, and neither may sit beside <c>AsksForPassword</c>. This
/// one carries the credential, because that is the newer of the two, plus a group and a pair of tags —
/// which are orthogonal to the binding and are what make this host reach the highest schema version a
/// valid host can.
/// </remarks>
[Fact]
public void AFullHost_RoundTrips()
@@ -35,7 +36,8 @@ public sealed class HostSecretCodecTests
options: [("ServerAliveInterval", "30"), ("Compression", "yes")],
relayEnabled: true,
credentialId: credentialId,
groupId: Production);
groupId: Production,
tags: [Pci, EuWest]);
HostSecretCodec.TryDecode(HostSecretCodec.Encode(host), out var document).ShouldBeTrue();
@@ -146,6 +148,149 @@ public sealed class HostSecretCodecTests
document.Host.ShouldBe(host);
}
[Fact]
public void AHostThatInheritsItsPort_IsWrittenAtTheVersionThatIntroducedInheritance()
{
HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(port: null)), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.SchemaVersion.ShouldBe(
HostSecretCodec.PortInheritSchemaVersion,
"a host stamped at 4 with its port omitted is not read-only on an older build — it is "
+ "undecodable there, because int Port reads 0 and TryValidate refuses it");
document.Host.Port.ShouldBeNull();
}
[Fact]
public void AHostPinnedToATypedPassword_IsWrittenAtTheSameVersionAsAnInheritedPort()
{
// Both halves of inheritance share a version because they arrive together and answer the same
// question: what a host says when it declines to take its group's answer.
HostSecretCodec
.TryDecode(HostSecretCodec.Encode(Host(asksForPassword: true)), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.SchemaVersion.ShouldBe(HostSecretCodec.PortInheritSchemaVersion);
document.Host.AsksForPassword.ShouldBe(true);
}
[Fact]
public void AHostWearingTags_IsWrittenAtTheVersionThatIntroducedThem()
{
HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(tags: [Pci])), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.SchemaVersion.ShouldBe(HostSecretCodec.TagIdsSchemaVersion);
document.Host.TagIds.ShouldBe(TagSet.Create([Pci]));
}
[Fact]
public void ATaggedHostThatPinsItsPort_IsNotDraggedOntoTheInheritanceVersionOrBelowIt()
{
// The maximum, from both directions. Tags are independent of inheritance, so a host that only wears
// one must not be written at 5 — and a host that only inherits must not be written at 6, which would
// make it undecodable on a build that could have read it.
HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(tags: [Pci])), out var tagged)
.ShouldBeTrue();
HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(port: null)), out var inheriting)
.ShouldBeTrue();
tagged.ShouldNotBeNull().SchemaVersion
.ShouldBeGreaterThan(HostSecretCodec.PortInheritSchemaVersion);
inheriting.ShouldNotBeNull().SchemaVersion
.ShouldBeLessThan(HostSecretCodec.TagIdsSchemaVersion);
}
[Fact]
public void AHostWithAnEmptyTagSet_IsWrittenAtTheVersionItWouldHaveHadWithout()
{
// Wearing no tags is not using the feature. If an empty set bumped the version, adding tags would
// have made every host in every vault read-only on every machine that had not upgraded.
HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(tags: [])), out var document)
.ShouldBeTrue();
document.ShouldNotBeNull();
document.SchemaVersion.ShouldBe(HostSecretCodec.BaseSchemaVersion);
}
[Fact]
public void AddingInheritanceAndTags_DidNotChangeTheBytesOfAHostUsingNeither()
{
// The companion to the pin below, and the one that matters most for these two fields: a set that
// serialised as [] and a flag that serialised as false would both land in every host in every vault,
// and the first sync after the upgrade would push all of them as changed.
var bytes = HostSecretCodec.Encode(Host(username: null, notes: null, tags: []));
Encoding.UTF8.GetString(bytes).ShouldBe(
"""
{"schemaVersion":1,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false}
""");
}
[Fact]
public void AHostThatInheritsItsPort_OmitsTheKeyRatherThanWritingANull()
{
// The companion pin the old one could not be edited into. What an inheriting host must produce is
// the absence of "port", not "port":null and not "port":22 — the first would decode as 0 on any
// build, and the second is what inheritance exists to stop writing.
var bytes = HostSecretCodec.Encode(Host(username: null, notes: null, port: null));
Encoding.UTF8.GetString(bytes).ShouldBe(
"""
{"schemaVersion":5,"label":"prod-db","hostname":"db.internal","jumpHostIds":[],"options":{},"relayEnabled":false}
""");
}
[Fact]
public void AHostWearingTags_WritesThemLastAndSorted()
{
// Last, so every field that existed before them keeps its bytes; sorted, so two users who tapped the
// same two chips in opposite orders produce one value and nothing to push.
var bytes = HostSecretCodec.Encode(
Host(username: null, notes: null, tags: [EuWest, Pci]));
Encoding.UTF8.GetString(bytes).ShouldBe(
"""
{"schemaVersion":6,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false,"tagIds":["0192f0c8-8888-7c3d-8e4f-5a6b7c8d9e08","0192f0c8-9999-7c3d-8e4f-5a6b7c8d9e09"]}
""");
}
[Fact]
public void APayloadSayingItAsksForNoPassword_DecodesAsHavingSaidNothing()
{
// False and null mean the same thing — take the group's binding — so only one may reach the record.
// Two spellings of one state is a difference the merge would report as a change nobody made.
var payload = Encoding.UTF8.GetBytes(
"""
{"schemaVersion":5,"label":"prod-db","hostname":"db.internal","port":22,"asksForPassword":false}
""");
HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue();
document.ShouldNotBeNull().Host.AsksForPassword.ShouldBeNull();
}
[Fact]
public void APayloadRepeatingATag_DecodesAsTheSetItMeant()
{
// Written by some other client, and it must not compare unequal to the same set written once — or
// the engine would push this host as changed on every pass for ever.
var payload = Encoding.UTF8.GetBytes(
$$"""
{"schemaVersion":6,"label":"prod-db","hostname":"db.internal","port":22,"tagIds":["{{Pci}}","{{Pci}}"]}
""");
HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue();
document.ShouldNotBeNull().Host.TagIds.ShouldBe(TagSet.Create([Pci]));
}
[Fact]
public void AddingTheKeyField_DidNotChangeTheBytesOfAHostWithoutOne()
{
tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs
+118
View File
@@ -56,6 +56,8 @@ public sealed class HostSecretMergeTests
Options = HostOptions.Create([new HostOption("Compression", "yes")]),
RelayEnabled = true,
SshKeyId = DeployKey,
GroupId = Production,
TagIds = TagSet.Create([Pci]),
};
var result = HostSecretMerge.Merge(ancestor, local, ancestor);
@@ -64,6 +66,122 @@ public sealed class HostSecretMergeTests
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void TheFieldsTheExclusionsKeepOutOfTheGuardAbove_AreAlsoRoutedThroughAMerge()
{
// AsksForPassword cannot sit beside SshKeyId in one valid host, and a null Port cannot sit beside
// an explicit one — so both get their own pass rather than being the fields the guard silently
// skips. A forgotten one here means a host put back on a typed password, or set to take its group's
// port, quietly reverting to the server's copy for ever.
var ancestor = Host();
var local = ancestor with { Port = null, AsksForPassword = true };
var result = HostSecretMerge.Merge(ancestor, local, ancestor);
result.Merged.ShouldBe(local);
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void TwoPeopleAddingDifferentTagsToOneHost_BothKeepTheirs()
{
// The single most visible difference between a field-level merge and last-writer-wins, and the
// reason TagIds merges per tag rather than as a whole value. A whole-value merge would take one
// side's set entire and drop the other's.
var ancestor = Host();
var result = HostSecretMerge.Merge(
ancestor,
ancestor with { TagIds = TagSet.Create([Pci]) },
ancestor with { TagIds = TagSet.Create([EuWest]) });
result.Merged.TagIds.ShouldBe(TagSet.Create([Pci, EuWest]));
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void OneSideRemovingATagWhileTheOtherAddsAnother_KeepsBothDecisions()
{
// Each tag is resolved on its own, so a removal on one side and an addition on the other are two
// independent answers rather than two versions of one. A whole-value merge would have to pick.
var ancestor = Host(tags: [Pci]);
var result = HostSecretMerge.Merge(
ancestor,
ancestor with { TagIds = TagSet.Empty },
ancestor with { TagIds = TagSet.Create([Pci, EuWest]) });
result.Merged.TagIds.ShouldBe(TagSet.Create([EuWest]));
result.HasConflicts.ShouldBeFalse();
}
[Fact]
public void ATagRemovedOnBothSides_IsNotResurrected()
{
var ancestor = Host(tags: [Pci, EuWest]);
var untagged = ancestor with { TagIds = TagSet.Create([EuWest]) };
var result = HostSecretMerge.Merge(ancestor, untagged, untagged);
result.Merged.TagIds.ShouldBe(TagSet.Create([EuWest]));
result.HasConflicts.ShouldBeFalse();
}
/// <remarks>
/// The property that makes a tag set the one field on a host which can never ask the user anything. A
/// tag is present or absent, so a key cannot hold two values, so the "both sides moved differently"
/// branch of the keyed merge is unreachable — see <c>HostSecretMerge.MergeTags</c>. Stated as a table
/// over every arrangement of one tag, because the claim is about the whole matrix rather than about any
/// one row of it.
/// </remarks>
[Theory]
[InlineData(true, true, true)]
[InlineData(true, true, false)]
[InlineData(true, false, true)]
[InlineData(true, false, false)]
[InlineData(false, true, true)]
[InlineData(false, true, false)]
[InlineData(false, false, true)]
[InlineData(false, false, false)]
public void NoArrangementOfOneTag_ProducesAConflict(bool inAncestor, bool inLocal, bool inRemote)
{
var result = HostSecretMerge.Merge(
Host(tags: Wearing(inAncestor)),
Host(tags: Wearing(inLocal)),
Host(tags: Wearing(inRemote)));
result.HasConflicts.ShouldBeFalse();
// And the outcome is the one a set should give: a side that moved gets its way, because the other
// one did not move.
result.Merged.TagIds.Contains(Pci).ShouldBe(inAncestor ? inLocal && inRemote : inLocal || inRemote);
}
private static Guid[] Wearing(bool tagged) => tagged ? [Pci] : [];
[Fact]
public void TwoSidesTakingDifferentPorts_NamesTheInheritedOneInTheConflict()
{
// The formatter has to run for the null side, and null here is not an absence — it is the decision
// to take the group's port. A conflict log printing an empty string in its place would leave the
// user unable to tell which of the two decisions was dropped.
var ancestor = Host(port: 22);
var result = HostSecretMerge.Merge(
ancestor,
ancestor with { Port = null },
ancestor with { Port = 2222 });
result.Merged.Port.ShouldBe(2222);
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(HostSecret.Port));
conflict.Kept.ShouldBe("2222");
conflict.Discarded.ShouldBe("the group's port");
}
[Fact]
public void ARemovedKeyBinding_IsNotResurrectedByTheOtherSide()
{
tests/DodoSSH.Client.Domain.Tests/TagSecretTests.cs
+117
View File
@@ -0,0 +1,117 @@
using System.Text;
namespace DodoSSH.Client.Domain.Tests;
/// <summary>
/// A tag: one name, and the reasons it is only that.
/// </summary>
/// <remarks>
/// <para>
/// There is very little behaviour here, which is the design — a tag exists to have an identity, so that
/// renaming it is one write instead of twenty. What these pin is that the envelope round-trips, that a
/// nameless tag cannot be stored, and that renaming the same tag on two machines is reported rather than
/// silently resolved.
/// </para>
/// <para>
/// The absent test worth naming: nothing here refuses a second tag called the same thing. Two people
/// creating "staging" offline is how it happens, and refusing the loser would mean discarding the tags a
/// colleague had already put on their hosts. See <see cref="TagSecret.TryValidate"/>.
/// </para>
/// </remarks>
public sealed class TagSecretTests
{
[Fact]
public void ATag_RoundTrips()
{
var tag = new TagSecret { Label = "pci" };
TagSecretCodec.TryDecode(TagSecretCodec.Encode(tag), out var document).ShouldBeTrue();
document.ShouldNotBeNull();
document.Tag.ShouldBe(tag);
document.SchemaVersion.ShouldBe(TagSecretCodec.CurrentSchemaVersion);
document.IsReadOnly.ShouldBeFalse();
}
[Fact]
public void ATag_EncodesAtTheBaseVersionWithNothingBesideItsName()
{
// Pinned as a literal, so the first field this type ever grows has to be appended and has to be
// omitted when null — the two properties that keep every tag already in every vault re-encoding to
// the bytes it was stored with, rather than looking like a change on the first sync after an upgrade.
Encoding.UTF8.GetString(TagSecretCodec.Encode(new TagSecret { Label = "pci" }))
.ShouldBe("""{"schemaVersion":1,"label":"pci"}""");
}
[Theory]
[InlineData("")]
[InlineData(" ")]
public void ATagWithNoName_IsRefused(string label)
{
new TagSecret { Label = label }.TryValidate(out var reason).ShouldBeFalse();
reason.ShouldNotBeNull();
}
[Fact]
public void ATagWrittenByANewerClient_IsReadableButNotWritableHere()
{
var payload = Encoding.UTF8.GetBytes(
"""
{"schemaVersion":99,"label":"pci","colour":"a field this build has never heard of"}
""");
TagSecretCodec.TryDecode(payload, out var document).ShouldBeTrue();
document.ShouldNotBeNull();
document.Tag.Label.ShouldBe("pci");
document.IsReadOnly.ShouldBeTrue();
}
[Fact]
public void AnUnnamedPayload_FailsToDecodeRatherThanProducingABlankTag()
{
// Failing closed, as every codec in this folder does: a nameless tag is an empty chip beside a host,
// filtering a set nobody can name.
var payload = Encoding.UTF8.GetBytes("""{"schemaVersion":1}""");
TagSecretCodec.TryDecode(payload, out var document).ShouldBeFalse();
document.ShouldBeNull();
}
[Fact]
public void TwoDifferentRenames_AreReportedWithBothNames()
{
var ancestor = new TagSecret { Label = "pci" };
var result = TagSecretMerge.Merge(
ancestor,
ancestor with { Label = "pci-dss" },
ancestor with { Label = "in-scope" });
result.Merged.Label.ShouldBe("in-scope");
var conflict = result.Conflicts.ShouldHaveSingleItem();
conflict.Field.ShouldBe(nameof(TagSecret.Label));
conflict.Kept.ShouldBe("in-scope");
conflict.Discarded.ShouldBe("pci-dss");
}
/// <remarks>
/// The case that would collide if membership were held on the tag instead of on each host: two people
/// putting one tag on two different machines at the same time. It cannot reach the merge at all, because
/// neither of those actions writes to this item.
/// </remarks>
[Fact]
public void TaggingHosts_DoesNotTouchTheTag()
{
var ancestor = new TagSecret { Label = "pci" };
var result = TagSecretMerge.Merge(ancestor, ancestor, ancestor);
result.HasConflicts.ShouldBeFalse();
result.Merged.ShouldBe(ancestor);
}
}
tests/DodoSSH.Client.Domain.Tests/ValueSemanticsTests.cs
+61
View File
@@ -149,18 +149,79 @@ public sealed class ValueSemanticsTests
(copy with { Notes = host.Notes }).ShouldBe(host);
}
[Fact]
public void ATagSet_ComparesByContentsAndNotByOrder()
{
// The half that differs from a jump chain, and the reason it is a separate type. A route reordered
// is a different route; a tag list reordered is the same host, so two users who tapped the same two
// chips in opposite orders must produce one value and nothing to push.
TagSet.Create([Pci, EuWest]).Equals(TagSet.Create([EuWest, Pci])).ShouldBeTrue();
(TagSet.Create([Pci, EuWest]) == TagSet.Create([EuWest, Pci])).ShouldBeTrue();
TagSet.Create([Pci, EuWest]).GetHashCode()
.ShouldBe(TagSet.Create([EuWest, Pci]).GetHashCode());
TagSet.Create([Pci]).Equals(TagSet.Create([EuWest])).ShouldBeFalse();
TagSet.Create([Pci]).Equals(TagSet.Create([Pci, EuWest])).ShouldBeFalse();
TagSet.Create([]).Equals(TagSet.Empty).ShouldBeTrue();
TagSet.Create([Pci]).Equals(null).ShouldBeFalse();
}
[Fact]
public void ATagSet_CollapsesARepeatedTag()
{
// A repeat arrives from a payload some other client wrote. Left alone it would compare unequal to
// the same set written once, and the engine would push the host as changed on every pass for ever.
TagSet.Create([Pci, Pci]).Equals(TagSet.Create([Pci])).ShouldBeTrue();
TagSet.Create([Pci, Pci]).Count.ShouldBe(1);
}
[Fact]
public void ATagSet_MapsToItsOwnIdsSoAKeyedMergeIsASetMerge()
{
// The value repeats the key on purpose: no key can then hold two different values, so the only
// disagreement a keyed merge can report is one side adding what the other removed.
var map = TagSet.Create([Pci, EuWest]).ToIdMap();
map.Keys.Order().ShouldBe(new[] { Pci, EuWest }.Order());
map[Pci].ShouldBe(Pci);
map[EuWest].ShouldBe(EuWest);
}
[Fact]
public void TryValidate_RejectsWhatCannotBeStored()
{
Host(label: "").TryValidate(out _).ShouldBeFalse();
Host(hostname: " ").TryValidate(out _).ShouldBeFalse();
Host(port: 0).TryValidate(out _).ShouldBeFalse();
Host(port: 65536).TryValidate(out _).ShouldBeFalse();
Host(jumps: [Guid.Empty]).TryValidate(out _).ShouldBeFalse();
Host(sshKeyId: Guid.Empty).TryValidate(out _).ShouldBeFalse();
Host(credentialId: Guid.Empty).TryValidate(out _).ShouldBeFalse();
Host(tags: [Guid.Empty]).TryValidate(out _).ShouldBeFalse();
// Null is "take the group's port", not an absent one, and it has to be storable — it is the whole
// of what inheritance stores.
Host(port: null).TryValidate(out _).ShouldBeTrue();
Host().TryValidate(out _).ShouldBeTrue();
}
[Fact]
public void AHostGivesOneAnswerAboutAuthentication_NotTwo()
{
// The same failure the key-or-credential rule catches, in the direction inheritance opened: a host
// that names a key and also says "ask me for a password" has answered one question twice, and the
// interface, the connect path and the user would each be free to pick a different answer.
var both = Host(sshKeyId: DeployKey, asksForPassword: true);
both.TryValidate(out var reason).ShouldBeFalse();
reason.ShouldNotBeNull().ShouldContain("not both");
Host(asksForPassword: true).TryValidate(out _).ShouldBeTrue();
Host(sshKeyId: DeployKey).TryValidate(out _).ShouldBeTrue();
}
[Fact]
public void AHostAuthenticatesOneWay_NotTwo()
{