fix: harden public RPC endpoints against replay, leaks and unbounded queries

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<String>,
@@ -359,6 +359,23 @@ pub struct GoatInitConfig {
     pub peg_btc_address: Option<Address>,
 }
 
+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(|_| "<redacted>"))
+            .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<GoatInitConfig> {
         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("<redacted>"));
+    }
+}
+
 #[cfg(test)]
 mod digest_tests {
     use super::*;

crates/store/src/utils.rs

@@ -134,8 +134,11 @@ impl QueryBuilder {
     }
 
     pub fn apply_pagination(&mut self, limit: Option<u32>, offset: Option<u32>) {
-        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<T>(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"));
+    }
+}

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<dyn Error>> {
         )
         .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() {

node/src/metrics_service.rs

@@ -162,7 +162,8 @@ struct BackendHealth {
     failed_calls: u64,
     unavailable_since: Option<i64>,
     last_failure_at: Option<i64>,
-    last_error: Option<String>,
+    /// 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<i64>,
     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<String>,
 }
 
@@ -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);
     }
 }

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) => {

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<ParsedAuth, (StatusCode, Json<ErrorResponse>)> {
     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<ErrorResponse>)> {
-        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();