From 1e9e43e496bdee19f095ddcfd5754837e7209659 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 11:55:01 +0200 Subject: [PATCH 01/44] fix(node-lib): keep secrets out of the command line log --- node-lib/src/runner.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 78fdc86765..d6dbd55cc9 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -41,7 +41,7 @@ use crate::{ config_files::{DEFAULT_P2P_NETWORKING_ENABLED, DEFAULT_RPC_ENABLED, NodeConfigFile}, mock_time::set_mock_time, node_controller::NodeController, - options::{OptionsWithResolvedCommand, RunOptions, default_data_dir}, + options::{Command, OptionsWithResolvedCommand, RunOptions, default_data_dir}, }; const LOCK_FILE_NAME: &str = ".lock"; @@ -303,7 +303,20 @@ pub async fn setup( return Ok(NodeSetupResult::DataDirCleanedUp); } - logging::log::info!("Command line options: {options:?}"); + // The full command line contains secrets (RPC credentials, staking keys), so only a + // curated subset of options that cannot hold secrets is logged. + let network = match options.command { + Command::Mainnet(_) => "mainnet", + Command::Testnet(_) => "testnet", + Command::Regtest(_) => "regtest", + }; + logging::log::info!( + "Command line options (secrets omitted): network: {network}, data directory: {:?}, \ + create data dir if missing: {:?}, log to file: {:?}", + options.top_level.data_dir, + options.top_level.create_data_dir_if_missing, + options.top_level.log_to_file, + ); let mut options = options; From e7ab8f42e2c57f5365b594de4e3152ed2eb0228d Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 11:55:07 +0200 Subject: [PATCH 02/44] fix(api-server): keep secrets out of the command line log --- api-server/scanner-daemon/src/main.rs | 16 +++++++++++++++- api-server/web-server/src/main.rs | 24 +++++++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/api-server/scanner-daemon/src/main.rs b/api-server/scanner-daemon/src/main.rs index 7480bb54f3..55a8d1ce99 100644 --- a/api-server/scanner-daemon/src/main.rs +++ b/api-server/scanner-daemon/src/main.rs @@ -31,7 +31,21 @@ async fn main() -> Result<(), ApiServerScannerError> { let args = ApiServerScannerArgs::parse(); logging::init_logging(); - logging::log::info!("Command line options: {args:?}"); + // The full command line contains secrets (RPC credentials, database password), so only a + // curated subset of options that cannot hold secrets is logged. + logging::log::info!( + "Command line options (secrets omitted): network: {:?}, node RPC address: {:?}, \ + node RPC cookie file: {:?}, postgres host: {}, port: {}, user: {}, database: {:?}, \ + max connections: {}", + args.network, + args.node_rpc_address, + args.node_rpc_cookie_file, + args.postgres_config.postgres_host, + args.postgres_config.postgres_port, + args.postgres_config.postgres_user, + args.postgres_config.postgres_database, + args.postgres_config.postgres_max_connections, + ); let ApiServerScannerArgs { network, diff --git a/api-server/web-server/src/main.rs b/api-server/web-server/src/main.rs index 19790092ef..e70a14c508 100644 --- a/api-server/web-server/src/main.rs +++ b/api-server/web-server/src/main.rs @@ -65,7 +65,29 @@ async fn main() -> Result<(), ApiServerWebServerInitError> { logging::init_logging(); let args = ApiServerWebServerConfig::parse(); - log::info!("Command line options: {args:?}"); + // The full command line contains secrets (RPC credentials, database password), so only a + // curated subset of options that cannot hold secrets is logged. + log::info!( + "Command line options (secrets omitted): network: {:?}, bind address: {:?}, \ + enable post routes: {}, node RPC address: {:?}, node RPC cookie file: {:?}, \ + postgres host: {}, port: {}, user: {}, database: {:?}, max connections: {}, \ + stream events broadcast capacity: {}, max subscribers: {}, poll interval: {}s, \ + keepalive interval: {}s", + args.network, + args.bind_address, + args.enable_post_routes, + args.node_rpc_address, + args.node_rpc_cookie_file, + args.postgres_config.postgres_host, + args.postgres_config.postgres_port, + args.postgres_config.postgres_user, + args.postgres_config.postgres_database, + args.postgres_config.postgres_max_connections, + args.stream_events_broadcast_capacity, + args.stream_events_max_subscribers, + args.stream_events_poll_interval_secs, + args.stream_events_keepalive_interval_secs, + ); let chain_type: ChainType = args.network.into(); let chain_config = Arc::new(Builder::new(chain_type).build()); From 386f224a4d0271b7e79d50c2d2eff8ff7944317d Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 11:58:57 +0200 Subject: [PATCH 03/44] fix(node-lib): reuse the existing network name helper in the options log --- node-lib/src/runner.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index d6dbd55cc9..8fc9ccac45 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -41,7 +41,7 @@ use crate::{ config_files::{DEFAULT_P2P_NETWORKING_ENABLED, DEFAULT_RPC_ENABLED, NodeConfigFile}, mock_time::set_mock_time, node_controller::NodeController, - options::{Command, OptionsWithResolvedCommand, RunOptions, default_data_dir}, + options::{OptionsWithResolvedCommand, RunOptions, default_data_dir}, }; const LOCK_FILE_NAME: &str = ".lock"; @@ -305,14 +305,10 @@ pub async fn setup( // The full command line contains secrets (RPC credentials, staking keys), so only a // curated subset of options that cannot hold secrets is logged. - let network = match options.command { - Command::Mainnet(_) => "mainnet", - Command::Testnet(_) => "testnet", - Command::Regtest(_) => "regtest", - }; logging::log::info!( - "Command line options (secrets omitted): network: {network}, data directory: {:?}, \ + "Command line options (secrets omitted): network: {}, data directory: {:?}, \ create data dir if missing: {:?}, log to file: {:?}", + options.command.chain_type().name(), options.top_level.data_dir, options.top_level.create_data_dir_if_missing, options.top_level.log_to_file, From 97023afaa3d854084c337809f902f73d925f59c0 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 13:49:26 +0200 Subject: [PATCH 04/44] fix(api-server): redact credentials in debug output and log summary --- api-server/api-server-common/src/lib.rs | 20 +++++- api-server/scanner-daemon/src/config.rs | 63 ++++++++++++++++- api-server/scanner-daemon/src/main.rs | 17 ++--- api-server/web-server/src/config.rs | 92 ++++++++++++++++++++++++- api-server/web-server/src/main.rs | 26 ++----- 5 files changed, 180 insertions(+), 38 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index b0ce699678..62280d308b 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -38,7 +38,7 @@ impl From for ChainType { } } -#[derive(Parser, Debug)] +#[derive(Parser)] pub struct PostgresConfig { /// Postgres host #[clap(long, default_value = "localhost")] @@ -65,6 +65,24 @@ pub struct PostgresConfig { pub postgres_max_connections: u32, } +// The password must never appear in debug output; anything that embeds this struct in a +// derived Debug (command line option dumps, error types) is safe by construction. +impl std::fmt::Debug for PostgresConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PostgresConfig") + .field("postgres_host", &self.postgres_host) + .field("postgres_port", &self.postgres_port) + .field("postgres_user", &self.postgres_user) + .field( + "postgres_password", + &self.postgres_password.as_ref().map(|_| ""), + ) + .field("postgres_database", &self.postgres_database) + .field("postgres_max_connections", &self.postgres_max_connections) + .finish() + } +} + impl Default for PostgresConfig { fn default() -> Self { Self::parse() diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index f112f35b65..db70bb0e2e 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -19,7 +19,7 @@ use api_server_common::{Network, PostgresConfig}; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; -#[derive(Parser, Debug)] +#[derive(Parser)] #[clap(mut_args(clap_utils::env_adder("API_SCANNER_DAEMON")))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerScannerArgs { @@ -47,3 +47,64 @@ pub struct ApiServerScannerArgs { #[clap(flatten)] pub postgres_config: PostgresConfig, } + +// RPC credentials and the database password must never appear in debug output; a debug +// dump of the whole struct is therefore safe by construction. +impl std::fmt::Debug for ApiServerScannerArgs { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ApiServerScannerArgs") + .field("network", &self.network) + .field("node_rpc_address", &self.node_rpc_address) + .field("node_rpc_cookie_file", &self.node_rpc_cookie_file) + .field( + "node_rpc_username", + &self.node_rpc_username.as_ref().map(|_| ""), + ) + .field( + "node_rpc_password", + &self.node_rpc_password.as_ref().map(|_| ""), + ) + .field("postgres_config", &self.postgres_config) + .finish() + } +} + +impl ApiServerScannerArgs { + /// A loggable summary of the options, omitting every value that can hold a secret. + /// Keep this method, not a struct dump, in log statements. + pub fn redacted_summary(&self) -> String { + // PostgresConfig's Debug output redacts the database password. + format!( + "network: {:?}, node RPC address: {:?}, node RPC cookie file: {:?}, postgres: {:?}", + self.network, self.node_rpc_address, self.node_rpc_cookie_file, self.postgres_config, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn redacted_summary_omits_secrets() { + let args = ApiServerScannerArgs::parse_from([ + "scanner-daemon", + "--node-rpc-username", + "alice", + "--node-rpc-password", + "supersecret", + "--postgres-password", + "dbsecret", + ]); + let summary = args.redacted_summary(); + assert!(summary.contains("network: Mainnet")); + assert!(summary.contains("postgres")); + assert!(!summary.contains("alice")); + assert!(!summary.contains("supersecret")); + assert!(!summary.contains("dbsecret")); + let debug_dump = format!("{args:?}"); + assert!(!debug_dump.contains("alice")); + assert!(!debug_dump.contains("supersecret")); + assert!(!debug_dump.contains("dbsecret")); + } +} diff --git a/api-server/scanner-daemon/src/main.rs b/api-server/scanner-daemon/src/main.rs index 55a8d1ce99..42c89c5de4 100644 --- a/api-server/scanner-daemon/src/main.rs +++ b/api-server/scanner-daemon/src/main.rs @@ -31,20 +31,11 @@ async fn main() -> Result<(), ApiServerScannerError> { let args = ApiServerScannerArgs::parse(); logging::init_logging(); - // The full command line contains secrets (RPC credentials, database password), so only a - // curated subset of options that cannot hold secrets is logged. + // The command line contains secrets (RPC credentials, database password), so the summary + // logged here omits them; see ApiServerScannerArgs::redacted_summary. logging::log::info!( - "Command line options (secrets omitted): network: {:?}, node RPC address: {:?}, \ - node RPC cookie file: {:?}, postgres host: {}, port: {}, user: {}, database: {:?}, \ - max connections: {}", - args.network, - args.node_rpc_address, - args.node_rpc_cookie_file, - args.postgres_config.postgres_host, - args.postgres_config.postgres_port, - args.postgres_config.postgres_user, - args.postgres_config.postgres_database, - args.postgres_config.postgres_max_connections, + "Command line options (secrets omitted): {}", + args.redacted_summary(), ); let ApiServerScannerArgs { diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 0a94a8f63e..6f10e81284 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -29,7 +29,7 @@ use api_server_common::streaming; const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; -#[derive(Debug, Parser)] +#[derive(Parser)] #[clap(mut_args(clap_utils::env_adder("API_WEB_SRV")))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerWebServerConfig { @@ -113,6 +113,96 @@ pub struct ApiServerWebServerConfig { pub stream_events_keepalive_interval_secs: u64, } +// RPC credentials and the database password must never appear in debug output; a debug +// dump of the whole struct is therefore safe by construction. +impl std::fmt::Debug for ApiServerWebServerConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ApiServerWebServerConfig") + .field("network", &self.network) + .field("bind_address", &self.bind_address) + .field("node_rpc_address", &self.node_rpc_address) + .field("node_rpc_cookie_file", &self.node_rpc_cookie_file) + .field( + "node_rpc_username", + &self.node_rpc_username.as_ref().map(|_| ""), + ) + .field( + "node_rpc_password", + &self.node_rpc_password.as_ref().map(|_| ""), + ) + .field("postgres_config", &self.postgres_config) + .field("enable_post_routes", &self.enable_post_routes) + .field( + "stream_events_broadcast_capacity", + &self.stream_events_broadcast_capacity, + ) + .field( + "stream_events_max_subscribers", + &self.stream_events_max_subscribers, + ) + .field( + "stream_events_poll_interval_secs", + &self.stream_events_poll_interval_secs, + ) + .field( + "stream_events_keepalive_interval_secs", + &self.stream_events_keepalive_interval_secs, + ) + .finish() + } +} + +impl ApiServerWebServerConfig { + /// A loggable summary of the options, omitting every value that can hold a secret. + /// Keep this method, not a struct dump, in log statements. + pub fn redacted_summary(&self) -> String { + // PostgresConfig's Debug output redacts the database password. + format!( + "network: {:?}, bind address: {:?}, enable post routes: {}, node RPC address: {:?}, \ + node RPC cookie file: {:?}, postgres: {:?}, stream events broadcast capacity: {}, \ + max subscribers: {}, poll interval: {}s, keepalive interval: {}s", + self.network, + self.bind_address, + self.enable_post_routes, + self.node_rpc_address, + self.node_rpc_cookie_file, + self.postgres_config, + self.stream_events_broadcast_capacity, + self.stream_events_max_subscribers, + self.stream_events_poll_interval_secs, + self.stream_events_keepalive_interval_secs, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn redacted_summary_omits_secrets() { + let args = ApiServerWebServerConfig::parse_from([ + "web-server", + "--node-rpc-username", + "alice", + "--node-rpc-password", + "supersecret", + "--postgres-password", + "dbsecret", + ]); + let summary = args.redacted_summary(); + assert!(summary.contains("network: Mainnet")); + assert!(summary.contains("postgres")); + assert!(!summary.contains("alice")); + assert!(!summary.contains("supersecret")); + assert!(!summary.contains("dbsecret")); + let debug_dump = format!("{args:?}"); + assert!(!debug_dump.contains("alice")); + assert!(!debug_dump.contains("supersecret")); + assert!(!debug_dump.contains("dbsecret")); + } +} + #[derive(Clone, Debug, Parser)] pub struct ListenAddress { socket: SocketAddr, diff --git a/api-server/web-server/src/main.rs b/api-server/web-server/src/main.rs index e70a14c508..b8c32e25b7 100644 --- a/api-server/web-server/src/main.rs +++ b/api-server/web-server/src/main.rs @@ -14,7 +14,6 @@ // limitations under the License. mod api; -mod config; mod error; use api_server_common::storage::impls::postgres::{ @@ -65,28 +64,11 @@ async fn main() -> Result<(), ApiServerWebServerInitError> { logging::init_logging(); let args = ApiServerWebServerConfig::parse(); - // The full command line contains secrets (RPC credentials, database password), so only a - // curated subset of options that cannot hold secrets is logged. + // The command line contains secrets (RPC credentials, database password), so the summary + // logged here omits them; see ApiServerWebServerConfig::redacted_summary. log::info!( - "Command line options (secrets omitted): network: {:?}, bind address: {:?}, \ - enable post routes: {}, node RPC address: {:?}, node RPC cookie file: {:?}, \ - postgres host: {}, port: {}, user: {}, database: {:?}, max connections: {}, \ - stream events broadcast capacity: {}, max subscribers: {}, poll interval: {}s, \ - keepalive interval: {}s", - args.network, - args.bind_address, - args.enable_post_routes, - args.node_rpc_address, - args.node_rpc_cookie_file, - args.postgres_config.postgres_host, - args.postgres_config.postgres_port, - args.postgres_config.postgres_user, - args.postgres_config.postgres_database, - args.postgres_config.postgres_max_connections, - args.stream_events_broadcast_capacity, - args.stream_events_max_subscribers, - args.stream_events_poll_interval_secs, - args.stream_events_keepalive_interval_secs, + "Command line options (secrets omitted): {}", + args.redacted_summary(), ); let chain_type: ChainType = args.network.into(); From 401b572b9b387f580519f805da4175cdc52f1942 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 14:25:28 +0200 Subject: [PATCH 05/44] test(api-server): assert option output keeps every expected field --- api-server/api-server-common/src/lib.rs | 13 +++++++++++++ api-server/scanner-daemon/src/config.rs | 6 ++++++ api-server/web-server/src/config.rs | 6 ++++++ 3 files changed, 25 insertions(+) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 62280d308b..b596db3358 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -88,3 +88,16 @@ impl Default for PostgresConfig { Self::parse() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn debug_output_redacts_postgres_password() { + let config = PostgresConfig::parse_from(["postgres-config", "--postgres-password", "dbsecret"]); + let dump = format!("{config:?}"); + assert!(dump.contains("postgres_password: Some(\"\")")); + assert!(!dump.contains("dbsecret")); + } +} diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index db70bb0e2e..324313ac3e 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -98,11 +98,17 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); + assert!(summary.contains("node RPC address: None")); + assert!(summary.contains("node RPC cookie file: None")); assert!(summary.contains("postgres")); + assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); + assert!(debug_dump.contains("ApiServerScannerArgs")); + assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); + assert!(debug_dump.contains("postgres_password: Some(\"\")")); assert!(!debug_dump.contains("alice")); assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 6f10e81284..49c1dcd474 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -192,11 +192,17 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); + assert!(summary.contains("bind address: None")); + assert!(summary.contains("enable post routes: false")); assert!(summary.contains("postgres")); + assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); + assert!(debug_dump.contains("ApiServerWebServerConfig")); + assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); + assert!(debug_dump.contains("postgres_password: Some(\"\")")); assert!(!debug_dump.contains("alice")); assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); From be305648058f589c78e6c1421f5fc4c6c8e21b2c Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 14:35:12 +0200 Subject: [PATCH 06/44] test(api-server): verify option output keeps non-secret fields visible --- api-server/api-server-common/src/lib.rs | 9 ++++++++- api-server/scanner-daemon/src/config.rs | 1 + api-server/web-server/src/config.rs | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index b596db3358..6f87707afa 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -95,8 +95,15 @@ mod tests { #[test] fn debug_output_redacts_postgres_password() { - let config = PostgresConfig::parse_from(["postgres-config", "--postgres-password", "dbsecret"]); + let config = PostgresConfig::parse_from([ + "postgres-config", + "--postgres-host", + "dbhost", + "--postgres-password", + "dbsecret", + ]); let dump = format!("{config:?}"); + assert!(dump.contains("postgres_host: \"dbhost\"")); assert!(dump.contains("postgres_password: Some(\"\")")); assert!(!dump.contains("dbsecret")); } diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 324313ac3e..21020dfcbe 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -107,6 +107,7 @@ mod tests { assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); assert!(debug_dump.contains("ApiServerScannerArgs")); + assert!(debug_dump.contains("node_rpc_username: Some(\"\")")); assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); assert!(debug_dump.contains("postgres_password: Some(\"\")")); assert!(!debug_dump.contains("alice")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 49c1dcd474..dfa09ca931 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -201,6 +201,7 @@ mod tests { assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); assert!(debug_dump.contains("ApiServerWebServerConfig")); + assert!(debug_dump.contains("node_rpc_username: Some(\"\")")); assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); assert!(debug_dump.contains("postgres_password: Some(\"\")")); assert!(!debug_dump.contains("alice")); From dc1f9710bbb4c254dcf6d10a33442f464427df8d Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 14:35:38 +0200 Subject: [PATCH 07/44] fix(node-lib): restrict log file permissions to the file owner --- node-lib/src/runner.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 8fc9ccac45..7f7d7964a7 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -276,7 +276,9 @@ pub async fn setup( ContentLimit::Bytes(10_000_000), // 10MB each Compression::None, #[cfg(unix)] - None, + // Logs can contain sensitive information, so they are only readable by the owner, + // like the RPC cookie file. + Some(0o600), ); logging::init_logging_generic( main_log_writer_settings, From 1f857f96fb9e689938ba07d4b8e64f754870c2a5 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 14:46:14 +0200 Subject: [PATCH 08/44] fix(node-lib): restrict permissions of existing log files on startup --- node-lib/src/runner.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 7f7d7964a7..7bd61fbe67 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -270,6 +270,21 @@ pub async fn setup( ) }, ); + // Log files written by earlier versions may have wider permissions; restrict them too. + // Failures are ignored: this is best-effort hardening of existing files. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Ok(entries) = std::fs::read_dir(data_dir.join("logs")) { + for entry in entries.flatten() { + let path = entry.path(); + if path.to_string_lossy().contains(".log") { + let _ = + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); + } + } + } + } let log_file = FileRotate::new( data_dir.join(format!("logs/{}", log_file_name)), AppendCount::new(13), // total 14 files From 8020c9c59c2e4303dd71bc55b4768e8bcf5dcf89 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 14:50:54 +0200 Subject: [PATCH 09/44] fix(node-lib): only adjust permissions of regular log files --- node-lib/src/runner.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 7bd61fbe67..797c0bb138 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -277,10 +277,16 @@ pub async fn setup( use std::os::unix::fs::PermissionsExt; if let Ok(entries) = std::fs::read_dir(data_dir.join("logs")) { for entry in entries.flatten() { - let path = entry.path(); - if path.to_string_lossy().contains(".log") { - let _ = - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); + // Skip directories and symlinks so the mode is never applied through a link + // or to anything but a regular file. + if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { + continue; + } + if entry.file_name().to_string_lossy().contains(".log") { + let _ = std::fs::set_permissions( + entry.path(), + std::fs::Permissions::from_mode(0o600), + ); } } } From ee0a614996043ec3eb314f1d58380550a9f89ea1 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 15:22:42 +0200 Subject: [PATCH 10/44] fix(options): redact rpc credentials and tighten debug output --- api-server/api-server-common/src/lib.rs | 24 +++++++--- api-server/scanner-daemon/src/config.rs | 33 +++++++++---- api-server/web-server/src/config.rs | 52 ++++++++++++++------ node-lib/src/config_files/rpc.rs | 6 +-- node-lib/src/lib.rs | 2 +- node-lib/src/options.rs | 64 +++++++++++++++++++++++-- node-lib/src/runner.rs | 9 ++-- node-lib/tests/cli.rs | 4 +- 8 files changed, 152 insertions(+), 42 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 6f87707afa..257fae5473 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -66,19 +66,29 @@ pub struct PostgresConfig { } // The password must never appear in debug output; anything that embeds this struct in a -// derived Debug (command line option dumps, error types) is safe by construction. +// derived Debug (command line option dumps, error types) is safe by construction. The +// destructure has no `..` so a newly added field fails to compile until it is either added +// here or shown to be safe. impl std::fmt::Debug for PostgresConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let Self { + postgres_host, + postgres_port, + postgres_user, + postgres_password, + postgres_database, + postgres_max_connections, + } = self; f.debug_struct("PostgresConfig") - .field("postgres_host", &self.postgres_host) - .field("postgres_port", &self.postgres_port) - .field("postgres_user", &self.postgres_user) + .field("postgres_host", postgres_host) + .field("postgres_port", postgres_port) + .field("postgres_user", postgres_user) .field( "postgres_password", - &self.postgres_password.as_ref().map(|_| ""), + &postgres_password.as_ref().map(|_| ""), ) - .field("postgres_database", &self.postgres_database) - .field("postgres_max_connections", &self.postgres_max_connections) + .field("postgres_database", postgres_database) + .field("postgres_max_connections", postgres_max_connections) .finish() } } diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 21020dfcbe..860cbdc76e 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -52,19 +52,29 @@ pub struct ApiServerScannerArgs { // dump of the whole struct is therefore safe by construction. impl std::fmt::Debug for ApiServerScannerArgs { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Destructure without `..` so a newly added field fails to compile until it is either + // added here or shown to be safe. + let Self { + network, + node_rpc_address, + node_rpc_cookie_file, + node_rpc_username, + node_rpc_password, + postgres_config, + } = self; f.debug_struct("ApiServerScannerArgs") - .field("network", &self.network) - .field("node_rpc_address", &self.node_rpc_address) - .field("node_rpc_cookie_file", &self.node_rpc_cookie_file) + .field("network", network) + .field("node_rpc_address", node_rpc_address) + .field("node_rpc_cookie_file", node_rpc_cookie_file) .field( "node_rpc_username", - &self.node_rpc_username.as_ref().map(|_| ""), + &node_rpc_username.as_ref().map(|_| ""), ) .field( "node_rpc_password", - &self.node_rpc_password.as_ref().map(|_| ""), + &node_rpc_password.as_ref().map(|_| ""), ) - .field("postgres_config", &self.postgres_config) + .field("postgres_config", postgres_config) .finish() } } @@ -87,8 +97,14 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { + // Every asserted value is passed explicitly so the test cannot be affected by + // ML_API_SCANNER_DAEMON_* environment variables. let args = ApiServerScannerArgs::parse_from([ "scanner-daemon", + "--network", + "mainnet", + "--node-rpc-cookie-file", + "/tmp/rpc-cookie-file", "--node-rpc-username", "alice", "--node-rpc-password", @@ -98,8 +114,9 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("node RPC address: None")); - assert!(summary.contains("node RPC cookie file: None")); + // The value of the address depends on the environment, so only its label is asserted. + assert!(summary.contains("node RPC address:")); + assert!(summary.contains("node RPC cookie file: Some(\"/tmp/rpc-cookie-file\")")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index dfa09ca931..02d6617e8b 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -117,36 +117,52 @@ pub struct ApiServerWebServerConfig { // dump of the whole struct is therefore safe by construction. impl std::fmt::Debug for ApiServerWebServerConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Destructure without `..` so a newly added field fails to compile until it is either + // added here or shown to be safe. + let Self { + network, + bind_address, + postgres_config, + enable_post_routes, + node_rpc_address, + node_rpc_cookie_file, + node_rpc_username, + node_rpc_password, + stream_events_broadcast_capacity, + stream_events_max_subscribers, + stream_events_poll_interval_secs, + stream_events_keepalive_interval_secs, + } = self; f.debug_struct("ApiServerWebServerConfig") - .field("network", &self.network) - .field("bind_address", &self.bind_address) - .field("node_rpc_address", &self.node_rpc_address) - .field("node_rpc_cookie_file", &self.node_rpc_cookie_file) + .field("network", network) + .field("bind_address", bind_address) + .field("node_rpc_address", node_rpc_address) + .field("node_rpc_cookie_file", node_rpc_cookie_file) .field( "node_rpc_username", - &self.node_rpc_username.as_ref().map(|_| ""), + &node_rpc_username.as_ref().map(|_| ""), ) .field( "node_rpc_password", - &self.node_rpc_password.as_ref().map(|_| ""), + &node_rpc_password.as_ref().map(|_| ""), ) - .field("postgres_config", &self.postgres_config) - .field("enable_post_routes", &self.enable_post_routes) + .field("postgres_config", postgres_config) + .field("enable_post_routes", enable_post_routes) .field( "stream_events_broadcast_capacity", - &self.stream_events_broadcast_capacity, + stream_events_broadcast_capacity, ) .field( "stream_events_max_subscribers", - &self.stream_events_max_subscribers, + stream_events_max_subscribers, ) .field( "stream_events_poll_interval_secs", - &self.stream_events_poll_interval_secs, + stream_events_poll_interval_secs, ) .field( "stream_events_keepalive_interval_secs", - &self.stream_events_keepalive_interval_secs, + stream_events_keepalive_interval_secs, ) .finish() } @@ -181,8 +197,15 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { + // Every asserted value is passed explicitly so the test cannot be affected by + // ML_API_WEB_SERVER_* environment variables. let args = ApiServerWebServerConfig::parse_from([ "web-server", + "--network", + "mainnet", + "--bind-address", + "127.0.0.1:3000", + "--enable-post-routes", "--node-rpc-username", "alice", "--node-rpc-password", @@ -192,8 +215,9 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("bind address: None")); - assert!(summary.contains("enable post routes: false")); + assert!(summary.contains("bind address: Some")); + assert!(summary.contains("127.0.0.1:3000")); + assert!(summary.contains("enable post routes: true")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); diff --git a/node-lib/src/config_files/rpc.rs b/node-lib/src/config_files/rpc.rs index fab55c463d..7428519b80 100644 --- a/node-lib/src/config_files/rpc.rs +++ b/node-lib/src/config_files/rpc.rs @@ -15,7 +15,7 @@ use std::{net::SocketAddr, str::FromStr}; -use crate::RunOptions; +use crate::{Redacted, RunOptions}; use chainstate_launcher::ChainConfig; use serde::{Deserialize, Serialize}; @@ -70,8 +70,8 @@ impl RpcConfigFile { .rpc_enabled .unwrap_or_else(|| rpc_enabled.unwrap_or(DEFAULT_RPC_ENABLED)); - let username = username.or(options.rpc_username.clone()); - let password = password.or(options.rpc_password.clone()); + let username = username.or(options.rpc_username.clone().map(Redacted::into_inner)); + let password = password.or(options.rpc_password.clone().map(Redacted::into_inner)); let cookie_file = cookie_file.or(options.rpc_cookie_file.clone()); RpcConfigFile { diff --git a/node-lib/src/lib.rs b/node-lib/src/lib.rs index 0bdb1a3df4..649914ba72 100644 --- a/node-lib/src/lib.rs +++ b/node-lib/src/lib.rs @@ -34,7 +34,7 @@ pub use config_files::{ pub use node_daemon_runner::{ExitCode, run_node_daemon}; pub use options::{ CLEAN_DATA_OPTION_LONG_NAME, Command, IMPORT_BOOTSTRAP_FILE_OPTION_LONG_NAME, Options, - OptionsWithResolvedCommand, RunOptions, TopLevelOptions, + OptionsWithResolvedCommand, Redacted, RunOptions, TopLevelOptions, }; pub use runner::{NodeSetupResult, NodeType, setup}; diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index da61692d88..42ab86e81d 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -17,9 +17,11 @@ use std::{ ffi::OsString, + fmt, net::{IpAddr, SocketAddr}, num::NonZeroU64, path::{Path, PathBuf}, + str::FromStr, }; use clap::{Args, CommandFactory, FromArgMatches, Parser, Subcommand}; @@ -237,6 +239,43 @@ pub struct RegtestOptions { pub chain_config: ChainConfigOptions, } +/// A value that must never appear in debug output, meant for command line options holding +/// credentials so that any debug formatting of the containing struct stays secret-safe. +#[derive(Clone, Default)] +pub struct Redacted(T); + +impl fmt::Debug for Redacted { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("") + } +} + +impl From for Redacted { + fn from(value: T) -> Self { + Self(value) + } +} + +impl FromStr for Redacted { + type Err = T::Err; + + fn from_str(s: &str) -> Result { + T::from_str(s).map(Self) + } +} + +impl Redacted { + pub fn into_inner(self) -> T { + self.0 + } +} + +impl AsRef for Redacted { + fn as_ref(&self) -> &T { + &self.0 + } +} + #[derive(Args, Clone, Debug, Default)] pub struct RunOptions { /// Minimum number of connected peers to enable block production. @@ -412,12 +451,12 @@ pub struct RunOptions { /// Username for RPC server basic authorization. /// If not set, the cookie file is created. #[clap(long, value_name = "USERNAME")] - pub rpc_username: Option, + pub rpc_username: Option>, /// Password for RPC server basic authorization. /// If not set, the RPC cookie file is created. #[clap(long, value_name = "PASSWORD")] - pub rpc_password: Option, + pub rpc_password: Option>, /// Custom file path for the RPC cookie file. /// If not set, the cookie file is created in the data dir. @@ -451,7 +490,7 @@ pub fn default_data_dir(chain_type: ChainType) -> PathBuf { #[cfg(test)] mod tests { - use std::{collections::BTreeMap, str::FromStr as _}; + use std::collections::BTreeMap; use rstest::rstest; @@ -576,3 +615,22 @@ mod tests { } } } + +#[cfg(test)] +mod redacted_tests { + use super::*; + + #[test] + fn run_options_debug_omits_rpc_credentials() { + let options = RunOptions { + rpc_username: Some("rpc_user_name".to_owned().into()), + rpc_password: Some("rpc_pass_word".to_owned().into()), + ..Default::default() + }; + + let debug_output = format!("{options:?}"); + assert!(debug_output.contains(""), "{debug_output}"); + assert!(!debug_output.contains("rpc_user_name"), "{debug_output}"); + assert!(!debug_output.contains("rpc_pass_word"), "{debug_output}"); + } +} diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 797c0bb138..e3091245ec 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -271,14 +271,15 @@ pub async fn setup( }, ); // Log files written by earlier versions may have wider permissions; restrict them too. - // Failures are ignored: this is best-effort hardening of existing files. + // This is best-effort hardening: the check and the mode change are separate steps, so a + // path swapped in between is only skipped or left unchanged, and failures are ignored. #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; if let Ok(entries) = std::fs::read_dir(data_dir.join("logs")) { for entry in entries.flatten() { - // Skip directories and symlinks so the mode is never applied through a link - // or to anything but a regular file. + // Skip directories and symlinks so the mode is normally never applied + // through a link or to anything but a regular file. if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { continue; } @@ -332,7 +333,7 @@ pub async fn setup( "Command line options (secrets omitted): network: {}, data directory: {:?}, \ create data dir if missing: {:?}, log to file: {:?}", options.command.chain_type().name(), - options.top_level.data_dir, + data_dir, options.top_level.create_data_dir_if_missing, options.top_level.log_to_file, ); diff --git a/node-lib/tests/cli.rs b/node-lib/tests/cli.rs index 7fbf9820b3..a86ff88fd3 100644 --- a/node-lib/tests/cli.rs +++ b/node-lib/tests/cli.rs @@ -173,8 +173,8 @@ fn read_config_override_values() { max_tip_age: Some(max_tip_age), rpc_bind_address: Some(rpc_bind_address), rpc_enabled: Some(true), - rpc_username: Some(rpc_username.to_owned()), - rpc_password: Some(rpc_password.to_owned()), + rpc_username: Some(rpc_username.to_owned().into()), + rpc_password: Some(rpc_password.to_owned().into()), rpc_cookie_file: Some(rpc_cookie_file.to_owned()), min_tx_relay_fee_rate: Some(min_tx_relay_fee_rate), force_allow_run_as_root_outer: Default::default(), From 2d7406444059a9937b5b355ff54e20ebe0ce4d48 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 15:50:11 +0200 Subject: [PATCH 11/44] fix(node-lib): keep rpc credentials out of the startup config log --- api-server/scanner-daemon/src/config.rs | 15 +++++++--- api-server/web-server/src/config.rs | 10 +++++-- node-lib/src/config_files/rpc.rs | 8 ++--- node-lib/src/options.rs | 40 +++++++++++++++++++++++++ node-lib/src/runner.rs | 15 ++++++---- node-lib/tests/cli.rs | 4 +-- 6 files changed, 74 insertions(+), 18 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 860cbdc76e..ca8e282039 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -97,25 +97,32 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { - // Every asserted value is passed explicitly so the test cannot be affected by - // ML_API_SCANNER_DAEMON_* environment variables. + // Every value that can also come from the ML_API_SCANNER_DAEMON_* environment + // variables is passed explicitly, so the test is fully hermetic. let args = ApiServerScannerArgs::parse_from([ "scanner-daemon", "--network", "mainnet", + "--node-rpc-address", + "127.0.0.1:12345", "--node-rpc-cookie-file", "/tmp/rpc-cookie-file", "--node-rpc-username", "alice", "--node-rpc-password", "supersecret", + "--postgres-host", + "dbhost", + "--postgres-user", + "dbuser", "--postgres-password", "dbsecret", ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); - // The value of the address depends on the environment, so only its label is asserted. - assert!(summary.contains("node RPC address:")); + assert!(summary.contains("node RPC address: Some(")); + assert!(summary.contains("127.0.0.1")); + assert!(summary.contains("12345")); assert!(summary.contains("node RPC cookie file: Some(\"/tmp/rpc-cookie-file\")")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 02d6617e8b..a95bd3197d 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -197,8 +197,8 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { - // Every asserted value is passed explicitly so the test cannot be affected by - // ML_API_WEB_SERVER_* environment variables. + // Every value that can also come from the ML_API_WEB_SRV_* environment variables + // is passed explicitly, so the test is fully hermetic. let args = ApiServerWebServerConfig::parse_from([ "web-server", "--network", @@ -206,10 +206,16 @@ mod tests { "--bind-address", "127.0.0.1:3000", "--enable-post-routes", + "--node-rpc-address", + "127.0.0.1:12345", "--node-rpc-username", "alice", "--node-rpc-password", "supersecret", + "--postgres-host", + "dbhost", + "--postgres-user", + "dbuser", "--postgres-password", "dbsecret", ]); diff --git a/node-lib/src/config_files/rpc.rs b/node-lib/src/config_files/rpc.rs index 7428519b80..a0f27a7235 100644 --- a/node-lib/src/config_files/rpc.rs +++ b/node-lib/src/config_files/rpc.rs @@ -33,10 +33,10 @@ pub struct RpcConfigFile { pub bind_address: Option, /// Username for RPC server basic authorization - pub username: Option, + pub username: Option>, /// Password for RPC server basic authorization - pub password: Option, + pub password: Option>, /// Custom file path for the RPC cookie file pub cookie_file: Option, @@ -70,8 +70,8 @@ impl RpcConfigFile { .rpc_enabled .unwrap_or_else(|| rpc_enabled.unwrap_or(DEFAULT_RPC_ENABLED)); - let username = username.or(options.rpc_username.clone().map(Redacted::into_inner)); - let password = password.or(options.rpc_password.clone().map(Redacted::into_inner)); + let username = username.or(options.rpc_username.clone()); + let password = password.or(options.rpc_password.clone()); let cookie_file = cookie_file.or(options.rpc_cookie_file.clone()); RpcConfigFile { diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 42ab86e81d..d104681440 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -276,6 +276,26 @@ impl AsRef for Redacted { } } +// Serialization writes the real value so configuration files can round-trip credentials; +// only debug output is redacted. +impl serde::Serialize for Redacted +where + T: serde::Serialize, +{ + fn serialize(&self, serializer: S) -> Result { + self.0.serialize(serializer) + } +} + +impl<'de, T> serde::Deserialize<'de> for Redacted +where + T: serde::Deserialize<'de>, +{ + fn deserialize>(deserializer: D) -> Result { + T::deserialize(deserializer).map(Self) + } +} + #[derive(Args, Clone, Debug, Default)] pub struct RunOptions { /// Minimum number of connected peers to enable block production. @@ -633,4 +653,24 @@ mod redacted_tests { assert!(!debug_output.contains("rpc_user_name"), "{debug_output}"); assert!(!debug_output.contains("rpc_pass_word"), "{debug_output}"); } + + #[test] + fn rpc_config_file_debug_omits_rpc_credentials() { + let options = RunOptions { + rpc_username: Some("rpc_user_name".to_owned().into()), + rpc_password: Some("rpc_pass_word".to_owned().into()), + ..Default::default() + }; + + let config = crate::RpcConfigFile::with_run_options( + &common::chain::config::create_testnet(), + crate::RpcConfigFile::default(), + &options, + ); + + let debug_output = format!("{config:#?}"); + assert!(debug_output.contains(""), "{debug_output}"); + assert!(!debug_output.contains("rpc_user_name"), "{debug_output}"); + assert!(!debug_output.contains("rpc_pass_word"), "{debug_output}"); + } } diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index e3091245ec..4bfc384502 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -202,8 +202,8 @@ async fn initialize( if rpc_config.rpc_enabled.unwrap_or(DEFAULT_RPC_ENABLED) { let rpc_creds = RpcCreds::new( data_dir, - rpc_config.username.as_deref(), - rpc_config.password.as_deref(), + rpc_config.username.as_ref().map(|value| value.as_ref().as_str()), + rpc_config.password.as_ref().map(|value| value.as_ref().as_str()), rpc_config.cookie_file.as_deref(), )?; @@ -284,10 +284,13 @@ pub async fn setup( continue; } if entry.file_name().to_string_lossy().contains(".log") { - let _ = std::fs::set_permissions( - entry.path(), - std::fs::Permissions::from_mode(0o600), - ); + // Open the file and change the mode through the descriptor so the + // change cannot be redirected to a different path after the open; + // failures are ignored. + let _ = + std::fs::OpenOptions::new().write(true).open(entry.path()).and_then( + |file| file.set_permissions(std::fs::Permissions::from_mode(0o600)), + ); } } } diff --git a/node-lib/tests/cli.rs b/node-lib/tests/cli.rs index a86ff88fd3..e969ad12e4 100644 --- a/node-lib/tests/cli.rs +++ b/node-lib/tests/cli.rs @@ -325,11 +325,11 @@ fn read_config_override_values() { assert!(config.rpc.clone().unwrap().rpc_enabled.unwrap()); assert_eq!( - config.rpc.as_ref().unwrap().username.as_deref(), + config.rpc.as_ref().unwrap().username.as_ref().map(|v| v.as_ref().as_str()), Some(rpc_username) ); assert_eq!( - config.rpc.as_ref().unwrap().password.as_deref(), + config.rpc.as_ref().unwrap().password.as_ref().map(|v| v.as_ref().as_str()), Some(rpc_password) ); assert_eq!( From 0d99e75a908989e8dbd76200099cac444e964830 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 16:00:55 +0200 Subject: [PATCH 12/44] fix(api-server): cover node rpc address in redacted summary tests --- api-server/scanner-daemon/src/config.rs | 3 ++- api-server/web-server/src/config.rs | 4 +++- node-lib/src/config_files/rpc.rs | 4 ++-- node-lib/src/runner.rs | 5 +++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index ca8e282039..19a567c20c 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -98,7 +98,8 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { // Every value that can also come from the ML_API_SCANNER_DAEMON_* environment - // variables is passed explicitly, so the test is fully hermetic. + // variables is passed explicitly; values not asserted on may still come from the + // environment. let args = ApiServerScannerArgs::parse_from([ "scanner-daemon", "--network", diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index a95bd3197d..584bdd284d 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -198,7 +198,7 @@ mod tests { #[test] fn redacted_summary_omits_secrets() { // Every value that can also come from the ML_API_WEB_SRV_* environment variables - // is passed explicitly, so the test is fully hermetic. + // is passed explicitly; values not asserted on may still come from the environment. let args = ApiServerWebServerConfig::parse_from([ "web-server", "--network", @@ -224,6 +224,8 @@ mod tests { assert!(summary.contains("bind address: Some")); assert!(summary.contains("127.0.0.1:3000")); assert!(summary.contains("enable post routes: true")); + assert!(summary.contains("node RPC address: Some(")); + assert!(summary.contains("12345")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); diff --git a/node-lib/src/config_files/rpc.rs b/node-lib/src/config_files/rpc.rs index a0f27a7235..b37f50afbc 100644 --- a/node-lib/src/config_files/rpc.rs +++ b/node-lib/src/config_files/rpc.rs @@ -70,8 +70,8 @@ impl RpcConfigFile { .rpc_enabled .unwrap_or_else(|| rpc_enabled.unwrap_or(DEFAULT_RPC_ENABLED)); - let username = username.or(options.rpc_username.clone()); - let password = password.or(options.rpc_password.clone()); + let username = username.or_else(|| options.rpc_username.clone()); + let password = password.or_else(|| options.rpc_password.clone()); let cookie_file = cookie_file.or(options.rpc_cookie_file.clone()); RpcConfigFile { diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 4bfc384502..53ab7c2f52 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -285,8 +285,9 @@ pub async fn setup( } if entry.file_name().to_string_lossy().contains(".log") { // Open the file and change the mode through the descriptor so the - // change cannot be redirected to a different path after the open; - // failures are ignored. + // change is not applied to a path that is swapped in afterwards; + // a path swapped in before the open can still be followed. Failures + // are ignored. let _ = std::fs::OpenOptions::new().write(true).open(entry.path()).and_then( |file| file.set_permissions(std::fs::Permissions::from_mode(0o600)), From a08d40f5420213224ab876260cd94789c1bb217a Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 16:06:10 +0200 Subject: [PATCH 13/44] fix(api-server): tie rpc address assertions to the port field --- api-server/scanner-daemon/src/config.rs | 2 +- api-server/web-server/src/config.rs | 3 ++- node-lib/src/config_files/rpc.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 19a567c20c..125700164e 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -123,7 +123,7 @@ mod tests { assert!(summary.contains("network: Mainnet")); assert!(summary.contains("node RPC address: Some(")); assert!(summary.contains("127.0.0.1")); - assert!(summary.contains("12345")); + assert!(summary.contains("port: 12345")); assert!(summary.contains("node RPC cookie file: Some(\"/tmp/rpc-cookie-file\")")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 584bdd284d..6cbe3b4fa2 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -225,7 +225,8 @@ mod tests { assert!(summary.contains("127.0.0.1:3000")); assert!(summary.contains("enable post routes: true")); assert!(summary.contains("node RPC address: Some(")); - assert!(summary.contains("12345")); + assert!(summary.contains("127.0.0.1")); + assert!(summary.contains("port: 12345")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(!summary.contains("alice")); diff --git a/node-lib/src/config_files/rpc.rs b/node-lib/src/config_files/rpc.rs index b37f50afbc..c7e8ff8b87 100644 --- a/node-lib/src/config_files/rpc.rs +++ b/node-lib/src/config_files/rpc.rs @@ -72,7 +72,7 @@ impl RpcConfigFile { let username = username.or_else(|| options.rpc_username.clone()); let password = password.or_else(|| options.rpc_password.clone()); - let cookie_file = cookie_file.or(options.rpc_cookie_file.clone()); + let cookie_file = cookie_file.or_else(|| options.rpc_cookie_file.clone()); RpcConfigFile { bind_address: Some(bind_address), From 5d4651384ddaad9ce253b35b36f8c8d4f02a4576 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 16:23:22 +0200 Subject: [PATCH 14/44] fix(node-lib): fix log file modes even when files are not writable --- node-lib/src/runner.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 53ab7c2f52..7f07c56b33 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -288,10 +288,9 @@ pub async fn setup( // change is not applied to a path that is swapped in afterwards; // a path swapped in before the open can still be followed. Failures // are ignored. - let _ = - std::fs::OpenOptions::new().write(true).open(entry.path()).and_then( - |file| file.set_permissions(std::fs::Permissions::from_mode(0o600)), - ); + let _ = std::fs::OpenOptions::new().read(true).open(entry.path()).and_then( + |file| file.set_permissions(std::fs::Permissions::from_mode(0o600)), + ); } } } From 6ce84084a6e22d7483f45a685af2d042f4d18d89 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 16:36:46 +0200 Subject: [PATCH 15/44] refactor(api-server): share the redaction marker between debug outputs --- api-server/api-server-common/src/lib.rs | 13 +++++++++---- api-server/scanner-daemon/src/config.rs | 12 +++--------- api-server/web-server/src/config.rs | 12 +++--------- node-lib/src/options.rs | 6 ++++++ 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 257fae5473..a56dfa669b 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -19,6 +19,14 @@ pub mod streaming; use clap::Parser; use common::chain::config::ChainType; +/// Marker printed in place of any secret-bearing value. +pub const REDACTED: &str = ""; + +/// Debug-safe replacement for an `Option` that may hold a secret. +pub fn redact(value: &Option) -> Option<&'static str> { + value.as_ref().map(|_| REDACTED) +} + #[derive(clap::ValueEnum, Clone, Debug)] pub enum Network { Mainnet, @@ -83,10 +91,7 @@ impl std::fmt::Debug for PostgresConfig { .field("postgres_host", postgres_host) .field("postgres_port", postgres_port) .field("postgres_user", postgres_user) - .field( - "postgres_password", - &postgres_password.as_ref().map(|_| ""), - ) + .field("postgres_password", &redact(postgres_password)) .field("postgres_database", postgres_database) .field("postgres_max_connections", postgres_max_connections) .finish() diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 125700164e..879b2e3dad 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -15,7 +15,7 @@ use clap::Parser; -use api_server_common::{Network, PostgresConfig}; +use api_server_common::{Network, PostgresConfig, redact}; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; @@ -66,14 +66,8 @@ impl std::fmt::Debug for ApiServerScannerArgs { .field("network", network) .field("node_rpc_address", node_rpc_address) .field("node_rpc_cookie_file", node_rpc_cookie_file) - .field( - "node_rpc_username", - &node_rpc_username.as_ref().map(|_| ""), - ) - .field( - "node_rpc_password", - &node_rpc_password.as_ref().map(|_| ""), - ) + .field("node_rpc_username", &redact(node_rpc_username)) + .field("node_rpc_password", &redact(node_rpc_password)) .field("postgres_config", postgres_config) .finish() } diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 6cbe3b4fa2..900143ce8e 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -17,7 +17,7 @@ use std::{net::SocketAddr, ops::Deref}; use clap::Parser; -use api_server_common::{Network, PostgresConfig}; +use api_server_common::{Network, PostgresConfig, redact}; use tokio::net::TcpListener; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; @@ -138,14 +138,8 @@ impl std::fmt::Debug for ApiServerWebServerConfig { .field("bind_address", bind_address) .field("node_rpc_address", node_rpc_address) .field("node_rpc_cookie_file", node_rpc_cookie_file) - .field( - "node_rpc_username", - &node_rpc_username.as_ref().map(|_| ""), - ) - .field( - "node_rpc_password", - &node_rpc_password.as_ref().map(|_| ""), - ) + .field("node_rpc_username", &redact(node_rpc_username)) + .field("node_rpc_password", &redact(node_rpc_password)) .field("postgres_config", postgres_config) .field("enable_post_routes", enable_post_routes) .field( diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index d104681440..544254faaa 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -241,6 +241,12 @@ pub struct RegtestOptions { /// A value that must never appear in debug output, meant for command line options holding /// credentials so that any debug formatting of the containing struct stays secret-safe. +/// A value that must never appear in debug output, meant for command line options holding +/// credentials so that any debug formatting of the containing struct stays secret-safe. +/// +/// Note: serialization emits the real value so configuration files can round-trip +/// credentials; do not embed this type in structs that are serialized to logs, +/// telemetry, or API responses. #[derive(Clone, Default)] pub struct Redacted(T); From ab86de0dab318e7b893377d13e29b94984ce3473 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 16:48:36 +0200 Subject: [PATCH 16/44] fix(node-lib): report and scope log permission fixes --- api-server/scanner-daemon/src/config.rs | 16 +++++------ api-server/web-server/src/config.rs | 29 ++++++------------- node-lib/src/options.rs | 5 ++-- node-lib/src/runner.rs | 37 ++++++++++++++++++++----- 4 files changed, 50 insertions(+), 37 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 879b2e3dad..c90e35e258 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -75,13 +75,11 @@ impl std::fmt::Debug for ApiServerScannerArgs { impl ApiServerScannerArgs { /// A loggable summary of the options, omitting every value that can hold a secret. - /// Keep this method, not a struct dump, in log statements. + /// Keep this method, not a struct dump, in log statements. The hand-written Debug + /// impl redacts the secret-bearing fields and guards new fields with an exhaustive + /// destructure, so delegating keeps the summary correct as fields are added. pub fn redacted_summary(&self) -> String { - // PostgresConfig's Debug output redacts the database password. - format!( - "network: {:?}, node RPC address: {:?}, node RPC cookie file: {:?}, postgres: {:?}", - self.network, self.node_rpc_address, self.node_rpc_cookie_file, self.postgres_config, - ) + format!("{self:?}") } } @@ -115,12 +113,14 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("node RPC address: Some(")); + assert!(summary.contains("node_rpc_address: Some(")); assert!(summary.contains("127.0.0.1")); assert!(summary.contains("port: 12345")); - assert!(summary.contains("node RPC cookie file: Some(\"/tmp/rpc-cookie-file\")")); + assert!(summary.contains("node_rpc_cookie_file: Some(\"/tmp/rpc-cookie-file\")")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); + assert!(summary.contains("node_rpc_username: Some(\"\")")); + assert!(summary.contains("node_rpc_password: Some(\"\")")); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 900143ce8e..1a508dd598 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -164,24 +164,11 @@ impl std::fmt::Debug for ApiServerWebServerConfig { impl ApiServerWebServerConfig { /// A loggable summary of the options, omitting every value that can hold a secret. - /// Keep this method, not a struct dump, in log statements. + /// Keep this method, not a struct dump, in log statements. The hand-written Debug + /// impl redacts the secret-bearing fields and guards new fields with an exhaustive + /// destructure, so delegating keeps the summary correct as fields are added. pub fn redacted_summary(&self) -> String { - // PostgresConfig's Debug output redacts the database password. - format!( - "network: {:?}, bind address: {:?}, enable post routes: {}, node RPC address: {:?}, \ - node RPC cookie file: {:?}, postgres: {:?}, stream events broadcast capacity: {}, \ - max subscribers: {}, poll interval: {}s, keepalive interval: {}s", - self.network, - self.bind_address, - self.enable_post_routes, - self.node_rpc_address, - self.node_rpc_cookie_file, - self.postgres_config, - self.stream_events_broadcast_capacity, - self.stream_events_max_subscribers, - self.stream_events_poll_interval_secs, - self.stream_events_keepalive_interval_secs, - ) + format!("{self:?}") } } @@ -215,14 +202,16 @@ mod tests { ]); let summary = args.redacted_summary(); assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("bind address: Some")); + assert!(summary.contains("bind_address: Some")); assert!(summary.contains("127.0.0.1:3000")); - assert!(summary.contains("enable post routes: true")); - assert!(summary.contains("node RPC address: Some(")); + assert!(summary.contains("enable_post_routes: true")); + assert!(summary.contains("node_rpc_address: Some(")); assert!(summary.contains("127.0.0.1")); assert!(summary.contains("port: 12345")); assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); + assert!(summary.contains("node_rpc_username: Some(\"\")")); + assert!(summary.contains("node_rpc_password: Some(\"\")")); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 544254faaa..f9e1a9b543 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -241,8 +241,9 @@ pub struct RegtestOptions { /// A value that must never appear in debug output, meant for command line options holding /// credentials so that any debug formatting of the containing struct stays secret-safe. -/// A value that must never appear in debug output, meant for command line options holding -/// credentials so that any debug formatting of the containing struct stays secret-safe. +/// +/// Note: redaction covers debug output only — values passed on the command line remain +/// visible in process listings and shell history. /// /// Note: serialization emits the real value so configuration files can round-trip /// credentials; do not embed this type in structs that are serialized to logs, diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 7f07c56b33..2d5945e042 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -272,7 +272,10 @@ pub async fn setup( ); // Log files written by earlier versions may have wider permissions; restrict them too. // This is best-effort hardening: the check and the mode change are separate steps, so a - // path swapped in between is only skipped or left unchanged, and failures are ignored. + // path swapped in between is only skipped or left unchanged. Failures are collected and + // reported once logging is initialized below. + #[cfg(unix)] + let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -283,14 +286,26 @@ pub async fn setup( if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { continue; } - if entry.file_name().to_string_lossy().contains(".log") { + // Only touch the log file of this executable and its rotated variants. + let name = entry.file_name().to_string_lossy().into_owned(); + let is_rotated_log = name.strip_prefix(&log_file_name).is_some_and(|suffix| { + suffix + .strip_prefix('.') + .is_some_and(|rest| !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit())) + }); + if name == log_file_name || is_rotated_log { // Open the file and change the mode through the descriptor so the // change is not applied to a path that is swapped in afterwards; - // a path swapped in before the open can still be followed. Failures - // are ignored. - let _ = std::fs::OpenOptions::new().read(true).open(entry.path()).and_then( - |file| file.set_permissions(std::fs::Permissions::from_mode(0o600)), - ); + // a path swapped in before the open can still be followed. + if let Err(err) = std::fs::OpenOptions::new() + .read(true) + .open(entry.path()) + .and_then(|file| { + file.set_permissions(std::fs::Permissions::from_mode(0o600)) + }) + { + permission_fix_errors.push((entry.path(), err)); + } } } } @@ -316,6 +331,14 @@ pub async fn setup( )), }), ); + #[cfg(unix)] + for (path, err) in &permission_fix_errors { + logging::log::warn!( + "Failed to restrict permissions of log file {}: {}", + path.display(), + err + ); + } } else { logging::init_logging_generic(main_log_writer_settings, logging::no_writer_settings()); } From 90b23f1dce8054ef58251279cec0e47986147c67 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 17:03:25 +0200 Subject: [PATCH 17/44] fix(api-server): keep clap errors from echoing secret arguments --- api-server/api-server-common/src/lib.rs | 31 ++++++++++++- api-server/scanner-daemon/src/config.rs | 15 ++----- api-server/scanner-daemon/src/main.rs | 5 ++- api-server/web-server/src/config.rs | 14 ++---- api-server/web-server/src/main.rs | 5 ++- node-lib/src/options.rs | 2 +- node-lib/src/runner.rs | 58 ++++++++++++++----------- 7 files changed, 77 insertions(+), 53 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index a56dfa669b..98178dd615 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -27,6 +27,33 @@ pub fn redact(value: &Option) -> Option<&'static str> { value.as_ref().map(|_| REDACTED) } +/// A loggable summary of the options, omitting every value that can hold a secret. +/// Implement this for types whose hand-written Debug impl redacts the secret-bearing +/// fields and guards new fields with an exhaustive destructure; the summary is then the +/// Debug output itself and stays correct as fields are added. +pub trait RedactedSummary: std::fmt::Debug { + fn redacted_summary(&self) -> String { + format!("{self:?}") + } +} + +/// Parses command line arguments, and on failure exits without echoing them: clap errors +/// can repeat raw `--flag=value` tokens, which may carry secrets. Help and version output +/// is rendered as-is. +pub fn parse_args_or_exit() -> T { + match T::try_parse() { + Ok(args) => args, + Err(err) => { + if err.use_stderr() { + eprintln!("Invalid command line arguments; run with --help for usage"); + std::process::exit(2); + } + let _ = err.print(); + std::process::exit(0); + } + } +} + #[derive(clap::ValueEnum, Clone, Debug)] pub enum Network { Mainnet, @@ -76,7 +103,9 @@ pub struct PostgresConfig { // The password must never appear in debug output; anything that embeds this struct in a // derived Debug (command line option dumps, error types) is safe by construction. The // destructure has no `..` so a newly added field fails to compile until it is either added -// here or shown to be safe. +// here or shown to be safe. The database role name in `postgres_user` is not a credential +// and is shown in clear text, unlike the RPC username, which is part of the basic-auth +// credential pair. impl std::fmt::Debug for PostgresConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let Self { diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index c90e35e258..db77de27b2 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -15,7 +15,7 @@ use clap::Parser; -use api_server_common::{Network, PostgresConfig, redact}; +use api_server_common::{Network, PostgresConfig, RedactedSummary, redact}; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; @@ -73,16 +73,9 @@ impl std::fmt::Debug for ApiServerScannerArgs { } } -impl ApiServerScannerArgs { - /// A loggable summary of the options, omitting every value that can hold a secret. - /// Keep this method, not a struct dump, in log statements. The hand-written Debug - /// impl redacts the secret-bearing fields and guards new fields with an exhaustive - /// destructure, so delegating keeps the summary correct as fields are added. - pub fn redacted_summary(&self) -> String { - format!("{self:?}") - } -} - +// The hand-written Debug impl redacts the secret-bearing fields and guards new fields +// with an exhaustive destructure, so the summary is the Debug output itself. +impl RedactedSummary for ApiServerScannerArgs {} #[cfg(test)] mod tests { use super::*; diff --git a/api-server/scanner-daemon/src/main.rs b/api-server/scanner-daemon/src/main.rs index 42c89c5de4..45e7e3d829 100644 --- a/api-server/scanner-daemon/src/main.rs +++ b/api-server/scanner-daemon/src/main.rs @@ -16,7 +16,7 @@ use std::sync::Arc; use api_blockchain_scanner_daemon::{ApiServerScannerError, make_postgres_storage, run}; -use clap::Parser; +use api_server_common::RedactedSummary; use common::chain::config::ChainType; use config::ApiServerScannerArgs; use rpc::RpcAuthData; @@ -28,7 +28,8 @@ utils::enable_rust_backtrace!(); #[tokio::main] async fn main() -> Result<(), ApiServerScannerError> { - let args = ApiServerScannerArgs::parse(); + // Parse errors are not echoed because they can repeat secret-bearing command line tokens. + let args = api_server_common::parse_args_or_exit::(); logging::init_logging(); // The command line contains secrets (RPC credentials, database password), so the summary diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 1a508dd598..79d35bb54d 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -17,7 +17,7 @@ use std::{net::SocketAddr, ops::Deref}; use clap::Parser; -use api_server_common::{Network, PostgresConfig, redact}; +use api_server_common::{Network, PostgresConfig, RedactedSummary, redact}; use tokio::net::TcpListener; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; @@ -162,15 +162,9 @@ impl std::fmt::Debug for ApiServerWebServerConfig { } } -impl ApiServerWebServerConfig { - /// A loggable summary of the options, omitting every value that can hold a secret. - /// Keep this method, not a struct dump, in log statements. The hand-written Debug - /// impl redacts the secret-bearing fields and guards new fields with an exhaustive - /// destructure, so delegating keeps the summary correct as fields are added. - pub fn redacted_summary(&self) -> String { - format!("{self:?}") - } -} +// The hand-written Debug impl redacts the secret-bearing fields and guards new fields +// with an exhaustive destructure, so the summary is the Debug output itself. +impl RedactedSummary for ApiServerWebServerConfig {} #[cfg(test)] mod tests { diff --git a/api-server/web-server/src/main.rs b/api-server/web-server/src/main.rs index b8c32e25b7..c8dce4898b 100644 --- a/api-server/web-server/src/main.rs +++ b/api-server/web-server/src/main.rs @@ -16,6 +16,7 @@ mod api; mod error; +use api_server_common::RedactedSummary; use api_server_common::storage::impls::postgres::{ PostgresStreamEventSource, TransactionalApiServerPostgresStorage, }; @@ -24,7 +25,6 @@ use api_web_server::{ ApiServerWebServerState, CachedValues, MempoolQueryClient, StreamEventsHandle, TxSubmitClient, api::web_server, config::ApiServerWebServerConfig, streaming, tx_dependency_ordering, }; -use clap::Parser; use common::{ chain::config::{Builder, ChainType}, primitives::time::Time, @@ -63,7 +63,8 @@ fn supervise(name: &'static str, handle: tokio::task::JoinHandle<()>) { async fn main() -> Result<(), ApiServerWebServerInitError> { logging::init_logging(); - let args = ApiServerWebServerConfig::parse(); + // Parse errors are not echoed because they can repeat secret-bearing command line tokens. + let args = api_server_common::parse_args_or_exit::(); // The command line contains secrets (RPC credentials, database password), so the summary // logged here omits them; see ApiServerWebServerConfig::redacted_summary. log::info!( diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index f9e1a9b543..6d22a04699 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -248,7 +248,7 @@ pub struct RegtestOptions { /// Note: serialization emits the real value so configuration files can round-trip /// credentials; do not embed this type in structs that are serialized to logs, /// telemetry, or API responses. -#[derive(Clone, Default)] +#[derive(Clone, Default, PartialEq, Eq)] pub struct Redacted(T); impl fmt::Debug for Redacted { diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 2d5945e042..9166b00037 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -279,32 +279,38 @@ pub async fn setup( #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Ok(entries) = std::fs::read_dir(data_dir.join("logs")) { - for entry in entries.flatten() { - // Skip directories and symlinks so the mode is normally never applied - // through a link or to anything but a regular file. - if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { - continue; - } - // Only touch the log file of this executable and its rotated variants. - let name = entry.file_name().to_string_lossy().into_owned(); - let is_rotated_log = name.strip_prefix(&log_file_name).is_some_and(|suffix| { - suffix - .strip_prefix('.') - .is_some_and(|rest| !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit())) - }); - if name == log_file_name || is_rotated_log { - // Open the file and change the mode through the descriptor so the - // change is not applied to a path that is swapped in afterwards; - // a path swapped in before the open can still be followed. - if let Err(err) = std::fs::OpenOptions::new() - .read(true) - .open(entry.path()) - .and_then(|file| { - file.set_permissions(std::fs::Permissions::from_mode(0o600)) - }) - { - permission_fix_errors.push((entry.path(), err)); + match std::fs::read_dir(data_dir.join("logs")) { + // The first run has no logs directory yet. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => permission_fix_errors.push((data_dir.join("logs"), err)), + Ok(entries) => { + for entry in entries.flatten() { + // Skip directories and symlinks so the mode is normally never applied + // through a link or to anything but a regular file. + if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { + continue; + } + // Only touch the log file of this executable and its rotated variants. + let name = entry.file_name().to_string_lossy().into_owned(); + let is_rotated_log = + name.strip_prefix(&log_file_name).is_some_and(|suffix| { + suffix.strip_prefix('.').is_some_and(|rest| { + !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit()) + }) + }); + if name == log_file_name || is_rotated_log { + // Open the file and change the mode through the descriptor so the + // change is not applied to a path that is swapped in afterwards; + // a path swapped in before the open can still be followed. + if let Err(err) = std::fs::OpenOptions::new() + .read(true) + .open(entry.path()) + .and_then(|file| { + file.set_permissions(std::fs::Permissions::from_mode(0o600)) + }) + { + permission_fix_errors.push((entry.path(), err)); + } } } } From 1e6d81d79cda381850f4d09c722c43508e177966 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 17:18:47 +0200 Subject: [PATCH 18/44] fix(node-lib): verify log file identity before changing permissions --- api-server/api-server-common/src/lib.rs | 3 ++- node-lib/src/config_files/mod.rs | 4 ++-- node-lib/src/config_files/rpc.rs | 4 ++-- node-lib/src/options.rs | 18 ++++-------------- node-lib/src/runner.rs | 23 +++++++++++++++++------ 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 98178dd615..2491f57fda 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -129,7 +129,8 @@ impl std::fmt::Debug for PostgresConfig { impl Default for PostgresConfig { fn default() -> Self { - Self::parse() + // Route through the non-echoing parse path. + parse_args_or_exit::() } } diff --git a/node-lib/src/config_files/mod.rs b/node-lib/src/config_files/mod.rs index aa0e75f51f..7d5d529906 100644 --- a/node-lib/src/config_files/mod.rs +++ b/node-lib/src/config_files/mod.rs @@ -33,7 +33,7 @@ use std::{fs, path::Path}; use ::chainstate_launcher::ChainConfig; use anyhow::{Context, Result}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use crate::RunOptions; @@ -45,7 +45,7 @@ use self::{ /// The node configuration. #[must_use] -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone)] #[serde(deny_unknown_fields)] pub struct NodeConfigFile { // Subsystems configurations. diff --git a/node-lib/src/config_files/rpc.rs b/node-lib/src/config_files/rpc.rs index c7e8ff8b87..8aeae3679c 100644 --- a/node-lib/src/config_files/rpc.rs +++ b/node-lib/src/config_files/rpc.rs @@ -17,13 +17,13 @@ use std::{net::SocketAddr, str::FromStr}; use crate::{Redacted, RunOptions}; use chainstate_launcher::ChainConfig; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use super::DEFAULT_RPC_ENABLED; /// The rpc subsystem configuration. #[must_use] -#[derive(Serialize, Deserialize, Debug, Default, Clone)] +#[derive(Deserialize, Debug, Default, Clone)] #[serde(deny_unknown_fields)] pub struct RpcConfigFile { /// Whether http RPC is enabled diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 6d22a04699..21339f1d18 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -245,9 +245,8 @@ pub struct RegtestOptions { /// Note: redaction covers debug output only — values passed on the command line remain /// visible in process listings and shell history. /// -/// Note: serialization emits the real value so configuration files can round-trip -/// credentials; do not embed this type in structs that are serialized to logs, -/// telemetry, or API responses. +/// Note: serialization is intentionally not supported so that configuration dumps cannot +/// emit credentials; the type is only deserialized when reading a configuration file. #[derive(Clone, Default, PartialEq, Eq)] pub struct Redacted(T); @@ -283,17 +282,8 @@ impl AsRef for Redacted { } } -// Serialization writes the real value so configuration files can round-trip credentials; -// only debug output is redacted. -impl serde::Serialize for Redacted -where - T: serde::Serialize, -{ - fn serialize(&self, serializer: S) -> Result { - self.0.serialize(serializer) - } -} - +// Serialization is intentionally not supported so that configuration dumps cannot emit +// credentials; only deserialization (reading a configuration file) is provided. impl<'de, T> serde::Deserialize<'de> for Redacted where T: serde::Deserialize<'de>, diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 9166b00037..54f842a1fa 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -271,9 +271,9 @@ pub async fn setup( }, ); // Log files written by earlier versions may have wider permissions; restrict them too. - // This is best-effort hardening: the check and the mode change are separate steps, so a - // path swapped in between is only skipped or left unchanged. Failures are collected and - // reported once logging is initialized below. + // This is best-effort hardening: the scan and the mode change are separate steps, so a + // path swapped in between is skipped or reported. Failures are collected and reported + // once logging is initialized below. #[cfg(unix)] let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); #[cfg(unix)] @@ -299,13 +299,24 @@ pub async fn setup( }) }); if name == log_file_name || is_rotated_log { - // Open the file and change the mode through the descriptor so the - // change is not applied to a path that is swapped in afterwards; - // a path swapped in before the open can still be followed. + // Open the file and change the mode through the descriptor. A + // path swapped in after the listing could still be followed, so + // compare the listed identity with the opened file and report a + // mismatch instead of changing the mode. if let Err(err) = std::fs::OpenOptions::new() .read(true) .open(entry.path()) .and_then(|file| { + use std::os::unix::fs::MetadataExt; + let listed = entry.metadata()?; + let opened = file.metadata()?; + if (listed.dev(), listed.ino()) != (opened.dev(), opened.ino()) + { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "log file changed while scanning", + )); + } file.set_permissions(std::fs::Permissions::from_mode(0o600)) }) { From 6e10fd234d01c785ab36c87ddf3af9eb976911d7 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 17:34:13 +0200 Subject: [PATCH 19/44] fix(api-server): redact the postgres password in the connection log --- api-server/api-server-common/src/lib.rs | 10 ++++------ .../src/storage/impls/postgres/mod.rs | 11 ++++++++++- api-server/scanner-daemon/src/config.rs | 17 +++++++++++++++-- api-server/web-server/src/config.rs | 17 +++++++++++++++-- node-lib/src/runner.rs | 12 +++++++++--- 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 2491f57fda..24dafc998b 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -28,13 +28,11 @@ pub fn redact(value: &Option) -> Option<&'static str> { } /// A loggable summary of the options, omitting every value that can hold a secret. -/// Implement this for types whose hand-written Debug impl redacts the secret-bearing -/// fields and guards new fields with an exhaustive destructure; the summary is then the -/// Debug output itself and stays correct as fields are added. +/// Implementors must consciously provide the summary; for types whose hand-written Debug +/// impl redacts the secret-bearing fields (with an exhaustive destructure guarding new +/// fields), the body is the Debug output itself. pub trait RedactedSummary: std::fmt::Debug { - fn redacted_summary(&self) -> String { - format!("{self:?}") - } + fn redacted_summary(&self) -> String; } /// Parses command line arguments, and on failure exits without echoing them: clap errors diff --git a/api-server/api-server-common/src/storage/impls/postgres/mod.rs b/api-server/api-server-common/src/storage/impls/postgres/mod.rs index 5b15fa77e7..e6c855d0ac 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/mod.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/mod.rs @@ -76,7 +76,16 @@ impl TransactionalApiServerPostgresStorage { let config_str = format!("host={host} port={port} user={user} {password_part} {database_part}"); - logging::log::debug!("Using postgres config connection string: {}", config_str); + // The connection string contains the database password, so only its redacted form is + // logged. + let logged_config_str = match password { + Some(p) => config_str.replacen(&format!("password={p}"), "password=", 1), + None => config_str.clone(), + }; + logging::log::debug!( + "Using postgres config connection string: {}", + logged_config_str + ); let config: tokio_postgres::Config = config_str.parse().map_err(|e: ::Err| { diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index db77de27b2..fc98996879 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -75,7 +75,11 @@ impl std::fmt::Debug for ApiServerScannerArgs { // The hand-written Debug impl redacts the secret-bearing fields and guards new fields // with an exhaustive destructure, so the summary is the Debug output itself. -impl RedactedSummary for ApiServerScannerArgs {} +impl RedactedSummary for ApiServerScannerArgs { + fn redacted_summary(&self) -> String { + format!("{self:?}") + } +} #[cfg(test)] mod tests { use super::*; @@ -110,7 +114,6 @@ mod tests { assert!(summary.contains("127.0.0.1")); assert!(summary.contains("port: 12345")); assert!(summary.contains("node_rpc_cookie_file: Some(\"/tmp/rpc-cookie-file\")")); - assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(summary.contains("node_rpc_username: Some(\"\")")); assert!(summary.contains("node_rpc_password: Some(\"\")")); @@ -126,4 +129,14 @@ mod tests { assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); } + + #[test] + fn redacted_summary_renders_absent_secrets_as_none() { + let args = ApiServerScannerArgs::parse_from(["scanner-daemon", "--network", "mainnet"]); + let summary = args.redacted_summary(); + assert!(summary.contains("node_rpc_username: None")); + assert!(summary.contains("node_rpc_password: None")); + assert!(summary.contains("postgres_password: None")); + assert!(!summary.contains("")); + } } diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 79d35bb54d..73c785b111 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -164,7 +164,11 @@ impl std::fmt::Debug for ApiServerWebServerConfig { // The hand-written Debug impl redacts the secret-bearing fields and guards new fields // with an exhaustive destructure, so the summary is the Debug output itself. -impl RedactedSummary for ApiServerWebServerConfig {} +impl RedactedSummary for ApiServerWebServerConfig { + fn redacted_summary(&self) -> String { + format!("{self:?}") + } +} #[cfg(test)] mod tests { @@ -202,7 +206,6 @@ mod tests { assert!(summary.contains("node_rpc_address: Some(")); assert!(summary.contains("127.0.0.1")); assert!(summary.contains("port: 12345")); - assert!(summary.contains("postgres")); assert!(summary.contains("postgres_password: Some(\"\")")); assert!(summary.contains("node_rpc_username: Some(\"\")")); assert!(summary.contains("node_rpc_password: Some(\"\")")); @@ -218,6 +221,16 @@ mod tests { assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); } + + #[test] + fn redacted_summary_renders_absent_secrets_as_none() { + let args = ApiServerWebServerConfig::parse_from(["web-server", "--network", "mainnet"]); + let summary = args.redacted_summary(); + assert!(summary.contains("node_rpc_username: None")); + assert!(summary.contains("node_rpc_password: None")); + assert!(summary.contains("postgres_password: None")); + assert!(!summary.contains("")); + } } #[derive(Clone, Debug, Parser)] diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 54f842a1fa..617a0cbdeb 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -284,7 +284,14 @@ pub async fn setup( Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} Err(err) => permission_fix_errors.push((data_dir.join("logs"), err)), Ok(entries) => { - for entry in entries.flatten() { + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + permission_fix_errors.push((data_dir.join("logs"), err)); + continue; + } + }; // Skip directories and symlinks so the mode is normally never applied // through a link or to anything but a regular file. if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { @@ -312,8 +319,7 @@ pub async fn setup( let opened = file.metadata()?; if (listed.dev(), listed.ino()) != (opened.dev(), opened.ino()) { - return Err(std::io::Error::new( - std::io::ErrorKind::NotFound, + return Err(std::io::Error::other( "log file changed while scanning", )); } From 677b78e9215136ade98ff8b5c755f1864f37b6d6 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 17:54:47 +0200 Subject: [PATCH 20/44] fix(api-server): keep clap diagnostics and tests free of secret echoes --- api-server/api-server-common/src/lib.rs | 19 +++- .../src/storage/impls/postgres/mod.rs | 12 +-- api-server/scanner-daemon/src/config.rs | 72 ++++++++------- api-server/web-server/src/config.rs | 90 +++++++++++-------- node-lib/src/options.rs | 5 +- 5 files changed, 114 insertions(+), 84 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 24dafc998b..1115259938 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -36,14 +36,27 @@ pub trait RedactedSummary: std::fmt::Debug { } /// Parses command line arguments, and on failure exits without echoing them: clap errors -/// can repeat raw `--flag=value` tokens, which may carry secrets. Help and version output -/// is rendered as-is. +/// for value-bearing arguments can repeat raw `--flag=value` tokens, which may carry +/// secrets. Errors that only name flags or render static text (missing arguments, +/// conflicts, help and version) are printed as-is. pub fn parse_args_or_exit() -> T { + use clap::error::ErrorKind; match T::try_parse() { Ok(args) => args, Err(err) => { + let value_free = matches!( + err.kind(), + ErrorKind::ArgumentConflict + | ErrorKind::MissingRequiredArgument + | ErrorKind::MissingSubcommand + | ErrorKind::NoEquals + ); if err.use_stderr() { - eprintln!("Invalid command line arguments; run with --help for usage"); + if value_free { + let _ = err.print(); + } else { + eprintln!("Invalid command line arguments; run with --help for usage"); + } std::process::exit(2); } let _ = err.print(); diff --git a/api-server/api-server-common/src/storage/impls/postgres/mod.rs b/api-server/api-server-common/src/storage/impls/postgres/mod.rs index e6c855d0ac..019abc895d 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/mod.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/mod.rs @@ -76,12 +76,14 @@ impl TransactionalApiServerPostgresStorage { let config_str = format!("host={host} port={port} user={user} {password_part} {database_part}"); - // The connection string contains the database password, so only its redacted form is - // logged. - let logged_config_str = match password { - Some(p) => config_str.replacen(&format!("password={p}"), "password=", 1), - None => config_str.clone(), + // The logged string is built from the same parts, so it stays parallel to the real + // one while the password never appears in it. + let logged_password_part = match password { + Some(_) => format!("password={}", crate::REDACTED), + None => String::new(), }; + let logged_config_str = + format!("host={host} port={port} user={user} {logged_password_part} {database_part}"); logging::log::debug!( "Using postgres config connection string: {}", logged_config_str diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index fc98996879..96e7ea1d35 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -83,48 +83,38 @@ impl RedactedSummary for ApiServerScannerArgs { #[cfg(test)] mod tests { use super::*; + use api_server_common::REDACTED; #[test] fn redacted_summary_omits_secrets() { - // Every value that can also come from the ML_API_SCANNER_DAEMON_* environment - // variables is passed explicitly; values not asserted on may still come from the - // environment. - let args = ApiServerScannerArgs::parse_from([ - "scanner-daemon", - "--network", - "mainnet", - "--node-rpc-address", - "127.0.0.1:12345", - "--node-rpc-cookie-file", - "/tmp/rpc-cookie-file", - "--node-rpc-username", - "alice", - "--node-rpc-password", - "supersecret", - "--postgres-host", - "dbhost", - "--postgres-user", - "dbuser", - "--postgres-password", - "dbsecret", - ]); + // Built directly so no ML_API_SCANNER_DAEMON_* environment fallback applies. + let args = ApiServerScannerArgs { + network: Network::Mainnet, + node_rpc_address: Some("127.0.0.1:12345".parse().unwrap()), + node_rpc_cookie_file: Some("/tmp/rpc-cookie-file".to_string()), + node_rpc_username: Some("alice".to_string()), + node_rpc_password: Some("supersecret".to_string()), + postgres_config: PostgresConfig { + postgres_host: "dbhost".to_string(), + postgres_port: 5432, + postgres_user: "dbuser".to_string(), + postgres_password: Some("dbsecret".to_string()), + postgres_database: None, + postgres_max_connections: 10, + }, + }; let summary = args.redacted_summary(); - assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("node_rpc_address: Some(")); + // Assertions are value-based rather than pinned to clap's Debug rendering. assert!(summary.contains("127.0.0.1")); - assert!(summary.contains("port: 12345")); - assert!(summary.contains("node_rpc_cookie_file: Some(\"/tmp/rpc-cookie-file\")")); - assert!(summary.contains("postgres_password: Some(\"\")")); - assert!(summary.contains("node_rpc_username: Some(\"\")")); - assert!(summary.contains("node_rpc_password: Some(\"\")")); + assert!(summary.contains(REDACTED)); + assert!(summary.contains("node_rpc_cookie_file")); + assert!(summary.contains("/tmp/rpc-cookie-file")); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); assert!(debug_dump.contains("ApiServerScannerArgs")); - assert!(debug_dump.contains("node_rpc_username: Some(\"\")")); - assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); - assert!(debug_dump.contains("postgres_password: Some(\"\")")); + assert!(debug_dump.contains(REDACTED)); assert!(!debug_dump.contains("alice")); assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); @@ -132,11 +122,19 @@ mod tests { #[test] fn redacted_summary_renders_absent_secrets_as_none() { - let args = ApiServerScannerArgs::parse_from(["scanner-daemon", "--network", "mainnet"]); + // Built directly so no ML_API_SCANNER_DAEMON_* environment fallback applies. + let args = ApiServerScannerArgs { + network: Network::Mainnet, + node_rpc_address: None, + node_rpc_cookie_file: None, + node_rpc_username: None, + node_rpc_password: None, + postgres_config: PostgresConfig::default(), + }; let summary = args.redacted_summary(); - assert!(summary.contains("node_rpc_username: None")); - assert!(summary.contains("node_rpc_password: None")); - assert!(summary.contains("postgres_password: None")); - assert!(!summary.contains("")); + assert!(!summary.contains(REDACTED)); + assert!(summary.contains("node_rpc_username")); + assert!(summary.contains("node_rpc_password")); + assert!(summary.contains("postgres_password")); } } diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 73c785b111..91135dcf85 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -173,50 +173,48 @@ impl RedactedSummary for ApiServerWebServerConfig { #[cfg(test)] mod tests { use super::*; + use api_server_common::REDACTED; #[test] fn redacted_summary_omits_secrets() { - // Every value that can also come from the ML_API_WEB_SRV_* environment variables - // is passed explicitly; values not asserted on may still come from the environment. - let args = ApiServerWebServerConfig::parse_from([ - "web-server", - "--network", - "mainnet", - "--bind-address", - "127.0.0.1:3000", - "--enable-post-routes", - "--node-rpc-address", - "127.0.0.1:12345", - "--node-rpc-username", - "alice", - "--node-rpc-password", - "supersecret", - "--postgres-host", - "dbhost", - "--postgres-user", - "dbuser", - "--postgres-password", - "dbsecret", - ]); + // Built directly so no ML_API_WEB_SRV_* environment fallback applies. + let args = ApiServerWebServerConfig { + network: Network::Mainnet, + bind_address: Some(ListenAddress { + socket: "127.0.0.1:3000".parse().unwrap(), + }), + postgres_config: PostgresConfig { + postgres_host: "dbhost".to_string(), + postgres_port: 5432, + postgres_user: "dbuser".to_string(), + postgres_password: Some("dbsecret".to_string()), + postgres_database: None, + postgres_max_connections: 10, + }, + enable_post_routes: true, + node_rpc_address: Some("127.0.0.1:12345".parse().unwrap()), + node_rpc_cookie_file: None, + node_rpc_username: Some("alice".to_string()), + node_rpc_password: Some("supersecret".to_string()), + stream_events_broadcast_capacity: streaming::DEFAULT_STREAM_EVENTS_BROADCAST_CAPACITY, + stream_events_max_subscribers: streaming::DEFAULT_STREAM_EVENTS_MAX_SUBSCRIBERS, + stream_events_poll_interval_secs: streaming::DEFAULT_STREAM_EVENTS_POLL_INTERVAL + .as_secs(), + stream_events_keepalive_interval_secs: + streaming::DEFAULT_STREAM_EVENTS_KEEPALIVE_INTERVAL.as_secs(), + }; let summary = args.redacted_summary(); - assert!(summary.contains("network: Mainnet")); - assert!(summary.contains("bind_address: Some")); + // Assertions are value-based rather than pinned to clap's Debug rendering. assert!(summary.contains("127.0.0.1:3000")); - assert!(summary.contains("enable_post_routes: true")); - assert!(summary.contains("node_rpc_address: Some(")); + assert!(summary.contains("enable_post_routes")); assert!(summary.contains("127.0.0.1")); - assert!(summary.contains("port: 12345")); - assert!(summary.contains("postgres_password: Some(\"\")")); - assert!(summary.contains("node_rpc_username: Some(\"\")")); - assert!(summary.contains("node_rpc_password: Some(\"\")")); + assert!(summary.contains(REDACTED)); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); assert!(!summary.contains("dbsecret")); let debug_dump = format!("{args:?}"); assert!(debug_dump.contains("ApiServerWebServerConfig")); - assert!(debug_dump.contains("node_rpc_username: Some(\"\")")); - assert!(debug_dump.contains("node_rpc_password: Some(\"\")")); - assert!(debug_dump.contains("postgres_password: Some(\"\")")); + assert!(debug_dump.contains(REDACTED)); assert!(!debug_dump.contains("alice")); assert!(!debug_dump.contains("supersecret")); assert!(!debug_dump.contains("dbsecret")); @@ -224,12 +222,28 @@ mod tests { #[test] fn redacted_summary_renders_absent_secrets_as_none() { - let args = ApiServerWebServerConfig::parse_from(["web-server", "--network", "mainnet"]); + // Built directly so no ML_API_WEB_SRV_* environment fallback applies. + let args = ApiServerWebServerConfig { + network: Network::Mainnet, + bind_address: None, + postgres_config: PostgresConfig::default(), + enable_post_routes: false, + node_rpc_address: None, + node_rpc_cookie_file: None, + node_rpc_username: None, + node_rpc_password: None, + stream_events_broadcast_capacity: streaming::DEFAULT_STREAM_EVENTS_BROADCAST_CAPACITY, + stream_events_max_subscribers: streaming::DEFAULT_STREAM_EVENTS_MAX_SUBSCRIBERS, + stream_events_poll_interval_secs: streaming::DEFAULT_STREAM_EVENTS_POLL_INTERVAL + .as_secs(), + stream_events_keepalive_interval_secs: + streaming::DEFAULT_STREAM_EVENTS_KEEPALIVE_INTERVAL.as_secs(), + }; let summary = args.redacted_summary(); - assert!(summary.contains("node_rpc_username: None")); - assert!(summary.contains("node_rpc_password: None")); - assert!(summary.contains("postgres_password: None")); - assert!(!summary.contains("")); + assert!(!summary.contains(REDACTED)); + assert!(summary.contains("node_rpc_username")); + assert!(summary.contains("node_rpc_password")); + assert!(summary.contains("postgres_password")); } } diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 21339f1d18..bc90c16aaf 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -247,7 +247,10 @@ pub struct RegtestOptions { /// /// Note: serialization is intentionally not supported so that configuration dumps cannot /// emit credentials; the type is only deserialized when reading a configuration file. -#[derive(Clone, Default, PartialEq, Eq)] +/// +/// Note: equality comparison is intentionally not derived; a derived comparison invites +/// non-constant-time checks on secret values in future authentication decisions. +#[derive(Clone, Default)] pub struct Redacted(T); impl fmt::Debug for Redacted { From 8bf85fff646b43e29939fe8a63627db29e55876e Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:02:32 +0200 Subject: [PATCH 21/44] fix(cli): hide secret env values and make defaults literal --- api-server/api-server-common/src/lib.rs | 64 +++++++++++++++---- .../src/storage/impls/postgres/mod.rs | 18 +++--- api-server/scanner-daemon/src/config.rs | 18 +++++- api-server/web-server/src/config.rs | 18 +++++- node-lib/src/options.rs | 5 +- utils/src/clap_utils.rs | 37 +++++++++++ 6 files changed, 132 insertions(+), 28 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 1115259938..b69a96a3b2 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -37,22 +37,16 @@ pub trait RedactedSummary: std::fmt::Debug { /// Parses command line arguments, and on failure exits without echoing them: clap errors /// for value-bearing arguments can repeat raw `--flag=value` tokens, which may carry -/// secrets. Errors that only name flags or render static text (missing arguments, -/// conflicts, help and version) are printed as-is. +/// secrets. Only errors whose clap rendering is guaranteed to name flags or static text +/// (ArgumentConflict, MissingRequiredArgument, MissingSubcommand, NoEquals, plus help +/// and version) are printed as-is; every other error, including UnknownArgument, is +/// replaced by a generic message. pub fn parse_args_or_exit() -> T { - use clap::error::ErrorKind; match T::try_parse() { Ok(args) => args, Err(err) => { - let value_free = matches!( - err.kind(), - ErrorKind::ArgumentConflict - | ErrorKind::MissingRequiredArgument - | ErrorKind::MissingSubcommand - | ErrorKind::NoEquals - ); if err.use_stderr() { - if value_free { + if error_is_value_free(err.kind()) { let _ = err.print(); } else { eprintln!("Invalid command line arguments; run with --help for usage"); @@ -65,6 +59,41 @@ pub fn parse_args_or_exit() -> T { } } +/// Whether the clap rendering of this error kind is guaranteed to name flags or static +/// text only, never raw argument values. +fn error_is_value_free(kind: clap::error::ErrorKind) -> bool { + use clap::error::ErrorKind; + matches!( + kind, + ErrorKind::ArgumentConflict + | ErrorKind::MissingRequiredArgument + | ErrorKind::MissingSubcommand + | ErrorKind::NoEquals + ) +} + +#[cfg(test)] +mod parse_args_tests { + use clap::error::ErrorKind; + + #[test] + fn value_bearing_error_kinds_are_suppressed() { + assert!(!super::error_is_value_free(ErrorKind::InvalidValue)); + assert!(!super::error_is_value_free(ErrorKind::ValueValidation)); + assert!(!super::error_is_value_free(ErrorKind::UnknownArgument)); + } + + #[test] + fn flag_naming_error_kinds_are_printed() { + assert!(super::error_is_value_free( + ErrorKind::MissingRequiredArgument + )); + assert!(super::error_is_value_free(ErrorKind::ArgumentConflict)); + assert!(super::error_is_value_free(ErrorKind::MissingSubcommand)); + assert!(super::error_is_value_free(ErrorKind::NoEquals)); + } +} + #[derive(clap::ValueEnum, Clone, Debug)] pub enum Network { Mainnet, @@ -140,8 +169,17 @@ impl std::fmt::Debug for PostgresConfig { impl Default for PostgresConfig { fn default() -> Self { - // Route through the non-echoing parse path. - parse_args_or_exit::() + // Literal defaults mirroring the clap `default_value` attributes above; parsing + // here would read the process's real arguments and is not safe outside of the + // actual CLI startup. + Self { + postgres_host: "localhost".to_string(), + postgres_port: 5432, + postgres_user: "postgres".to_string(), + postgres_password: None, + postgres_database: None, + postgres_max_connections: 10, + } } } diff --git a/api-server/api-server-common/src/storage/impls/postgres/mod.rs b/api-server/api-server-common/src/storage/impls/postgres/mod.rs index 019abc895d..8fa0b3b5a2 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/mod.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/mod.rs @@ -76,17 +76,15 @@ impl TransactionalApiServerPostgresStorage { let config_str = format!("host={host} port={port} user={user} {password_part} {database_part}"); - // The logged string is built from the same parts, so it stays parallel to the real - // one while the password never appears in it. - let logged_password_part = match password { - Some(_) => format!("password={}", crate::REDACTED), - None => String::new(), - }; - let logged_config_str = - format!("host={host} port={port} user={user} {logged_password_part} {database_part}"); + // The discrete fields are logged instead of the connection string, so the log + // format does not have to be kept parallel to the real one. logging::log::debug!( - "Using postgres config connection string: {}", - logged_config_str + "Using postgres config: host={host} port={port} user={user} password={} {database_part}", + if password.is_some() { + crate::REDACTED + } else { + "" + } ); let config: tokio_postgres::Config = diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 96e7ea1d35..3b251bc17a 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -20,7 +20,10 @@ use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; #[derive(Parser)] -#[clap(mut_args(clap_utils::env_adder("API_SCANNER_DAEMON")))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "API_SCANNER_DAEMON", + &["node-rpc-username", "node-rpc-password", "postgres-password"] +)))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerScannerArgs { /// Network @@ -105,7 +108,11 @@ mod tests { }; let summary = args.redacted_summary(); // Assertions are value-based rather than pinned to clap's Debug rendering. + // NetworkAddressWithPort renders the address and port as separate Debug fields, + // so they are asserted separately. + assert!(summary.contains("node_rpc_address")); assert!(summary.contains("127.0.0.1")); + assert!(summary.contains("port: 12345")); assert!(summary.contains(REDACTED)); assert!(summary.contains("node_rpc_cookie_file")); assert!(summary.contains("/tmp/rpc-cookie-file")); @@ -129,7 +136,14 @@ mod tests { node_rpc_cookie_file: None, node_rpc_username: None, node_rpc_password: None, - postgres_config: PostgresConfig::default(), + postgres_config: PostgresConfig { + postgres_host: "dbhost".to_string(), + postgres_port: 5432, + postgres_user: "dbuser".to_string(), + postgres_password: None, + postgres_database: None, + postgres_max_connections: 10, + }, }; let summary = args.redacted_summary(); assert!(!summary.contains(REDACTED)); diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 91135dcf85..112f52b234 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -30,7 +30,10 @@ use api_server_common::streaming; const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; #[derive(Parser)] -#[clap(mut_args(clap_utils::env_adder("API_WEB_SRV")))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "API_WEB_SRV", + &["node-rpc-username", "node-rpc-password", "postgres-password"] +)))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerWebServerConfig { /// Network @@ -207,7 +210,11 @@ mod tests { // Assertions are value-based rather than pinned to clap's Debug rendering. assert!(summary.contains("127.0.0.1:3000")); assert!(summary.contains("enable_post_routes")); + // NetworkAddressWithPort renders the address and port as separate Debug fields, + // so they are asserted separately. + assert!(summary.contains("node_rpc_address")); assert!(summary.contains("127.0.0.1")); + assert!(summary.contains("port: 12345")); assert!(summary.contains(REDACTED)); assert!(!summary.contains("alice")); assert!(!summary.contains("supersecret")); @@ -226,7 +233,14 @@ mod tests { let args = ApiServerWebServerConfig { network: Network::Mainnet, bind_address: None, - postgres_config: PostgresConfig::default(), + postgres_config: PostgresConfig { + postgres_host: "localhost".to_string(), + postgres_port: 5432, + postgres_user: "postgres".to_string(), + postgres_password: None, + postgres_database: None, + postgres_max_connections: 10, + }, enable_post_routes: false, node_rpc_address: None, node_rpc_cookie_file: None, diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index bc90c16aaf..053f7386b8 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -59,7 +59,10 @@ pub const IMPORT_BOOTSTRAP_FILE_OPTION_ID: &str = "import_bootstrap_file"; // Note: this struct is shared between different node executables, namely, node-daemon and node-gui, // so the env vars for both of them will use the same infix; this is intended. #[derive(Parser, Debug, Clone)] -#[clap(mut_args(clap_utils::env_adder("NODE")))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "NODE", + &["rpc-username", "rpc-password"] +)))] #[clap(author, version(app_version_with_git_info!().to_pretty_string()), about)] pub struct Options { #[clap(flatten)] diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index f31c6f5b6b..5d45746c11 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -47,3 +47,40 @@ pub fn add_env(infix: &str, mut arg: Arg) -> Arg { pub fn env_adder(infix: &str) -> impl FnMut(Arg) -> Arg + '_ { |arg| add_env(infix, arg) } + +/// Same as [`env_adder`], but the values of the options named in `hidden` are excluded +/// from the help output, so secrets provided through the environment are not rendered +/// as `[env: NAME=value]`. +pub fn env_adder_hiding_env_values<'a>( + infix: &'a str, + hidden: &'static [&'static str], +) -> impl FnMut(Arg) -> Arg + 'a { + move |mut arg| { + arg = add_env(infix, arg); + if hidden.contains(&arg.get_long().unwrap_or_default()) { + arg = arg.hide_env_values(true); + } + arg + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn env_values_are_hidden_only_for_the_named_options() { + let mut wrap = env_adder_hiding_env_values("NODE", &["rpc-username", "rpc-password"]); + + let arg = wrap(clap::Arg::new("rpc_password").long("rpc-password")); + assert!(arg.is_hide_env_values_set()); + assert!(arg.get_env().is_some()); + + let arg = wrap(clap::Arg::new("rpc_username").long("rpc-username")); + assert!(arg.is_hide_env_values_set()); + + let arg = wrap(clap::Arg::new("datadir").long("datadir")); + assert!(!arg.is_hide_env_values_set()); + assert!(arg.get_env().is_some()); + } +} From 655105806e81c239b9278d0193c98a068eaea1d5 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:19:54 +0200 Subject: [PATCH 22/44] fix(cli): hide secret env values on the commands that use them --- api-server/api-server-common/src/lib.rs | 15 +++++++++++++++ node-lib/src/options.rs | 20 +++++++++++++------- utils/src/clap_utils.rs | 2 +- wallet/wallet-cli-lib/src/config.rs | 20 ++++++++++++++++---- wallet/wallet-rpc-lib/src/cmdline.rs | 15 ++++++++++++--- 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index b69a96a3b2..b8b15234f9 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -187,6 +187,21 @@ impl Default for PostgresConfig { mod tests { use super::*; + #[test] + fn default_matches_clap_defaults() { + let from_clap = PostgresConfig::parse_from(["postgres-config"]); + let default = PostgresConfig::default(); + assert_eq!(from_clap.postgres_host, default.postgres_host); + assert_eq!(from_clap.postgres_port, default.postgres_port); + assert_eq!(from_clap.postgres_user, default.postgres_user); + assert_eq!(from_clap.postgres_password, default.postgres_password); + assert_eq!(from_clap.postgres_database, default.postgres_database); + assert_eq!( + from_clap.postgres_max_connections, + default.postgres_max_connections + ); + } + #[test] fn debug_output_redacts_postgres_password() { let config = PostgresConfig::parse_from([ diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 053f7386b8..0560308367 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -59,10 +59,7 @@ pub const IMPORT_BOOTSTRAP_FILE_OPTION_ID: &str = "import_bootstrap_file"; // Note: this struct is shared between different node executables, namely, node-daemon and node-gui, // so the env vars for both of them will use the same infix; this is intended. #[derive(Parser, Debug, Clone)] -#[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "NODE", - &["rpc-username", "rpc-password"] -)))] +#[clap(mut_args(clap_utils::env_adder("NODE")))] #[clap(author, version(app_version_with_git_info!().to_pretty_string()), about)] pub struct Options { #[clap(flatten)] @@ -178,13 +175,22 @@ impl TopLevelOptions { #[derive(Subcommand, Clone, Debug)] pub enum Command { /// Run the mainnet node. - #[clap(mut_args(clap_utils::env_adder("MAINNET_NODE")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "MAINNET_NODE", + &["rpc-username", "rpc-password"] + )))] Mainnet(RunOptions), /// Run the testnet node. - #[clap(mut_args(clap_utils::env_adder("TESTNET_NODE")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "TESTNET_NODE", + &["rpc-username", "rpc-password"] + )))] Testnet(RunOptions), /// Run the regtest node. - #[clap(mut_args(clap_utils::env_adder("REGTEST_NODE")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "REGTEST_NODE", + &["rpc-username", "rpc-password"] + )))] Regtest(Box), } diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 5d45746c11..41b2b3dc3f 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -53,7 +53,7 @@ pub fn env_adder(infix: &str) -> impl FnMut(Arg) -> Arg + '_ { /// as `[env: NAME=value]`. pub fn env_adder_hiding_env_values<'a>( infix: &'a str, - hidden: &'static [&'static str], + hidden: &'a [&'a str], ) -> impl FnMut(Arg) -> Arg + 'a { move |mut arg| { arg = add_env(infix, arg); diff --git a/wallet/wallet-cli-lib/src/config.rs b/wallet/wallet-cli-lib/src/config.rs index dc984bfab3..a8ec741406 100644 --- a/wallet/wallet-cli-lib/src/config.rs +++ b/wallet/wallet-cli-lib/src/config.rs @@ -25,13 +25,25 @@ use wallet_rpc_lib::cmdline::CliHardwareWalletType; #[derive(Subcommand, Clone, Debug)] pub enum Network { - #[clap(mut_args(clap_utils::env_adder("MAINNET_WALLET")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "MAINNET_WALLET", + &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + )))] Mainnet(CliArgs), - #[clap(mut_args(clap_utils::env_adder("TESTNET_WALLET")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "TESTNET_WALLET", + &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + )))] Testnet(CliArgs), - #[clap(mut_args(clap_utils::env_adder("REGTEST_WALLET")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "REGTEST_WALLET", + &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + )))] Regtest(Box), - #[clap(mut_args(clap_utils::env_adder("SIGNET_WALLET")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "SIGNET_WALLET", + &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + )))] Signet(CliArgs), } diff --git a/wallet/wallet-rpc-lib/src/cmdline.rs b/wallet/wallet-rpc-lib/src/cmdline.rs index 3762d10d6e..66a69de433 100644 --- a/wallet/wallet-rpc-lib/src/cmdline.rs +++ b/wallet/wallet-rpc-lib/src/cmdline.rs @@ -51,15 +51,24 @@ impl WalletRpcDaemonArgs { #[derive(clap::Subcommand)] pub enum WalletRpcDaemonCommand { /// Run the mainnet wallet. - #[clap(mut_args(clap_utils::env_adder("MAINNET_WALLET_RPC_DAEMON")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "MAINNET_WALLET_RPC_DAEMON", + &["rpc-username", "rpc-password"] + )))] Mainnet(WalletRpcDaemonChainArgs), /// Run the testnet wallet. - #[clap(mut_args(clap_utils::env_adder("TESTNET_WALLET_RPC_DAEMON")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "TESTNET_WALLET_RPC_DAEMON", + &["rpc-username", "rpc-password"] + )))] Testnet(WalletRpcDaemonChainArgs), /// Run the regtest wallet. - #[clap(mut_args(clap_utils::env_adder("REGTEST_WALLET_RPC_DAEMON")))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values( + "REGTEST_WALLET_RPC_DAEMON", + &["rpc-username", "rpc-password"] + )))] Regtest { #[command(flatten)] args: WalletRpcDaemonChainArgs, From 77e589d479d3d4072be2cfb950ff0d90a0dbbfff Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:30:27 +0200 Subject: [PATCH 23/44] fix(cli): hide every secret-bearing option from the help output --- api-server/api-server-common/src/lib.rs | 13 ++----------- wallet/wallet-cli-lib/src/config.rs | 22 +++++++++++++++++----- wallet/wallet-rpc-lib/src/cmdline.rs | 6 +++--- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index b8b15234f9..5ba9f87111 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -113,7 +113,7 @@ impl From for ChainType { } } -#[derive(Parser)] +#[derive(Parser, PartialEq)] pub struct PostgresConfig { /// Postgres host #[clap(long, default_value = "localhost")] @@ -190,16 +190,7 @@ mod tests { #[test] fn default_matches_clap_defaults() { let from_clap = PostgresConfig::parse_from(["postgres-config"]); - let default = PostgresConfig::default(); - assert_eq!(from_clap.postgres_host, default.postgres_host); - assert_eq!(from_clap.postgres_port, default.postgres_port); - assert_eq!(from_clap.postgres_user, default.postgres_user); - assert_eq!(from_clap.postgres_password, default.postgres_password); - assert_eq!(from_clap.postgres_database, default.postgres_database); - assert_eq!( - from_clap.postgres_max_connections, - default.postgres_max_connections - ); + assert_eq!(from_clap, PostgresConfig::default()); } #[test] diff --git a/wallet/wallet-cli-lib/src/config.rs b/wallet/wallet-cli-lib/src/config.rs index a8ec741406..df1145db6c 100644 --- a/wallet/wallet-cli-lib/src/config.rs +++ b/wallet/wallet-cli-lib/src/config.rs @@ -27,26 +27,38 @@ use wallet_rpc_lib::cmdline::CliHardwareWalletType; pub enum Network { #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "MAINNET_WALLET", - &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + WALLET_SECRET_OPTIONS )))] Mainnet(CliArgs), #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "TESTNET_WALLET", - &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + WALLET_SECRET_OPTIONS )))] Testnet(CliArgs), #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "REGTEST_WALLET", - &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + WALLET_SECRET_OPTIONS )))] Regtest(Box), #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "SIGNET_WALLET", - &["node-rpc-username", "node-rpc-password", "wallet-rpc-username", "wallet-rpc-password"] + WALLET_SECRET_OPTIONS )))] Signet(CliArgs), } +/// Options whose values can hold a secret; their environment-sourced values are hidden +/// from the help output. +const WALLET_SECRET_OPTIONS: &[&str] = &[ + "node-rpc-username", + "node-rpc-password", + "wallet-rpc-username", + "wallet-rpc-password", + "wallet-password", + "remote-rpc-wallet-username", + "remote-rpc-wallet-password", +]; + #[derive(Args, Clone, Debug)] pub struct RegtestOptions { #[clap(flatten)] @@ -56,7 +68,7 @@ pub struct RegtestOptions { } #[derive(Parser, Debug)] -#[clap(mut_args(clap_utils::env_adder("WALLET")))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values("WALLET", WALLET_SECRET_OPTIONS)))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] #[command(args_conflicts_with_subcommands = true)] pub struct WalletCliArgs { diff --git a/wallet/wallet-rpc-lib/src/cmdline.rs b/wallet/wallet-rpc-lib/src/cmdline.rs index 66a69de433..3ad75a2de7 100644 --- a/wallet/wallet-rpc-lib/src/cmdline.rs +++ b/wallet/wallet-rpc-lib/src/cmdline.rs @@ -53,21 +53,21 @@ pub enum WalletRpcDaemonCommand { /// Run the mainnet wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "MAINNET_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password"] + &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] )))] Mainnet(WalletRpcDaemonChainArgs), /// Run the testnet wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "TESTNET_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password"] + &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] )))] Testnet(WalletRpcDaemonChainArgs), /// Run the regtest wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "REGTEST_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password"] + &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] )))] Regtest { #[command(flatten)] From 0fd290e795793a768b166a7a858c7a407d2a5f28 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:36:50 +0200 Subject: [PATCH 24/44] test(cli): pin the env value hiding of wallet secret options --- wallet/wallet-cli-lib/src/config.rs | 22 ++++++++++++++++++++++ wallet/wallet-rpc-lib/src/cmdline.rs | 11 ++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/wallet/wallet-cli-lib/src/config.rs b/wallet/wallet-cli-lib/src/config.rs index df1145db6c..f19e8a06ee 100644 --- a/wallet/wallet-cli-lib/src/config.rs +++ b/wallet/wallet-cli-lib/src/config.rs @@ -59,6 +59,28 @@ const WALLET_SECRET_OPTIONS: &[&str] = &[ "remote-rpc-wallet-password", ]; +#[cfg(test)] +mod tests { + use super::*; + use clap::CommandFactory; + + #[test] + fn every_secret_option_has_its_env_value_hidden() { + let cmd = WalletCliArgs::command(); + let all_args = cmd + .get_arguments() + .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); + for arg in all_args { + if let Some(long) = arg.get_long().filter(|long| WALLET_SECRET_OPTIONS.contains(long)) { + assert!( + arg.is_hide_env_values_set(), + "env value of --{long} is not hidden from help" + ); + } + } + } +} + #[derive(Args, Clone, Debug)] pub struct RegtestOptions { #[clap(flatten)] diff --git a/wallet/wallet-rpc-lib/src/cmdline.rs b/wallet/wallet-rpc-lib/src/cmdline.rs index 3ad75a2de7..47404c7a45 100644 --- a/wallet/wallet-rpc-lib/src/cmdline.rs +++ b/wallet/wallet-rpc-lib/src/cmdline.rs @@ -48,26 +48,31 @@ impl WalletRpcDaemonArgs { } } +/// Options whose values can hold a secret; their environment-sourced values are hidden +/// from the help output. +const WALLET_RPC_SECRET_OPTIONS: &[&str] = + &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"]; + #[derive(clap::Subcommand)] pub enum WalletRpcDaemonCommand { /// Run the mainnet wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "MAINNET_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] + WALLET_RPC_SECRET_OPTIONS )))] Mainnet(WalletRpcDaemonChainArgs), /// Run the testnet wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "TESTNET_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] + WALLET_RPC_SECRET_OPTIONS )))] Testnet(WalletRpcDaemonChainArgs), /// Run the regtest wallet. #[clap(mut_args(clap_utils::env_adder_hiding_env_values( "REGTEST_WALLET_RPC_DAEMON", - &["rpc-username", "rpc-password", "node-rpc-username", "node-rpc-password"] + WALLET_RPC_SECRET_OPTIONS )))] Regtest { #[command(flatten)] From d2c80575d0d61e81e98f68094419d89049cc5e7e Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:40:53 +0200 Subject: [PATCH 25/44] test(cli): match the secret option lists against real arguments --- wallet/wallet-cli-lib/src/config.rs | 8 +++++++ wallet/wallet-rpc-lib/src/cmdline.rs | 35 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/wallet/wallet-cli-lib/src/config.rs b/wallet/wallet-cli-lib/src/config.rs index f19e8a06ee..d66d10f845 100644 --- a/wallet/wallet-cli-lib/src/config.rs +++ b/wallet/wallet-cli-lib/src/config.rs @@ -70,14 +70,22 @@ mod tests { let all_args = cmd .get_arguments() .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); + let mut matched = std::collections::HashSet::new(); for arg in all_args { if let Some(long) = arg.get_long().filter(|long| WALLET_SECRET_OPTIONS.contains(long)) { assert!( arg.is_hide_env_values_set(), "env value of --{long} is not hidden from help" ); + matched.insert(long); } } + assert_eq!( + matched, + WALLET_SECRET_OPTIONS.iter().copied().collect::>(), + "WALLET_SECRET_OPTIONS does not match the actual secret options; \ + stale entries pass silently and missing entries leak env values in --help" + ); } } diff --git a/wallet/wallet-rpc-lib/src/cmdline.rs b/wallet/wallet-rpc-lib/src/cmdline.rs index 47404c7a45..f5def77a16 100644 --- a/wallet/wallet-rpc-lib/src/cmdline.rs +++ b/wallet/wallet-rpc-lib/src/cmdline.rs @@ -330,3 +330,38 @@ pub enum ConfigError { #[error(transparent)] RunningAsRoot(anyhow::Error), } + +#[cfg(test)] +mod tests { + use super::*; + use clap::CommandFactory; + + #[test] + fn every_secret_option_has_its_env_value_hidden() { + let cmd = WalletRpcDaemonArgs::command(); + let all_args = cmd + .get_arguments() + .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); + let mut matched = std::collections::HashSet::new(); + for arg in all_args { + if let Some(long) = + arg.get_long().filter(|long| WALLET_RPC_SECRET_OPTIONS.contains(long)) + { + assert!( + arg.is_hide_env_values_set(), + "env value of --{long} is not hidden from help" + ); + matched.insert(long); + } + } + assert_eq!( + matched, + WALLET_RPC_SECRET_OPTIONS + .iter() + .copied() + .collect::>(), + "WALLET_RPC_SECRET_OPTIONS does not match the actual secret options; \ + stale entries pass silently and missing entries leak env values in --help" + ); + } +} From 938bcddcfc8cd80d27e25a9e469f8ba25765c945 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sat, 17 Oct 2026 23:46:21 +0200 Subject: [PATCH 26/44] refactor(utils): share the secret option pin between wallet crates --- utils/src/clap_utils.rs | 25 +++++++++++++++++++++++++ wallet/wallet-cli-lib/src/config.rs | 21 +-------------------- wallet/wallet-rpc-lib/src/cmdline.rs | 27 +++------------------------ 3 files changed, 29 insertions(+), 44 deletions(-) diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 41b2b3dc3f..7ca23d7db7 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -64,6 +64,31 @@ pub fn env_adder_hiding_env_values<'a>( } } +/// Test helper: asserts that every option named in `secret_options` exists in `cmd` and +/// has its environment-sourced value hidden from the help output. +#[doc(hidden)] +pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { + let all_args = cmd + .get_arguments() + .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); + let mut matched = std::collections::HashSet::new(); + for arg in all_args { + if let Some(long) = arg.get_long().filter(|long| secret_options.contains(long)) { + assert!( + arg.is_hide_env_values_set(), + "env value of --{long} is not hidden from help" + ); + matched.insert(long); + } + } + assert_eq!( + matched, + secret_options.iter().copied().collect::>(), + "the secret options list does not match the actual secret options; \ + stale entries pass silently and missing entries leak env values in --help" + ); +} + #[cfg(test)] mod tests { use super::*; diff --git a/wallet/wallet-cli-lib/src/config.rs b/wallet/wallet-cli-lib/src/config.rs index d66d10f845..57074c11c6 100644 --- a/wallet/wallet-cli-lib/src/config.rs +++ b/wallet/wallet-cli-lib/src/config.rs @@ -66,26 +66,7 @@ mod tests { #[test] fn every_secret_option_has_its_env_value_hidden() { - let cmd = WalletCliArgs::command(); - let all_args = cmd - .get_arguments() - .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); - let mut matched = std::collections::HashSet::new(); - for arg in all_args { - if let Some(long) = arg.get_long().filter(|long| WALLET_SECRET_OPTIONS.contains(long)) { - assert!( - arg.is_hide_env_values_set(), - "env value of --{long} is not hidden from help" - ); - matched.insert(long); - } - } - assert_eq!( - matched, - WALLET_SECRET_OPTIONS.iter().copied().collect::>(), - "WALLET_SECRET_OPTIONS does not match the actual secret options; \ - stale entries pass silently and missing entries leak env values in --help" - ); + clap_utils::assert_secret_options_hidden(&WalletCliArgs::command(), WALLET_SECRET_OPTIONS); } } diff --git a/wallet/wallet-rpc-lib/src/cmdline.rs b/wallet/wallet-rpc-lib/src/cmdline.rs index f5def77a16..e5fbdfcb66 100644 --- a/wallet/wallet-rpc-lib/src/cmdline.rs +++ b/wallet/wallet-rpc-lib/src/cmdline.rs @@ -338,30 +338,9 @@ mod tests { #[test] fn every_secret_option_has_its_env_value_hidden() { - let cmd = WalletRpcDaemonArgs::command(); - let all_args = cmd - .get_arguments() - .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); - let mut matched = std::collections::HashSet::new(); - for arg in all_args { - if let Some(long) = - arg.get_long().filter(|long| WALLET_RPC_SECRET_OPTIONS.contains(long)) - { - assert!( - arg.is_hide_env_values_set(), - "env value of --{long} is not hidden from help" - ); - matched.insert(long); - } - } - assert_eq!( - matched, - WALLET_RPC_SECRET_OPTIONS - .iter() - .copied() - .collect::>(), - "WALLET_RPC_SECRET_OPTIONS does not match the actual secret options; \ - stale entries pass silently and missing entries leak env values in --help" + clap_utils::assert_secret_options_hidden( + &WalletRpcDaemonArgs::command(), + WALLET_RPC_SECRET_OPTIONS, ); } } From 15126c61bfe47f4481e71d9e11785e65e5d84c1d Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 00:04:13 +0200 Subject: [PATCH 27/44] fix(api-server): pin the help output against secret env values --- api-server/api-server-common/src/lib.rs | 60 ++++++++++++++++++++++++- api-server/scanner-daemon/src/config.rs | 15 +++++-- api-server/web-server/src/config.rs | 18 +++++++- node-lib/src/runner.rs | 15 +++++-- utils/src/clap_utils.rs | 4 +- 5 files changed, 100 insertions(+), 12 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 5ba9f87111..af11c0d079 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -41,6 +41,14 @@ pub trait RedactedSummary: std::fmt::Debug { /// (ArgumentConflict, MissingRequiredArgument, MissingSubcommand, NoEquals, plus help /// and version) are printed as-is; every other error, including UnknownArgument, is /// replaced by a generic message. +/// +/// # Contract with the argument definitions +/// +/// Help and version output is echoed verbatim, and the error fallback tells users to +/// run `--help`. Any secret-bearing argument bound to an environment variable must +/// therefore set `hide_env_values(true)` (see +/// `clap_utils::env_adder_hiding_env_values`), otherwise clap renders +/// `[env: NAME=value]` into the help text and this function prints the secret. pub fn parse_args_or_exit() -> T { match T::try_parse() { Ok(args) => args, @@ -68,7 +76,10 @@ fn error_is_value_free(kind: clap::error::ErrorKind) -> bool { ErrorKind::ArgumentConflict | ErrorKind::MissingRequiredArgument | ErrorKind::MissingSubcommand - | ErrorKind::NoEquals + | ErrorKind::NoEquals // DisplayHelpOnMissingArgumentOrSubcommand (stderr-routed, value-free under the + // hide_env_values contract) is deliberately excluded, so a binary configured + // with `arg_required_else_help` prints the generic message rather than its help + // on empty arguments. ) } @@ -92,6 +103,53 @@ mod parse_args_tests { assert!(super::error_is_value_free(ErrorKind::MissingSubcommand)); assert!(super::error_is_value_free(ErrorKind::NoEquals)); } + + /// Pins that the clap rendering of the allowlisted kinds never embeds raw argument + /// values, so a clap upgrade that starts echoing them fails here instead of leaking + /// secrets in production. + #[test] + fn allowlisted_kinds_never_render_raw_values() { + let secret = "s3cret-canary-value"; + let cmd = clap::Command::new("canary") + .arg(clap::Arg::new("a").long("a").num_args(1)) + .arg( + clap::Arg::new("b") + .long("b") + .conflicts_with("a") + .action(clap::ArgAction::SetTrue), + ) + .arg(clap::Arg::new("c").long("c").required(true)); + + let conflict = + cmd.clone().try_get_matches_from(["canary", "--a", secret, "--b"]).unwrap_err(); + assert_eq!(conflict.kind(), clap::error::ErrorKind::ArgumentConflict); + assert!(!conflict.render().to_string().contains(secret)); + + let missing_required = + cmd.clone().try_get_matches_from(["canary", "--a", secret]).unwrap_err(); + assert_eq!( + missing_required.kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + assert!(!missing_required.render().to_string().contains(secret)); + + let requires_equals = clap::Command::new("canary") + .arg(clap::Arg::new("a").long("a").require_equals(true).num_args(1)); + let no_equals = + requires_equals.try_get_matches_from(["canary", "--a", secret]).unwrap_err(); + assert_eq!(no_equals.kind(), clap::error::ErrorKind::NoEquals); + assert!(!no_equals.render().to_string().contains(secret)); + + let subcommanded = clap::Command::new("canary") + .subcommand_required(true) + .subcommand(clap::Command::new("run")); + let missing_subcommand = subcommanded.try_get_matches_from(["canary"]).unwrap_err(); + assert_eq!( + missing_subcommand.kind(), + clap::error::ErrorKind::MissingSubcommand + ); + assert!(!missing_subcommand.render().to_string().contains(secret)); + } } #[derive(clap::ValueEnum, Clone, Debug)] diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 3b251bc17a..9b00519c39 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -19,11 +19,12 @@ use api_server_common::{Network, PostgresConfig, RedactedSummary, redact}; use utils::{app_version_with_git_info, clap_utils}; use utils_networking::NetworkAddressWithPort; +/// Options whose values can hold a secret; their environment-sourced values are hidden +/// from the help output. +const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; + #[derive(Parser)] -#[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "API_SCANNER_DAEMON", - &["node-rpc-username", "node-rpc-password", "postgres-password"] -)))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values("API_SCANNER_DAEMON", SECRET_OPTIONS)))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerScannerArgs { /// Network @@ -87,6 +88,12 @@ impl RedactedSummary for ApiServerScannerArgs { mod tests { use super::*; use api_server_common::REDACTED; + use clap::CommandFactory; + + #[test] + fn secret_options_are_hidden_from_help() { + clap_utils::assert_secret_options_hidden(&ApiServerScannerArgs::command(), SECRET_OPTIONS); + } #[test] fn redacted_summary_omits_secrets() { diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 112f52b234..be6727d7ca 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -139,12 +139,12 @@ impl std::fmt::Debug for ApiServerWebServerConfig { f.debug_struct("ApiServerWebServerConfig") .field("network", network) .field("bind_address", bind_address) + .field("postgres_config", postgres_config) + .field("enable_post_routes", enable_post_routes) .field("node_rpc_address", node_rpc_address) .field("node_rpc_cookie_file", node_rpc_cookie_file) .field("node_rpc_username", &redact(node_rpc_username)) .field("node_rpc_password", &redact(node_rpc_password)) - .field("postgres_config", postgres_config) - .field("enable_post_routes", enable_post_routes) .field( "stream_events_broadcast_capacity", stream_events_broadcast_capacity, @@ -177,6 +177,20 @@ impl RedactedSummary for ApiServerWebServerConfig { mod tests { use super::*; use api_server_common::REDACTED; + use clap::CommandFactory; + + /// Options whose values can hold a secret; their environment-sourced values are hidden + /// from the help output (see the `mut_args` attribute above). + const SECRET_OPTIONS: &[&str] = + &["node-rpc-username", "node-rpc-password", "postgres-password"]; + + #[test] + fn secret_options_are_hidden_from_help() { + clap_utils::assert_secret_options_hidden( + &ApiServerWebServerConfig::command(), + SECRET_OPTIONS, + ); + } #[test] fn redacted_summary_omits_secrets() { diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 617a0cbdeb..7001cd1bd2 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -306,16 +306,23 @@ pub async fn setup( }) }); if name == log_file_name || is_rotated_log { + // Capture the identity at listing time so a swap between the + // listing and the open is detected by the check below. + let listed = match entry.metadata() { + Ok(listed) => listed, + Err(err) => { + permission_fix_errors.push((entry.path(), err)); + continue; + } + }; // Open the file and change the mode through the descriptor. A - // path swapped in after the listing could still be followed, so - // compare the listed identity with the opened file and report a - // mismatch instead of changing the mode. + // path swapped in after this point is reported as a mismatch + // instead of having its mode changed. if let Err(err) = std::fs::OpenOptions::new() .read(true) .open(entry.path()) .and_then(|file| { use std::os::unix::fs::MetadataExt; - let listed = entry.metadata()?; let opened = file.metadata()?; if (listed.dev(), listed.ino()) != (opened.dev(), opened.ino()) { diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 7ca23d7db7..f5d6fb08c0 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -57,7 +57,7 @@ pub fn env_adder_hiding_env_values<'a>( ) -> impl FnMut(Arg) -> Arg + 'a { move |mut arg| { arg = add_env(infix, arg); - if hidden.contains(&arg.get_long().unwrap_or_default()) { + if arg.get_long().is_some_and(|long| hidden.contains(&long)) { arg = arg.hide_env_values(true); } arg @@ -68,6 +68,8 @@ pub fn env_adder_hiding_env_values<'a>( /// has its environment-sourced value hidden from the help output. #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { + // Note: only the top-level command and its immediate subcommands are inspected; + // args under deeper nested subcommands are not verified. let all_args = cmd .get_arguments() .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); From a86b6bdbd834cf4553587f5ce19a4def0ca7bd87 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 00:13:43 +0200 Subject: [PATCH 28/44] test(cli): search nested subcommands when pinning hidden env values --- api-server/api-server-common/src/lib.rs | 9 +++++---- api-server/web-server/src/config.rs | 14 +++++--------- utils/src/clap_utils.rs | 18 +++++++++++------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index af11c0d079..0aa76b802a 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -71,15 +71,16 @@ pub fn parse_args_or_exit() -> T { /// text only, never raw argument values. fn error_is_value_free(kind: clap::error::ErrorKind) -> bool { use clap::error::ErrorKind; + // `DisplayHelpOnMissingArgumentOrSubcommand` is deliberately excluded even though its + // rendering is stderr-routed and value-free under the `hide_env_values` contract: a + // binary configured with `arg_required_else_help` should print the generic message + // rather than its full help when invoked with no arguments. matches!( kind, ErrorKind::ArgumentConflict | ErrorKind::MissingRequiredArgument | ErrorKind::MissingSubcommand - | ErrorKind::NoEquals // DisplayHelpOnMissingArgumentOrSubcommand (stderr-routed, value-free under the - // hide_env_values contract) is deliberately excluded, so a binary configured - // with `arg_required_else_help` prints the generic message rather than its help - // on empty arguments. + | ErrorKind::NoEquals ) } diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index be6727d7ca..e6a83a95e4 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -29,11 +29,12 @@ use api_server_common::streaming; const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; +/// Options whose values can hold a secret; their environment-sourced values are hidden +/// from the help output (see the `mut_args` attribute and the guard test below). +const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; + #[derive(Parser)] -#[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "API_WEB_SRV", - &["node-rpc-username", "node-rpc-password", "postgres-password"] -)))] +#[clap(mut_args(clap_utils::env_adder_hiding_env_values("API_WEB_SRV", SECRET_OPTIONS)))] #[clap(version(app_version_with_git_info!().to_pretty_string()))] pub struct ApiServerWebServerConfig { /// Network @@ -179,11 +180,6 @@ mod tests { use api_server_common::REDACTED; use clap::CommandFactory; - /// Options whose values can hold a secret; their environment-sourced values are hidden - /// from the help output (see the `mut_args` attribute above). - const SECRET_OPTIONS: &[&str] = - &["node-rpc-username", "node-rpc-password", "postgres-password"]; - #[test] fn secret_options_are_hidden_from_help() { clap_utils::assert_secret_options_hidden( diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index f5d6fb08c0..60af9e6d7b 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -64,15 +64,19 @@ pub fn env_adder_hiding_env_values<'a>( } } -/// Test helper: asserts that every option named in `secret_options` exists in `cmd` and -/// has its environment-sourced value hidden from the help output. +/// Test helper: asserts that every option named in `secret_options` exists in `cmd` +/// (recursively, including all nested subcommands) and has its environment-sourced value +/// hidden from the help output. #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { - // Note: only the top-level command and its immediate subcommands are inspected; - // args under deeper nested subcommands are not verified. - let all_args = cmd - .get_arguments() - .chain(cmd.get_subcommands().flat_map(|sub| sub.get_arguments())); + fn collect_args<'a>(cmd: &'a clap::Command, args: &mut Vec<&'a clap::Arg>) { + args.extend(cmd.get_arguments()); + for sub in cmd.get_subcommands() { + collect_args(sub, args); + } + } + let mut all_args = Vec::new(); + collect_args(cmd, &mut all_args); let mut matched = std::collections::HashSet::new(); for arg in all_args { if let Some(long) = arg.get_long().filter(|long| secret_options.contains(long)) { From 854d4352552deb7657f0d6a8ec786cd262cfdf5c Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 00:40:50 +0200 Subject: [PATCH 29/44] fix(cli): build the postgres config without a password string --- .../src/storage/impls/postgres/mod.rs | 33 +++------ api-server/scanner-daemon/Cargo.toml | 1 + api-server/scanner-daemon/src/config.rs | 1 + api-server/web-server/Cargo.toml | 1 + node-lib/Cargo.toml | 1 + node-lib/src/options.rs | 11 +++ node-lib/src/runner.rs | 69 +++++++++++-------- utils/Cargo.toml | 3 + utils/src/clap_utils.rs | 62 ++++++++++++++++- wallet/wallet-cli-lib/Cargo.toml | 1 + wallet/wallet-rpc-lib/Cargo.toml | 1 + 11 files changed, 131 insertions(+), 53 deletions(-) diff --git a/api-server/api-server-common/src/storage/impls/postgres/mod.rs b/api-server/api-server-common/src/storage/impls/postgres/mod.rs index 8fa0b3b5a2..5b99e76716 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/mod.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/mod.rs @@ -18,7 +18,6 @@ pub mod transactional; mod queries; -use std::str::FromStr; use std::sync::Arc; use bb8_postgres::PostgresConnectionManager; @@ -64,22 +63,14 @@ impl TransactionalApiServerPostgresStorage { max_connections: u32, chain_config: Arc, ) -> Result { - let password_part = match password { - Some(p) => format!("password={}", p), - None => "".to_string(), + // The configuration is built from the discrete fields instead of a connection + // string, so the password never exists as a string that a parse error could echo. + let database = match database { + Some(d) => d.to_owned(), + None => format!("mintlayer-{}", chain_config.chain_type().name()), }; - - let database_part = match database { - Some(d) => format!("dbname={}", d), - None => format!("dbname=mintlayer-{}", chain_config.chain_type().name()), - }; - - let config_str = - format!("host={host} port={port} user={user} {password_part} {database_part}"); - // The discrete fields are logged instead of the connection string, so the log - // format does not have to be kept parallel to the real one. logging::log::debug!( - "Using postgres config: host={host} port={port} user={user} password={} {database_part}", + "Using postgres config: host={host} port={port} user={user} password={} database={database}", if password.is_some() { crate::REDACTED } else { @@ -87,13 +78,11 @@ impl TransactionalApiServerPostgresStorage { } ); - let config: tokio_postgres::Config = - config_str.parse().map_err(|e: ::Err| { - ApiServerStorageError::InitializationError(format!( - "Postgres configuration parsing error: {}", - e - )) - })?; + let mut config = tokio_postgres::Config::new(); + config.host(host).port(port).user(user).dbname(&database); + if let Some(password) = password { + config.password(password); + } let manager = PostgresConnectionManager::new(config.clone(), NoTls); let pool = Pool::builder().max_size(max_connections).build(manager).await.map_err(|e| { diff --git a/api-server/scanner-daemon/Cargo.toml b/api-server/scanner-daemon/Cargo.toml index d3641056a7..0ab0f819ec 100644 --- a/api-server/scanner-daemon/Cargo.toml +++ b/api-server/scanner-daemon/Cargo.toml @@ -24,6 +24,7 @@ tokio = { workspace = true, features = ["full"] } [dev-dependencies] test-utils = { path = "../../test-utils" } +utils = { path = "../../utils", features = ["test-util"] } [build-dependencies] build_utils = { path = "../../utils/build_utils" } diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 9b00519c39..2efd389ff2 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -84,6 +84,7 @@ impl RedactedSummary for ApiServerScannerArgs { format!("{self:?}") } } + #[cfg(test)] mod tests { use super::*; diff --git a/api-server/web-server/Cargo.toml b/api-server/web-server/Cargo.toml index a85649a59c..d22556616f 100644 --- a/api-server/web-server/Cargo.toml +++ b/api-server/web-server/Cargo.toml @@ -37,6 +37,7 @@ randomness = { path = "../../randomness" } rstest.workspace = true strum.workspace = true test-utils = { path = "../../test-utils" } +utils = { path = "../../utils", features = ["test-util"] } [build-dependencies] build_utils = { path = "../../utils/build_utils" } diff --git a/node-lib/Cargo.toml b/node-lib/Cargo.toml index 47fa715edb..debdeda35f 100644 --- a/node-lib/Cargo.toml +++ b/node-lib/Cargo.toml @@ -36,6 +36,7 @@ toml.workspace = true [dev-dependencies] crypto = { path = "../crypto" } +utils = { path = "../utils", features = ["test-util"] } randomness = { path = "../randomness" } ctor.workspace = true diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 0560308367..a477a9f983 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -529,6 +529,17 @@ mod tests { }; use utils::concatln; + /// Pins the secret option list against the derived command: a renamed field would + /// silently stop matching the hiding list in the `mut_args` attributes and leak its + /// environment value into `--help`. + #[test] + fn secret_options_env_values_are_hidden() { + utils::clap_utils::assert_secret_options_hidden( + &::command(), + &["rpc-username", "rpc-password"], + ); + } + use super::*; #[rstest] diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 7001cd1bd2..4881f668fb 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -279,6 +279,14 @@ pub async fn setup( #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; + // A rotated log file carries the base name plus a dot and a numeric suffix. + fn is_rotated_log_name(log_file_name: &str, name: &str) -> bool { + name.strip_prefix(log_file_name).is_some_and(|suffix| { + suffix.strip_prefix('.').is_some_and(|rest| { + !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit()) + }) + }) + } match std::fs::read_dir(data_dir.join("logs")) { // The first run has no logs directory yet. Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} @@ -292,22 +300,24 @@ pub async fn setup( continue; } }; - // Skip directories and symlinks so the mode is normally never applied - // through a link or to anything but a regular file. + // Skip entries that are not regular files so the mode is normally + // never applied through a link or to anything but a regular file; + // a matching name left as a directory, symlink, or special file is + // reported below instead of being silently ignored. + let name = entry.file_name().to_string_lossy().into_owned(); if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { + if name == log_file_name || is_rotated_log_name(&log_file_name, &name) { + permission_fix_errors.push(( + entry.path(), + std::io::Error::other("skipped: not a regular file"), + )); + } continue; } // Only touch the log file of this executable and its rotated variants. - let name = entry.file_name().to_string_lossy().into_owned(); - let is_rotated_log = - name.strip_prefix(&log_file_name).is_some_and(|suffix| { - suffix.strip_prefix('.').is_some_and(|rest| { - !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit()) - }) - }); - if name == log_file_name || is_rotated_log { + if name == log_file_name || is_rotated_log_name(&log_file_name, &name) { // Capture the identity at listing time so a swap between the - // listing and the open is detected by the check below. + // listing and the mode change is detected by the check below. let listed = match entry.metadata() { Ok(listed) => listed, Err(err) => { @@ -315,24 +325,25 @@ pub async fn setup( continue; } }; - // Open the file and change the mode through the descriptor. A - // path swapped in after this point is reported as a mismatch - // instead of having its mode changed. - if let Err(err) = std::fs::OpenOptions::new() - .read(true) - .open(entry.path()) - .and_then(|file| { - use std::os::unix::fs::MetadataExt; - let opened = file.metadata()?; - if (listed.dev(), listed.ino()) != (opened.dev(), opened.ino()) - { - return Err(std::io::Error::other( - "log file changed while scanning", - )); - } - file.set_permissions(std::fs::Permissions::from_mode(0o600)) - }) - { + // Change the mode through the path rather than an open file: + // chmod never blocks, while opening a path swapped in for a FIFO + // would wait for a writer. A swap is instead detected after the + // fact by comparing the listed identity with the path's current + // identity and reported as an error. + if let Err(err) = std::fs::set_permissions( + entry.path(), + std::fs::Permissions::from_mode(0o600), + ) + .and_then(|()| { + use std::os::unix::fs::MetadataExt; + let current = std::fs::symlink_metadata(entry.path())?; + if (listed.dev(), listed.ino()) != (current.dev(), current.ino()) { + return Err(std::io::Error::other( + "log file changed while scanning", + )); + } + Ok(()) + }) { permission_fix_errors.push((entry.path(), err)); } } diff --git a/utils/Cargo.toml b/utils/Cargo.toml index eac3709fe8..b49cb44974 100644 --- a/utils/Cargo.toml +++ b/utils/Cargo.toml @@ -46,6 +46,9 @@ loom.workspace = true [features] tokio-console = ["logging/tokio-console"] dev = [] # used by fixed-hash +# Gates the clap test helpers; enabled through a dev-dependency entry by the crates +# whose test suites use them. +test-util = [] [[bench]] name = "benches" diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 60af9e6d7b..5fe2f8010c 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -65,8 +65,9 @@ pub fn env_adder_hiding_env_values<'a>( } /// Test helper: asserts that every option named in `secret_options` exists in `cmd` -/// (recursively, including all nested subcommands) and has its environment-sourced value -/// hidden from the help output. +/// (recursively, including all nested subcommands), is bound to an environment variable, +/// and has its environment-sourced value hidden from the help output. +#[cfg(feature = "test-util")] #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { fn collect_args<'a>(cmd: &'a clap::Command, args: &mut Vec<&'a clap::Arg>) { @@ -84,6 +85,10 @@ pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str] arg.is_hide_env_values_set(), "env value of --{long} is not hidden from help" ); + assert!( + arg.get_env().is_some(), + "--{long} has no environment variable bound; env wiring was dropped" + ); matched.insert(long); } } @@ -115,3 +120,56 @@ mod tests { assert!(arg.get_env().is_some()); } } + +#[cfg(all(test, feature = "test-util"))] +mod secret_options_guard_tests { + use super::*; + + #[test] + fn guard_accepts_a_wired_command_tree() { + let cmd = clap::Command::new("root").subcommand( + clap::Command::new("sub").arg( + add_env("NODE", clap::Arg::new("rpc_password").long("rpc-password")) + .hide_env_values(true), + ), + ); + assert_secret_options_hidden(&cmd, &["rpc-password"]); + } + + #[test] + #[should_panic(expected = "does not match the actual secret options")] + fn guard_rejects_a_stale_entry() { + let cmd = clap::Command::new("root").arg( + add_env("NODE", clap::Arg::new("rpc_password").long("rpc-password")) + .hide_env_values(true), + ); + assert_secret_options_hidden(&cmd, &["rpc-password", "old-option"]); + } + + #[test] + #[should_panic(expected = "does not match the actual secret options")] + fn guard_rejects_a_missing_entry() { + let cmd = clap::Command::new("root") + .arg(add_env("NODE", clap::Arg::new("datadir").long("datadir"))); + assert_secret_options_hidden(&cmd, &["rpc-password"]); + } + + #[test] + #[should_panic(expected = "not hidden from help")] + fn guard_rejects_an_unhidden_option() { + let cmd = clap::Command::new("root").arg( + add_env("NODE", clap::Arg::new("rpc_password").long("rpc-password")) + // The env wiring is present but the value is not hidden. + .hide_env_values(false), + ); + assert_secret_options_hidden(&cmd, &["rpc-password"]); + } + + #[test] + #[should_panic(expected = "no environment variable bound")] + fn guard_rejects_a_dropped_env_binding() { + let cmd = clap::Command::new("root") + .arg(clap::Arg::new("rpc_password").long("rpc-password").hide_env_values(true)); + assert_secret_options_hidden(&cmd, &["rpc-password"]); + } +} diff --git a/wallet/wallet-cli-lib/Cargo.toml b/wallet/wallet-cli-lib/Cargo.toml index 2b9534cc15..a0f456158a 100644 --- a/wallet/wallet-cli-lib/Cargo.toml +++ b/wallet/wallet-cli-lib/Cargo.toml @@ -51,6 +51,7 @@ futures.workspace = true prettytable-rs = "0.10" [dev-dependencies] +utils = { path = "../../utils", features = ["test-util"] } blockprod = { path = "../../blockprod" } chainstate-storage = { path = "../../chainstate/storage" } crypto = { path = "../../crypto" } diff --git a/wallet/wallet-rpc-lib/Cargo.toml b/wallet/wallet-rpc-lib/Cargo.toml index ebd4a3280f..b4217d8ba1 100644 --- a/wallet/wallet-rpc-lib/Cargo.toml +++ b/wallet/wallet-rpc-lib/Cargo.toml @@ -42,6 +42,7 @@ tokio.workspace = true [dev-dependencies] +utils = { path = "../../utils", features = ["test-util"] } consensus = { path = "../../consensus" } mempool = { path = "../../mempool" } rpc = { path = "../../rpc", features = ["test-support"] } From 1313fa74063af62fe882a7c222fea3b8279634ce Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 00:48:21 +0200 Subject: [PATCH 30/44] refactor(api-server): pass the database name by value --- api-server/api-server-common/src/storage/impls/postgres/mod.rs | 2 +- utils/src/clap_utils.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/api-server/api-server-common/src/storage/impls/postgres/mod.rs b/api-server/api-server-common/src/storage/impls/postgres/mod.rs index 5b99e76716..c03353b039 100644 --- a/api-server/api-server-common/src/storage/impls/postgres/mod.rs +++ b/api-server/api-server-common/src/storage/impls/postgres/mod.rs @@ -79,7 +79,7 @@ impl TransactionalApiServerPostgresStorage { ); let mut config = tokio_postgres::Config::new(); - config.host(host).port(port).user(user).dbname(&database); + config.host(host).port(port).user(user).dbname(database); if let Some(password) = password { config.password(password); } diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 5fe2f8010c..0927afcf73 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -121,6 +121,9 @@ mod tests { } } +// NOTE: `test-util` is not enabled by default, so `cargo test -p utils` skips these +// guard tests; they run under `cargo test --workspace` (via the consumer crates' +// dev-dependencies) or `cargo test -p utils --features test-util`. #[cfg(all(test, feature = "test-util"))] mod secret_options_guard_tests { use super::*; From 88e3d308035d51267fc4b2ab0d74dceeb1b67294 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 00:54:35 +0200 Subject: [PATCH 31/44] docs(utils): correct how the guard tests are selected --- utils/src/clap_utils.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 0927afcf73..12e0b21a48 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -122,8 +122,11 @@ mod tests { } // NOTE: `test-util` is not enabled by default, so `cargo test -p utils` skips these -// guard tests; they run under `cargo test --workspace` (via the consumer crates' -// dev-dependencies) or `cargo test -p utils --features test-util`. +// guard tests, and `cargo test --workspace` does not run them either: with resolver +// v2/v3, `test-util` enabled by consumer crates' dev-dependencies applies only to the +// lib units built for those consumers (compiled without `cfg(test)`), never to this +// crate's own test units. They run only with an explicit +// `cargo test -p utils --features test-util` (or `--all-features`). #[cfg(all(test, feature = "test-util"))] mod secret_options_guard_tests { use super::*; From 15b7c9a67308014f5af656b7084b3910dfb305c3 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:12:58 +0200 Subject: [PATCH 32/44] fix(node-lib): restrict old log files even without file logging --- api-server/api-server-common/src/lib.rs | 7 +- node-lib/src/runner.rs | 204 ++++++++++++------------ utils/src/clap_utils.rs | 15 +- 3 files changed, 119 insertions(+), 107 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 0aa76b802a..3e60c47edf 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -57,7 +57,12 @@ pub fn parse_args_or_exit() -> T { if error_is_value_free(err.kind()) { let _ = err.print(); } else { - eprintln!("Invalid command line arguments; run with --help for usage"); + // The error kind's rendering is a static category name and can never + // contain argument values, so it is safe to include here. + eprintln!( + "Invalid command line arguments ({:?}); run with --help for usage", + err.kind() + ); } std::process::exit(2); } diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 4881f668fb..a9e3b993c7 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -240,6 +240,89 @@ async fn initialize( Ok((manager, controller)) } +/// Restricts the permissions of log files left by earlier versions (which may hold +/// material dumped from the command line) to owner-only. Returns per-file failures so +/// the caller can report them once logging is initialized. Matching entries that are +/// not regular files are reported as skipped. +#[cfg(unix)] +fn collect_log_permission_fix_errors( + data_dir: &std::path::Path, + log_file_name: &str, +) -> Vec<(std::path::PathBuf, std::io::Error)> { + use std::os::unix::fs::PermissionsExt; + let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); + // A rotated log file carries the base name plus a dot and a numeric suffix. + fn is_rotated_log_name(log_file_name: &str, name: &str) -> bool { + name.strip_prefix(log_file_name).is_some_and(|suffix| { + suffix + .strip_prefix('.') + .is_some_and(|rest| !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit())) + }) + } + match std::fs::read_dir(data_dir.join("logs")) { + // The first run has no logs directory yet. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => permission_fix_errors.push((data_dir.join("logs"), err)), + Ok(entries) => { + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + permission_fix_errors.push((data_dir.join("logs"), err)); + continue; + } + }; + // Skip entries that are not regular files so the mode is normally never + // applied through a link or to anything but a regular file; a matching + // name left as a directory, symlink, or special file is reported below + // instead of being silently ignored. + let name = entry.file_name().to_string_lossy().into_owned(); + if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { + if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + permission_fix_errors.push(( + entry.path(), + std::io::Error::other("skipped: not a regular file"), + )); + } + continue; + } + // Only touch the log file of this executable and its rotated variants. + if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + // Capture the identity at listing time so a swap between the listing + // and the mode change is detected by the check below. + let listed = match entry.metadata() { + Ok(listed) => listed, + Err(err) => { + permission_fix_errors.push((entry.path(), err)); + continue; + } + }; + // Change the mode through the path rather than an open file: chmod + // never blocks, while opening a path swapped in for a FIFO would wait + // for a writer. A swap is instead detected after the fact by + // comparing the listed identity with the path's current identity and + // reported as an error. + if let Err(err) = std::fs::set_permissions( + entry.path(), + std::fs::Permissions::from_mode(0o600), + ) + .and_then(|()| { + use std::os::unix::fs::MetadataExt; + let current = std::fs::symlink_metadata(entry.path())?; + if (listed.dev(), listed.ino()) != (current.dev(), current.ino()) { + return Err(std::io::Error::other("log file changed while scanning")); + } + Ok(()) + }) { + permission_fix_errors.push((entry.path(), err)); + } + } + } + } + } + permission_fix_errors +} + /// Processes options and potentially runs the node. pub async fn setup( options: OptionsWithResolvedCommand, @@ -260,97 +343,22 @@ pub async fn setup( // Init logging let main_log_writer_settings = logging::default_writer_settings(default_log_filter.to_owned()); + let log_file_name = std::env::current_exe().map_or_else( + |_| DEFAULT_LOG_FILE_NAME.to_owned(), + |exe| { + exe.as_path().file_stem().and_then(|stem| stem.to_str()).map_or_else( + || DEFAULT_LOG_FILE_NAME.to_owned(), + |s| format!("{}.log", s.to_owned()), + ) + }, + ); + // Log files written by earlier versions may have wider permissions; restrict them too. + // This best-effort hardening runs even when this invocation does not write logs to a + // file. The scan and the mode change are separate steps, so a path swapped in between + // is skipped or reported. Failures are reported once logging is initialized below. + #[cfg(unix)] + let permission_fix_errors = collect_log_permission_fix_errors(&data_dir, &log_file_name); if options.log_to_file_option_set() { - let log_file_name = std::env::current_exe().map_or_else( - |_| DEFAULT_LOG_FILE_NAME.to_owned(), - |exe| { - exe.as_path().file_stem().and_then(|stem| stem.to_str()).map_or_else( - || DEFAULT_LOG_FILE_NAME.to_owned(), - |s| format!("{}.log", s.to_owned()), - ) - }, - ); - // Log files written by earlier versions may have wider permissions; restrict them too. - // This is best-effort hardening: the scan and the mode change are separate steps, so a - // path swapped in between is skipped or reported. Failures are collected and reported - // once logging is initialized below. - #[cfg(unix)] - let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - // A rotated log file carries the base name plus a dot and a numeric suffix. - fn is_rotated_log_name(log_file_name: &str, name: &str) -> bool { - name.strip_prefix(log_file_name).is_some_and(|suffix| { - suffix.strip_prefix('.').is_some_and(|rest| { - !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit()) - }) - }) - } - match std::fs::read_dir(data_dir.join("logs")) { - // The first run has no logs directory yet. - Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} - Err(err) => permission_fix_errors.push((data_dir.join("logs"), err)), - Ok(entries) => { - for entry in entries { - let entry = match entry { - Ok(entry) => entry, - Err(err) => { - permission_fix_errors.push((data_dir.join("logs"), err)); - continue; - } - }; - // Skip entries that are not regular files so the mode is normally - // never applied through a link or to anything but a regular file; - // a matching name left as a directory, symlink, or special file is - // reported below instead of being silently ignored. - let name = entry.file_name().to_string_lossy().into_owned(); - if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { - if name == log_file_name || is_rotated_log_name(&log_file_name, &name) { - permission_fix_errors.push(( - entry.path(), - std::io::Error::other("skipped: not a regular file"), - )); - } - continue; - } - // Only touch the log file of this executable and its rotated variants. - if name == log_file_name || is_rotated_log_name(&log_file_name, &name) { - // Capture the identity at listing time so a swap between the - // listing and the mode change is detected by the check below. - let listed = match entry.metadata() { - Ok(listed) => listed, - Err(err) => { - permission_fix_errors.push((entry.path(), err)); - continue; - } - }; - // Change the mode through the path rather than an open file: - // chmod never blocks, while opening a path swapped in for a FIFO - // would wait for a writer. A swap is instead detected after the - // fact by comparing the listed identity with the path's current - // identity and reported as an error. - if let Err(err) = std::fs::set_permissions( - entry.path(), - std::fs::Permissions::from_mode(0o600), - ) - .and_then(|()| { - use std::os::unix::fs::MetadataExt; - let current = std::fs::symlink_metadata(entry.path())?; - if (listed.dev(), listed.ino()) != (current.dev(), current.ino()) { - return Err(std::io::Error::other( - "log file changed while scanning", - )); - } - Ok(()) - }) { - permission_fix_errors.push((entry.path(), err)); - } - } - } - } - } - } let log_file = FileRotate::new( data_dir.join(format!("logs/{}", log_file_name)), AppendCount::new(13), // total 14 files @@ -372,17 +380,17 @@ pub async fn setup( )), }), ); - #[cfg(unix)] - for (path, err) in &permission_fix_errors { - logging::log::warn!( - "Failed to restrict permissions of log file {}: {}", - path.display(), - err - ); - } } else { logging::init_logging_generic(main_log_writer_settings, logging::no_writer_settings()); } + #[cfg(unix)] + for (path, err) in &permission_fix_errors { + logging::log::warn!( + "Failed to restrict permissions of log file {}: {}", + path.display(), + err + ); + } // Clean data dir if needed if options.clean_data_option_set() { diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 12e0b21a48..a3abcfa239 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -67,7 +67,7 @@ pub fn env_adder_hiding_env_values<'a>( /// Test helper: asserts that every option named in `secret_options` exists in `cmd` /// (recursively, including all nested subcommands), is bound to an environment variable, /// and has its environment-sourced value hidden from the help output. -#[cfg(feature = "test-util")] +#[cfg(any(test, feature = "test-util"))] #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { fn collect_args<'a>(cmd: &'a clap::Command, args: &mut Vec<&'a clap::Arg>) { @@ -121,13 +121,12 @@ mod tests { } } -// NOTE: `test-util` is not enabled by default, so `cargo test -p utils` skips these -// guard tests, and `cargo test --workspace` does not run them either: with resolver -// v2/v3, `test-util` enabled by consumer crates' dev-dependencies applies only to the -// lib units built for those consumers (compiled without `cfg(test)`), never to this -// crate's own test units. They run only with an explicit -// `cargo test -p utils --features test-util` (or `--all-features`). -#[cfg(all(test, feature = "test-util"))] +// The helper above is additionally gated on `test-util` for non-test builds; in this +// crate's own test units plain `cfg(test)` suffices, so the guard tests run under a +// plain `cargo test -p utils` and in `cargo test --workspace`. Consumer test suites +// reach the helper through the `test-util` feature enabled in their dev-dependency +// entries. +#[cfg(test)] mod secret_options_guard_tests { use super::*; From 0606708d223157bbb7bbf1cbe7f5aa96ffec3acd Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:19:28 +0200 Subject: [PATCH 33/44] fix(node-lib): report log listing failures with their cause --- node-lib/src/runner.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index a9e3b993c7..4ba6d7c5d1 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -277,7 +277,16 @@ fn collect_log_permission_fix_errors( // name left as a directory, symlink, or special file is reported below // instead of being silently ignored. let name = entry.file_name().to_string_lossy().into_owned(); - if !entry.file_type().is_ok_and(|file_type| file_type.is_file()) { + let is_regular_file = match entry.file_type() { + Ok(file_type) => file_type.is_file(), + Err(err) => { + if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + permission_fix_errors.push((entry.path(), err)); + } + continue; + } + }; + if !is_regular_file { if name == log_file_name || is_rotated_log_name(log_file_name, &name) { permission_fix_errors.push(( entry.path(), From fd41e31f53678ec12c310845b6bce033045c5ec4 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:21:36 +0200 Subject: [PATCH 34/44] refactor(node-lib): compute the log name match once per entry --- node-lib/src/runner.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 4ba6d7c5d1..41bdbf47b8 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -277,17 +277,19 @@ fn collect_log_permission_fix_errors( // name left as a directory, symlink, or special file is reported below // instead of being silently ignored. let name = entry.file_name().to_string_lossy().into_owned(); + let is_matching_log_name = + name == log_file_name || is_rotated_log_name(log_file_name, &name); let is_regular_file = match entry.file_type() { Ok(file_type) => file_type.is_file(), Err(err) => { - if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + if is_matching_log_name { permission_fix_errors.push((entry.path(), err)); } continue; } }; if !is_regular_file { - if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + if is_matching_log_name { permission_fix_errors.push(( entry.path(), std::io::Error::other("skipped: not a regular file"), @@ -296,7 +298,7 @@ fn collect_log_permission_fix_errors( continue; } // Only touch the log file of this executable and its rotated variants. - if name == log_file_name || is_rotated_log_name(log_file_name, &name) { + if is_matching_log_name { // Capture the identity at listing time so a swap between the listing // and the mode change is detected by the check below. let listed = match entry.metadata() { From 2bb65262dbad639af8e73aab83c7e040d86c97cd Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:24:57 +0200 Subject: [PATCH 35/44] fix(node-lib): declare the log name helper before its first use --- node-lib/src/runner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 41bdbf47b8..b888f69801 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -250,7 +250,6 @@ fn collect_log_permission_fix_errors( log_file_name: &str, ) -> Vec<(std::path::PathBuf, std::io::Error)> { use std::os::unix::fs::PermissionsExt; - let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); // A rotated log file carries the base name plus a dot and a numeric suffix. fn is_rotated_log_name(log_file_name: &str, name: &str) -> bool { name.strip_prefix(log_file_name).is_some_and(|suffix| { @@ -259,6 +258,7 @@ fn collect_log_permission_fix_errors( .is_some_and(|rest| !rest.is_empty() && rest.bytes().all(|b| b.is_ascii_digit())) }) } + let mut permission_fix_errors: Vec<(std::path::PathBuf, std::io::Error)> = Vec::new(); match std::fs::read_dir(data_dir.join("logs")) { // The first run has no logs directory yet. Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} From 4f38b9ce322703dbe884747bc3b93b314621ab6f Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:46:31 +0200 Subject: [PATCH 36/44] fix(cli): hide staking settings secrets and zeroize rpc credentials --- Cargo.lock | 1 + api-server/scanner-daemon/src/config.rs | 3 ++ api-server/web-server/src/config.rs | 3 ++ node-lib/Cargo.toml | 1 + node-lib/src/options.rs | 55 ++++++++++++++----------- node-lib/src/runner.rs | 8 ++-- utils/src/clap_utils.rs | 15 ++++--- 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f2aa860e9..09fd31b5d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5679,6 +5679,7 @@ dependencies = [ "toml 0.8.23", "utils", "utils-networking", + "zeroize", ] [[package]] diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 2efd389ff2..ee620ad75a 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -21,6 +21,9 @@ use utils_networking::NetworkAddressWithPort; /// Options whose values can hold a secret; their environment-sourced values are hidden /// from the help output. +// NOTE: when adding a secret-bearing field, update BOTH the Debug impl below (the +// exhaustive destructure enforces this) AND this list (nothing enforces it; the guard +// test only rejects stale entries, not omissions). const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; #[derive(Parser)] diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index e6a83a95e4..d731f9a6d6 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -31,6 +31,9 @@ const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; /// Options whose values can hold a secret; their environment-sourced values are hidden /// from the help output (see the `mut_args` attribute and the guard test below). +// NOTE: when adding a secret-bearing field, update BOTH the Debug impl below (the +// exhaustive destructure enforces this) AND this list (nothing enforces it; the guard +// test only rejects stale entries, not omissions). const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; #[derive(Parser)] diff --git a/node-lib/Cargo.toml b/node-lib/Cargo.toml index debdeda35f..09b97fa39b 100644 --- a/node-lib/Cargo.toml +++ b/node-lib/Cargo.toml @@ -18,6 +18,7 @@ storage-lmdb = { path = "../storage/lmdb" } subsystem = { path = "../subsystem" } test-rpc-functions = { path = "../test-rpc-functions" } utils = { path = "../utils" } +zeroize.workspace = true utils-networking = { path = "../utils/networking" } anyhow.workspace = true diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index a477a9f983..65bc320bfc 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -172,25 +172,21 @@ impl TopLevelOptions { } } +// Options whose values can hold a secret: RPC credentials and the genesis staking +// settings, which carry staking and VRF private keys. Their environment-sourced values +// are hidden from the help output (see the `mut_args` attributes and the guard test). +const SECRET_OPTIONS: &[&str] = &["rpc-username", "rpc-password", "chain-genesis-staking-settings"]; + #[derive(Subcommand, Clone, Debug)] pub enum Command { /// Run the mainnet node. - #[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "MAINNET_NODE", - &["rpc-username", "rpc-password"] - )))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values("MAINNET_NODE", SECRET_OPTIONS)))] Mainnet(RunOptions), /// Run the testnet node. - #[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "TESTNET_NODE", - &["rpc-username", "rpc-password"] - )))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values("TESTNET_NODE", SECRET_OPTIONS)))] Testnet(RunOptions), /// Run the regtest node. - #[clap(mut_args(clap_utils::env_adder_hiding_env_values( - "REGTEST_NODE", - &["rpc-username", "rpc-password"] - )))] + #[clap(mut_args(clap_utils::env_adder_hiding_env_values("REGTEST_NODE", SECRET_OPTIONS)))] Regtest(Box), } @@ -259,22 +255,34 @@ pub struct RegtestOptions { /// /// Note: equality comparison is intentionally not derived; a derived comparison invites /// non-constant-time checks on secret values in future authentication decisions. +/// +/// Note: equality comparison is intentionally not derived; a derived comparison invites +/// non-constant-time checks on secret values in future authentication decisions. +/// +/// Note: the inner value is zeroized when the value is dropped, so the memory holding +/// the credential is wiped instead of lingering in freed memory (for zeroizable types). #[derive(Clone, Default)] -pub struct Redacted(T); +pub struct Redacted(T); -impl fmt::Debug for Redacted { +impl Drop for Redacted { + fn drop(&mut self) { + self.0.zeroize(); + } +} + +impl fmt::Debug for Redacted { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("") } } -impl From for Redacted { +impl From for Redacted { fn from(value: T) -> Self { Self(value) } } -impl FromStr for Redacted { +impl FromStr for Redacted { type Err = T::Err; fn from_str(s: &str) -> Result { @@ -282,13 +290,7 @@ impl FromStr for Redacted { } } -impl Redacted { - pub fn into_inner(self) -> T { - self.0 - } -} - -impl AsRef for Redacted { +impl AsRef for Redacted { fn as_ref(&self) -> &T { &self.0 } @@ -296,9 +298,12 @@ impl AsRef for Redacted { // Serialization is intentionally not supported so that configuration dumps cannot emit // credentials; only deserialization (reading a configuration file) is provided. +// Consuming access is intentionally not provided either: `into_inner` would move the +// value out of a type with a `Drop` impl, and dropped values are zeroized, not handed +// out. Read access goes through `AsRef`. impl<'de, T> serde::Deserialize<'de> for Redacted where - T: serde::Deserialize<'de>, + T: serde::Deserialize<'de> + zeroize::Zeroize, { fn deserialize>(deserializer: D) -> Result { T::deserialize(deserializer).map(Self) @@ -536,7 +541,7 @@ mod tests { fn secret_options_env_values_are_hidden() { utils::clap_utils::assert_secret_options_hidden( &::command(), - &["rpc-username", "rpc-password"], + SECRET_OPTIONS, ); } diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index b888f69801..8cd03ea1c8 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -310,9 +310,11 @@ fn collect_log_permission_fix_errors( }; // Change the mode through the path rather than an open file: chmod // never blocks, while opening a path swapped in for a FIFO would wait - // for a writer. A swap is instead detected after the fact by - // comparing the listed identity with the path's current identity and - // reported as an error. + // for a writer. The pre-checks above do not follow links, but chmod + // itself does, so a path swapped in between the listing and this call + // could still redirect the mode change; such a swap is detected after + // the fact by comparing the listed identity with the path's current + // identity and reported as an error rather than silently accepted. if let Err(err) = std::fs::set_permissions( entry.path(), std::fs::Permissions::from_mode(0o600), diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index a3abcfa239..93bf4d2a8e 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -51,6 +51,11 @@ pub fn env_adder(infix: &str) -> impl FnMut(Arg) -> Arg + '_ { /// Same as [`env_adder`], but the values of the options named in `hidden` are excluded /// from the help output, so secrets provided through the environment are not rendered /// as `[env: NAME=value]`. +/// +/// Matching is by exact long name: an option missing from (or misspelled in) `hidden` +/// silently keeps rendering its environment value in `--help`. Always pair this helper +/// with [`assert_secret_options_hidden`] in a test so the list is pinned against the +/// derived command. pub fn env_adder_hiding_env_values<'a>( infix: &'a str, hidden: &'a [&'a str], @@ -67,6 +72,9 @@ pub fn env_adder_hiding_env_values<'a>( /// Test helper: asserts that every option named in `secret_options` exists in `cmd` /// (recursively, including all nested subcommands), is bound to an environment variable, /// and has its environment-sourced value hidden from the help output. +// Gated additionally on `test-util` so consumer test suites can reach it through a +// dev-dependency feature entry, while this crate's own test units get it from plain +// `cfg(test)`. #[cfg(any(test, feature = "test-util"))] #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { @@ -121,11 +129,8 @@ mod tests { } } -// The helper above is additionally gated on `test-util` for non-test builds; in this -// crate's own test units plain `cfg(test)` suffices, so the guard tests run under a -// plain `cargo test -p utils` and in `cargo test --workspace`. Consumer test suites -// reach the helper through the `test-util` feature enabled in their dev-dependency -// entries. +// The helper carries the `test-util` gate (see its definition); these tests exercise it +// in this crate's own test units, which get it from plain `cfg(test)`. #[cfg(test)] mod secret_options_guard_tests { use super::*; From 02b583b3b88f3624561e46714d9f76d54c07c40a Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 01:53:14 +0200 Subject: [PATCH 37/44] docs(api-server): scope the secret option list notes --- api-server/scanner-daemon/src/config.rs | 9 ++++++--- api-server/web-server/src/config.rs | 9 ++++++--- node-lib/src/options.rs | 3 --- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index ee620ad75a..84e0ffa74f 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -21,9 +21,12 @@ use utils_networking::NetworkAddressWithPort; /// Options whose values can hold a secret; their environment-sourced values are hidden /// from the help output. -// NOTE: when adding a secret-bearing field, update BOTH the Debug impl below (the -// exhaustive destructure enforces this) AND this list (nothing enforces it; the guard -// test only rejects stale entries, not omissions). +// NOTE: when adding a secret-bearing field to this struct, update BOTH the Debug impl +// below (the exhaustive destructure enforces this) AND this list (nothing enforces it; +// the guard test only rejects stale entries, not omissions). A secret-bearing field +// added to the flattened `PostgresConfig` escapes the destructure here: it is guarded +// by `PostgresConfig`'s own Debug impl in api-server-common, but it must still be +// added to this list. const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; #[derive(Parser)] diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index d731f9a6d6..6912b0b213 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -31,9 +31,12 @@ const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; /// Options whose values can hold a secret; their environment-sourced values are hidden /// from the help output (see the `mut_args` attribute and the guard test below). -// NOTE: when adding a secret-bearing field, update BOTH the Debug impl below (the -// exhaustive destructure enforces this) AND this list (nothing enforces it; the guard -// test only rejects stale entries, not omissions). +// NOTE: when adding a secret-bearing field to this struct, update BOTH the Debug impl +// below (the exhaustive destructure enforces this) AND this list (nothing enforces it; +// the guard test only rejects stale entries, not omissions). A secret-bearing field +// added to the flattened `PostgresConfig` escapes the destructure here: it is guarded +// by `PostgresConfig`'s own Debug impl in api-server-common, but it must still be +// added to this list. const SECRET_OPTIONS: &[&str] = &["node-rpc-username", "node-rpc-password", "postgres-password"]; #[derive(Parser)] diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 65bc320bfc..d47e2dbe8b 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -256,9 +256,6 @@ pub struct RegtestOptions { /// Note: equality comparison is intentionally not derived; a derived comparison invites /// non-constant-time checks on secret values in future authentication decisions. /// -/// Note: equality comparison is intentionally not derived; a derived comparison invites -/// non-constant-time checks on secret values in future authentication decisions. -/// /// Note: the inner value is zeroized when the value is dropped, so the memory holding /// the credential is wiped instead of lingering in freed memory (for zeroizable types). #[derive(Clone, Default)] From b0bb3d2ea8cda60e4a9c3acef71cb768633e0769 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:07:06 +0200 Subject: [PATCH 38/44] test(cli): classify every help option as secret or non-secret --- api-server/api-server-common/src/lib.rs | 26 +++++++++++--------- api-server/scanner-daemon/src/config.rs | 25 ++++++++++++++++++- api-server/web-server/src/config.rs | 32 ++++++++++++++++++++++--- utils/src/clap_utils.rs | 8 +++++-- 4 files changed, 74 insertions(+), 17 deletions(-) diff --git a/api-server/api-server-common/src/lib.rs b/api-server/api-server-common/src/lib.rs index 3e60c47edf..9086995f64 100644 --- a/api-server/api-server-common/src/lib.rs +++ b/api-server/api-server-common/src/lib.rs @@ -177,18 +177,23 @@ impl From for ChainType { } } +const DEFAULT_POSTGRES_HOST: &str = "localhost"; +const DEFAULT_POSTGRES_PORT: u16 = 5432; +const DEFAULT_POSTGRES_USER: &str = "postgres"; +const DEFAULT_POSTGRES_MAX_CONNECTIONS: u32 = 10; + #[derive(Parser, PartialEq)] pub struct PostgresConfig { /// Postgres host - #[clap(long, default_value = "localhost")] + #[clap(long, default_value = DEFAULT_POSTGRES_HOST)] pub postgres_host: String, /// Postgres port - #[clap(long, default_value = "5432")] + #[clap(long, default_value_t = DEFAULT_POSTGRES_PORT)] pub postgres_port: u16, /// Postgres user - #[clap(long, default_value = "postgres")] + #[clap(long, default_value = DEFAULT_POSTGRES_USER)] pub postgres_user: String, /// Postgres password @@ -200,7 +205,7 @@ pub struct PostgresConfig { pub postgres_database: Option, /// Postgres max connections - #[clap(long, default_value = "10")] + #[clap(long, default_value_t = DEFAULT_POSTGRES_MAX_CONNECTIONS)] pub postgres_max_connections: u32, } @@ -233,16 +238,15 @@ impl std::fmt::Debug for PostgresConfig { impl Default for PostgresConfig { fn default() -> Self { - // Literal defaults mirroring the clap `default_value` attributes above; parsing - // here would read the process's real arguments and is not safe outside of the - // actual CLI startup. + // Shared constants, not parsing: parsing here would read the process's real + // arguments and is not safe outside of the actual CLI startup. Self { - postgres_host: "localhost".to_string(), - postgres_port: 5432, - postgres_user: "postgres".to_string(), + postgres_host: DEFAULT_POSTGRES_HOST.to_string(), + postgres_port: DEFAULT_POSTGRES_PORT, + postgres_user: DEFAULT_POSTGRES_USER.to_string(), postgres_password: None, postgres_database: None, - postgres_max_connections: 10, + postgres_max_connections: DEFAULT_POSTGRES_MAX_CONNECTIONS, } } } diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 84e0ffa74f..c5aae7a5d3 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -99,7 +99,30 @@ mod tests { #[test] fn secret_options_are_hidden_from_help() { - clap_utils::assert_secret_options_hidden(&ApiServerScannerArgs::command(), SECRET_OPTIONS); + let cmd = ApiServerScannerArgs::command(); + clap_utils::assert_secret_options_hidden(&cmd, SECRET_OPTIONS); + + // Options explicitly classified as non-secret; anything else must be listed in + // SECRET_OPTIONS or the assertion below fails. + const NON_SECRET_OPTIONS: &[&str] = &[ + "network", + "node-rpc-address", + "node-rpc-cookie-file", + "postgres-host", + "postgres-port", + "postgres-user", + "postgres-database", + "postgres-max-connections", + ]; + let unclassified: Vec<_> = cmd + .get_arguments() + .filter_map(|arg| arg.get_long()) + .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) + .collect(); + assert!( + unclassified.is_empty(), + "env-bound options not classified as secret or non-secret: {unclassified:?}" + ); } #[test] diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 6912b0b213..2f93c4f9ef 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -188,9 +188,35 @@ mod tests { #[test] fn secret_options_are_hidden_from_help() { - clap_utils::assert_secret_options_hidden( - &ApiServerWebServerConfig::command(), - SECRET_OPTIONS, + let cmd = ApiServerWebServerConfig::command(); + clap_utils::assert_secret_options_hidden(&cmd, SECRET_OPTIONS); + + // Options explicitly classified as non-secret; anything else must be listed in + // SECRET_OPTIONS or the assertion below fails. + const NON_SECRET_OPTIONS: &[&str] = &[ + "network", + "bind-address", + "enable-post-routes", + "node-rpc-address", + "node-rpc-cookie-file", + "postgres-host", + "postgres-port", + "postgres-user", + "postgres-database", + "postgres-max-connections", + "stream-events-broadcast-capacity", + "stream-events-max-subscribers", + "stream-events-poll-interval-secs", + "stream-events-keepalive-interval-secs", + ]; + let unclassified: Vec<_> = cmd + .get_arguments() + .filter_map(|arg| arg.get_long()) + .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) + .collect(); + assert!( + unclassified.is_empty(), + "env-bound options not classified as secret or non-secret: {unclassified:?}" ); } diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 93bf4d2a8e..cbcdc8afce 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -100,9 +100,12 @@ pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str] matched.insert(long); } } + let mut matched: Vec<_> = matched.into_iter().collect(); + let mut expected: Vec<_> = secret_options.to_vec(); + matched.sort_unstable(); + expected.sort_unstable(); assert_eq!( - matched, - secret_options.iter().copied().collect::>(), + matched, expected, "the secret options list does not match the actual secret options; \ stale entries pass silently and missing entries leak env values in --help" ); @@ -122,6 +125,7 @@ mod tests { let arg = wrap(clap::Arg::new("rpc_username").long("rpc-username")); assert!(arg.is_hide_env_values_set()); + assert!(arg.get_env().is_some()); let arg = wrap(clap::Arg::new("datadir").long("datadir")); assert!(!arg.is_hide_env_values_set()); From 13459fb3a86efa26455785c96d7494b5dd4b667d Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:13:37 +0200 Subject: [PATCH 39/44] test(api-server): flag stale non-secret option entries --- api-server/scanner-daemon/src/config.rs | 17 ++++++++++++++--- api-server/web-server/src/config.rs | 17 ++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index c5aae7a5d3..6924bcd2d2 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -114,15 +114,26 @@ mod tests { "postgres-database", "postgres-max-connections", ]; - let unclassified: Vec<_> = cmd - .get_arguments() - .filter_map(|arg| arg.get_long()) + let longs: std::collections::HashSet<_> = + cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); + let unclassified: Vec<_> = longs + .iter() + .copied() .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) .collect(); assert!( unclassified.is_empty(), "env-bound options not classified as secret or non-secret: {unclassified:?}" ); + let stale: Vec<_> = NON_SECRET_OPTIONS + .iter() + .copied() + .filter(|long| !longs.contains(long)) + .collect(); + assert!( + stale.is_empty(), + "non-secret options not found in the command (stale entries): {stale:?}" + ); } #[test] diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 2f93c4f9ef..7ebf718562 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -209,15 +209,26 @@ mod tests { "stream-events-poll-interval-secs", "stream-events-keepalive-interval-secs", ]; - let unclassified: Vec<_> = cmd - .get_arguments() - .filter_map(|arg| arg.get_long()) + let longs: std::collections::HashSet<_> = + cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); + let unclassified: Vec<_> = longs + .iter() + .copied() .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) .collect(); assert!( unclassified.is_empty(), "env-bound options not classified as secret or non-secret: {unclassified:?}" ); + let stale: Vec<_> = NON_SECRET_OPTIONS + .iter() + .copied() + .filter(|long| !longs.contains(long)) + .collect(); + assert!( + stale.is_empty(), + "non-secret options not found in the command (stale entries): {stale:?}" + ); } #[test] From a0ba911e7aef3c70050133481b241704c11d8130 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:16:21 +0200 Subject: [PATCH 40/44] refactor(utils): share the option classification check --- api-server/scanner-daemon/src/config.rs | 27 +++----------------- api-server/web-server/src/config.rs | 27 +++----------------- utils/src/clap_utils.rs | 33 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 46 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 6924bcd2d2..0bc64dbd32 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -100,10 +100,8 @@ mod tests { #[test] fn secret_options_are_hidden_from_help() { let cmd = ApiServerScannerArgs::command(); - clap_utils::assert_secret_options_hidden(&cmd, SECRET_OPTIONS); - // Options explicitly classified as non-secret; anything else must be listed in - // SECRET_OPTIONS or the assertion below fails. + // SECRET_OPTIONS or the classification check below fails. const NON_SECRET_OPTIONS: &[&str] = &[ "network", "node-rpc-address", @@ -114,26 +112,9 @@ mod tests { "postgres-database", "postgres-max-connections", ]; - let longs: std::collections::HashSet<_> = - cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); - let unclassified: Vec<_> = longs - .iter() - .copied() - .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) - .collect(); - assert!( - unclassified.is_empty(), - "env-bound options not classified as secret or non-secret: {unclassified:?}" - ); - let stale: Vec<_> = NON_SECRET_OPTIONS - .iter() - .copied() - .filter(|long| !longs.contains(long)) - .collect(); - assert!( - stale.is_empty(), - "non-secret options not found in the command (stale entries): {stale:?}" - ); + // Every option must be classified as secret or non-secret; the helper includes + // the secret pin above and rejects stale non-secret entries. + clap_utils::assert_options_classified(&cmd, SECRET_OPTIONS, NON_SECRET_OPTIONS); } #[test] diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 7ebf718562..4d05133279 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -189,10 +189,8 @@ mod tests { #[test] fn secret_options_are_hidden_from_help() { let cmd = ApiServerWebServerConfig::command(); - clap_utils::assert_secret_options_hidden(&cmd, SECRET_OPTIONS); - // Options explicitly classified as non-secret; anything else must be listed in - // SECRET_OPTIONS or the assertion below fails. + // SECRET_OPTIONS or the classification check below fails. const NON_SECRET_OPTIONS: &[&str] = &[ "network", "bind-address", @@ -209,26 +207,9 @@ mod tests { "stream-events-poll-interval-secs", "stream-events-keepalive-interval-secs", ]; - let longs: std::collections::HashSet<_> = - cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); - let unclassified: Vec<_> = longs - .iter() - .copied() - .filter(|long| !SECRET_OPTIONS.contains(long) && !NON_SECRET_OPTIONS.contains(long)) - .collect(); - assert!( - unclassified.is_empty(), - "env-bound options not classified as secret or non-secret: {unclassified:?}" - ); - let stale: Vec<_> = NON_SECRET_OPTIONS - .iter() - .copied() - .filter(|long| !longs.contains(long)) - .collect(); - assert!( - stale.is_empty(), - "non-secret options not found in the command (stale entries): {stale:?}" - ); + // Every option must be classified as secret or non-secret; the helper includes + // the secret pin above and rejects stale non-secret entries. + clap_utils::assert_options_classified(&cmd, SECRET_OPTIONS, NON_SECRET_OPTIONS); } #[test] diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index cbcdc8afce..7964e3070b 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -111,6 +111,39 @@ pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str] ); } +/// Test helper: same as [`assert_secret_options_hidden`], plus an exhaustive +/// classification check: every option of `cmd` must be listed in `secret_options` or +/// `non_secret_options`, and every non-secret entry must exist in the command. +#[cfg(any(test, feature = "test-util"))] +#[doc(hidden)] +pub fn assert_options_classified( + cmd: &clap::Command, + secret_options: &[&str], + non_secret_options: &[&str], +) { + assert_secret_options_hidden(cmd, secret_options); + let longs: std::collections::HashSet<_> = + cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); + let unclassified: Vec<_> = longs + .iter() + .copied() + .filter(|long| !secret_options.contains(long) && !non_secret_options.contains(long)) + .collect(); + assert!( + unclassified.is_empty(), + "env-bound options not classified as secret or non-secret: {unclassified:?}" + ); + let stale: Vec<_> = non_secret_options + .iter() + .copied() + .filter(|long| !longs.contains(long)) + .collect(); + assert!( + stale.is_empty(), + "non-secret options not found in the command (stale entries): {stale:?}" + ); +} + #[cfg(test)] mod tests { use super::*; From 21eb9e0a2a459430857603e60b4bd5a6b1723d90 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:34:37 +0200 Subject: [PATCH 41/44] fix(utils): classify subcommand options too --- node-lib/src/options.rs | 3 +++ node-lib/src/runner.rs | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index d47e2dbe8b..ad311c2802 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -175,6 +175,9 @@ impl TopLevelOptions { // Options whose values can hold a secret: RPC credentials and the genesis staking // settings, which carry staking and VRF private keys. Their environment-sourced values // are hidden from the help output (see the `mut_args` attributes and the guard test). +// For chain-genesis-staking-settings this hides the help rendering only; the options +// dump that debug-printed the staking keys was removed rather than redacted, so debug +// output stays secret-safe only as long as no full-options debug formatting returns. const SECRET_OPTIONS: &[&str] = &["rpc-username", "rpc-password", "chain-genesis-staking-settings"]; #[derive(Subcommand, Clone, Debug)] diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 8cd03ea1c8..9e13877c17 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -312,9 +312,11 @@ fn collect_log_permission_fix_errors( // never blocks, while opening a path swapped in for a FIFO would wait // for a writer. The pre-checks above do not follow links, but chmod // itself does, so a path swapped in between the listing and this call - // could still redirect the mode change; such a swap is detected after - // the fact by comparing the listed identity with the path's current - // identity and reported as an error rather than silently accepted. + // still redirects the mode change to whatever it now references; the + // identity comparison below then reports the swap. Fully closing the + // window would need platform-specific open flags (no-follow, or + // non-blocking to keep the FIFO case non-blocking), which would add a + // platform dependency this workspace does not carry. if let Err(err) = std::fs::set_permissions( entry.path(), std::fs::Permissions::from_mode(0o600), From 6c42c77b77b6190d7ffd8257a9b2322f83622f50 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:57:52 +0200 Subject: [PATCH 42/44] fix(common): keep staking keys out of debug output --- common/src/chain/config/regtest.rs | 37 +++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/common/src/chain/config/regtest.rs b/common/src/chain/config/regtest.rs index e042834d5c..9d78f7f975 100644 --- a/common/src/chain/config/regtest.rs +++ b/common/src/chain/config/regtest.rs @@ -42,13 +42,25 @@ const GENESIS_STAKE_PRIVATE_KEY: &str = "008717e6946febd3a33ccdc3f3a27629ec80c33461c33a0fc56b4836fcedd26638"; const GENESIS_VRF_PRIVATE_KEY: &str = "003fcf7b813bec2a293f574b842988895278b396dd72471de2583b242097a59f06e9f3cd7b78d45750afd17292031373fddb5e7a8090db51221038f5e05f29998e"; -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, PartialEq)] pub struct GenesisStakingSettings { pool_id: PoolId, stake_private_key: PrivateKey, vrf_private_key: VRFPrivateKey, } +// The private keys are omitted so debug formatting of the chain options cannot disclose +// staking key material; the pool id is not sensitive and stays visible. +impl std::fmt::Debug for GenesisStakingSettings { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("GenesisStakingSettings") + .field("pool_id", &self.pool_id) + .field("stake_private_key", &"") + .field("vrf_private_key", &"") + .finish() + } +} + #[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] pub enum GenesisStakingSettingsInputErrors { #[error("Duplicate staking setting {0}")] @@ -180,6 +192,29 @@ impl FromStr for GenesisStakingSettings { } } +#[cfg(test)] +mod genesis_staking_settings_tests { + use super::*; + + #[test] + fn debug_output_omits_private_keys() { + let settings = GenesisStakingSettings::from_str(&format!( + "pool_id:{GENESIS_POOL_ID},stake_private_key:{GENESIS_STAKE_PRIVATE_KEY},\ + vrf_private_key:{GENESIS_VRF_PRIVATE_KEY}" + )) + .unwrap(); + let rendering = format!("{settings:?}"); + assert!(rendering.contains(""), "{rendering}"); + assert!( + !rendering.contains("8717e6946febd3a33ccdc3f3a27629ec80c33461c33a0fc56b4836fcedd26638"), + "{rendering}" + ); + assert!(!rendering.contains("3fcf7b813bec2a29"), "{rendering}"); + assert!(!rendering.contains("Scalar"), "{rendering}"); + assert!(!rendering.contains("Schnorrkel"), "{rendering}"); + } +} + impl Display for GenesisStakingSettings { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { let mut values = vec![]; From 69c2acfd59a153166f81ef31ad470f43b8ef7ec6 Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 02:57:52 +0200 Subject: [PATCH 43/44] fix(node-lib): anchor the log scan identity in the directory entry --- node-lib/src/runner.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 9e13877c17..bdf7bec941 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -299,15 +299,11 @@ fn collect_log_permission_fix_errors( } // Only touch the log file of this executable and its rotated variants. if is_matching_log_name { - // Capture the identity at listing time so a swap between the listing - // and the mode change is detected by the check below. - let listed = match entry.metadata() { - Ok(listed) => listed, - Err(err) => { - permission_fix_errors.push((entry.path(), err)); - continue; - } - }; + // Anchor the identity in the directory entry itself: the inode comes + // from the readdir buffer, so it reflects the listing rather than the + // current state of the path. + use std::os::unix::fs::DirEntryExt; + let listed_ino = entry.ino(); // Change the mode through the path rather than an open file: chmod // never blocks, while opening a path swapped in for a FIFO would wait // for a writer. The pre-checks above do not follow links, but chmod @@ -324,7 +320,7 @@ fn collect_log_permission_fix_errors( .and_then(|()| { use std::os::unix::fs::MetadataExt; let current = std::fs::symlink_metadata(entry.path())?; - if (listed.dev(), listed.ino()) != (current.dev(), current.ino()) { + if listed_ino != current.ino() { return Err(std::io::Error::other("log file changed while scanning")); } Ok(()) From 62af497c86b144fd2375ffe5b76920d4319d67ec Mon Sep 17 00:00:00 2001 From: nullPointerEnjoyer Date: Sun, 18 Oct 2026 03:00:21 +0200 Subject: [PATCH 44/44] fix(utils): classify subcommand options too --- api-server/scanner-daemon/src/config.rs | 4 ++- api-server/web-server/src/config.rs | 4 ++- utils/src/clap_utils.rs | 36 +++++++++++++++++++------ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/api-server/scanner-daemon/src/config.rs b/api-server/scanner-daemon/src/config.rs index 0bc64dbd32..551f929c91 100644 --- a/api-server/scanner-daemon/src/config.rs +++ b/api-server/scanner-daemon/src/config.rs @@ -23,7 +23,9 @@ use utils_networking::NetworkAddressWithPort; /// from the help output. // NOTE: when adding a secret-bearing field to this struct, update BOTH the Debug impl // below (the exhaustive destructure enforces this) AND this list (nothing enforces it; -// the guard test only rejects stale entries, not omissions). A secret-bearing field +// the classification check in the guard test rejects omissions: any new option left +// out of both SECRET_OPTIONS and NON_SECRET_OPTIONS fails the test, though it cannot +// tell which list an option belongs in). A secret-bearing field // added to the flattened `PostgresConfig` escapes the destructure here: it is guarded // by `PostgresConfig`'s own Debug impl in api-server-common, but it must still be // added to this list. diff --git a/api-server/web-server/src/config.rs b/api-server/web-server/src/config.rs index 4d05133279..8073901c1e 100644 --- a/api-server/web-server/src/config.rs +++ b/api-server/web-server/src/config.rs @@ -33,7 +33,9 @@ const LISTEN_ADDRESS: &str = "127.0.0.1:3000"; /// from the help output (see the `mut_args` attribute and the guard test below). // NOTE: when adding a secret-bearing field to this struct, update BOTH the Debug impl // below (the exhaustive destructure enforces this) AND this list (nothing enforces it; -// the guard test only rejects stale entries, not omissions). A secret-bearing field +// the classification check in the guard test rejects omissions: any new option left +// out of both SECRET_OPTIONS and NON_SECRET_OPTIONS fails the test, though it cannot +// tell which list an option belongs in). A secret-bearing field // added to the flattened `PostgresConfig` escapes the destructure here: it is guarded // by `PostgresConfig`'s own Debug impl in api-server-common, but it must still be // added to this list. diff --git a/utils/src/clap_utils.rs b/utils/src/clap_utils.rs index 7964e3070b..d24d5bed57 100644 --- a/utils/src/clap_utils.rs +++ b/utils/src/clap_utils.rs @@ -75,15 +75,17 @@ pub fn env_adder_hiding_env_values<'a>( // Gated additionally on `test-util` so consumer test suites can reach it through a // dev-dependency feature entry, while this crate's own test units get it from plain // `cfg(test)`. +#[cfg(any(test, feature = "test-util"))] +fn collect_args<'a>(cmd: &'a clap::Command, args: &mut Vec<&'a clap::Arg>) { + args.extend(cmd.get_arguments()); + for sub in cmd.get_subcommands() { + collect_args(sub, args); + } +} + #[cfg(any(test, feature = "test-util"))] #[doc(hidden)] pub fn assert_secret_options_hidden(cmd: &clap::Command, secret_options: &[&str]) { - fn collect_args<'a>(cmd: &'a clap::Command, args: &mut Vec<&'a clap::Arg>) { - args.extend(cmd.get_arguments()); - for sub in cmd.get_subcommands() { - collect_args(sub, args); - } - } let mut all_args = Vec::new(); collect_args(cmd, &mut all_args); let mut matched = std::collections::HashSet::new(); @@ -122,8 +124,10 @@ pub fn assert_options_classified( non_secret_options: &[&str], ) { assert_secret_options_hidden(cmd, secret_options); + let mut all_args = Vec::new(); + collect_args(cmd, &mut all_args); let longs: std::collections::HashSet<_> = - cmd.get_arguments().filter_map(|arg| arg.get_long()).collect(); + all_args.iter().filter_map(|arg| arg.get_long()).collect(); let unclassified: Vec<_> = longs .iter() .copied() @@ -131,7 +135,7 @@ pub fn assert_options_classified( .collect(); assert!( unclassified.is_empty(), - "env-bound options not classified as secret or non-secret: {unclassified:?}" + "options not classified as secret or non-secret: {unclassified:?}" ); let stale: Vec<_> = non_secret_options .iter() @@ -219,4 +223,20 @@ mod secret_options_guard_tests { .arg(clap::Arg::new("rpc_password").long("rpc-password").hide_env_values(true)); assert_secret_options_hidden(&cmd, &["rpc-password"]); } + + #[test] + #[should_panic(expected = "not classified as secret or non-secret")] + fn classification_rejects_an_unclassified_option() { + let cmd = clap::Command::new("root") + .arg(add_env("NODE", clap::Arg::new("datadir").long("datadir"))); + assert_options_classified(&cmd, &[], &[]); + } + + #[test] + #[should_panic(expected = "stale entries")] + fn classification_rejects_a_stale_non_secret_entry() { + let cmd = clap::Command::new("root") + .arg(add_env("NODE", clap::Arg::new("datadir").long("datadir"))); + assert_options_classified(&cmd, &[], &["datadir", "old-option"]); + } }