Skip to content

Harden Config Parsing / Validation #268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benthecarman wants to merge 4 commits into
base: main
Choose a base branch
from
+154 −30
Open

Harden Config Parsing / Validation #268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c66065
Reject invalid client networks
benthecarman Aug 28, 2026
90071e5
Keep API keys within one instance
benthecarman Aug 28, 2026
d0d84d9
Reject missing explicit CLI configs
benthecarman Aug 28, 2026
72341f0
Reject missing explicit MCP configs
benthecarman Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions ldk-server-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ 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,
resolve_cert_path, Config, DEFAULT_GRPC_SERVICE_ADDRESS,
};
use ldk_server_client::error::LdkServerError;
use ldk_server_client::error::LdkServerErrorCode::{
Expand Down Expand Up @@ -639,21 +639,10 @@ 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| {
Expand Down Expand Up @@ -1263,6 +1252,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>,
Expand Down Expand Up @@ -1446,6 +1446,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();
Expand Down
112 changes: 100 additions & 12 deletions ldk-server-client/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,17 +168,19 @@ 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) {
Some(path) => read_api_key(&path),
None => Ok(None),
let network = match config {
Some(config) => match config.network() {
Ok(network) => network,
Err(_) => return Ok(None),
},
None => "bitcoin".to_string(),
};
match storage_dir(config) {
Some(dir) => read_api_key(&api_key_path_for_storage_dir(dir, &network)),
None => match get_default_api_key_path(&network) {
Some(path) => read_api_key(&path),
None => Ok(None),
},
}
}

Expand Down Expand Up @@ -246,11 +248,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_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(
Expand Down Expand Up @@ -345,6 +375,64 @@ 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();

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()
Expand Down
28 changes: 26 additions & 2 deletions ldk-server-mcp/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -136,6 +144,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();
Expand All @@ -144,16 +153,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();
Expand Down
Loading