Conversation
This broke invocations like:
NIX_SSHOPTS='-p2222 -oUserKnownHostsFile=/dev/null -oStrictHostKeyChecking=no' nix copy /nix/store/......-foo --to ssh-ng://root@localhost
In Nix 2.30.2, fakeSSH was enabled when the "thing I want to connect to"
was plain old "localhost". Previously, this check was written as:
, fakeSSH(host == "localhost")
Given the above invocation, `host` would have been `root@localhost`, and
thus `fakeSSH` would be `false` because `root@localhost` != `localhost`.
However, since 49ba061, `authority.host`
returned _just_ the host (`localhost`, no user) and erroneously enabled
`fakeSSH` in this case, causing `NIX_SSHOPTS` to be ignored (since,
when `fakeSSH` is `true`, `SSHMaster::startCommand` doesn't call
`addCommonSSHOpts`).
`authority.to_string()` accurately returns the expected `root@localhost`
format (given the above invocation), fixing this.
WalkthroughThe localhost detection logic in src/libstore/ssh.cc was updated to compare authority.to_string() to "localhost" instead of only authority.host. This changes when fakeSSH (and thus useMaster) is enabled. No other logic, signatures, or exports were modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant SSH as SSH Config
Caller->>SSH: configureConnection(authority)
note right of SSH: Determine localhost
rect rgba(200,230,255,0.4)
SSH->>SSH: fakeSSH = (authority.to_string() == "localhost")
end
alt fakeSSH is true
SSH->>SSH: useMaster = false (or dependent logic)
note right of SSH: Behavior tied to fakeSSH
else fakeSSH is false
SSH->>SSH: useMaster computed normally
end
SSH-->>Caller: connection parameters
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Comment |
|
Curious for other reviewers - what is fakeSSH exactly? Sniffing out Does this code (w/ this patch), account for this? |
This broke invocations like:
In Nix 2.30.2, fakeSSH was enabled when the "thing I want to connect to" was plain old "localhost". Previously, this check was written as:
Given the above invocation,
hostwould have beenroot@localhost, and thusfakeSSHwould befalsebecauseroot@localhost!=localhost.However, since 49ba061,
authority.hostreturned just the host (localhost, no user) and erroneously enabledfakeSSHin this case, causingNIX_SSHOPTSto be ignored (since, whenfakeSSHistrue,SSHMaster::startCommanddoesn't calladdCommonSSHOpts).authority.to_string()accurately returns the expectedroot@localhostformat (given the above invocation), fixing this.Motivation
Context
ref NixOS#14150
Summary by CodeRabbit