fix: handle SFTP host key confirmation - #2015
Conversation
AI-assisted: OpenAI Codex helped investigate, implement, test, and review this change. I reviewed and understand the final changes.
AI-assisted: OpenAI Codex helped investigate, implement, test, and review this change. I reviewed and understand the final changes.
c499cb9 to
e3abc2b
Compare
jacobgkau
left a comment
There was a problem hiding this comment.
This is the new dialog:
I have some problems with the grammar of this message. However, it appears it's coming from GVFS, not COSMIC Files: https://github.com/GNOME/gvfs/blob/061802831d94e8aff45172f3ca18f953cf348b61/daemon/gvfsbackendsftp.c#L1350 So it may not be trivial for us to customize. And passing along GVFS's flawed message is objectively better than our current behavior of erroring out with no way to actually connect without resorting to other apps.
With that said, having both a Cancel Login and a Cancel button is awkward. I understand the former comes from GVFS and the latter comes from COSMIC Files, but users might not understand the difference. Does GVFS have any questions that wouldn't include a Cancel option?
It appears Nautilus, Thunar, and Nemo all pop the dialog with the expectation one of the options will be sufficient (and a lower-level option to abort is unnecessary):
So we can probably just drop the second Cancel button and rely on GVFS to provide a cancel option in these questions, at least for now.
It would probably also be best to lay the buttons out horizontally, to better match our other dialogs. And if we can get the order flipped so the affirmative is farthest to the right (as those other three GVFS-using apps do), and add an appropriate icon, then that would also better match our other dialogs, e.g.:
Summary
GIO asks the mount operation to confirm an uncached SFTP host key, but COSMIC Files did not handle that question and reported
Login dialog cancelled.Fixes #580.
Testing
cargo test --offline --all-features --lib --bins— 45 passedcargo check --offline --features gvfsgit diff HEAD^ --checkLive verification on
c499cb9was captured across two runs: one confirmed that Escape leaves the first-connection fingerprint question visible, and another confirmed that the same build can open the remote SFTP root. The combined Escape → continued mount sequence was not captured as one run. Automated coverage verifies that Escape preserves the active question and cannot dismiss the next queued question.