Public Access
Stop a reconnect dialling a different machine of the same name
A tab remembered what its host was called and nothing else, and the reconnect under a stopped tab looked the host up by that name: HostFor took the first row in the keychain whose label matched. Nothing in this application stops two hosts being called the same thing — a team's vault beside a personal one makes it ordinary, and "prod-db" is what the database server gets called in both — so the button dialled whichever namesake sat earliest in the list. A dropped session on one machine came back as a live shell on another, with the tab, the address and the status bar all still naming the first. There was nothing on screen to read the substitution off. The name match was not an oversight; it was an argument, written into the remark on HostFor and repeated on RefreshConnectedHosts: a tab outlives the vault that opened it, so it "cannot hold an entity id that would still mean anything after a lock". That is not so. EntityId is the item's own sync identity rather than a reference to a row object, so a vault that is locked and opened again rebuilds every row around the same ids. The one handle that was rejected as too fragile to survive a lock is the handle that survives it. So the id now travels with the connection. ConnectionAttemptEventArgs and TerminalSessionEventArgs carry it, taken from ConnectionTarget.HostId, which the connect path has had all along for the log; TerminalTabViewModel keeps it; and HostFor matches on it. A host renamed since the tab opened is found too, which a name match by definition cannot. Null is not missing data — it is the whole of what makes a typed-in address different, and it is why such a tab is refused a reconnect rather than offered one that would dial a stranger sharing its name. Three other lookups had grown their own copy of the same match and so had the same fault, where it is read rather than dialled: the green connected dot, which lit on every machine answering to the name and claimed a shell on hosts nobody had connected to; the pinned paths QUICK ACCESS offers, which come from whichever row that same match found; and the sidebar's "Pin folder" and the SFTP tab row, which would have edited and browsed the wrong host. All four now go through HostFor, so "which machine is this tab" has one answer. Both new tests assert on the address actually dialled rather than on anything the tab says about itself. The two rows agree on every string a tab carries, which is exactly why this went unnoticed: a test written against the label would have passed on the day the wrong machine was being opened.
This commit is contained in:
1 parent
9ead42171c
commit
2d95366552
4 files changed
+177
-36
No files matched your search
@@ -947,6 +947,7 @@ internal static class ItemBadge
|
||||
/// <param name="identityLabel">
|
||||
/// The display name of the key or credential that authenticated, or null when a typed password did.
|
||||
/// </param>
|
||||
/// <param name="hostId">The keychain item this was dialled from, or null for a typed-in address.</param>
|
||||
internal sealed class TerminalSessionEventArgs(
|
||||
Guid attemptId,
|
||||
uint sessionId,
|
||||
@@ -954,7 +955,8 @@ internal sealed class TerminalSessionEventArgs(
|
||||
string address,
|
||||
string cipher,
|
||||
string hostKeyAlgorithm,
|
||||
string? identityLabel) : EventArgs
|
||||
string? identityLabel,
|
||||
Guid? hostId) : EventArgs
|
||||
{
|
||||
/// <summary>Which attempt this session came out of.</summary>
|
||||
/// <inheritdoc cref="ConnectionAttemptEventArgs.AttemptId" path="/remarks" />
|
||||
@@ -971,19 +973,27 @@ internal sealed class TerminalSessionEventArgs(
|
||||
internal string HostKeyAlgorithm { get; } = hostKeyAlgorithm;
|
||||
|
||||
internal string? IdentityLabel { get; } = identityLabel;
|
||||
|
||||
/// <inheritdoc cref="ConnectionAttemptEventArgs.HostId" />
|
||||
internal Guid? HostId { get; } = hostId;
|
||||
}
|
||||
|
||||
/// <summary>A connection that has been asked for, and has not answered yet.</summary>
|
||||
/// <param name="attemptId">Identifies this attempt for the whole of its life.</param>
|
||||
/// <param name="label">The host's name, as the vault has it.</param>
|
||||
/// <param name="address">Who this will be logged in as, and where.</param>
|
||||
/// <param name="hostId">The keychain item this is being dialled from, or null for a typed-in address.</param>
|
||||
/// <remarks>
|
||||
/// The vault says a connection has started before it says whether it worked, so that the shell can put a
|
||||
/// tab in the strip at the moment the user asks for one rather than however many seconds later a handshake
|
||||
/// takes. Everything a tab needs to name itself is here, because the name is a decrypted item and the shell
|
||||
/// has no vault to read it from.
|
||||
/// </remarks>
|
||||
internal sealed class ConnectionAttemptEventArgs(Guid attemptId, string label, string address) : EventArgs
|
||||
internal sealed class ConnectionAttemptEventArgs(
|
||||
Guid attemptId,
|
||||
string label,
|
||||
string address,
|
||||
Guid? hostId) : EventArgs
|
||||
{
|
||||
/// <summary>Identifies this attempt for the whole of its life.</summary>
|
||||
/// <remarks>
|
||||
@@ -995,6 +1005,19 @@ internal sealed class ConnectionAttemptEventArgs(Guid attemptId, string label, s
|
||||
internal string Label { get; } = label;
|
||||
|
||||
internal string Address { get; } = address;
|
||||
|
||||
/// <summary>
|
||||
/// The keychain item this connection was made from, or null for a machine that is not in the keychain.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// ◆ <b>The name is not an identifier and this is.</b> Two hosts may be called the same thing — nothing
|
||||
/// stops it, and a team vault next to a personal one makes it ordinary — so a tab that remembered only
|
||||
/// what it was called could be matched back to the wrong machine. That is not a cosmetic mix-up: the
|
||||
/// reconnect below the tab dials whichever row the match found, so the one thing that must not be
|
||||
/// guessed is which machine a reconnect goes to. Carried from here because a tab has no vault to ask —
|
||||
/// see <c>TerminalTabViewModel.HostId</c>.
|
||||
/// </remarks>
|
||||
internal Guid? HostId { get; } = hostId;
|
||||
}
|
||||
|
||||
/// <summary>A connection that has got as far as a named step.</summary>
|
||||
@@ -11326,7 +11349,8 @@ internal sealed partial class VaultViewModel(
|
||||
var attempt = new ConnectionAttemptEventArgs(
|
||||
Guid.CreateVersion7(),
|
||||
target.Label,
|
||||
Dialled(target, authentication));
|
||||
Dialled(target, authentication),
|
||||
target.HostId);
|
||||
|
||||
ConnectionStarting?.Invoke(this, attempt);
|
||||
Status = $"Connecting to {target.Label}…";
|
||||
@@ -11784,7 +11808,8 @@ internal sealed partial class VaultViewModel(
|
||||
Dialled(target, authentication),
|
||||
facts?.Cipher ?? string.Empty,
|
||||
facts?.HostKeyAlgorithm ?? string.Empty,
|
||||
authentication.IdentityLabel));
|
||||
authentication.IdentityLabel,
|
||||
target.HostId));
|
||||
|
||||
// Last, and after the tab exists: keeping the password is a favour, and the session the user asked
|
||||
// for must not wait on a vault write to appear.
|
||||
|
||||
Reference in new issue
Block a user