Harden Config Parsing / Validation

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,29 @@ 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 = 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 resolve_api_key_path(config.as_ref()).unwrap_or_else(|e| {
+				eprintln!("Failed to resolve API key: {e}");
+				std::process::exit(1);
+			}) {
+				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 +1264,17 @@ async fn main() {
 	}
 }
 
+fn load_client_config(explicit_path: Option<PathBuf>) -> Result<Option<Config>, 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<u32>, forwarding_fee_base_msat: Option<u32>,
 	cltv_expiry_delta: Option<u32>,
@@ -1446,6 +1458,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();

ldk-server-client/src/config.rs

@@ -155,33 +155,43 @@ pub fn resolve_base_url(override_url: Option<String>, 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.
+/// Returns an error if the configured network is unsupported, or if a candidate API key file
+/// exists but cannot be read or does not contain exactly 32 bytes.
 pub fn resolve_api_key(
 	override_key: Option<String>, config: Option<&Config>,
 ) -> Result<Option<String>, String> {
 	if override_key.is_some() {
 		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.
+///
+/// Returns an error if the configured network is unsupported.
+pub fn resolve_api_key_path(config: Option<&Config>) -> Result<Option<PathBuf>, String> {
+	let network = match config {
+		Some(config) => config.network()?,
+		None => "bitcoin".to_string(),
+	};
+	Ok(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<Option<String>, String> {
 	let file = match std::fs::File::open(path) {
 		Ok(file) => file,
@@ -246,11 +256,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<String>) {
+		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<String>) {
+		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<String>) {
+		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 +383,75 @@ 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 = "invalid-network"
+
+				[storage.disk]
+				dir_path = "{}"
+			"#,
+			storage_dir.display()
+		))
+		.unwrap();
+
+		assert_eq!(
+			resolve_api_key_path(Some(&config)).unwrap_err(),
+			"Unsupported network: invalid-network"
+		);
+		assert_eq!(
+			resolve_api_key(None, Some(&config)).unwrap_err(),
+			"Unsupported network: invalid-network"
+		);
+
+		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)).unwrap(),
+			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()