diff --git a/ldk-server-cli/src/main.rs b/ldk-server-cli/src/main.rs index 630a5084..ce661fc9 100644 --- a/ldk-server-cli/src/main.rs +++ b/ldk-server-cli/src/main.rs @@ -15,8 +15,9 @@ use clap_complete::{generate, Shell}; use hex_conservative::{DisplayHex, FromHex}; use ldk_server_client::client::LdkServerClient; use ldk_server_client::config::{ - get_default_config_path, load_config, read_tls_certificate, resolve_api_key, resolve_base_url, - resolve_cert_path, DEFAULT_GRPC_SERVICE_ADDRESS, + get_default_config_path, load_config, read_tls_certificate, resolve_api_key, + resolve_api_key_path, resolve_base_url, resolve_cert_path, Config, + DEFAULT_GRPC_SERVICE_ADDRESS, }; use ldk_server_client::error::LdkServerError; use ldk_server_client::error::LdkServerErrorCode::{ @@ -639,29 +640,27 @@ async fn main() { return; } - let config_path = cli.config.map(PathBuf::from).or_else(get_default_config_path); - let config = match config_path.as_ref() { - None => None, - Some(path) => { - if path.is_file() { - let cfg = load_config(path).unwrap_or_else(|e| { - eprintln!("Failed to load config file '{}': {}", path.display(), e); - std::process::exit(1); - }); - Some(cfg) - } else { - None - } - }, - }; + let config = load_client_config(cli.config.map(PathBuf::from)).unwrap_or_else(|e| { + eprintln!("{e}"); + std::process::exit(1); + }); + let api_key_path = resolve_api_key_path(config.as_ref()); let api_key = resolve_api_key(cli.api_key, config.as_ref()) .unwrap_or_else(|e| { eprintln!("Failed to resolve API key: {e}"); std::process::exit(1); }) .unwrap_or_else(|| { - eprintln!("API key not provided. Use --api-key or ensure the api_key file exists at {DEFAULT_DIR}/[network]/api_key"); + match api_key_path { + Some(path) => eprintln!( + "API key not provided. Use --api-key or ensure the api_key file exists at '{}'", + path.display() + ), + None => eprintln!( + "API key not provided. Use --api-key; no API key file path could be resolved from the configuration" + ), + } std::process::exit(1); }); @@ -1263,6 +1262,17 @@ async fn main() { } } +fn load_client_config(explicit_path: Option) -> Result, String> { + let config_path = explicit_path.clone().or_else(get_default_config_path); + match config_path { + Some(path) if path.is_file() => load_config(&path).map(Some), + Some(path) if explicit_path.is_some() => { + Err(format!("Config file '{}' does not exist or is not a file", path.display())) + }, + _ => Ok(None), + } +} + fn build_open_channel_config( forwarding_fee_proportional_millionths: Option, forwarding_fee_base_msat: Option, cltv_expiry_delta: Option, @@ -1446,6 +1456,18 @@ fn handle_error(e: LdkServerError) -> ! { mod tests { use super::*; + #[test] + fn load_client_config_rejects_missing_explicit_path() { + let nonce = + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let path = std::env::temp_dir() + .join(format!("ldk-server-cli-missing-config-{}-{nonce}.toml", std::process::id())); + + let error = load_client_config(Some(path.clone())).unwrap_err(); + + assert!(error.contains(&path.display().to_string())); + } + #[test] fn parse_custom_tlv_accepts_valid_record() { let (type_num, value) = parse_custom_tlv("65537:deadbeef").unwrap(); diff --git a/ldk-server-client/src/config.rs b/ldk-server-client/src/config.rs index 243ab18a..10e5b048 100644 --- a/ldk-server-client/src/config.rs +++ b/ldk-server-client/src/config.rs @@ -155,9 +155,10 @@ pub fn resolve_base_url(override_url: Option, config: Option<&Config>) - /// Resolves the API key used to authenticate against the `ldk-server` gRPC endpoint. /// -/// Prefers `override_key`, falls back to reading the API key file from the configured storage -/// directory, and finally from the OS-specific default data directory. The raw bytes read from -/// disk are lower-hex encoded before being returned. +/// Prefers `override_key`. Otherwise, reads the API key file from the configured storage +/// directory when one is set, or from the OS-specific default data directory when one is not. +/// A failed read from a configured storage directory does not fall back to the default data +/// directory. The raw bytes read from disk are lower-hex encoded before being returned. /// /// Returns an error if a candidate API key file exists but cannot be read or does not contain /// exactly 32 bytes. @@ -168,20 +169,27 @@ pub fn resolve_api_key( return Ok(override_key); } - let network = config.and_then(|c| c.network().ok()).unwrap_or_else(|| "bitcoin".to_string()); - if let Some(dir) = storage_dir(config) { - let path = api_key_path_for_storage_dir(dir, &network); - if let Some(api_key) = read_api_key(&path)? { - return Ok(Some(api_key)); - } - } - - match get_default_api_key_path(&network) { + match resolve_api_key_path(config) { Some(path) => read_api_key(&path), None => Ok(None), } } +/// Resolves the API key file path selected by [`resolve_api_key`] when no override is given. +/// +/// Uses the configured storage directory when one is set. Uses the OS-specific default data +/// directory only when no storage directory is configured. +pub fn resolve_api_key_path(config: Option<&Config>) -> Option { + let network = match config { + Some(config) => config.network().ok()?, + None => "bitcoin".to_string(), + }; + match storage_dir(config) { + Some(dir) => Some(api_key_path_for_storage_dir(dir, &network)), + None => get_default_api_key_path(&network), + } +} + fn read_api_key(path: &Path) -> Result, String> { let file = match std::fs::File::open(path) { Ok(file) => file, @@ -246,11 +254,39 @@ fn default_grpc_service_address() -> String { #[cfg(test)] mod tests { + use std::fs; + use std::sync::Mutex; + use std::time::{SystemTime, UNIX_EPOCH}; + use super::{ - load_config, read_tls_certificate, resolve_base_url, Config, CONFIG_FILE_SIZE_LIMIT, + get_default_api_key_path, load_config, read_tls_certificate, resolve_api_key, + resolve_api_key_path, resolve_base_url, Config, API_KEY_FILE, CONFIG_FILE_SIZE_LIMIT, DEFAULT_GRPC_SERVICE_ADDRESS, TLS_CERT_FILE_SIZE_LIMIT, }; + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + #[cfg(target_os = "windows")] + fn set_default_data_dir(temp_dir: &std::path::Path) -> (String, Option) { + let old_value = std::env::var("APPDATA").ok(); + std::env::set_var("APPDATA", temp_dir); + ("APPDATA".to_string(), old_value) + } + + #[cfg(not(target_os = "windows"))] + fn set_default_data_dir(temp_dir: &std::path::Path) -> (String, Option) { + let old_value = std::env::var("HOME").ok(); + std::env::set_var("HOME", temp_dir); + ("HOME".to_string(), old_value) + } + + fn restore_env_var(name: &str, value: Option) { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + #[test] fn config_defaults_grpc_service_address() { let config: Config = toml::from_str( @@ -345,6 +381,68 @@ mod tests { assert_eq!(resolve_base_url(None, None), DEFAULT_GRPC_SERVICE_ADDRESS); } + #[test] + fn resolve_api_key_rejects_unsupported_network() { + let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos(); + let storage_dir = std::env::temp_dir() + .join(format!("ldk-server-client-invalid-network-{}-{nonce}", std::process::id())); + fs::create_dir_all(storage_dir.join("bitcoin")).unwrap(); + fs::write(storage_dir.join("bitcoin").join(API_KEY_FILE), [0xAB; 32]).unwrap(); + + let config: Config = toml::from_str(&format!( + r#" + [node] + network = "bitcion" + + [storage.disk] + dir_path = "{}" + "#, + storage_dir.display() + )) + .unwrap(); + + assert!(resolve_api_key(None, Some(&config)).unwrap().is_none()); + + fs::remove_dir_all(storage_dir).unwrap(); + } + + #[test] + fn resolve_api_key_does_not_cross_storage_instances() { + let _lock = ENV_LOCK.lock().unwrap(); + let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos(); + let temp_dir = std::env::temp_dir() + .join(format!("ldk-server-client-key-instance-{}-{nonce}", std::process::id())); + let configured_storage = temp_dir.join("configured"); + fs::create_dir_all(&configured_storage).unwrap(); + + let (default_dir_env_var, old_default_dir) = set_default_data_dir(&temp_dir); + let default_api_key = get_default_api_key_path("regtest").unwrap(); + fs::create_dir_all(default_api_key.parent().unwrap()).unwrap(); + fs::write(&default_api_key, [0xAB; 32]).unwrap(); + + let config: Config = toml::from_str(&format!( + r#" + [node] + network = "regtest" + + [storage.disk] + dir_path = "{}" + "#, + configured_storage.display() + )) + .unwrap(); + + assert_eq!( + resolve_api_key_path(Some(&config)), + Some(configured_storage.join("regtest").join(API_KEY_FILE)) + ); + let resolved = resolve_api_key(None, Some(&config)); + + restore_env_var(&default_dir_env_var, old_default_dir); + fs::remove_dir_all(temp_dir).unwrap(); + assert!(resolved.unwrap().is_none()); + } + #[test] fn read_tls_certificate_rejects_oversized_file() { let path = std::env::temp_dir() diff --git a/ldk-server-mcp/src/config.rs b/ldk-server-mcp/src/config.rs index f8c066d9..2255bca2 100644 --- a/ldk-server-mcp/src/config.rs +++ b/ldk-server-mcp/src/config.rs @@ -10,8 +10,8 @@ use std::path::PathBuf; use ldk_server_client::config::{ - get_default_config_path, load_config, read_tls_certificate, resolve_api_key, resolve_base_url, - resolve_cert_path, + get_default_config_path, load_config, read_tls_certificate, resolve_api_key, + resolve_api_key_path, resolve_base_url, resolve_cert_path, }; pub struct ResolvedConfig { @@ -28,6 +28,14 @@ pub fn resolve_config(config_path: Option) -> Result) -> Result format!( + "API key not provided. Set LDK_API_KEY or ensure the api_key file exists at '{}'", + path.display() + ), + None => "API key not provided. Set LDK_API_KEY; no API key file path could be resolved from the configuration".to_string(), + })?; let tls_cert_path = resolve_cert_path(env_tls_cert_path, config.as_ref()).ok_or_else(|| { "TLS cert path not provided. Set LDK_TLS_CERT_PATH or ensure config file exists at ~/.ldk-server/config.toml" @@ -136,6 +149,7 @@ mod tests { let temp_dir = std::env::temp_dir() .join(format!("ldk-server-mcp-config-fallback-{}", std::process::id())); std::fs::create_dir_all(&temp_dir).unwrap(); + let (default_dir_env_var, old_default_dir) = set_default_data_dir(&temp_dir); let cert_path = temp_dir.join("tls.crt"); std::fs::write(&cert_path, b"test-cert").unwrap(); @@ -144,16 +158,31 @@ mod tests { std::env::set_var("LDK_API_KEY", "deadbeef"); std::env::set_var("LDK_TLS_CERT_PATH", &cert_path); std::env::remove_var("LDK_BASE_URL"); - let resolved = - resolve_config(Some(temp_dir.join("nonexistent.toml").display().to_string())).unwrap(); + let resolved = resolve_config(None).unwrap(); std::env::remove_var("LDK_API_KEY"); std::env::remove_var("LDK_TLS_CERT_PATH"); + restore_env_var(&default_dir_env_var, old_default_dir); assert_eq!(resolved.base_url, DEFAULT_GRPC_SERVICE_ADDRESS); std::fs::remove_dir_all(temp_dir).unwrap(); } + #[test] + fn resolve_config_rejects_missing_explicit_config() { + let _lock = ENV_LOCK.lock().unwrap(); + let temp_dir = std::env::temp_dir() + .join(format!("ldk-server-mcp-missing-config-{}", std::process::id())); + std::fs::create_dir_all(&temp_dir).unwrap(); + let missing_path = temp_dir.join("missing.toml"); + + let result = resolve_config(Some(missing_path.display().to_string())); + + std::fs::remove_dir_all(temp_dir).unwrap(); + let error = result.err().unwrap(); + assert!(error.contains(&missing_path.display().to_string())); + } + #[test] fn resolve_config_ignores_malformed_default_config_when_env_complete() { let _lock = ENV_LOCK.lock().unwrap(); @@ -230,4 +259,41 @@ mod tests { std::fs::remove_dir_all(temp_dir).unwrap(); } + + #[test] + fn resolve_config_reports_missing_key_in_storage_dir() { + let _lock = ENV_LOCK.lock().unwrap(); + + let temp_dir = std::env::temp_dir() + .join(format!("ldk-server-mcp-missing-storage-key-{}", std::process::id())); + let custom_storage = temp_dir.join("custom-storage"); + std::fs::create_dir_all(&custom_storage).unwrap(); + + let config_path = temp_dir.join("config.toml"); + std::fs::write( + &config_path, + format!( + r#" + [node] + network = "regtest" + + [storage.disk] + dir_path = "{}" + "#, + custom_storage.display() + ), + ) + .unwrap(); + + std::env::remove_var("LDK_API_KEY"); + std::env::remove_var("LDK_TLS_CERT_PATH"); + std::env::remove_var("LDK_BASE_URL"); + let error = resolve_config(Some(config_path.display().to_string())).err().unwrap(); + + assert!( + error.contains(&custom_storage.join("regtest").join("api_key").display().to_string()) + ); + + std::fs::remove_dir_all(temp_dir).unwrap(); + } }