Rpc header secret leak fix (#2522)

Author: fnando

cmd/crates/soroban-test/tests/it/config.rs

@@ -720,6 +720,29 @@ fn cannot_create_key_with_alias() {
         .failure();
 }
 
+#[test]
+fn malformed_rpc_header_error_does_not_expose_secret() {
+    let sandbox = TestEnv::default();
+    let secret = "Authorization Bearer secret_poc_token_12345";
+    sandbox
+        .new_assert_cmd("network")
+        .args([
+            "add",
+            "--rpc-url",
+            "https://example.invalid",
+            "--network-passphrase",
+            "Test SDF Network ; September 2015",
+            "testcorp",
+        ])
+        .env("STELLAR_RPC_HEADERS", secret)
+        .assert()
+        .stderr(predicate::str::contains("secret_poc_token_12345").not())
+        .stderr(predicate::str::contains(
+            "invalid HTTP header: must be in the form 'key:value'",
+        ))
+        .failure();
+}
+
 #[test]
 fn env_does_not_display_rpc_headers() {
     let sandbox = TestEnv::default();

cmd/soroban-cli/src/commands/network/add.rs

@@ -7,6 +7,9 @@ pub enum Error {
 
     #[error(transparent)]
     Config(#[from] locator::Error),
+
+    #[error(transparent)]
+    Network(#[from] network::Error),
 }
 
 #[derive(Debug, clap::Parser, Clone)]
@@ -24,6 +27,7 @@ pub struct Cmd {
 
 impl Cmd {
     pub fn run(&self) -> Result<(), Error> {
+        self.network.validate_headers()?;
         self.config_locator
             .write_network(&self.name, &self.network)?;
         Ok(())

cmd/soroban-cli/src/config/network.rs

@@ -76,10 +76,9 @@ pub struct Args {
         num_args = 1,
         action = clap::ArgAction::Append,
         value_delimiter = '\n',
-        value_parser = parse_http_header,
         hide_env_values = true,
     )]
-    pub rpc_headers: Vec<(String, String)>,
+    pub rpc_headers: Vec<String>,
     /// Network passphrase to sign the transaction sent to the rpc server
     #[arg(
         long = "network-passphrase",
@@ -111,11 +110,18 @@ impl Args {
             (_, Some(_), None) => Err(Error::MissingNetworkPassphrase),
             (_, None, Some(_)) => Err(Error::MissingRpcUrl),
             (Some(network), None, None) => Ok(locator.read_network(network)?),
-            (_, Some(rpc_url), Some(network_passphrase)) => Ok(Network {
-                rpc_url,
-                rpc_headers: self.rpc_headers.clone(),
-                network_passphrase,
-            }),
+            (_, Some(rpc_url), Some(network_passphrase)) => {
+                let rpc_headers = self
+                    .rpc_headers
+                    .iter()
+                    .map(|h| parse_http_header(h))
+                    .collect::<Result<Vec<_>, _>>()?;
+                Ok(Network {
+                    rpc_url,
+                    rpc_headers,
+                    network_passphrase,
+                })
+            }
         }
     }
 }
@@ -138,7 +144,7 @@ pub struct Network {
         num_args = 1,
         action = clap::ArgAction::Append,
         value_delimiter = '\n',
-        value_parser = parse_http_header,
+        value_parser = accept_raw_rpc_header,
         hide_env_values = true,
     )]
     pub rpc_headers: Vec<(String, String)>,
@@ -174,14 +180,35 @@ fn parse_http_header(header: &str) -> Result<(String, String), Error> {
         .next_tuple()
         .ok_or_else(|| Error::InvalidHeader)?;
 
-    // Check that the headers are properly formatted
     HeaderName::from_str(key)?;
     HeaderValue::from_str(value)?;
 
     Ok((key.to_string(), value.to_string()))
 }
 
+/// Clap value_parser for `Network::rpc_headers` that always succeeds, deferring
+/// validation to application code so clap never echoes the raw value in error messages.
+#[allow(clippy::unnecessary_wraps)]
+fn accept_raw_rpc_header(header: &str) -> Result<(String, String), std::convert::Infallible> {
+    match header.split_once(':') {
+        Some((key, value)) => Ok((key.trim().to_string(), value.trim().to_string())),
+        None => Ok((String::new(), header.to_string())),
+    }
+}
+
+fn validate_rpc_headers(headers: &[(String, String)]) -> Result<(), Error> {
+    for (key, value) in headers {
+        HeaderName::from_str(key).map_err(|_| Error::InvalidHeader)?;
+        HeaderValue::from_str(value).map_err(|_| Error::InvalidHeader)?;
+    }
+    Ok(())
+}
+
 impl Network {
+    pub fn validate_headers(&self) -> Result<(), Error> {
+        validate_rpc_headers(&self.rpc_headers)
+    }
+
     pub async fn helper_url(&self, addr: &str) -> Result<Url, Error> {
         tracing::debug!("address {addr:?}");
         let rpc_url =
@@ -554,6 +581,74 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_malformed_rpc_header_accepted_by_clap_without_error() {
+        use crate::test_utils::with_env_guard;
+        use clap::Parser;
+
+        #[derive(clap::Parser)]
+        struct TestCmd {
+            #[command(flatten)]
+            args: Args,
+        }
+
+        let secret = "Authorization Bearer secret_poc_token_12345";
+        with_env_guard(&["STELLAR_RPC_HEADERS"], || {
+            std::env::set_var("STELLAR_RPC_HEADERS", secret);
+            let result = TestCmd::try_parse_from(["stellar"]);
+            assert!(
+                result.is_ok(),
+                "Clap must accept malformed RPC headers without error — validation is deferred to application code to prevent secrets from being echoed in clap error messages"
+            );
+        });
+    }
+
+    #[test]
+    fn test_validate_headers_rejects_missing_colon_without_exposing_value() {
+        // Simulates what accept_raw_rpc_header stores when no ':' is present.
+        let network = Network {
+            rpc_url: "http://localhost:8000".to_string(),
+            network_passphrase: "Test".to_string(),
+            rpc_headers: vec![(
+                String::new(),
+                "Authorization Bearer secret_token_xyz".to_string(),
+            )],
+        };
+
+        let result = network.validate_headers();
+        assert!(result.is_err());
+        let error_msg = result.unwrap_err().to_string();
+        assert_eq!(
+            error_msg,
+            "invalid HTTP header: must be in the form 'key:value'"
+        );
+        assert!(
+            !error_msg.contains("secret_token_xyz"),
+            "Error must not expose the raw header value, got: {error_msg}"
+        );
+    }
+
+    #[test]
+    fn test_malformed_rpc_header_app_error_does_not_expose_value() {
+        use super::super::locator;
+
+        let secret = "Authorization Bearer secret_poc_token_12345";
+        let args = Args {
+            rpc_url: Some("https://example.com".to_string()),
+            rpc_headers: vec![secret.to_string()],
+            network_passphrase: Some("Test SDF Network ; September 2015".to_string()),
+            network: None,
+        };
+
+        let result = args.get(&locator::Args::default());
+        assert!(result.is_err());
+        let error_msg = result.unwrap_err().to_string();
+        assert!(
+            !error_msg.contains("secret_poc_token_12345"),
+            "Application error must not expose secret header value, got: {error_msg}"
+        );
+    }
+
     #[test]
     fn test_debug_conceals_rpc_header_values() {
         let network = Network {