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,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<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 +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();

ldk-server-client/src/config.rs

@@ -155,9 +155,10 @@ 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.
@@ -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<PathBuf> {
+	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<Option<String>, 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<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 +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()

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<String>) -> Result<ResolvedConfig, Str
 		env_base_url.is_some() && env_api_key.is_some() && env_tls_cert_path.is_some();
 
 	let explicit_config_path = config_path.map(PathBuf::from);
+	if let Some(path) = &explicit_config_path {
+		if !path.is_file() {
+			return Err(format!(
+				"Config file '{}' does not exist or is not a file",
+				path.display()
+			));
+		}
+	}
 	let config_path = explicit_config_path.clone().or_else(get_default_config_path);
 	let config = match config_path {
 		Some(ref path)
@@ -40,9 +48,14 @@ pub fn resolve_config(config_path: Option<String>) -> Result<ResolvedConfig, Str
 
 	let base_url = resolve_base_url(env_base_url, config.as_ref());
 
-	let api_key = resolve_api_key(env_api_key, config.as_ref())?.ok_or_else(
-		|| "API key not provided. Set LDK_API_KEY or ensure the api_key file exists at ~/.ldk-server/[network]/api_key".to_string()
-	)?;
+	let api_key_path = resolve_api_key_path(config.as_ref());
+	let api_key = resolve_api_key(env_api_key, config.as_ref())?.ok_or_else(|| match api_key_path {
+		Some(path) => 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();
+	}
 }