Redact endpoint secrets in logs (#3989)

* chore: moved redact code to utils * feat: use SecretUrl type for endpoints * refactor: Addressed PR comments
chainflip-io · Sep 14, 2023 · 52551a0 · 52551a0
1 parent 5c37f77
commit 52551a0
Show file tree

Hide file tree

Showing 15 changed files with 253 additions and 162 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/api/bin/chainflip-cli/src/settings.rs b/api/bin/chainflip-cli/src/settings.rs
@@ -252,7 +252,7 @@ mod tests {
 		.unwrap();
 
 		assert_eq!(settings.state_chain.ws_endpoint, "ws://localhost:9944");
-		assert_eq!(settings.eth.nodes.primary.ws_node_endpoint, "ws://localhost:8545");
+		assert_eq!(settings.eth.nodes.primary.ws_node_endpoint.as_ref(), "ws://localhost:8545");
 	}
 
 	#[test]
@@ -294,21 +294,21 @@ mod tests {
 		);
 		assert_eq!(
 			opts.eth_opts.eth_ws_node_endpoint.unwrap(),
-			settings.eth.nodes.primary.ws_node_endpoint
+			settings.eth.nodes.primary.ws_node_endpoint.as_ref()
 		);
 		assert_eq!(
 			opts.eth_opts.eth_http_node_endpoint.unwrap(),
-			settings.eth.nodes.primary.http_node_endpoint
+			settings.eth.nodes.primary.http_node_endpoint.as_ref()
 		);
 
 		let eth_backup_node = settings.eth.nodes.backup.unwrap();
 		assert_eq!(
 			opts.eth_opts.eth_backup_ws_node_endpoint.unwrap(),
-			eth_backup_node.ws_node_endpoint
+			eth_backup_node.ws_node_endpoint.as_ref()
 		);
 		assert_eq!(
 			opts.eth_opts.eth_backup_http_node_endpoint.unwrap(),
-			eth_backup_node.http_node_endpoint
+			eth_backup_node.http_node_endpoint.as_ref()
 		);
 
 		assert_eq!(opts.eth_opts.eth_private_key_file.unwrap(), settings.eth.private_key_file);

diff --git a/engine/Cargo.toml b/engine/Cargo.toml
@@ -60,7 +60,6 @@ lazy_static = "1.4"
 num-bigint = "0.4"
 num-derive = "0.4"
 num-traits = "0.2"
-regex = "1"
 secp256k1 = "0.27"
 serde = { version = "1.0", features = ["derive", "rc"] }
 serde_json = "1.0"

diff --git a/engine/src/btc/rpc.rs b/engine/src/btc/rpc.rs
@@ -7,6 +7,7 @@ use serde;
 use serde_json::json;
 
 use bitcoin::{block::Version, Amount, Block, BlockHash, Txid};
+use utilities::redact_endpoint_secret::SecretUrl;
 
 use crate::settings::HttpBasicAuthEndpoint;
 
@@ -59,7 +60,7 @@ struct FeeRateResponse {
 pub struct BtcRpcClient {
 	// internally the Client is Arc'd
 	client: Client,
-	url: String,
+	url: SecretUrl,
 	user: String,
 	password: String,
 }
@@ -88,7 +89,7 @@ impl BtcRpcClient {
 
 		let response = &self
 			.client
-			.post(&self.url)
+			.post(self.url.as_ref())
 			.basic_auth(&self.user, Some(&self.password))
 			.json(&request_body)
 			.send()
@@ -223,7 +224,7 @@ mod tests {
 	#[ignore = "requires local node, useful for manual testing"]
 	async fn test_btc_async() {
 		let client = BtcRpcClient::new(HttpBasicAuthEndpoint {
-			http_node_endpoint: "http://localhost:8332".to_string(),
+			http_node_endpoint: "http://localhost:8332".into(),
 			rpc_user: "flip".to_string(),
 			rpc_password: "flip".to_string(),
 		})

diff --git a/engine/src/dot/http_rpc.rs b/engine/src/dot/http_rpc.rs
@@ -21,7 +21,7 @@ use subxt::{
 };
 
 use anyhow::Result;
-use utilities::make_periodic_tick;
+use utilities::{make_periodic_tick, redact_endpoint_secret::SecretUrl};
 
 use crate::constants::DOT_AVERAGE_BLOCK_TIME;
 
@@ -30,7 +30,7 @@ use super::rpc::DotRpcApi;
 pub struct PolkadotHttpClient(HttpClient);
 
 impl PolkadotHttpClient {
-	pub fn new(url: &str) -> Result<Self> {
+	pub fn new(url: &SecretUrl) -> Result<Self> {
 		let token = format!("Bearer {}", "TOKEN");
 		let mut headers = HeaderMap::new();
 		headers.insert(AUTHORIZATION, token.parse().unwrap());
@@ -80,7 +80,7 @@ pub struct DotHttpRpcClient {
 }
 
 impl DotHttpRpcClient {
-	pub fn new(url: String) -> Result<impl Future<Output = Self>> {
+	pub fn new(url: SecretUrl) -> Result<impl Future<Output = Self>> {
 		let polkadot_http_client = Arc::new(PolkadotHttpClient::new(&url)?);
 
 		Ok(async move {
@@ -201,8 +201,7 @@ mod tests {
 	#[ignore = "requires local node"]
 	#[tokio::test]
 	async fn test_http_rpc() {
-		let url = "http://localhost:9945";
-		let dot_http_rpc = DotHttpRpcClient::new(url.to_string()).unwrap().await;
+		let dot_http_rpc = DotHttpRpcClient::new("http://localhost:9945".into()).unwrap().await;
 		let block_hash = dot_http_rpc.block_hash(1).await.unwrap();
 		println!("block_hash: {:?}", block_hash);
 	}

diff --git a/engine/src/dot/retry_rpc.rs b/engine/src/dot/retry_rpc.rs
@@ -48,7 +48,7 @@ impl DotRetryRpcClient {
 		let f_create_clients = |endpoints: WsHttpEndpoints| {
 			Result::<_, anyhow::Error>::Ok((
 				DotHttpRpcClient::new(endpoints.http_node_endpoint)?,
-				DotSubClient::new(&endpoints.ws_node_endpoint),
+				DotSubClient::new(endpoints.ws_node_endpoint),
 			))
 		};
 
@@ -310,8 +310,8 @@ mod tests {
 					scope,
 					NodeContainer {
 						primary: WsHttpEndpoints {
-							http_node_endpoint: "http://127.0.0.1:9945".to_string(),
-							ws_node_endpoint: "ws://127.0.0.1:9945".to_string(),
+							http_node_endpoint: "http://127.0.0.1:9945".into(),
+							ws_node_endpoint: "ws://127.0.0.1:9945".into(),
 						},
 						backup: None,
 					},

diff --git a/engine/src/dot/rpc.rs b/engine/src/dot/rpc.rs
@@ -12,6 +12,7 @@ use subxt::{
 	Config, OnlineClient, PolkadotConfig,
 };
 use tokio::sync::RwLock;
+use utilities::redact_endpoint_secret::SecretUrl;
 
 use anyhow::{anyhow, Result};
 
@@ -134,12 +135,12 @@ impl DotRpcApi for DotRpcClient {
 
 #[derive(Clone)]
 pub struct DotSubClient {
-	pub ws_endpoint: String,
+	pub ws_endpoint: SecretUrl,
 }
 
 impl DotSubClient {
-	pub fn new(ws_endpoint: &str) -> Self {
-		Self { ws_endpoint: ws_endpoint.to_string() }
+	pub fn new(ws_endpoint: SecretUrl) -> Self {
+		Self { ws_endpoint }
 	}
 }
 
@@ -148,7 +149,7 @@ impl DotSubscribeApi for DotSubClient {
 	async fn subscribe_best_heads(
 		&self,
 	) -> Result<Pin<Box<dyn Stream<Item = Result<PolkadotHeader>> + Send>>> {
-		let client = OnlineClient::<PolkadotConfig>::from_url(self.ws_endpoint.clone()).await?;
+		let client = OnlineClient::<PolkadotConfig>::from_url(&self.ws_endpoint).await?;
 		Ok(Box::pin(
 			client
 				.blocks()
@@ -162,7 +163,7 @@ impl DotSubscribeApi for DotSubClient {
 	async fn subscribe_finalized_heads(
 		&self,
 	) -> Result<Pin<Box<dyn Stream<Item = Result<PolkadotHeader>> + Send>>> {
-		let client = OnlineClient::<PolkadotConfig>::from_url(self.ws_endpoint.clone()).await?;
+		let client = OnlineClient::<PolkadotConfig>::from_url(&self.ws_endpoint).await?;
 		Ok(Box::pin(
 			client
 				.blocks()

diff --git a/engine/src/eth/mod.rs b/engine/src/eth/mod.rs
@@ -1,5 +1,4 @@
 pub mod event;
-pub mod redact_endpoint_secret;
 pub mod retry_rpc;
 pub mod rpc;
 

diff --git a/engine/src/eth/redact_endpoint_secret.rs b/engine/src/eth/redact_endpoint_secret.rs
diff --git a/engine/src/eth/rpc.rs b/engine/src/eth/rpc.rs
@@ -2,6 +2,7 @@ pub mod address_checker;
 
 use ethers::{prelude::*, signers::Signer, types::transaction::eip2718::TypedTransaction};
 use futures_core::Future;
+use utilities::redact_endpoint_secret::SecretUrl;
 
 use crate::constants::{ETH_AVERAGE_BLOCK_TIME, SYNC_POLL_INTERVAL};
 use anyhow::{anyhow, Context, Result};
@@ -25,10 +26,10 @@ pub struct EthRpcClient {
 impl EthRpcClient {
 	pub fn new(
 		private_key_file: PathBuf,
-		http_node_endpoint: String,
+		http_node_endpoint: SecretUrl,
 		expected_chain_id: u64,
 	) -> Result<impl Future<Output = Self>> {
-		let provider = Arc::new(Provider::<Http>::try_from(http_node_endpoint.clone())?);
+		let provider = Arc::new(Provider::<Http>::try_from(http_node_endpoint.as_ref())?);
 		let wallet =
 			read_clean_and_decode_hex_str_file(&private_key_file, "Ethereum Private Key", |key| {
 				ethers::signers::Wallet::from_str(key).map_err(anyhow::Error::new)
@@ -49,17 +50,14 @@ impl EthRpcClient {
 					Ok(chain_id) if chain_id == expected_chain_id.into() => break client,
 					Ok(chain_id) => {
 						tracing::warn!(
-						"Connected to Ethereum node but with chain_id {}, expected {}. Please check your CFE
-						configuration file...",
-						chain_id,
-						expected_chain_id
-					);
+								"Connected to Ethereum node but with chain_id {chain_id}, expected {expected_chain_id}. Please check your CFE
+								configuration file...",
+							);
 					},
 					Err(e) => tracing::error!(
-					"Cannot connect to an Ethereum node at {} with error: {e}. Please check your CFE
-					configuration file. Retrying...",
-					http_node_endpoint
-				),
+							"Cannot connect to an Ethereum node at {http_node_endpoint} with error: {e}. Please check your CFE
+							configuration file. Retrying...",
+						),
 				}
 			}
 		})
@@ -191,13 +189,13 @@ impl EthRpcApi for EthRpcClient {
 /// On each subscription this will create a new WS connection.
 #[derive(Clone)]
 pub struct ReconnectSubscriptionClient {
-	ws_node_endpoint: String,
+	ws_node_endpoint: SecretUrl,
 	// This value comes from the SC.
 	chain_id: web3::types::U256,
 }
 
 impl ReconnectSubscriptionClient {
-	pub fn new(ws_node_endpoint: String, chain_id: web3::types::U256) -> Self {
+	pub fn new(ws_node_endpoint: SecretUrl, chain_id: web3::types::U256) -> Self {
 		Self { ws_node_endpoint, chain_id }
 	}
 }
@@ -212,7 +210,9 @@ use crate::eth::ConscientiousEthWebsocketBlockHeaderStream;
 #[async_trait::async_trait]
 impl ReconnectSubscribeApi for ReconnectSubscriptionClient {
 	async fn subscribe_blocks(&self) -> Result<ConscientiousEthWebsocketBlockHeaderStream> {
-		let web3 = web3::Web3::new(web3::transports::WebSocket::new(&self.ws_node_endpoint).await?);
+		let web3 = web3::Web3::new(
+			web3::transports::WebSocket::new(self.ws_node_endpoint.as_ref()).await?,
+		);
 
 		let mut poll_interval = make_periodic_tick(SYNC_POLL_INTERVAL, false);