diff --git a/crates/client/src/goat_chain/goat_adaptor.rs b/crates/client/src/goat_chain/goat_adaptor.rs index 6ffce2de7..cb378e21e 100644 --- a/crates/client/src/goat_chain/goat_adaptor.rs +++ b/crates/client/src/goat_chain/goat_adaptor.rs @@ -345,7 +345,7 @@ sol!( } ); -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct GoatInitConfig { pub rpc_url: Url, pub private_key: Option, @@ -359,6 +359,23 @@ pub struct GoatInitConfig { pub peg_btc_address: Option
, } +impl std::fmt::Debug for GoatInitConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("GoatInitConfig") + .field("rpc_url", &self.rpc_url) + .field("private_key", &self.private_key.as_ref().map(|_| "")) + .field("chain_id", &self.chain_id) + .field("gateway_address", &self.gateway_address) + .field("sequencer_set_publisher_address", &self.sequencer_set_publisher_address) + .field("committee_management_address", &self.committee_management_address) + .field("stake_management_address", &self.stake_management_address) + .field("multi_sig_verifier_address", &self.multi_sig_verifier_address) + .field("btc_spv_address", &self.btc_spv_address) + .field("peg_btc_address", &self.peg_btc_address) + .finish() + } +} + impl GoatInitConfig { pub async fn new(rpc_url: Url) -> anyhow::Result { let provider = @@ -1726,6 +1743,30 @@ impl GoatAdaptor { } } +#[cfg(test)] +mod config_tests { + use super::*; + + #[test] + fn debug_output_hides_the_private_key() { + let config = GoatInitConfig { + rpc_url: Url::parse("http://127.0.0.1:8545").unwrap(), + private_key: Some("deadbeefcafe".to_string()), + chain_id: 1, + gateway_address: None, + sequencer_set_publisher_address: None, + committee_management_address: None, + stake_management_address: None, + multi_sig_verifier_address: None, + btc_spv_address: None, + peg_btc_address: None, + }; + let debug = format!("{config:?}"); + assert!(!debug.contains("deadbeefcafe")); + assert!(debug.contains("")); + } +} + #[cfg(test)] mod digest_tests { use super::*; diff --git a/crates/store/src/utils.rs b/crates/store/src/utils.rs index 0a29560bf..44637a1d8 100644 --- a/crates/store/src/utils.rs +++ b/crates/store/src/utils.rs @@ -134,8 +134,11 @@ impl QueryBuilder { } pub fn apply_pagination(&mut self, limit: Option, offset: Option) { - if let Some(limit) = limit { - self.sql.push_str(&format!(" LIMIT {limit}")); + // SQLite only accepts OFFSET after a LIMIT; -1 means no limit. + match (limit, offset) { + (Some(limit), _) => self.sql.push_str(&format!(" LIMIT {limit}")), + (None, Some(_)) => self.sql.push_str(" LIMIT -1"), + (None, None) => {} } if let Some(offset) = offset { @@ -205,3 +208,19 @@ impl QueryBuilder { pub fn create_place_holders(inputs: &[T]) -> String { vec!["?"; inputs.len()].join(",") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn offset_without_limit_still_produces_valid_sql() { + let mut builder = QueryBuilder::new("SELECT * FROM graph"); + builder.apply_pagination(None, Some(20)); + assert!(builder.sql.ends_with(" LIMIT -1 OFFSET 20")); + + let mut builder = QueryBuilder::new("SELECT * FROM graph"); + builder.apply_pagination(Some(10), Some(20)); + assert!(builder.sql.ends_with(" LIMIT 10 OFFSET 20")); + } +} diff --git a/node/src/main.rs b/node/src/main.rs index 0b0f1cc12..8a2a80206 100644 --- a/node/src/main.rs +++ b/node/src/main.rs @@ -25,7 +25,7 @@ use bitvm_noded::{ run_watch_event_task, }; -use anyhow::Result; +use anyhow::{Context, Result}; use bitvm_noded::metrics_service::{MetricsState, set_node_metrics_state}; use bitvm_noded::middleware::swarm::{BitvmNetworkManager, BitvmSwarmConfig}; use bitvm_noded::p2p_admission::load_persisted_peer_bindings; @@ -152,6 +152,10 @@ async fn main() -> Result<(), Box> { ) .into()); } + // A malformed CORS setting stops startup here instead of failing the RPC task later. + let _ = rpc_service::cors_config::CorsConfig::from_env() + .create_cors_layer() + .context("invalid CORS configuration")?; // Bind the metrics listener before any background task starts, so an invalid path // or an unavailable metrics port aborts startup with an explicit error. let metrics_listener = match opt.metrics_addr.as_deref() { diff --git a/node/src/metrics_service.rs b/node/src/metrics_service.rs index c20130d06..bbf1cc9e6 100644 --- a/node/src/metrics_service.rs +++ b/node/src/metrics_service.rs @@ -162,7 +162,8 @@ struct BackendHealth { failed_calls: u64, unavailable_since: Option, last_failure_at: Option, - last_error: Option, + /// The operation that failed last; the error text itself only goes to the log. + last_failed_operation: Option<&'static str>, } /// One backend's availability as served by `/v1/health`. @@ -172,6 +173,8 @@ pub struct BackendHealthSnapshot { pub up: bool, pub unavailable_since: Option, pub consecutive_failures: u32, + /// Which operation failed last, e.g. `"get_block_height failed"`. Never the raw + /// error, which can carry backend URLs and API keys. pub last_error: Option, } @@ -589,16 +592,22 @@ impl MetricsState { } /// Records one call to a backend and logs only the transitions: one WARN - /// when it becomes unavailable, one INFO when it answers again. - pub fn record_backend_call(&self, backend: &'static str, success: bool, error: Option<&str>) { - self.record_backend_call_at(backend, success, error, current_time_secs()); + /// when it becomes unavailable, one INFO when it answers again. A failure may + /// name the failed operation and its error. + pub fn record_backend_call( + &self, + backend: &'static str, + success: bool, + failure: Option<(&'static str, &str)>, + ) { + self.record_backend_call_at(backend, success, failure, current_time_secs()); } fn record_backend_call_at( &self, backend: &'static str, success: bool, - error: Option<&str>, + failure: Option<(&'static str, &str)>, now: i64, ) { let labels = BackendLabels { backend: backend.to_owned() }; @@ -621,13 +630,13 @@ impl MetricsState { self.backend_up.get_or_create(&labels).set(1); self.backend_unavailable_since_seconds.get_or_create(&labels).set(0); entry.failed_calls = 0; - entry.last_error = None; + entry.last_failed_operation = None; } } else { entry.consecutive_failures += 1; entry.failed_calls += 1; entry.last_failure_at = Some(now); - entry.last_error = error.map(str::to_owned); + entry.last_failed_operation = failure.map(|(operation, _)| operation); if entry.unavailable_since.is_none() && entry.consecutive_failures >= BACKEND_UNAVAILABLE_AFTER_FAILURES { @@ -637,7 +646,7 @@ impl MetricsState { backend, outcome = "unavailable", consecutive_failures = entry.consecutive_failures, - error = error.unwrap_or("unknown"), + error = failure.map_or("unknown", |(_, error)| error), "backend unavailable; further failures are logged at debug until it recovers" ); self.backend_up.get_or_create(&labels).set(0); @@ -682,7 +691,7 @@ impl MetricsState { _ => 0, }; let message = error.to_string(); - self.record_backend_call(backend, false, Some(&message)); + self.record_backend_call(backend, false, Some((operation, &message))); if known_down { tracing::debug!(event = "metrics_backend_probe", backend, operation, error = %message, "backend probe failed (backend already unavailable)"); } else { @@ -701,7 +710,9 @@ impl MetricsState { up: entry.is_none_or(|h| h.unavailable_since.is_none()), unavailable_since: entry.and_then(|h| h.unavailable_since), consecutive_failures: entry.map_or(0, |h| h.consecutive_failures), - last_error: entry.and_then(|h| h.last_error.clone()), + last_error: entry + .and_then(|h| h.last_failed_operation) + .map(|operation| format!("{operation} failed")), } }) .collect() @@ -1230,19 +1241,19 @@ mod tests { #[test] fn backend_availability_transitions_after_consecutive_failures() { let state = MetricsState::new(Arc::new(Mutex::new(Registry::default()))); - state.record_backend_call_at("goat", false, Some("timeout"), 1_000); + state.record_backend_call_at("goat", false, Some(("get_height", "timeout")), 1_000); assert!(!state.backend_unavailable("goat"), "one failure is not an outage"); - state.record_backend_call_at("goat", false, Some("timeout"), 1_005); + state.record_backend_call_at("goat", false, Some(("get_height", "timeout")), 1_005); assert!(state.backend_unavailable("goat")); let snapshot = state.backend_health_snapshot(); let goat = snapshot.iter().find(|b| b.backend == "goat").unwrap(); assert!(!goat.up); assert_eq!(goat.unavailable_since, Some(1_005)); - assert_eq!(goat.last_error.as_deref(), Some("timeout")); + assert_eq!(goat.last_error.as_deref(), Some("get_height failed")); // a success right after a failure is a flap, not a recovery state.record_backend_call_at("goat", true, None, 1_010); assert!(state.backend_unavailable("goat")); - state.record_backend_call_at("goat", false, Some("502"), 1_015); + state.record_backend_call_at("goat", false, Some(("get_height", "502")), 1_015); // recovered once no call failed for the quiet window state.record_backend_call_at("goat", true, None, 1_015 + BACKEND_RECOVERY_QUIET_SECS); assert!(!state.backend_unavailable("goat")); @@ -1255,6 +1266,16 @@ mod tests { assert!(!state.backend_unavailable("goat")); } + #[test] + fn health_snapshot_names_the_operation_but_not_the_error() { + let state = MetricsState::new(Arc::new(Mutex::new(Registry::default()))); + let error = "error sending request for url (https://rpc.example/v1/SECRET_API_KEY)"; + state.backend_probe_failed("btc", "get_block_height", &error); + let btc = state.backend_health_snapshot().into_iter().find(|b| b.backend == "btc").unwrap(); + assert_eq!(btc.last_error.as_deref(), Some("get_block_height failed")); + assert!(!serde_json::to_string(&btc).unwrap().contains("SECRET_API_KEY")); + } + #[test] fn readiness_follows_backend_availability() { let state = MetricsState::new(Arc::new(Mutex::new(Registry::default()))); @@ -1263,14 +1284,14 @@ mod tests { state.mark_backend_ready(true); state.set_event_watch_state(EventWatchState::Healthy); assert!(state.readiness_snapshot().ready); - state.record_backend_call_at("graph", false, Some("down"), 100); - state.record_backend_call_at("graph", false, Some("down"), 105); + state.record_backend_call_at("graph", false, Some(("get_height", "down")), 100); + state.record_backend_call_at("graph", false, Some(("get_height", "down")), 105); assert!(!state.readiness_snapshot().ready, "an unavailable subgraph is not ready"); assert!(encoded(&state).contains("bitvm_node_ready 0")); // the proof builder is informational only state.record_backend_call_at("graph", true, None, 200); - state.record_backend_call_at("proof", false, Some("down"), 200); - state.record_backend_call_at("proof", false, Some("down"), 205); + state.record_backend_call_at("proof", false, Some(("get_height", "down")), 200); + state.record_backend_call_at("proof", false, Some(("get_height", "down")), 205); assert!(state.readiness_snapshot().ready); } } diff --git a/node/src/p2p_msg_handler.rs b/node/src/p2p_msg_handler.rs index c5cbcdc54..2351aa6b6 100644 --- a/node/src/p2p_msg_handler.rs +++ b/node/src/p2p_msg_handler.rs @@ -210,12 +210,19 @@ impl P2pMessageHandler for BitvmNodeProcessor { async fn handle_tick_message( &self, swarm: &mut BitvmSwarmWrapper, - _peer_id: PeerId, + peer_id: PeerId, actor: Actor, msg_type: TickMessageType, ) -> anyhow::Result<()> { match msg_type { TickMessageType::HeartBeat => { + // Keep this node's own row fresh so it counts as online in the + // node list; the public RPC handlers no longer write it. + if let Err(error) = + update_node_timestamp(&self.local_db, &peer_id.to_string()).await + { + tracing::warn!("refresh local node timestamp: {error}"); + } match detect_heart_beat(swarm).await { Ok(_) => {} Err(e) => { diff --git a/node/src/rpc_service/auth.rs b/node/src/rpc_service/auth.rs index e54a50bea..ee484926e 100644 --- a/node/src/rpc_service/auth.rs +++ b/node/src/rpc_service/auth.rs @@ -38,7 +38,7 @@ pub async fn require_request_auth( ) -> Response { let (parts, body) = request.into_parts(); let now = current_time_secs(); - let parsed_auth = match parse_auth_headers(&parts.headers, now) { + let parsed_auth = match parse_auth_headers(&parts.headers, now, app_state.rpc_auth_not_before) { Ok(auth) => auth, Err(error) => return error.into_response(), }; @@ -116,9 +116,12 @@ pub fn sign_request_auth( (timestamp, nonce, signature) } +/// `not_before` is the node start: the nonce cache is in memory, so a request +/// stamped before it may already have been accepted by the previous process. fn parse_auth_headers( headers: &HeaderMap, now: i64, + not_before: i64, ) -> Result)> { let timestamp_str = header_value(headers, AUTH_TIMESTAMP_HEADER)?.to_string(); let timestamp = timestamp_str @@ -127,6 +130,13 @@ fn parse_auth_headers( if now.abs_diff(timestamp) > AUTH_WINDOW_SECS as u64 { return Err(auth_error(StatusCode::UNAUTHORIZED, "AUTH_ERROR", "timestamp expired")); } + if timestamp < not_before { + return Err(auth_error( + StatusCode::UNAUTHORIZED, + "AUTH_ERROR", + "timestamp predates the node start; sign the request again", + )); + } let nonce = header_value(headers, AUTH_NONCE_HEADER)?.to_string(); let parsed_nonce = Uuid::parse_str(&nonce) @@ -257,7 +267,7 @@ mod tests { body: &[u8], now: i64, ) -> Result<(), (StatusCode, Json)> { - let auth = parse_auth_headers(headers, now)?; + let auth = parse_auth_headers(headers, now, 0)?; let (x_only_pubkey, _) = test_keypair().x_only_public_key(); verify_auth_signature(&auth, method, target, body, &x_only_pubkey) } @@ -279,6 +289,18 @@ mod tests { assert!(verify(&non_v4_headers, &Method::POST, TARGET, BODY, 2_000_000_000).is_ok()); } + #[test] + fn requests_signed_before_the_node_started_are_rejected() { + let keypair = test_keypair(); + let headers = signed_headers(&keypair, TIMESTAMP, NONCE, &Method::POST, TARGET, BODY); + let now = 2_000_000_060; + + assert!(parse_auth_headers(&headers, now, 2_000_000_000).is_ok()); + let (status, body) = parse_auth_headers(&headers, now, 2_000_000_001).err().unwrap(); + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert!(body.message.contains("predates the node start")); + } + #[test] fn signature_is_bound_to_the_entire_request() { let keypair = test_keypair(); diff --git a/node/src/rpc_service/cors_config.rs b/node/src/rpc_service/cors_config.rs index 7cc148763..e20033974 100644 --- a/node/src/rpc_service/cors_config.rs +++ b/node/src/rpc_service/cors_config.rs @@ -108,56 +108,54 @@ impl CorsConfig { } } - /// Create CORS layer - pub fn create_cors_layer(&self) -> CorsLayer { - let mut cors_layer = CorsLayer::new(); - - // Set allowed origins - if self.allowed_origins.is_empty() { - cors_layer = cors_layer.allow_origin(Any); - } else { - let origins: Result, _> = - self.allowed_origins.iter().map(|origin| HeaderValue::from_str(origin)).collect(); - - match origins { - Ok(header_values) => { - cors_layer = cors_layer.allow_origin(header_values); - } - Err(_) => { - tracing::warn!("Invalid CORS origins, falling back to Any"); - cors_layer = cors_layer.allow_origin(Any); - } - } + /// Create the CORS layer. A malformed origin or header is an error rather than a + /// silent widening to `*`, and so is a combination tower-http would panic on. + pub fn create_cors_layer(&self) -> anyhow::Result { + // An empty list or a literal `*` means any value. + let any_origin = + self.allowed_origins.is_empty() || self.allowed_origins.iter().any(|o| o == "*"); + let any_headers = + self.allowed_headers.is_empty() || self.allowed_headers.iter().any(|h| h == "*"); + if self.allow_credentials && (any_origin || any_headers) { + anyhow::bail!( + "CORS_ALLOW_CREDENTIALS=true needs explicit CORS_ALLOWED_ORIGINS and \ + CORS_ALLOWED_HEADERS (no empty list, no `*`)" + ); } - // Set allowed headers - if self.allowed_headers.is_empty() { - cors_layer = cors_layer.allow_headers(Any); + let mut cors_layer = CorsLayer::new(); + cors_layer = if any_origin { + cors_layer.allow_origin(Any) } else { - let headers: Result, _> = - self.allowed_headers.iter().map(|header| HeaderName::from_str(header)).collect(); - - match headers { - Ok(header_names) => { - cors_layer = cors_layer.allow_headers(header_names); - } - Err(_) => { - tracing::warn!("Invalid CORS headers, falling back to Any"); - cors_layer = cors_layer.allow_headers(Any); - } - } - } + let origins = self + .allowed_origins + .iter() + .map(|origin| { + HeaderValue::from_str(origin) + .map_err(|_| anyhow::anyhow!("invalid CORS origin {origin:?}")) + }) + .collect::>>()?; + cors_layer.allow_origin(origins) + }; + cors_layer = if any_headers { + cors_layer.allow_headers(Any) + } else { + let headers = self + .allowed_headers + .iter() + .map(|header| { + HeaderName::from_str(header) + .map_err(|_| anyhow::anyhow!("invalid CORS header {header:?}")) + }) + .collect::>>()?; + cors_layer.allow_headers(headers) + }; - // Set allowed HTTP methods; - // Set credentials allowance; - // Set preflight request cache time; - // Set private network allowance - cors_layer = cors_layer + Ok(cors_layer .allow_methods(self.allowed_methods.clone()) .allow_credentials(self.allow_credentials) .max_age(std::time::Duration::from_secs(self.max_age)) - .allow_private_network(self.allow_private_network); - cors_layer + .allow_private_network(self.allow_private_network)) } /// Validate configuration security @@ -311,6 +309,42 @@ mod tests { assert!(!warnings.iter().any(|w| w.contains("long"))); } + #[test] + fn bad_cors_settings_are_errors_not_wildcards_or_panics() { + let invalid_origin = CorsConfig { + allowed_origins: vec!["https://ok.example".to_string(), "bad\norigin".to_string()], + ..Default::default() + }; + assert!(invalid_origin.create_cors_layer().is_err()); + + let invalid_header = + CorsConfig { allowed_headers: vec!["bad header".to_string()], ..Default::default() }; + assert!(invalid_header.create_cors_layer().is_err()); + + for (origins, headers) in [ + (vec![], vec!["content-type".to_string()]), + (vec!["*".to_string()], vec!["content-type".to_string()]), + (vec!["https://ok.example".to_string()], vec![]), + ] { + let config = CorsConfig { + allowed_origins: origins, + allowed_headers: headers, + allow_credentials: true, + ..Default::default() + }; + assert!(config.create_cors_layer().is_err()); + } + + let wildcard = CorsConfig { allowed_origins: vec!["*".to_string()], ..Default::default() }; + assert!(wildcard.create_cors_layer().is_ok()); + let credentials = CorsConfig { + allowed_origins: vec!["https://ok.example".to_string()], + allow_credentials: true, + ..Default::default() + }; + assert!(credentials.create_cors_layer().is_ok()); + } + #[test] fn test_credentials_security() { // Test credentials with wildcard origin (should warn) diff --git a/node/src/rpc_service/handler/bitvm_handler.rs b/node/src/rpc_service/handler/bitvm_handler.rs index 599745d30..a598e9153 100644 --- a/node/src/rpc_service/handler/bitvm_handler.rs +++ b/node/src/rpc_service/handler/bitvm_handler.rs @@ -559,9 +559,12 @@ pub async fn get_graph( /// ``` #[axum::debug_handler] pub async fn get_graphs( - Query(params): Query, + Query(mut params): Query, State(app_state): State>, ) -> ApiResult { + let (offset, limit) = InputValidator::validate_pagination(params.offset, params.limit)?; + params.offset = Some(offset); + params.limit = Some(limit); let mut resp = GraphListResponse::default(); let mut storage_process = app_state.local_db.acquire().await.api_error("GET_GRAPHS_ERROR")?; diff --git a/node/src/rpc_service/handler/node_handler.rs b/node/src/rpc_service/handler/node_handler.rs index 5f51125e5..54b740974 100644 --- a/node/src/rpc_service/handler/node_handler.rs +++ b/node/src/rpc_service/handler/node_handler.rs @@ -15,7 +15,7 @@ use store::localdb::NodeQuery; /// Get node list /// /// Returns a paginated list of BitVM network nodes based on query parameters. Supports filtering by -/// GOAT address, actor type, and online status. Automatically updates the current node's timestamp. +/// GOAT address, actor type, and online status. /// /// # Query Parameters /// @@ -84,10 +84,6 @@ pub async fn get_nodes( let mut storage_process = app_state.local_db.acquire().await.api_error("GET_NODES_ERROR")?; - storage_process - .update_node_timestamp(&app_state.peer_id, current_time_secs()) - .await - .api_error("GET_NODES_ERROR")?; let time_threshold = current_time_secs() - ALIVE_TIME_JUDGE_THRESHOLD; let (_, goat_addr) = reflect_goat_address(query_params.goat_addr); let (nodes, total) = storage_process @@ -97,7 +93,7 @@ pub async fn get_nodes( time_threshold: query_params.status.clone().map(|_| time_threshold), is_in_time_threshold: query_params .status - .map(|status| status != NODE_STATUS_OFFLINE) + .map(|status| !status.eq_ignore_ascii_case(NODE_STATUS_OFFLINE)) .unwrap_or(true), order: None, offset: Some(offset), @@ -115,7 +111,7 @@ pub async fn get_nodes( /// Get nodes overview statistics /// /// Returns statistical overview of all nodes in the BitVM network, including counts by actor type -/// and online/offline status. Automatically updates the current node's timestamp. +/// and online/offline status. /// /// # Returns /// @@ -157,11 +153,6 @@ pub async fn get_nodes_overview( let mut storage_process = app_state.local_db.acquire().await.api_error("NODES_OVERVIEW_ERROR")?; - storage_process - .update_node_timestamp(&app_state.peer_id, current_time_secs()) - .await - .api_error("NODES_OVERVIEW_ERROR")?; - let time_threshold = current_time_secs() - ALIVE_TIME_JUDGE_THRESHOLD; let nodes_overview = storage_process.node_overview(time_threshold).await.api_error("NODES_OVERVIEW_ERROR")?; @@ -171,8 +162,7 @@ pub async fn get_nodes_overview( /// Get node by peer ID /// -/// Returns detailed information for a specific node based on its peer_id. If the requested -/// node is the current node, automatically updates its timestamp. +/// Returns detailed information for a specific node based on its peer_id. /// /// # Path Parameters /// @@ -221,13 +211,6 @@ pub async fn get_node( let mut storage_process = app_state.local_db.acquire().await.api_error("GET_NODE_ERROR")?; - if peer_id == app_state.peer_id { - storage_process - .update_node_timestamp(&app_state.peer_id, current_time_secs()) - .await - .api_error("GET_NODE_ERROR")?; - } - let res = storage_process.node_by_id(peer_id.as_str()).await.api_error("GET_NODE_ERROR")?.map(|v| { v.to_node_desc(current_time_secs() - ALIVE_TIME_JUDGE_THRESHOLD, &app_state.peer_id) diff --git a/node/src/rpc_service/handler/proof_handler.rs b/node/src/rpc_service/handler/proof_handler.rs index c5ed3d7ef..67c217014 100644 --- a/node/src/rpc_service/handler/proof_handler.rs +++ b/node/src/rpc_service/handler/proof_handler.rs @@ -64,20 +64,16 @@ async fn handle_proof_desc_forwarding( let Some(env_url) = Url::parse(&host).ok().filter(|u| u.scheme() == "http" || u.scheme() == "https") else { - tracing::warn!( - "GOAT_PROOF_BUILD_URL is invalid (must be valid http/https URL): {host}" - ); + tracing::warn!("GOAT_PROOF_BUILD_URL is invalid (must be valid http/https URL)"); return ok_response(ProofDescResponse { proof_desc: None, - error: Some(format!("Invalid GOAT_PROOF_BUILD_URL configuration: {host}")), + error: Some("proof builder URL is misconfigured".to_string()), }); }; let request_host = headers.get("host").and_then(|h| h.to_str().ok()); if is_loop_detected(&env_url, request_host) { - tracing::warn!( - "GOAT_PROOF_BUILD_URL points to self ({host}), returning mock data to avoid loop" - ); + tracing::warn!("GOAT_PROOF_BUILD_URL points to this node; not forwarding"); return ok_response(ProofDescResponse { proof_desc: None, error: Some("Request host matches self, loop detected".to_string()), @@ -90,10 +86,10 @@ async fn handle_proof_desc_forwarding( let resp = app_state.http_client.get_response_json::(&url).await.map_err( |e| { - to_api_error( - error_code, - format!("fail to get proof from url: {host}, error: {e}"), - ) + // The URL and the upstream error stay in the log: the URL can + // carry credentials and this endpoint is public. + tracing::warn!(error = %e, "proof builder request failed"); + to_api_error(error_code, "proof builder unavailable".to_string()) }, )?; ok_response(resp) diff --git a/node/src/rpc_service/mod.rs b/node/src/rpc_service/mod.rs index 39faf9ed6..49ea02449 100644 --- a/node/src/rpc_service/mod.rs +++ b/node/src/rpc_service/mod.rs @@ -1,6 +1,6 @@ pub mod auth; mod bitvm; -mod cors_config; +pub mod cors_config; pub mod handler; mod node; mod response; @@ -11,6 +11,7 @@ mod tests; pub(super) mod utils; pub mod validation; +use crate::clock::current_time_secs; use crate::env::{get_btc_url_from_env, get_goat_network, get_network, goat_config_from_env}; use crate::metrics_service::{MetricsState, metrics_handler, metrics_middleware}; use crate::rpc_service::auth::require_request_auth; @@ -49,7 +50,7 @@ use tower_http::cors::CorsLayer; use tower_http::trace::TraceLayer; /// Create secure CORS layer -pub fn create_secure_cors_layer() -> CorsLayer { +pub fn create_secure_cors_layer() -> anyhow::Result { let cors_config = CorsConfig::from_env(); // Validate configuration security @@ -58,7 +59,7 @@ pub fn create_secure_cors_layer() -> CorsLayer { tracing::warn!("CORS security warning: {}", warning); } - cors_config.create_cors_layer() + cors_config.create_cors_layer().context("invalid CORS configuration") } pub struct AppState { @@ -72,6 +73,10 @@ pub struct AppState { /// Present when the execution engine is enabled (debug and status endpoints). pub engine: Option, rpc_auth_nonces: Mutex>, + /// Signed requests stamped before this second are rejected. The nonce cache + /// starts empty after a restart, so a request captured earlier could + /// otherwise be replayed until its timestamp expires. + rpc_auth_not_before: i64, } impl AppState { @@ -84,7 +89,9 @@ impl AppState { ) -> anyhow::Result> { let btc_client = BTCClient::new(get_network(), get_btc_url_from_env().as_deref()); let goat_client = GOATClient::new(goat_config_from_env().await, get_goat_network()); - let http_client = HttpAsyncClient::new(None); + // The public proof-desc proxy is the only user: one attempt per request, + // so an unauthenticated caller cannot multiply load on the proof builder. + let http_client = HttpAsyncClient::new(Some(0)); Ok(Arc::new(AppState { local_db, btc_client, @@ -95,6 +102,7 @@ impl AppState { engine, http_client, rpc_auth_nonces: Mutex::new(HashMap::new()), + rpc_auth_not_before: current_time_secs(), })) } @@ -119,8 +127,9 @@ impl AppState { actor, peer_id, engine: None, - http_client: HttpAsyncClient::new(None), + http_client: HttpAsyncClient::new(Some(0)), rpc_auth_nonces: Mutex::new(HashMap::new()), + rpc_auth_not_before: current_time_secs(), })) } } @@ -145,7 +154,7 @@ async fn root() -> &'static str { "Hello, World!" } -pub(crate) fn build_business_router(app_state: Arc) -> Router { +pub(crate) fn build_business_router(app_state: Arc) -> anyhow::Result { let public_routes = Router::new() .route(routes::ROOT, get(root)) .route(routes::v1::HEALTH, get(get_health)) @@ -184,7 +193,7 @@ pub(crate) fn build_business_router(app_state: Arc) -> Router { let signed_routes = signed_routes .route_layer(middleware::from_fn_with_state(app_state.clone(), require_request_auth)); - public_routes.merge(signed_routes).layer(create_secure_cors_layer()).with_state(app_state) + Ok(public_routes.merge(signed_routes).layer(create_secure_cors_layer()?).with_state(app_state)) } pub async fn serve_with_app_state( @@ -204,7 +213,7 @@ async fn serve_with_listener( cancellation_token: CancellationToken, ) -> anyhow::Result { let node_span = tracing::Span::current(); - let server = build_business_router(app_state.clone()) + let server = build_business_router(app_state.clone())? .layer( TraceLayer::new_for_http() .make_span_with(move |request: &Request| { diff --git a/node/src/rpc_service/tests.rs b/node/src/rpc_service/tests.rs index d80db4694..f200eea29 100644 --- a/node/src/rpc_service/tests.rs +++ b/node/src/rpc_service/tests.rs @@ -161,7 +161,7 @@ async fn spawn_business_listener( let app_state = mock_app_state().await?; let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?.to_string(); - let server = rpc_service::build_business_router(app_state); + let server = rpc_service::build_business_router(app_state)?; let handle = tokio::spawn(async move { let _ = tokio::select! { result = axum::serve(listener, server) => result, @@ -208,6 +208,23 @@ async fn signed_request_nonce_is_rejected_after_first_use() -> Result<(), Box Result<(), Box> { + let cancellation_token = CancellationToken::new(); + let (addr, server) = spawn_business_listener(cancellation_token.clone()).await?; + let client = Client::new(); + let base = format!("http://{addr}{}", routes::v1::GRAPHS_BASE); + + let too_many = client.get(format!("{base}?limit=1000")).send().await?; + assert_eq!(too_many.status().as_u16(), 400); + let offset_only = client.get(format!("{base}?offset=5")).send().await?; + assert_eq!(offset_only.status().as_u16(), 200); + + cancellation_token.cancel(); + server.await?; + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn debug_routes_follow_rpc_debug_endpoints_feature() -> Result<(), Box> {