refactor(cli): remove hardcoded SSH key paths from config

[itzlambda]

Removes ssh.key_path and ssh.private_key_path from CLI config. Instead, the private key is dynamically located by matching against the public key registered with Basilica using find_private_key_for_public_key().

This simplifies configuration and enables SSH to work correctly when the user has multiple SSH keys.

Summary by CodeRabbit

Release Notes

• Refactor

  • SSH authentication mechanism refactored to derive keys dynamically from API, eliminating the need for SSH key configuration in files.
  • Added SSH connection timeout setting (30 seconds default) for improved connection stability.

• Chores

  • Removed legacy SSH key path configuration options from config files.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR refactors SSH key path handling by removing configuration-based storage and switching to dynamic discovery via API. Instead of reading SSH key paths from config, the CLI now derives the private key for the current SSH public key and explicitly passes it through SSH operation handlers.

Changes

Cohort / File(s)	Summary
Configuration removal config/cli.toml.example, crates/basilica-cli/src/config/mod.rs	Deleted SSH key-related config fields (key_path, private_key_path) and helper methods (ssh_keys_exist, ssh_keys_missing, ssh_keys_incomplete); removed path expansion/compression logic. Added connection_timeout to SSH config.
SSH client API refactoring crates/basilica-cli/src/ssh/mod.rs	Changed all SSH method signatures to accept a concrete private_key_path: PathBuf parameter instead of optional override; methods affected: ssh_access_to_connection_details, execute_command, test_connection, interactive_session, interactive_session_with_options, upload_file, download_file.
GPU rental handlers crates/basilica-cli/src/cli/handlers/gpu_rental.rs	Refactored SSH display and operation flows to derive private key path dynamically via API lookup (get_ssh_key, find_private_key_for_public_key), added private key awareness to status display, updated handler signatures (handle_status, handle_exec, handle_ssh, handle_cp) to compute and propagate private key paths; added is_private_ip helper for polling logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Verify all call sites updated to pass the concrete private_key_path instead of optional/config-based paths, particularly in GPU rental handlers (handle_status, handle_exec, handle_ssh, handle_cp)
• Examine dynamic key discovery logic (get_ssh_key, find_private_key_for_public_key) and fallback behavior when private key is not found
• Check polling and status display logic in GPU rental handlers to ensure is_private_ip helper is applied correctly
• Ensure SSH client test connection and error handling remain robust with the new mandatory path parameter

Possibly related PRs

• fix: store SSH public key on rental to enable SSH after key deletion #290: Modifies per-rental ssh_public_key surfacing and private key resolution, complementing how the main PR determines which key to use
• feat(cli): show local SSH key path in status and ssh-keys list #268: Adds local key discovery logic (find_private_key_for_public_key, find_local_public_key_path) that this PR now depends on
• feat(cli): auto-register SSH key during basilica up #231: Refactors SSH key handling away from config-stored paths toward explicit lookup/registration, aligned with the same architectural shift

Suggested reviewers

• distributedstatemachine

Poem

🐰 Keys no longer tucked in config files, now they hop through API aisles,
With dynamic discovery and paths threaded with care through each SSH mile,
Cleaner configs, explicit flows—the rabbit hops with renewed style! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing hardcoded SSH key paths from the CLI configuration and replacing them with dynamic discovery.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch juba

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

crates/basilica-cli/src/cli/handlers/gpu_rental.rs (1)

479-494: Consider extracting repeated private key lookup into a helper function.

This 12-line pattern for looking up the private key via get_ssh_key() and find_private_key_for_public_key() is duplicated at least 4 times in this file (lines 479-494, 523-537, 664-679, 699-713). Consider extracting to a helper like:

async fn get_private_key_path(api_client: &BasilicaClient) -> Result<PathBuf, CliError> {
    let ssh_key = api_client
        .get_ssh_key()
        .await
        .map_err(|e| CliError::Internal(eyre!(e)))?
        .ok_or_else(|| {
            CliError::Internal(
                eyre!("No SSH key registered with Basilica")
                    .suggestion("Run 'basilica ssh-keys add' to register your SSH key"),
            )
        })?;
    
    find_private_key_for_public_key(&ssh_key.public_key)
        .map_err(CliError::Internal)
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6019a6c and 34e63b2.

📒 Files selected for processing (4)

• config/cli.toml.example (1 hunks)
• crates/basilica-cli/src/cli/handlers/gpu_rental.rs (18 hunks)
• crates/basilica-cli/src/config/mod.rs (0 hunks)
• crates/basilica-cli/src/ssh/mod.rs (7 hunks)

💤 Files with no reviewable changes (1)

• crates/basilica-cli/src/config/mod.rs

🧰 Additional context used

🧬 Code graph analysis (1)

crates/basilica-cli/src/cli/handlers/gpu_rental.rs (2)

crates/basilica-cli/src/ssh/key_matcher.rs (1)

• find_private_key_for_public_key (24-103)

crates/basilica-cli/src/output/mod.rs (1)

• compress_path (45-53)

🔇 Additional comments (7)

crates/basilica-cli/src/ssh/mod.rs (1)

53-78: LGTM! Clean refactor to explicit private key path.

The change from optional override to required PathBuf simplifies the API contract. The existence check at line 58 with actionable suggestions is a good UX improvement. All callers are now forced at compile-time to provide a valid path.

config/cli.toml.example (1)

12-15: LGTM! Config simplified by removing SSH key path settings.

The removal of key_path and private_key_path aligns with the PR objective to use dynamic key discovery. The connection_timeout is appropriately retained for operational configuration.

crates/basilica-cli/src/cli/handlers/gpu_rental.rs (5)

1259-1269: Good use of graceful degradation for status display.

The use of ok().flatten().and_then() appropriately allows the status command to succeed even when the private key cannot be found locally. This is the right pattern for a read-only informational command.

---

1768-1769: LGTM! Efficient private key lookup using rental's public key.

Using the public key from resolve_rental_with_ssh rather than making an additional API call is efficient. The ownership transfer of private_key_path to execute_command is correct.

---

2002-2019: LGTM! Correct RFC 1918 private IP detection.

The implementation correctly checks all three RFC 1918 private address ranges. The safe default of returning false for unparseable addresses is appropriate for this use case.

---

2127-2158: LGTM! Correct handling of PathBuf in retry loop.

The use of private_key_path.clone() in the loop (line 2150) is necessary since we may retry multiple times. The final successful connection appropriately moves ownership to interactive_session (line 2157).

---

2335-2358: Good graceful degradation for SSH display when key is unavailable.

The conditional logic properly handles both cases: showing the full SSH command with private key path when available, or a basic command with a helpful warning when the key isn't found locally. This provides a good user experience.