Skip to content

Commit 4810592

Browse files
josecelanojosecelano
committed
Merge #226: feat: validate SSH key paths must be absolute
0ded857 feat: [#225] validate SSH key paths must be absolute (Jose Celano) Pull request description: Fixes #225 ## Summary This PR implements validation to ensure SSH key paths are absolute during `create environment` command, preventing failures later during `configure` and other commands. ## Changes - **Validation Logic**: Added `is_absolute()` checks for both private and public SSH key paths in `SshCredentialsConfig::new()` - **Error Handling**: Added two new error variants: - `RelativePrivateKeyPath` - for relative private key paths - `RelativePublicKeyPath` - for relative public key paths - **Error Messages**: Comprehensive help text with: - Clear explanation of the problem - `realpath` command examples for fixing - Rationale for why absolute paths are required - **Tests**: Added 5 new unit tests demonstrating validation behavior - **Test Updates**: Updated 23 existing tests to use absolute paths via `CARGO_MANIFEST_DIR` - **Doc Examples**: Marked 6 doc examples as `no_run` to prevent execution ## Validation ✅ All 1429 unit tests passing ✅ All 315 doctests passing ✅ All pre-commit checks passing ✅ Manual E2E testing confirmed: - Relative private key path → rejected with clear error - Relative public key path → rejected with clear error - Absolute paths → accepted and environment created successfully ## Example Error Output ``` ❌ Create command failed: Configuration validation failed: SSH private key path must be absolute: "fixtures/testing_rsa" SSH private key path must be absolute. Fix: 1. Convert relative path to absolute path: Use the `realpath` command to get the absolute path: realpath <your-relative-path> Example: - Current (relative): fixtures/testing_rsa - Command: realpath fixtures/testing_rsa - Result: /home/user/project/fixtures/testing_rsa 2. Update your configuration file with the absolute path ``` ## Benefits - **Fail Fast**: Errors caught immediately during config parsing, before any infrastructure operations - **Clear Guidance**: Users get actionable error messages with specific fix instructions - **Prevents Later Failures**: Eliminates cryptic SSH failures during `configure` and subsequent commands - **Multi-command Safety**: Ensures paths work correctly across different working directories ACKs for top commit: josecelano: ACK 0ded857 Tree-SHA512: db3327215db34f23da59010b5b11cb4c72e5a9d23b2f66ea74fa8d1b03fa2f6b173ffcda10e2d84fe908a90494bf67e23b76b5c6224e4a892c990f9de1ee71ff
2 parents bc056fa + 0ded857 commit 4810592

File tree

4 files changed

+318
-56
lines changed

4 files changed

+318
-56
lines changed

‎src/application/command_handlers/create/config/environment_config.rs‎

Lines changed: 63 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl EnvironmentCreationConfig {
121121
///
122122
/// # Examples
123123
///
124-
/// ```rust
124+
/// ```no_run
125125
/// use torrust_tracker_deployer_lib::application::command_handlers::create::config::{
126126
/// EnvironmentCreationConfig, EnvironmentSection, SshCredentialsConfig,
127127
/// ProviderSection, LxdProviderSection
@@ -195,7 +195,7 @@ impl EnvironmentCreationConfig {
195195
///
196196
/// # Examples
197197
///
198-
/// ```rust
198+
/// ```no_run
199199
/// use torrust_tracker_deployer_lib::application::command_handlers::create::config::{
200200
/// EnvironmentCreationConfig, EnvironmentSection, SshCredentialsConfig,
201201
/// ProviderSection, LxdProviderSection
@@ -611,17 +611,18 @@ mod tests {
611611

612612
#[test]
613613
fn test_convert_to_environment_params_success_auto_generated_instance_name() {
614+
use std::env;
615+
616+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
617+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
618+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
619+
614620
let config = EnvironmentCreationConfig::new(
615621
EnvironmentSection {
616622
name: "dev".to_string(),
617623
instance_name: None, // Auto-generate
618624
},
619-
SshCredentialsConfig::new(
620-
"fixtures/testing_rsa".to_string(),
621-
"fixtures/testing_rsa.pub".to_string(),
622-
"torrust".to_string(),
623-
22,
624-
),
625+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
625626
default_lxd_provider("torrust-profile-dev"),
626627
TrackerSection::default(),
627628
);
@@ -640,17 +641,18 @@ mod tests {
640641

641642
#[test]
642643
fn test_convert_to_environment_params_success_custom_instance_name() {
644+
use std::env;
645+
646+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
647+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
648+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
649+
643650
let config = EnvironmentCreationConfig::new(
644651
EnvironmentSection {
645652
name: "prod".to_string(),
646653
instance_name: Some("my-custom-instance".to_string()),
647654
},
648-
SshCredentialsConfig::new(
649-
"fixtures/testing_rsa".to_string(),
650-
"fixtures/testing_rsa.pub".to_string(),
651-
"torrust".to_string(),
652-
22,
653-
),
655+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
654656
default_lxd_provider("torrust-profile-prod"),
655657
TrackerSection::default(),
656658
);
@@ -667,17 +669,18 @@ mod tests {
667669

668670
#[test]
669671
fn test_convert_to_environment_params_invalid_environment_name() {
672+
use std::env;
673+
674+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
675+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
676+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
677+
670678
let config = EnvironmentCreationConfig::new(
671679
EnvironmentSection {
672680
name: "Invalid_Name".to_string(), // uppercase - invalid
673681
instance_name: None,
674682
},
675-
SshCredentialsConfig::new(
676-
"fixtures/testing_rsa".to_string(),
677-
"fixtures/testing_rsa.pub".to_string(),
678-
"torrust".to_string(),
679-
22,
680-
),
683+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
681684
default_lxd_provider("torrust-profile"),
682685
TrackerSection::default(),
683686
);
@@ -695,17 +698,18 @@ mod tests {
695698

696699
#[test]
697700
fn test_convert_to_environment_params_invalid_instance_name() {
701+
use std::env;
702+
703+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
704+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
705+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
706+
698707
let config = EnvironmentCreationConfig::new(
699708
EnvironmentSection {
700709
name: "dev".to_string(),
701710
instance_name: Some("invalid-".to_string()), // ends with dash - invalid
702711
},
703-
SshCredentialsConfig::new(
704-
"fixtures/testing_rsa".to_string(),
705-
"fixtures/testing_rsa.pub".to_string(),
706-
"torrust".to_string(),
707-
22,
708-
),
712+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
709713
default_lxd_provider("torrust-profile"),
710714
TrackerSection::default(),
711715
);
@@ -724,17 +728,18 @@ mod tests {
724728

725729
#[test]
726730
fn test_convert_to_environment_params_invalid_profile_name() {
731+
use std::env;
732+
733+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
734+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
735+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
736+
727737
let config = EnvironmentCreationConfig::new(
728738
EnvironmentSection {
729739
name: "dev".to_string(),
730740
instance_name: None,
731741
},
732-
SshCredentialsConfig::new(
733-
"fixtures/testing_rsa".to_string(),
734-
"fixtures/testing_rsa.pub".to_string(),
735-
"torrust".to_string(),
736-
22,
737-
),
742+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
738743
ProviderSection::Lxd(LxdProviderSection {
739744
profile_name: "invalid-".to_string(), // ends with dash - invalid
740745
}),
@@ -754,14 +759,20 @@ mod tests {
754759

755760
#[test]
756761
fn test_convert_to_environment_params_invalid_username() {
762+
use std::env;
763+
764+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
765+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
766+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
767+
757768
let config = EnvironmentCreationConfig::new(
758769
EnvironmentSection {
759770
name: "dev".to_string(),
760771
instance_name: None,
761772
},
762773
SshCredentialsConfig::new(
763-
"fixtures/testing_rsa".to_string(),
764-
"fixtures/testing_rsa.pub".to_string(),
774+
private_key_path,
775+
public_key_path,
765776
"123invalid".to_string(), // starts with number - invalid
766777
22,
767778
),
@@ -782,14 +793,19 @@ mod tests {
782793

783794
#[test]
784795
fn test_convert_to_environment_params_private_key_not_found() {
796+
use std::env;
797+
798+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
799+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
800+
785801
let config = EnvironmentCreationConfig::new(
786802
EnvironmentSection {
787803
name: "dev".to_string(),
788804
instance_name: None,
789805
},
790806
SshCredentialsConfig::new(
791807
"/nonexistent/key".to_string(),
792-
"fixtures/testing_rsa.pub".to_string(),
808+
public_key_path,
793809
"torrust".to_string(),
794810
22,
795811
),
@@ -810,13 +826,18 @@ mod tests {
810826

811827
#[test]
812828
fn test_convert_to_environment_params_public_key_not_found() {
829+
use std::env;
830+
831+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
832+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
833+
813834
let config = EnvironmentCreationConfig::new(
814835
EnvironmentSection {
815836
name: "dev".to_string(),
816837
instance_name: None,
817838
},
818839
SshCredentialsConfig::new(
819-
"fixtures/testing_rsa".to_string(),
840+
private_key_path,
820841
"/nonexistent/key.pub".to_string(),
821842
"torrust".to_string(),
822843
22,
@@ -840,18 +861,18 @@ mod tests {
840861
fn test_integration_with_environment_new() {
841862
// This test verifies that the converted parameters work with Environment::new()
842863
use crate::domain::Environment;
864+
use std::env;
865+
866+
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
867+
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
868+
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
843869

844870
let config = EnvironmentCreationConfig::new(
845871
EnvironmentSection {
846872
name: "test-env".to_string(),
847873
instance_name: None,
848874
},
849-
SshCredentialsConfig::new(
850-
"fixtures/testing_rsa".to_string(),
851-
"fixtures/testing_rsa.pub".to_string(),
852-
"torrust".to_string(),
853-
22,
854-
),
875+
SshCredentialsConfig::new(private_key_path, public_key_path, "torrust".to_string(), 22),
855876
default_lxd_provider("torrust-profile-test-env"),
856877
TrackerSection::default(),
857878
);

‎src/application/command_handlers/create/config/errors.rs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ pub enum CreateConfigError {
4646
#[error("SSH public key file not found: {path}")]
4747
PublicKeyNotFound { path: PathBuf },
4848

49+
/// SSH private key path must be absolute
50+
#[error("SSH private key path must be absolute: {path:?}")]
51+
RelativePrivateKeyPath { path: PathBuf },
52+
53+
/// SSH public key path must be absolute
54+
#[error("SSH public key path must be absolute: {path:?}")]
55+
RelativePublicKeyPath { path: PathBuf },
56+
4957
/// Invalid SSH port (must be 1-65535)
5058
#[error("Invalid SSH port: {port} (must be between 1 and 65535)")]
5159
InvalidPort { port: u16 },
@@ -201,6 +209,66 @@ impl CreateConfigError {
201209
3. Ensure you have read permissions on the file\n\
202210
4. Generate public key from private key if needed: ssh-keygen -y -f <private_key> > <public_key>"
203211
}
212+
Self::RelativePrivateKeyPath { .. } => {
213+
// Note: Can't use format! in const context, so we use a static message
214+
// The actual path will be shown in the error message itself
215+
"SSH private key path must be absolute.\n\
216+
\n\
217+
SSH key paths must be absolute to ensure they work correctly across\n\
218+
different working directories and command invocations.\n\
219+
\n\
220+
Fix:\n\
221+
1. Convert relative path to absolute path:\n\
222+
\n\
223+
Use the `realpath` command to get the absolute path:\n\
224+
\n\
225+
realpath <your-relative-path>\n\
226+
\n\
227+
Example:\n\
228+
- Current (relative): fixtures/testing_rsa\n\
229+
- Command: realpath fixtures/testing_rsa\n\
230+
- Result: /home/user/project/fixtures/testing_rsa\n\
231+
\n\
232+
2. Update your configuration file with the absolute path\n\
233+
\n\
234+
3. Alternative approaches:\n\
235+
- Use ~ for home directory (e.g., ~/.ssh/id_rsa)\n\
236+
- Use environment variables (e.g., $HOME/.ssh/id_rsa)\n\
237+
\n\
238+
Why absolute paths?\n\
239+
- Commands may run from different working directories\n\
240+
- Environment state persists paths that must remain valid\n\
241+
- Multi-command workflows (create → provision → configure)"
242+
}
243+
Self::RelativePublicKeyPath { .. } => {
244+
"SSH public key path must be absolute.\n\
245+
\n\
246+
SSH key paths must be absolute to ensure they work correctly across\n\
247+
different working directories and command invocations.\n\
248+
\n\
249+
Fix:\n\
250+
1. Convert relative path to absolute path:\n\
251+
\n\
252+
Use the `realpath` command to get the absolute path:\n\
253+
\n\
254+
realpath <your-relative-path>\n\
255+
\n\
256+
Example:\n\
257+
- Current (relative): fixtures/testing_rsa.pub\n\
258+
- Command: realpath fixtures/testing_rsa.pub\n\
259+
- Result: /home/user/project/fixtures/testing_rsa.pub\n\
260+
\n\
261+
2. Update your configuration file with the absolute path\n\
262+
\n\
263+
3. Alternative approaches:\n\
264+
- Use ~ for home directory (e.g., ~/.ssh/id_rsa.pub)\n\
265+
- Use environment variables (e.g., $HOME/.ssh/id_rsa.pub)\n\
266+
\n\
267+
Why absolute paths?\n\
268+
- Commands may run from different working directories\n\
269+
- Environment state persists paths that must remain valid\n\
270+
- Multi-command workflows (create → provision → configure)"
271+
}
204272
Self::InvalidPort { .. } => {
205273
"Invalid SSH port number.\n\
206274
\n\

‎src/application/command_handlers/create/config/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
//!
5151
//! ## Usage Example
5252
//!
53-
//! ```rust
53+
//! ```no_run
5454
//! use torrust_tracker_deployer_lib::application::command_handlers::create::config::{
5555
//! EnvironmentCreationConfig, EnvironmentSection, SshCredentialsConfig,
5656
//! ProviderSection, LxdProviderSection

0 commit comments

Comments
 (0)