diff --git a/src/DodoSSH.Client.Domain/HostSecret.cs b/src/DodoSSH.Client.Domain/HostSecret.cs index 4e7d88c..ce3e60d 100644 --- a/src/DodoSSH.Client.Domain/HostSecret.cs +++ b/src/DodoSSH.Client.Domain/HostSecret.cs @@ -29,6 +29,22 @@ public sealed record HostSecret : IVaultSecret /// The default SSH port, used when a host does not say otherwise. public const int DefaultPort = 22; + /// The longest a single pinned path may be. + /// + /// A remote path is opaque to this client, so nothing about its shape is checked beyond this — but + /// opaque is not unbounded. This is inside an encrypted payload every device decrypts and renders, and + /// a length nobody would type by hand is the shape a corrupted or hostile payload takes. + /// + public const int MaxPinnedPathLength = 1024; + + /// The most paths a host may pin. + /// + /// A quick-access list a person actually uses tops out well short of this; the limit exists to bound + /// the payload rather than to constrain anyone's workflow. See for + /// the parallel reasoning about a single entry. + /// + public const int MaxPinnedPaths = 32; + /// Display name. The only name this host has anywhere. public required string Label { get; init; } @@ -188,6 +204,25 @@ public sealed record HostSecret : IVaultSecret /// public TagSet TagIds { get; init; } = TagSet.Empty; + /// + /// The remote directories pinned on this host, in the order the user arranged them. + /// + /// + /// + /// Plain strings rather than any parsed path type, because the path means something only to the + /// remote shell — this client never resolves it, never lists it, never checks that it exists. What + /// "pinned" buys is a shortcut into wherever the user already knows to go: a project checkout, a log + /// directory, the place a deploy lands. + /// + /// + /// Order is kept rather than sorted, unlike , because order is what a pinned-path + /// list is for — it is what a quick-access menu draws top to bottom, and a user who moves their + /// most-used path to the top has expressed something a sorted set would erase. See + /// for how that survives the merge. + /// + /// + public PinnedPathList PinnedPaths { get; init; } = PinnedPathList.Empty; + /// /// The group this host is filed under, or null for none. /// @@ -279,7 +314,62 @@ public sealed record HostSecret : IVaultSecret return false; } - return ReferencesAreStorable(out reason); + return PinnedPathsAreStorable(out reason) && ReferencesAreStorable(out reason); + } + + /// + /// Checks the paths pinned on this host. + /// + /// + /// + /// A path here is opaque to this client: it is resolved by the remote shell, not by this one, so + /// nothing about leading slashes or . components is checked, and a relative path — one meant to + /// resolve against the account's home directory — is exactly as valid as an absolute one. + /// + /// + /// What is refused is narrower. A path that is blank once trimmed pins nothing to jump to. A control + /// character, NUL included, is never part of a real path and is the shape a corrupted or hand-crafted + /// payload takes — this client renders these directly in a shortcut list, and a stray escape sequence + /// or embedded NUL there is a bug at best. And and + /// bound the payload the same way bounds + /// directives: not because a real workflow approaches either limit, but because an encrypted payload + /// every device must decrypt and render should not be allowed to demand an unbounded amount of either. + /// + /// + private bool PinnedPathsAreStorable([NotNullWhen(false)] out string? reason) + { + if (PinnedPaths.Count > MaxPinnedPaths) + { + reason = $"A host cannot pin more than {MaxPinnedPaths} paths."; + return false; + } + + foreach (var path in PinnedPaths) + { + if (string.IsNullOrWhiteSpace(path)) + { + reason = "A pinned path cannot be blank."; + return false; + } + + if (path.Length > MaxPinnedPathLength) + { + reason = $"A pinned path cannot be longer than {MaxPinnedPathLength} characters."; + return false; + } + + foreach (var ch in path) + { + if (char.IsControl(ch)) + { + reason = "A pinned path cannot contain a control character."; + return false; + } + } + } + + reason = null; + return true; } /// diff --git a/src/DodoSSH.Client.Domain/HostSecretCodec.cs b/src/DodoSSH.Client.Domain/HostSecretCodec.cs index a636b3e..a78d295 100644 --- a/src/DodoSSH.Client.Domain/HostSecretCodec.cs +++ b/src/DodoSSH.Client.Domain/HostSecretCodec.cs @@ -102,12 +102,22 @@ public static class HostSecretCodec /// public const int TagIdsSchemaVersion = 6; + /// The version that introduced . + /// + /// One past tags rather than sharing with them, for the reason gives + /// for sharing with inheritance instead of standing alone: the two fields are independent, a host can + /// pin paths without wearing a single tag, and stating the version such a host would actually need + /// keeps the rule in a maximum over what is genuinely present rather + /// than a coincidence of what shipped together. + /// + public const int PinnedPathsSchemaVersion = 7; + /// The highest schema version this build can write. /// /// Names the highest constant above, which assumes when it takes a /// maximum. A new field added below this line has to be named here too. /// - public const int CurrentSchemaVersion = TagIdsSchemaVersion; + public const int CurrentSchemaVersion = PinnedPathsSchemaVersion; /// Serialises a host to the bytes that get sealed. /// The host is not valid for storage. @@ -149,6 +159,10 @@ public static class HostSecretCodec // [] here would land in every host in every vault and make the first sync after the upgrade // read as though every one of them had changed. TagIds = host.TagIds.Count == 0 ? null : [.. host.TagIds], + + // Last, and null for the same reason TagIds is: a host that pins nothing must encode exactly + // as it did before this field existed. + PinnedPaths = host.PinnedPaths.Count == 0 ? null : [.. host.PinnedPaths], }; return JsonSerializer.SerializeToUtf8Bytes( @@ -219,6 +233,11 @@ public static class HostSecretCodec version = Math.Max(version, TagIdsSchemaVersion); } + if (host.PinnedPaths.Count > 0) + { + version = Math.Max(version, PinnedPathsSchemaVersion); + } + return version; } @@ -295,6 +314,8 @@ public static class HostSecretCodec AsksForPassword = parsed.AsksForPassword is true ? true : null, TagIds = TagSet.Create(parsed.TagIds ?? []), + + PinnedPaths = PinnedPathList.Create(parsed.PinnedPaths ?? []), }; if (!candidate.TryValidate(out _)) @@ -369,6 +390,12 @@ internal sealed class HostPayloadDocument /// which the sync engine would read as every host having changed. /// public Guid[]? TagIds { get; set; } + + /// + /// Last, and null rather than [], for exactly the reasons gives — it is the + /// newer of the two and follows the same rule. + /// + public string[]? PinnedPaths { get; set; } } [JsonSourceGenerationOptions( diff --git a/src/DodoSSH.Client.Domain/HostSecretMerge.cs b/src/DodoSSH.Client.Domain/HostSecretMerge.cs index 871d402..65e4b95 100644 --- a/src/DodoSSH.Client.Domain/HostSecretMerge.cs +++ b/src/DodoSSH.Client.Domain/HostSecretMerge.cs @@ -97,6 +97,8 @@ public static class HostSecretMerge FormatChain), Options = MergeOptions(ancestor.Options, local.Options, remote.Options, conflicts), TagIds = MergeTags(ancestor.TagIds, local.TagIds, remote.TagIds, conflicts), + PinnedPaths = MergePinnedPaths( + ancestor.PinnedPaths, local.PinnedPaths, remote.PinnedPaths, conflicts), RelayEnabled = Field( nameof(HostSecret.RelayEnabled), ancestor.RelayEnabled, @@ -290,6 +292,56 @@ public static class HostSecretMerge return TagSet.Create(merge.Merged.Keys); } + /// + /// Merges the pinned paths per path, so two people each pinning a different one both keep theirs. + /// + /// + /// + /// The same reasoning as , and the same shape: a whole-value merge would take + /// one side's list entire and drop the other's, so a colleague pinning /var/log while you pinned + /// /var/www/app would silently lose one of the two. + /// + /// + /// Unlike a tag set, order survives. is built in list + /// order rather than a canonical one, and unions keys ancestor first, + /// then local, then remote — so the merged keys come out base order, then this machine's additions, + /// then the server's, which is what then + /// preserves rather than re-sorting. + /// + /// + /// No conflict is reachable here either, for the reason gives. The value + /// in the map is the key, so a path can only be present or absent, and the "both sides moved + /// differently" branch of the keyed merge needs one key to hold two values — which this map cannot + /// express. The loop stays for the same reason it stays there: the proof depends on + /// keying by value, and that is one edit away from no longer + /// being true. + /// + /// + private static PinnedPathList MergePinnedPaths( + PinnedPathList ancestor, + PinnedPathList local, + PinnedPathList remote, + List conflicts) + { + var merge = ThreeWayMerge.Map( + ancestor.ToPathMap(), + local.ToPathMap(), + remote.ToPathMap(), + StringComparer.Ordinal); + + foreach (var conflict in merge.Conflicts) + { + conflicts.Add(new HostFieldConflict( + $"{nameof(HostSecret.PinnedPaths)}[{conflict.Key}]", + conflict.DiscardedSide, + conflict.Kept is null ? "not pinned" : "pinned", + conflict.DiscardedWasRemoval ? "not pinned" : "pinned", + conflict.DiscardedWasRemoval)); + } + + return PinnedPathList.Create(merge.Merged.Keys); + } + private static string FormatChain(JumpChain chain) => chain.Count == 0 ? "(none)" : string.Join(" → ", chain); } diff --git a/src/DodoSSH.Client.Domain/PinnedPathList.cs b/src/DodoSSH.Client.Domain/PinnedPathList.cs new file mode 100644 index 0000000..07f9b63 --- /dev/null +++ b/src/DodoSSH.Client.Domain/PinnedPathList.cs @@ -0,0 +1,175 @@ +using System.Collections; +using System.Diagnostics.CodeAnalysis; + +namespace DodoSSH.Client.Domain; + +/// +/// The remote directory paths pinned on a host, in the order the user arranged them. +/// +/// +/// +/// A dedicated type rather than a list of strings, for the reason and +/// both give: a plain on a record gets reference +/// equality from the compiler-generated Equals, so every host would read as changed on every sync +/// pass and two identical edits would register as a conflict. +/// +/// +/// Ordered, unlike ; deduplicated, unlike . Order is +/// exactly what a pinned-path list is for: it is what a quick-access menu draws top to bottom, and a user +/// who drags their most-used path to the top has expressed something a sorted set would erase. So this +/// preserves insertion order rather than sorting at construction. But two pins of the same path are not +/// two shortcuts, only one typed twice, so this still dedupes — keeping the first occurrence — by ordinal +/// comparison, because the path is meaningful only to the remote shell and that shell may well be running +/// on a case-sensitive filesystem; folding case here would silently collapse two different directories +/// into one entry. +/// +/// +/// Empty and over-length paths are not rejected at construction, for the reason +/// gives for its empty ids: they arrive from a decrypted payload written by another client, and a +/// constructor that threw would turn one bad item into a failed sync pass for every other item behind it. +/// is where that is caught. +/// +/// +public sealed class PinnedPathList : IReadOnlyList, IEquatable +{ + private readonly string[] paths; + private readonly int hash; + + private PinnedPathList(string[] paths) + { + this.paths = paths; + hash = ComputeHash(paths); + } + + /// No pinned paths. + public static PinnedPathList Empty { get; } = new([]); + + /// + public int Count => paths.Length; + + /// + public string this[int index] => paths[index]; + + /// Copies a sequence of paths, keeping order and the first occurrence of any repeat. + public static PinnedPathList Create(IEnumerable paths) + { + ArgumentNullException.ThrowIfNull(paths); + + return Canonicalise([.. paths]); + } + + /// Copies a span of paths, keeping order and the first occurrence of any repeat. + public static PinnedPathList Create(ReadOnlySpan paths) => Canonicalise(paths.ToArray()); + + /// Whether this host pins the given path, compared ordinally. + public bool Contains(string path) => Array.IndexOf(paths, path) >= 0; + + /// + /// The list as a map from path to path, in list order, which is the shape + /// takes. + /// + /// + /// + /// The value repeats the key for the reason 's does: a per-key merge + /// resolves presence and absence independently, which is set semantics with removals, so two machines + /// pinning different paths on the same host both keep theirs, and a removal on one side is reported + /// rather than silently undone. + /// + /// + /// Built in list order rather than any canonical one, and that is load-bearing rather than incidental. + /// unions keys by walking the ancestor's map, then the local side's, + /// then the remote's, each in the order its dictionary enumerates — so an ordered map here is what lets + /// reproduce "base order, then additions" after the merge without doing + /// any ordering of its own. + /// + /// + public IReadOnlyDictionary ToPathMap() + { + var map = new Dictionary(paths.Length, StringComparer.Ordinal); + + foreach (var path in paths) + { + map[path] = path; + } + + return map; + } + + /// + public bool Equals(PinnedPathList? other) + { + if (ReferenceEquals(this, other)) + { + return true; + } + + return other is not null + && other.hash == hash + && paths.AsSpan().SequenceEqual(other.paths); + } + + /// + public override bool Equals(object? obj) => Equals(obj as PinnedPathList); + + /// + public override int GetHashCode() => hash; + + /// + public IEnumerator GetEnumerator() => ((IEnumerable)paths).GetEnumerator(); + + /// + IEnumerator IEnumerable.GetEnumerator() => paths.GetEnumerator(); + + /// The paths, without copying, in list order. + public ReadOnlySpan AsSpan() => paths; + + /// Contents equality, tolerating nulls on either side. + [SuppressMessage( + "Usage", + "CA2225:Operator overloads have named alternates", + Justification = "Equals(PinnedPathList) is the named alternate.")] + public static bool operator ==(PinnedPathList? left, PinnedPathList? right) => + left is null ? right is null : left.Equals(right); + + /// Contents inequality. + public static bool operator !=(PinnedPathList? left, PinnedPathList? right) => !(left == right); + + /// + /// Order-preserving, unlike 's canonicalisation: the first occurrence of a path is + /// kept in place and later repeats are dropped, rather than the whole array being sorted. + /// + private static PinnedPathList Canonicalise(string[] paths) + { + if (paths.Length == 0) + { + return Empty; + } + + var seen = new HashSet(paths.Length, StringComparer.Ordinal); + var written = 0; + + for (var read = 0; read < paths.Length; read++) + { + if (seen.Add(paths[read])) + { + paths[written++] = paths[read]; + } + } + + return new PinnedPathList(written == paths.Length ? paths : paths[..written]); + } + + private static int ComputeHash(string[] paths) + { + // Order-sensitive, because order is the thing this type exists to preserve. + var accumulator = new HashCode(); + accumulator.Add(paths.Length); + + foreach (var path in paths) + { + accumulator.Add(path, StringComparer.Ordinal); + } + + return accumulator.ToHashCode(); + } +} diff --git a/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs b/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs index 6416e3c..a419a3a 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs @@ -39,7 +39,8 @@ internal static class HostFactory Guid? credentialId = null, bool? asksForPassword = null, Guid? groupId = null, - Guid[]? tags = null) => + Guid[]? tags = null, + string[]? pinnedPaths = null) => new() { Label = label, @@ -57,6 +58,7 @@ internal static class HostFactory AsksForPassword = asksForPassword, GroupId = groupId, TagIds = tags is null ? TagSet.Empty : TagSet.Create(tags), + PinnedPaths = pinnedPaths is null ? PinnedPathList.Empty : PinnedPathList.Create(pinnedPaths), }; /// diff --git a/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs b/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs index 3fbe358..127ed3a 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs @@ -17,9 +17,9 @@ public sealed class HostSecretCodecTests /// /// "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, and neither may sit beside AsksForPassword. 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. + /// one carries the credential, because that is the newer of the two, plus a group, a pair of tags and a + /// pair of pinned paths — which are orthogonal to the binding and are what make this host reach the + /// highest schema version a valid host can. /// [Fact] public void AFullHost_RoundTrips() @@ -37,7 +37,8 @@ public sealed class HostSecretCodecTests relayEnabled: true, credentialId: credentialId, groupId: Production, - tags: [Pci, EuWest]); + tags: [Pci, EuWest], + pinnedPaths: ["/var/www/app", "/var/log"]); HostSecretCodec.TryDecode(HostSecretCodec.Encode(host), out var document).ShouldBeTrue(); @@ -219,6 +220,90 @@ public sealed class HostSecretCodecTests document.SchemaVersion.ShouldBe(HostSecretCodec.BaseSchemaVersion); } + [Fact] + public void AHostPinningPaths_IsWrittenAtTheVersionThatIntroducedThem() + { + HostSecretCodec + .TryDecode(HostSecretCodec.Encode(Host(pinnedPaths: ["/var/www/app"])), out var document) + .ShouldBeTrue(); + + document.ShouldNotBeNull(); + document.SchemaVersion.ShouldBe(HostSecretCodec.PinnedPathsSchemaVersion); + document.Host.PinnedPaths.ShouldBe(PinnedPathList.Create(["/var/www/app"])); + } + + [Fact] + public void AHostWithNoPinnedPaths_IsWrittenAtTheVersionItWouldHaveHadWithout() + { + // Pinning nothing is not using the feature, for the same reason an empty tag set is not: bumping + // the version for it would have made every host in every vault read-only on every machine that had + // not upgraded yet. + HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(pinnedPaths: [])), out var document) + .ShouldBeTrue(); + + document.ShouldNotBeNull(); + document.SchemaVersion.ShouldBe(HostSecretCodec.BaseSchemaVersion); + } + + [Fact] + public void ATaggedHostThatAlsoPinsPaths_IsWrittenAtTheHigherOfTheTwoVersions() + { + // Independent fields, so the version is a maximum over both rather than whichever this switch + // happened to check last — the same defect the group-and-binding case guards against above. + HostSecretCodec + .TryDecode(HostSecretCodec.Encode(Host(tags: [Pci], pinnedPaths: ["/var/www/app"])), out var document) + .ShouldBeTrue(); + + document.ShouldNotBeNull(); + document.SchemaVersion.ShouldBe(HostSecretCodec.PinnedPathsSchemaVersion); + } + + [Fact] + public void AHostPinningPaths_WritesThemLastAndInListOrder() + { + // Last, so every field that existed before them keeps its bytes; in list order rather than sorted, + // unlike tags — order is the meaning of a pinned-path list, so canonicalising it away would lose + // exactly what the feature is for. + var bytes = HostSecretCodec.Encode( + Host(username: null, notes: null, pinnedPaths: ["/var/www/app", "/var/log"])); + + Encoding.UTF8.GetString(bytes).ShouldBe( + """ + {"schemaVersion":7,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false,"pinnedPaths":["/var/www/app","/var/log"]} + """); + } + + [Fact] + public void APayloadWithoutPinnedPaths_DecodesAsPinningNothing() + { + // The compatibility case this field exists to pass: a payload written before pinned paths existed + // must read cleanly as a host that pins nothing, not fail to decode. + var payload = Encoding.UTF8.GetBytes( + """ + {"schemaVersion":6,"label":"prod-db","hostname":"db.internal","port":22,"tagIds":["0192f0c8-8888-7c3d-8e4f-5a6b7c8d9e08"]} + """); + + HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue(); + + document.ShouldNotBeNull().Host.PinnedPaths.ShouldBe(PinnedPathList.Empty); + } + + [Fact] + public void APayloadRepeatingAPinnedPath_DecodesKeepingTheFirstOccurrence() + { + // Written by some other client, and it must not compare unequal to the same list written once — + // or the engine would push this host as changed on every pass for ever. + var payload = Encoding.UTF8.GetBytes( + """ + {"schemaVersion":7,"label":"prod-db","hostname":"db.internal","port":22,"pinnedPaths":["/var/www/app","/var/log","/var/www/app"]} + """); + + HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue(); + + document.ShouldNotBeNull().Host.PinnedPaths + .ShouldBe(PinnedPathList.Create(["/var/www/app", "/var/log"])); + } + [Fact] public void AddingInheritanceAndTags_DidNotChangeTheBytesOfAHostUsingNeither() { diff --git a/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs b/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs index e2c85e0..d581df8 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs @@ -58,6 +58,7 @@ public sealed class HostSecretMergeTests SshKeyId = DeployKey, GroupId = Production, TagIds = TagSet.Create([Pci]), + PinnedPaths = PinnedPathList.Create(["/var/www/app"]), }; var result = HostSecretMerge.Merge(ancestor, local, ancestor); @@ -160,6 +161,102 @@ public sealed class HostSecretMergeTests private static Guid[] Wearing(bool tagged) => tagged ? [Pci] : []; + [Fact] + public void TwoPeoplePinningDifferentPathsToOneHost_BothKeepTheirs() + { + // The same reason TagIds merges per tag rather than as a whole value: a whole-value merge would + // take one side's list entire and drop the other's. + var ancestor = Host(); + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PinnedPaths = PinnedPathList.Create(["/var/www/app"]) }, + ancestor with { PinnedPaths = PinnedPathList.Create(["/var/log"]) }); + + result.Merged.PinnedPaths.ShouldBe(PinnedPathList.Create(["/var/www/app", "/var/log"])); + result.HasConflicts.ShouldBeFalse(); + } + + [Fact] + public void PinnedPathAdditions_ComeOutInBaseOrderThenLocalThenRemote() + { + // The order guarantee this merge exists to keep: unlike a tag chip, a pinned path's position is + // part of what the user set, so the merge must produce a deterministic order rather than whatever + // a set union happens to yield. + var ancestor = Host(pinnedPaths: ["/base/one", "/base/two"]); + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with + { + PinnedPaths = PinnedPathList.Create(["/base/one", "/base/two", "/local/added"]), + }, + ancestor with + { + PinnedPaths = PinnedPathList.Create(["/base/one", "/base/two", "/remote/added"]), + }); + + result.Merged.PinnedPaths.ShouldBe( + PinnedPathList.Create(["/base/one", "/base/two", "/local/added", "/remote/added"])); + result.HasConflicts.ShouldBeFalse(); + } + + [Fact] + public void OneSideRemovingAPinnedPathWhileTheOtherAddsAnother_KeepsBothDecisions() + { + // Each path 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(pinnedPaths: ["/var/www/app"]); + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PinnedPaths = PinnedPathList.Empty }, + ancestor with + { + PinnedPaths = PinnedPathList.Create(["/var/www/app", "/var/log"]), + }); + + result.Merged.PinnedPaths.ShouldBe(PinnedPathList.Create(["/var/log"])); + result.HasConflicts.ShouldBeFalse(); + } + + [Fact] + public void APinnedPathRemovedOnBothSides_IsNotResurrected() + { + var ancestor = Host(pinnedPaths: ["/var/www/app", "/var/log"]); + var withoutApp = ancestor with { PinnedPaths = PinnedPathList.Create(["/var/log"]) }; + + var result = HostSecretMerge.Merge(ancestor, withoutApp, withoutApp); + + result.Merged.PinnedPaths.ShouldBe(PinnedPathList.Create(["/var/log"])); + result.HasConflicts.ShouldBeFalse(); + } + + /// + [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 NoArrangementOfOnePinnedPath_ProducesAConflict(bool inAncestor, bool inLocal, bool inRemote) + { + var result = HostSecretMerge.Merge( + Host(pinnedPaths: Pinning(inAncestor)), + Host(pinnedPaths: Pinning(inLocal)), + Host(pinnedPaths: Pinning(inRemote))); + + result.HasConflicts.ShouldBeFalse(); + + result.Merged.PinnedPaths.Contains("/var/www/app") + .ShouldBe(inAncestor ? inLocal && inRemote : inLocal || inRemote); + } + + private static string[] Pinning(bool pinned) => pinned ? ["/var/www/app"] : []; + [Fact] public void TwoSidesTakingDifferentPorts_NamesTheInheritedOneInTheConflict() { diff --git a/tests/DodoSSH.Client.Domain.Tests/ValueSemanticsTests.cs b/tests/DodoSSH.Client.Domain.Tests/ValueSemanticsTests.cs index 32ede4a..ab9d9aa 100644 --- a/tests/DodoSSH.Client.Domain.Tests/ValueSemanticsTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/ValueSemanticsTests.cs @@ -189,6 +189,76 @@ public sealed class ValueSemanticsTests map[EuWest].ShouldBe(EuWest); } + [Fact] + public void APinnedPathList_ComparesByContentsAndOrder() + { + // The half that differs from a tag set and matches a jump chain: a pinned-path list is what a + // shortcut menu draws top to bottom, so reordering it is a change, not a no-op. + PinnedPathList.Create(["/var/log", "/var/www/app"]) + .Equals(PinnedPathList.Create(["/var/log", "/var/www/app"])) + .ShouldBeTrue(); + + (PinnedPathList.Create(["/var/log", "/var/www/app"]) + == PinnedPathList.Create(["/var/log", "/var/www/app"])).ShouldBeTrue(); + + PinnedPathList.Create(["/var/log", "/var/www/app"]) + .Equals(PinnedPathList.Create(["/var/www/app", "/var/log"])) + .ShouldBeFalse(); + + PinnedPathList.Create(["/var/log"]) + .Equals(PinnedPathList.Create(["/var/log", "/var/www/app"])) + .ShouldBeFalse(); + + PinnedPathList.Create([]).Equals(PinnedPathList.Empty).ShouldBeTrue(); + PinnedPathList.Create(["/var/log"]).Equals(null).ShouldBeFalse(); + } + + [Fact] + public void APinnedPathList_HashesByContentsAndOrder() + { + PinnedPathList.Create(["/var/log", "/var/www/app"]).GetHashCode() + .ShouldBe(PinnedPathList.Create(["/var/log", "/var/www/app"]).GetHashCode()); + } + + [Fact] + public void APinnedPathList_ComparesPathsOrdinally() + { + // A remote path is meaningful only to the far end, and that end may run a case-sensitive + // filesystem — so folding case here would be a decision this client has no business making. + PinnedPathList.Create(["/var/log"]).Equals(PinnedPathList.Create(["/VAR/LOG"])).ShouldBeFalse(); + } + + [Fact] + public void APinnedPathList_CollapsesARepeatedPathKeepingTheFirstOccurrence() + { + // A repeat arrives from a payload some other client wrote, or from the same path pinned twice on + // one machine. Left alone it would compare unequal to the same list written once, and the engine + // would push the host as changed on every pass for ever. + PinnedPathList.Create(["/var/log", "/var/log"]).Equals(PinnedPathList.Create(["/var/log"])) + .ShouldBeTrue(); + + PinnedPathList.Create(["/var/log", "/var/log"]).Count.ShouldBe(1); + + // The first occurrence survives rather than the last, so a later repeat cannot silently move an + // earlier entry to a new position in the list. + PinnedPathList.Create(["/var/log", "/var/www/app", "/var/log"])[0].ShouldBe("/var/log"); + PinnedPathList.Create(["/var/log", "/var/www/app", "/var/log"]).Count.ShouldBe(2); + } + + [Fact] + public void APinnedPathList_MapsToItsOwnPathsInListOrder() + { + // The value repeats the key on purpose, exactly as TagSet.ToIdMap's does: 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. Unlike TagSet, the map is built in list order rather than a canonical one, which + // is what lets the merge reproduce "base order, then additions". + var map = PinnedPathList.Create(["/var/log", "/var/www/app"]).ToPathMap(); + + map.Keys.ShouldBe(new[] { "/var/log", "/var/www/app" }); + map["/var/log"].ShouldBe("/var/log"); + map["/var/www/app"].ShouldBe("/var/www/app"); + } + [Fact] public void TryValidate_RejectsWhatCannotBeStored() { @@ -200,11 +270,39 @@ public sealed class ValueSemanticsTests Host(sshKeyId: Guid.Empty).TryValidate(out _).ShouldBeFalse(); Host(credentialId: Guid.Empty).TryValidate(out _).ShouldBeFalse(); Host(tags: [Guid.Empty]).TryValidate(out _).ShouldBeFalse(); + Host(pinnedPaths: [" "]).TryValidate(out _).ShouldBeFalse(); + Host(pinnedPaths: ["/has\0nul"]).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(); + + // A relative path is exactly as valid as an absolute one: it resolves against the account's home, + // which this client never needs to know. + Host(pinnedPaths: ["relative/within/home"]).TryValidate(out _).ShouldBeTrue(); + } + + [Fact] + public void TryValidate_BoundsThePinnedPaths() + { + Host(pinnedPaths: [new string('a', HostSecret.MaxPinnedPathLength)]) + .TryValidate(out _).ShouldBeTrue(); + + Host(pinnedPaths: [new string('a', HostSecret.MaxPinnedPathLength + 1)]) + .TryValidate(out var tooLong).ShouldBeFalse(); + tooLong.ShouldNotBeNull().ShouldContain("longer than"); + + var atLimit = Enumerable.Range(0, HostSecret.MaxPinnedPaths) + .Select(i => $"/path/{i}") + .ToArray(); + Host(pinnedPaths: atLimit).TryValidate(out _).ShouldBeTrue(); + + var overLimit = Enumerable.Range(0, HostSecret.MaxPinnedPaths + 1) + .Select(i => $"/path/{i}") + .ToArray(); + Host(pinnedPaths: overLimit).TryValidate(out var tooMany).ShouldBeFalse(); + tooMany.ShouldNotBeNull().ShouldContain("cannot pin more than"); } [Fact]