Skip to content

feat(ssh-keys): implement SSH key management UI and functionality#112

Merged
Cleboost merged 8 commits intomasterfrom
ssh-keys
Apr 11, 2026
Merged

feat(ssh-keys): implement SSH key management UI and functionality#112
Cleboost merged 8 commits intomasterfrom
ssh-keys

Conversation

@Cleboost
Copy link
Copy Markdown
Owner

This pull request adds support for selecting and using SSH private keys for server connections throughout the application. The changes include updating the SSH host configuration to store an identity file, updating the UI to allow users to pick a key when adding or editing servers, and ensuring that SSH and SFTP connections use the selected key if provided. Additionally, the SSH keys management UI is now implemented.

Key changes:

SSH host configuration and parsing:

  • Added an identity_file field to the SshHost struct and updated SSH config parsing and writing to support the IdentityFile option. [1] [2] [3] [4]

Connection logic (SFTP/SSH):

  • Updated SFTP and SSH connection logic to attempt authentication using the configured key file first, then fall back to password or agent-based authentication, with clear error handling if all methods fail. [1] [2]
  • Added a new deploy_pubkey function for deploying public keys to remote servers, supporting all authentication methods.

User interface enhancements:

  • Updated the "Add Server" dialog to include a dropdown for selecting an SSH key, and ensured the selected key is saved to the host configuration. [1] [2] [3]
  • When launching a terminal session, the SSH command now includes the -i option if a key is specified for the host.

SSH key management UI:

  • Replaced the placeholder "SSH Keys Management - WIP" UI with a real SSH keys management UI, and added the new module to the project. [1] [2]

These changes provide users with more flexible and secure authentication options when connecting to servers.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end SSH key selection/management so server connections (terminal, SFTP, and key deployment) can use a configured private key via IdentityFile, plus a new UI for managing keys under ~/.ssh.

Changes:

  • Extend SshHost + SSH config parsing/writing to persist IdentityFile as identity_file.
  • Update connection flows (VTE ssh, SFTP, and new pubkey deployment) to attempt key-based auth first, then fall back to password/agent.
  • Implement the SSH Keys management screen (list/import/generate/delete/deploy).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/config_observer.rs Adds identity_file to host model and parses/writes IdentityFile in SSH config.
src/sftp_engine.rs Attempts SFTP auth via configured key, then password, then agent.
src/ssh_engine.rs Adds deploy_pubkey and key/password/agent auth fallback for deployments.
src/ui/add_server_dialog.rs Adds SSH key dropdown and saves the selected key path into host config.
src/ui/window.rs Uses -i when launching terminal SSH sessions; wires in SSH keys UI screen.
src/ui/ssh_keys.rs New SSH keys management UI (list/import/generate/delete/deploy).
src/ui/mod.rs Exposes the new ssh_keys module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Cleboost
Copy link
Copy Markdown
Owner Author

@copilot apply changes based on the comments in this thread

Repository owner deleted a comment from Copilot AI Apr 10, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Cleboost
Copy link
Copy Markdown
Owner Author

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits April 10, 2026 19:45
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 10, 2026

@copilot apply changes based on the comments in this thread

All review comments addressed in commits 0e9a376 and b705b33:

  • window.rs SSH_ASKPASS with -i: When identity_file is configured, SSH_ASKPASS_REQUIRE=force and SSH_ASKPASS are no longer added to the environment. This lets ssh handle key passphrase prompts natively in the VTE terminal instead of routing them through the app's askpass helper.

  • add_server_dialog.rs key dropdown preselection: The stored identity_file value is now passed through expand_tilde() before comparing with the absolute priv_path from load_ssh_keys(), so existing configs using ~/.ssh/id_rsa-style paths preselect correctly.

  • ssh_keys.rs import #[cfg(unix)]: The private key file is now written unconditionally on all platforms with explicit error handling. Unix permissions (0600) are set after a successful write.

  • ssh_keys.rs generate: ssh-keygen now runs via tokio::task::spawn_blocking in a glib::MainContext::spawn_local so the GTK main thread is not blocked. The dialog closes before the task starts, and a pre-existing file check prevents silent overwrites. stderr is included in the error dialog.

  • ssh_keys.rs import keygen: ssh-keygen -y now runs in the background the same way, with stderr surfaced in the error dialog.

  • sftp_engine.rs / ssh_engine.rs passphrase-protected keys: Auth order changed to key (no passphrase) → agent (handles passphrase-protected keys) → password, so encrypted keys loaded in ssh-agent work automatically.

  • config_observer.rs: expand_tilde() moved here as a public function shared by all modules; the duplicate private copies in ssh_engine.rs and sftp_engine.rs have been removed.

  • Bonus: All unwrap() calls on d.transient_for() replaced with a new make_error_alert(parent: Option<&gtk4::Window>, …) helper that gracefully handles a missing parent window.

@Cleboost
Copy link
Copy Markdown
Owner Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive SSH key management, allowing users to generate, import, delete, and deploy SSH keys to remote servers. The SshHost configuration has been expanded to support the IdentityFile property, and both the SFTP and SSH engines now prioritize key-based authentication and SSH agent usage over passwords. Additionally, the UI has been updated with a dedicated SSH keys management view and a key selection dropdown in the server configuration dialog. Review feedback identifies a potential data loss risk during key import due to missing file-existence checks, a lack of atomicity when writing to the SSH configuration file, and the absence of a connection timeout when establishing TCP streams.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive SSH key management, enabling users to generate, import, and deploy SSH keys directly within the application. The SshHost configuration now includes an identity_file field, which is integrated into the SFTP and terminal connection logic to support key-based authentication alongside the SSH agent and password methods. The UI has been updated with a dedicated SSH keys management view and an enhanced server dialog for key selection. Review feedback highlights critical security improvements for key file creation on Unix systems and the need for more robust checks to prevent accidental file overwrites during key generation and import.

@Cleboost Cleboost merged commit 7f71f8f into master Apr 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants