Redact password in URLs at remaining display sites.

cmd/soroban-cli/src/commands/doctor.rs

@@ -9,11 +9,12 @@ use crate::{
     config::{
         self, data,
         locator::{self, KeyType},
-        network::{redact_rpc_url, Network, DEFAULTS as DEFAULT_NETWORKS},
+        network::{Network, DEFAULTS as DEFAULT_NETWORKS},
     },
     print::Print,
     rpc,
     upgrade_check::has_available_upgrade,
+    utils::url::redact_url,
 };
 
 #[derive(Parser, Debug, Clone)]
@@ -100,7 +101,7 @@ async fn print_network(
 
     print.globeln(format!(
         "{prefix} {name:?} ({})",
-        redact_rpc_url(&network.rpc_url)
+        redact_url(&network.rpc_url)
     ));
     print.blankln(format!("protocol {}", version_info.protocol_version));
     print.blankln(format!("rpc {}", version_info.version));
@@ -123,7 +124,7 @@ async fn inspect_networks(print: &Print, config_locator: &locator::Args) -> Resu
         if print_network(true, print, &name, &network).await.is_err() {
             print.warnln(format!(
                 "Default network {name:?} ({}) is unreachable",
-                redact_rpc_url(&network.rpc_url)
+                redact_url(&network.rpc_url)
             ));
         }
     }
@@ -133,7 +134,7 @@ async fn inspect_networks(print: &Print, config_locator: &locator::Args) -> Resu
             if print_network(false, print, name, &network).await.is_err() {
                 print.warnln(format!(
                     "Network {name:?} ({}) is unreachable",
-                    redact_rpc_url(&network.rpc_url)
+                    redact_url(&network.rpc_url)
                 ));
             }
         }

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

@@ -2,6 +2,7 @@ use sha2::{Digest, Sha256};
 
 use crate::commands::global;
 use crate::config::network;
+use crate::utils::url::redact_url;
 use crate::{config, print, rpc};
 
 #[derive(thiserror::Error, Debug)]
@@ -93,7 +94,7 @@ impl Cmd {
             build_timestamp: version_result.build_timestamp,
             captive_core_version: version_result.captive_core_version,
             protocol_version: network_result.protocol_version,
-            friendbot_url: network_result.friendbot_url,
+            friendbot_url: network_result.friendbot_url.map(|u| redact_url(&u)),
             passphrase: network_result.passphrase,
         };
 

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

@@ -47,7 +47,7 @@ impl Cmd {
 
                 format!(
                     "Name: {name}\nRPC url: {rpc_url}\nRPC headers:{headers}\nNetwork passphrase: {passphrase}",
-                    rpc_url = crate::config::network::redact_rpc_url(&network.rpc_url),
+                    rpc_url = crate::utils::url::redact_url(&network.rpc_url),
                     passphrase = network.network_passphrase,
                 )
             })

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

@@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize};
 use std::str::FromStr;
 use url::Url;
 
-use super::network::redact_rpc_url;
+use crate::utils::url::redact_url;
 use crate::xdr::{self, WriteXdr};
 
 #[derive(thiserror::Error, Debug)]
@@ -61,7 +61,7 @@ pub fn bucket_dir() -> Result<std::path::PathBuf, Error> {
 pub fn write(action: Action, rpc_url: &Url) -> Result<ulid::Ulid, Error> {
     let data = Data {
         action,
-        rpc_url: redact_rpc_url(rpc_url.as_str()),
+        rpc_url: redact_url(rpc_url.as_str()),
     };
     let id = ulid::Ulid::new();
     let file = actions_dir()?.join(id.to_string()).with_extension("json");
@@ -132,7 +132,12 @@ impl std::fmt::Display for DatedAction {
                 .map_or_else(|| "SUCCESS".to_string(), |_| "ERROR".to_string()),
             Action::Send { response } => response.status.clone(),
         };
-        write!(f, "{id} {} {status} {datetime} {uri} ", a.type_str())
+        write!(
+            f,
+            "{id} {} {status} {datetime} {} ",
+            a.type_str(),
+            redact_url(uri.as_str()),
+        )
     }
 }
 

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

@@ -10,7 +10,7 @@ use stellar_strkey::ed25519::PublicKey;
 use url::Url;
 
 use super::locator;
-use crate::utils::http;
+use crate::utils::{http, url::redact_url};
 use crate::{
     commands::HEADING_RPC,
     rpc::{self, Client},
@@ -165,23 +165,13 @@ impl std::fmt::Debug for Network {
             .map(|(k, _)| (k.as_str(), "<concealed>"))
             .collect();
         f.debug_struct("Network")
-            .field("rpc_url", &redact_rpc_url(&self.rpc_url))
+            .field("rpc_url", &redact_url(&self.rpc_url))
             .field("rpc_headers", &concealed)
             .field("network_passphrase", &self.network_passphrase)
             .finish()
     }
 }
 
-pub fn redact_rpc_url(rpc_url: &str) -> String {
-    let Ok(mut url) = Url::parse(rpc_url) else {
-        return rpc_url.to_string();
-    };
-    if url.password().is_some() {
-        let _ = url.set_password(Some("redacted"));
-    }
-    url.to_string()
-}
-
 fn parse_http_header(header: &str) -> Result<(String, String), Error> {
     let header_components = header.splitn(2, ':');
 
@@ -221,8 +211,8 @@ impl Network {
 
     pub async fn helper_url(&self, addr: &str) -> Result<Url, Error> {
         tracing::debug!("address {addr:?}");
-        let rpc_url =
-            Url::from_str(&self.rpc_url).map_err(|_| Error::InvalidUrl(self.rpc_url.clone()))?;
+        let rpc_url = Url::from_str(&self.rpc_url)
+            .map_err(|_| Error::InvalidUrl(redact_url(&self.rpc_url)))?;
         if self.network_passphrase.as_str() == passphrase::LOCAL {
             let mut local_url = rpc_url;
             local_url.set_path("/friendbot");
@@ -231,12 +221,17 @@ impl Network {
         } else {
             let client = self.rpc_client()?;
             let network = client.get_network().await?;
-            tracing::debug!("network {network:?}");
+            tracing::debug!(
+                "network passphrase={:?} protocol_version={} friendbot_url={:?}",
+                network.passphrase,
+                network.protocol_version,
+                network.friendbot_url.as_deref().map(redact_url),
+            );
             let url = client.friendbot_url().await?;
-            tracing::debug!("URL {url:?}");
+            tracing::debug!("URL {}", redact_url(&url));
             let mut url = Url::from_str(&url).map_err(|e| {
                 tracing::error!("{e}");
-                Error::InvalidUrl(url.clone())
+                Error::InvalidUrl(redact_url(&url))
             })?;
             url.query_pairs_mut().append_pair("addr", addr);
             Ok(url)
@@ -246,13 +241,13 @@ impl Network {
     #[allow(clippy::similar_names)]
     pub async fn fund_address(&self, addr: &PublicKey) -> Result<(), Error> {
         let uri = self.helper_url(&addr.to_string()).await?;
-        tracing::debug!("URL {uri:?}");
+        tracing::debug!("URL {}", redact_url(uri.as_str()));
         let response = http::client().get(uri.as_str()).send().await?;
 
         let request_successful = response.status().is_success();
         let body = response.bytes().await?;
         let res = serde_json::from_slice::<serde_json::Value>(&body)
-            .map_err(|e| Error::FailedToParseJSON(uri.to_string(), e))?;
+            .map_err(|e| Error::FailedToParseJSON(redact_url(uri.as_str()), e))?;
         tracing::debug!("{res:#?}");
         if !request_successful {
             if let Some(detail) = res.get("detail").and_then(Value::as_str) {
@@ -273,7 +268,7 @@ impl Network {
     }
 
     pub fn rpc_uri(&self) -> Result<Url, Error> {
-        Url::from_str(&self.rpc_url).map_err(|_| Error::InvalidUrl(self.rpc_url.clone()))
+        Url::from_str(&self.rpc_url).map_err(|_| Error::InvalidUrl(redact_url(&self.rpc_url)))
     }
 
     pub fn rpc_client(&self) -> Result<Client, Error> {
@@ -288,7 +283,7 @@ impl Network {
 
         rpc::Client::new_with_headers(&self.rpc_url, header_map).map_err(|e| match e {
             rpc::Error::InvalidRpcUrl(..) | rpc::Error::InvalidRpcUrlFromUriParts(..) => {
-                Error::InvalidUrl(self.rpc_url.clone())
+                Error::InvalidUrl(redact_url(&self.rpc_url))
             }
             other => Error::Rpc(other),
         })
@@ -693,36 +688,98 @@ mod tests {
         );
     }
 
-    #[test]
-    fn redact_rpc_url_leaves_url_without_password_unchanged() {
-        let plain = "https://rpc.example.com/soroban";
-        assert_eq!(redact_rpc_url(plain), plain);
+    #[tokio::test]
+    async fn fund_address_failed_to_parse_json_does_not_leak_credentialed_rpc_url() {
+        let mut server = Server::new_async().await;
+        // Friendbot returns a non-JSON body so serde_json::from_slice fails,
+        // triggering Error::FailedToParseJSON at the line we want to verify.
+        let _mock = server
+            .mock("GET", mockito::Matcher::Any)
+            .with_status(200)
+            .with_body("not valid json")
+            .create_async()
+            .await;
 
-        let user_only = "https://alice@rpc.example.com/soroban";
-        assert_eq!(redact_rpc_url(user_only), user_only);
-    }
+        let host_port = server
+            .url()
+            .strip_prefix("http://")
+            .expect("mockito url starts with http://")
+            .to_string();
+        let credentialed_rpc_url = format!("http://alice:supersecret@{host_port}");
 
-    #[test]
-    fn redact_rpc_url_replaces_password_with_placeholder() {
-        let with_password = "https://alice:supersecret@rpc.example.com/soroban";
-        let redacted = redact_rpc_url(with_password);
-        assert!(
-            !redacted.contains("supersecret"),
-            "password leaked: {redacted}"
-        );
+        let network = Network {
+            rpc_url: credentialed_rpc_url,
+            network_passphrase: passphrase::LOCAL.to_string(),
+            rpc_headers: Vec::new(),
+        };
+
+        let addr =
+            PublicKey::from_string("GBZXN7PIRZGNMHGA7MUUUF4GWPY5AYPV6LY4UV2GL6VJGIQRXFDNMADI")
+                .unwrap();
+        let err = network
+            .fund_address(&addr)
+            .await
+            .expect_err("fund_address must return Err when friendbot replies with non-JSON body");
+        let rendered = err.to_string();
         assert!(
-            redacted.contains("alice:redacted"),
-            "expected `alice:redacted`: {redacted}"
+            !rendered.contains("supersecret"),
+            "password leaked into error display: {rendered}"
         );
         assert!(
-            redacted.contains("rpc.example.com/soroban"),
-            "expected host and path preserved: {redacted}"
+            rendered.contains("alice:redacted"),
+            "expected `alice:redacted` placeholder in error display: {rendered}"
         );
     }
 
-    #[test]
-    fn redact_rpc_url_returns_input_when_unparseable() {
-        let bad = "not a url";
-        assert_eq!(redact_rpc_url(bad), bad);
+    #[tokio::test]
+    async fn helper_url_returned_credentialed_url_is_redactable_at_display_sinks() {
+        // Non-LOCAL passphrase branch: helper_url asks the RPC for the friendbot URL.
+        // The mocked RPC returns a parseable URL carrying userinfo, so Url::from_str
+        // succeeds and helper_url returns Ok(url). The InvalidUrl branch is therefore
+        // not exercised here — driving it would require an unparseable URL, which by
+        // design leaks unchanged (see PR discussion). This test only documents that
+        // the parseable URL returned from helper_url can be safely run through
+        // redact_url at any subsequent display sink.
+        let mut server = Server::new_async().await;
+        let _mock = server
+            .mock("POST", "/")
+            .with_body_from_request(|req| {
+                let body: Value = serde_json::from_slice(req.body().unwrap()).unwrap();
+                let id = body["id"].clone();
+                // Returned friendbot URL has userinfo + is parseable by url::Url.
+                // Url::from_str inside helper_url accepts it, so the InvalidUrl
+                // path at line 239 isn't exercised. Instead the URL flows into
+                // the tracing line and (after fund_address) into FailedToParseJSON.
+                json!({
+                    "jsonrpc": "2.0",
+                    "id": id,
+                    "result": {
+                        "friendbotUrl": "https://alice:supersecret@friendbot.example/",
+                        "passphrase": passphrase::TESTNET.to_string(),
+                        "protocolVersion": 21,
+                    }
+                })
+                .to_string()
+                .into()
+            })
+            .create_async()
+            .await;
+
+        let network = Network {
+            rpc_url: server.url(),
+            network_passphrase: passphrase::TESTNET.to_string(),
+            rpc_headers: Vec::new(),
+        };
+        let returned = network
+            .helper_url("GBZXN7PIRZGNMHGA7MUUUF4GWPY5AYPV6LY4UV2GL6VJGIQRXFDNMADI")
+            .await
+            .expect("helper_url should accept a parseable credentialed friendbot URL");
+        // The Url returned still carries the password — callers need it to authenticate.
+        assert_eq!(returned.password(), Some("supersecret"));
+        let redacted_for_display = redact_url(returned.as_str());
+        assert!(
+            !redacted_for_display.contains("supersecret"),
+            "redact_url failed to redact a parseable friendbot URL: {redacted_for_display}"
+        );
     }
 }

cmd/soroban-cli/src/utils.rs

@@ -267,6 +267,60 @@ pub mod http {
     }
 }
 
+pub mod url {
+    use url::Url;
+
+    /// Returns the given URL with any password component replaced by the literal
+    /// `redacted`. If the URL is not parseable, it is returned unchanged.
+    pub fn redact_url(url: &str) -> String {
+        let Ok(mut url) = Url::parse(url) else {
+            return url.to_string();
+        };
+        if url.password().is_some() {
+            let _ = url.set_password(Some("redacted"));
+        }
+        url.to_string()
+    }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        #[test]
+        fn leaves_url_without_password_unchanged() {
+            let plain = "https://rpc.example.com/soroban";
+            assert_eq!(redact_url(plain), plain);
+
+            let user_only = "https://alice@rpc.example.com/soroban";
+            assert_eq!(redact_url(user_only), user_only);
+        }
+
+        #[test]
+        fn replaces_password_with_placeholder() {
+            let with_password = "https://alice:supersecret@rpc.example.com/soroban";
+            let redacted = redact_url(with_password);
+            assert!(
+                !redacted.contains("supersecret"),
+                "password leaked: {redacted}"
+            );
+            assert!(
+                redacted.contains("alice:redacted"),
+                "expected `alice:redacted`: {redacted}"
+            );
+            assert!(
+                redacted.contains("rpc.example.com/soroban"),
+                "expected host and path preserved: {redacted}"
+            );
+        }
+
+        #[test]
+        fn returns_input_when_unparseable() {
+            let bad = "not a url";
+            assert_eq!(redact_url(bad), bad);
+        }
+    }
+}
+
 pub mod args {
     #[derive(thiserror::Error, Debug)]
     pub enum DeprecatedError<'a> {