fix: comprehensive security and reliability improvements (#4)

* fix(p2p-consensus): improve PBFT consensus security and reliability

- Add double-vote prevention in handle_prepare and handle_commit
- Validate ViewChange messages match NewView's view number
- Fix race conditions in consensus round handling
- Replace unwrap with proper error handling in finalize_evaluation
- Improve merkle proof generation with is_multiple_of check

* fix(storage): add size limits to bincode deserialization for security

- Add MAX_ENTRY_SIZE limits to prevent memory exhaustion attacks
- Use bincode::options().with_limit() for safe deserialization
- Ensure bincode compatibility with serialize defaults
- Fix ConflictResolution to use derive(Default)
- Apply clippy suggestions for improved code quality

* fix(secure-container-runtime): add container security hardening

- Add enhanced path traversal protection in policy validation
- Improve WebSocket transport security with better authentication
- Update container broker with improved error handling
- Add additional integration tests for security features

* fix(platform-server): secure API endpoints with signature verification

- Add signature verification to submit_evaluation endpoint
- Add signature verification to all validator API endpoints
- Secure CORS configuration and HTTP client error handling
- Fix broadcast authentication bypass vulnerability
- Add historical_weights pruning and JWT secret warning
- Implement Display trait instead of ToString for SubmissionStatus

* fix(validator): require authentication and improve security

- Require authentication for validator node operations
- Update container tests for security compliance
- Fix clippy warnings for iterator patterns

* chore: fix clippy warnings and apply code formatting

- Add allow attributes for await_holding_lock in p2p_client
- Apply cargo fmt formatting to all files
- Fix CORS configuration formatting in platform server

Author: echobt

bins/platform/src/server.rs

@@ -257,7 +257,7 @@ pub async fn run(args: ServerArgs) -> Result<()> {
             get(api::evaluations::get_evaluations),
         )
         .with_state(state)
-        .layer(CorsLayer::permissive())
+        .layer(build_cors_layer())
         .layer(TraceLayer::new_for_http());
 
     info!("╔══════════════════════════════════════════════════════════════╗");
@@ -437,10 +437,20 @@ async fn proxy_to_challenge(
         }
     };
 
-    let client = reqwest::Client::builder()
+    let client = match reqwest::Client::builder()
         .timeout(std::time::Duration::from_secs(600))
         .build()
-        .unwrap();
+    {
+        Ok(c) => c,
+        Err(e) => {
+            error!("Failed to create HTTP client: {}", e);
+            return (
+                StatusCode::INTERNAL_SERVER_ERROR,
+                "Failed to create HTTP client",
+            )
+                .into_response();
+        }
+    };
 
     let mut req_builder = client.request(method, &url);
     for (key, value) in headers.iter() {
@@ -481,6 +491,43 @@ async fn proxy_to_challenge(
     }
 }
 
+/// Build CORS layer based on environment configuration.
+/// In development mode (DEVELOPMENT_MODE env var set), allows any origin.
+/// In production, only whitelisted origins are allowed.
+fn build_cors_layer() -> CorsLayer {
+    let allowed_origins = std::env::var("CORS_ALLOWED_ORIGINS")
+        .unwrap_or_else(|_| "https://platform.network,https://chain.platform.network".to_string());
+
+    if allowed_origins == "*" || std::env::var("DEVELOPMENT_MODE").is_ok() {
+        tracing::warn!("CORS allowing all origins - this should only be used in development!");
+        CorsLayer::new()
+            .allow_origin(tower_http::cors::Any)
+            .allow_methods(tower_http::cors::Any)
+            .allow_headers(tower_http::cors::Any)
+    } else {
+        let origins: Vec<axum::http::HeaderValue> = allowed_origins
+            .split(',')
+            .filter_map(|s| s.trim().parse().ok())
+            .collect();
+
+        CorsLayer::new()
+            .allow_origin(origins)
+            .allow_methods([
+                axum::http::Method::GET,
+                axum::http::Method::POST,
+                axum::http::Method::PUT,
+                axum::http::Method::DELETE,
+                axum::http::Method::OPTIONS,
+            ])
+            .allow_headers([
+                axum::http::header::AUTHORIZATION,
+                axum::http::header::CONTENT_TYPE,
+                axum::http::header::ACCEPT,
+            ])
+            .allow_credentials(true)
+    }
+}
+
 /// Start the container broker WebSocket server
 ///
 /// This allows challenges to create sandboxed containers without direct Docker access.
@@ -507,11 +554,25 @@ async fn start_container_broker(port: u16) -> Result<()> {
     // WebSocket config with JWT auth
     let jwt_secret = std::env::var("BROKER_JWT_SECRET").ok();
 
+    // Determine if authentication should be required
+    let is_dev_mode = std::env::var("DEVELOPMENT_MODE").is_ok();
+    if is_dev_mode {
+        warn!("SECURITY WARNING: DEVELOPMENT_MODE is set - WebSocket authentication is DISABLED!");
+        warn!("DO NOT use DEVELOPMENT_MODE in production environments!");
+    }
+
+    if jwt_secret.is_none() && !is_dev_mode {
+        warn!("BROKER_JWT_SECRET not set - a random JWT secret will be generated");
+        warn!("This means JWT tokens will be invalidated on server restart!");
+        warn!("Set BROKER_JWT_SECRET environment variable for production use");
+    }
+
     let ws_config = WsConfig {
         bind_addr: format!("0.0.0.0:{}", port),
         jwt_secret,
         allowed_challenges: vec![], // Allow all challenges
         max_connections_per_challenge: 10,
+        allow_unauthenticated: is_dev_mode, // Only allow unauthenticated in dev mode
     };
 
     info!(

bins/validator-decentralized/src/main.rs

@@ -414,7 +414,7 @@ async fn persist_state_to_storage(
 
 /// Update validator set from metagraph data
 fn update_validator_set_from_metagraph(metagraph: &Metagraph, validator_set: &Arc<ValidatorSet>) {
-    for (_uid, neuron) in &metagraph.neurons {
+    for neuron in metagraph.neurons.values() {
         let hotkey_bytes: [u8; 32] = neuron.hotkey.clone().into();
         let hotkey = Hotkey(hotkey_bytes);
         // Get effective stake capped to u64::MAX (neuron.stake is u128)

bins/validator-node/src/main.rs

@@ -478,6 +478,7 @@ async fn main() -> Result<()> {
         jwt_secret: Some(jwt_secret),
         allowed_challenges: vec![],
         max_connections_per_challenge: 10,
+        allow_unauthenticated: false, // Require authentication in production
     };
     let broker_clone = broker.clone();
     tokio::spawn(async move {
@@ -951,6 +952,7 @@ fn load_keypair(args: &Args) -> Result<Keypair> {
 
 /// Submit weights for a given epoch
 /// This is the core weight submission logic, extracted to be reusable
+#[allow(clippy::too_many_arguments)]
 async fn submit_weights_for_epoch(
     epoch: u64,
     platform_client: &Arc<PlatformServerClient>,
@@ -1188,6 +1190,7 @@ async fn submit_weights_for_epoch(
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 async fn handle_block_event(
     event: BlockSyncEvent,
     platform_client: &Arc<PlatformServerClient>,

crates/challenge-sdk/src/p2p_client.rs

@@ -251,6 +251,7 @@ impl P2PChallengeClient {
     /// # Returns
     ///
     /// Returns a list of pending submissions or an error on timeout/failure.
+    #[allow(clippy::await_holding_lock)]
     pub async fn get_pending_submissions(
         &self,
         limit: usize,
@@ -355,6 +356,7 @@ impl P2PChallengeClient {
     /// # Returns
     ///
     /// Returns aggregated weights as (hotkey, weight) pairs or an error on timeout.
+    #[allow(clippy::await_holding_lock)]
     pub async fn get_weights(&self, epoch: u64) -> Result<Vec<(String, f64)>, ChallengeError> {
         let msg = P2PChallengeMessage::RequestWeights {
             challenge_id: self.config.challenge_id.clone(),
@@ -458,6 +460,7 @@ impl P2PChallengeClient {
     /// Returns a tuple of (evaluations, final_score) where:
     /// - `evaluations` is a list of validator evaluation results
     /// - `final_score` is `Some(score)` if consensus was reached, `None` otherwise
+    #[allow(clippy::await_holding_lock)]
     pub async fn get_evaluation_status(
         &self,
         submission_hash: &str,

crates/distributed-storage/src/challenge_store.rs

@@ -741,7 +741,7 @@ fn build_merkle_proof(leaves: &[[u8; 32]], leaf_index: usize, data: &[u8]) -> Me
     let mut index = leaf_index;
 
     while level.len() > 1 {
-        let sibling_index = if index % 2 == 0 {
+        let sibling_index = if index.is_multiple_of(2) {
             if index + 1 < level.len() {
                 index + 1
             } else {

crates/distributed-storage/src/local.rs

@@ -4,6 +4,7 @@
 //! It stores data with replication metadata to track which remote nodes have copies.
 
 use async_trait::async_trait;
+use bincode::Options;
 use parking_lot::RwLock;
 use serde::{Deserialize, Serialize};
 use sled::{Db, Tree};
@@ -12,6 +13,10 @@ use std::path::Path;
 use std::sync::Arc;
 use tracing::{debug, trace};
 
+/// Maximum size for deserializing storage entries (100MB).
+/// This limit prevents DoS attacks from malformed data causing excessive memory allocation.
+const MAX_ENTRY_SIZE: u64 = 100 * 1024 * 1024;
+
 use crate::error::{StorageError, StorageResult};
 use crate::query::{
     block_index_key, block_range_end, block_range_start, parse_block_index_key, QueryBuilder,
@@ -160,7 +165,12 @@ impl LocalStorage {
 
         match self.data_tree.get(&db_key)? {
             Some(bytes) => {
-                let entry: LocalEntry = bincode::deserialize(&bytes)?;
+                // Use options compatible with bincode::serialize (legacy format with fixint encoding)
+                let entry: LocalEntry = bincode::options()
+                    .with_limit(MAX_ENTRY_SIZE)
+                    .with_fixint_encoding()
+                    .allow_trailing_bytes()
+                    .deserialize(&bytes)?;
                 Ok(Some(entry))
             }
             None => Ok(None),
@@ -260,7 +270,12 @@ impl LocalStorage {
         for result in self.data_tree.iter() {
             let (key_bytes, value_bytes) = result?;
 
-            let entry: LocalEntry = bincode::deserialize(&value_bytes)?;
+            // Use options compatible with bincode::serialize (legacy format with fixint encoding)
+            let entry: LocalEntry = bincode::options()
+                .with_limit(MAX_ENTRY_SIZE)
+                .with_fixint_encoding()
+                .allow_trailing_bytes()
+                .deserialize(&value_bytes)?;
 
             if entry.replication.needs_replication {
                 // Parse the key back into a StorageKey
@@ -466,7 +481,12 @@ impl LocalStorage {
 
             // Get the actual data
             if let Some(value_bytes) = self.data_tree.get(&data_key_bytes)? {
-                let entry: LocalEntry = bincode::deserialize(&value_bytes)?;
+                // Use options compatible with bincode::serialize (legacy format with fixint encoding)
+                let entry: LocalEntry = bincode::options()
+                    .with_limit(MAX_ENTRY_SIZE)
+                    .with_fixint_encoding()
+                    .allow_trailing_bytes()
+                    .deserialize(&value_bytes)?;
 
                 // Skip expired entries
                 if entry.value.metadata.is_expired() {
@@ -648,7 +668,12 @@ impl DistributedStore for LocalStorage {
 
             // Get the actual data
             if let Some(value_bytes) = self.data_tree.get(&data_key)? {
-                let entry: LocalEntry = bincode::deserialize(&value_bytes)?;
+                // Use options compatible with bincode::serialize (legacy format with fixint encoding)
+                let entry: LocalEntry = bincode::options()
+                    .with_limit(MAX_ENTRY_SIZE)
+                    .with_fixint_encoding()
+                    .allow_trailing_bytes()
+                    .deserialize(&value_bytes)?;
 
                 // Skip expired entries
                 if entry.value.metadata.is_expired() {

crates/distributed-storage/src/query.rs

@@ -3,10 +3,25 @@
 //! This module provides SQL-like query capabilities using sled's range iterators.
 //! It supports block-based filtering, pagination, and fluent query construction.
 
+use bincode::Options;
 use serde::{Deserialize, Serialize};
 
 use crate::store::{StorageKey, StoredValue};
 
+/// Maximum size for deserializing query cursor data (1MB).
+/// This limit prevents DoS attacks from malformed data causing excessive memory allocation.
+/// Cursors are small structures, so 1MB is more than sufficient.
+const MAX_CURSOR_SIZE: u64 = 1024 * 1024;
+
+/// Create bincode options with size limit for safe deserialization.
+/// Uses fixint encoding and allows trailing bytes for compatibility with `bincode::serialize()`.
+fn bincode_options() -> impl Options {
+    bincode::options()
+        .with_limit(MAX_CURSOR_SIZE)
+        .with_fixint_encoding()
+        .allow_trailing_bytes()
+}
+
 /// Result of a query operation with pagination support
 #[derive(Clone, Debug)]
 pub struct QueryResult {
@@ -112,9 +127,10 @@ impl QueryCursor {
         bincode::serialize(self).unwrap_or_default()
     }
 
-    /// Decode cursor from bytes
+    /// Decode cursor from bytes with size limit protection.
+    /// Limits deserialization to MAX_CURSOR_SIZE bytes to prevent DoS via memory exhaustion.
     pub fn from_bytes(bytes: &[u8]) -> Option<Self> {
-        bincode::deserialize(bytes).ok()
+        bincode_options().deserialize(bytes).ok()
     }
 
     /// Encode cursor to base64 string
@@ -156,10 +172,10 @@ impl QueryFilter {
     /// Check if an entry matches this filter
     pub fn matches(&self, block_id: Option<u64>, created_at: i64, key: &[u8]) -> bool {
         match self {
-            QueryFilter::BlockBefore(max_block) => block_id.map_or(true, |b| b < *max_block),
-            QueryFilter::BlockAfter(min_block) => block_id.map_or(false, |b| b > *min_block),
+            QueryFilter::BlockBefore(max_block) => block_id.is_none_or(|b| b < *max_block),
+            QueryFilter::BlockAfter(min_block) => block_id.is_some_and(|b| b > *min_block),
             QueryFilter::BlockRange { start, end } => {
-                block_id.map_or(false, |b| b >= *start && b <= *end)
+                block_id.is_some_and(|b| b >= *start && b <= *end)
             }
             QueryFilter::CreatedBefore(timestamp) => created_at < *timestamp,
             QueryFilter::CreatedAfter(timestamp) => created_at > *timestamp,

crates/distributed-storage/src/replication.rs

@@ -102,22 +102,17 @@ impl ReplicationConfig {
 }
 
 /// Strategy for resolving conflicts between divergent values
-#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
 pub enum ConflictResolution {
     /// Last write wins (based on timestamp)
+    #[default]
     LastWriteWins,
     /// Highest version number wins
     HighestVersion,
     /// Custom merge function (caller provides)
     Custom,
 }
 
-impl Default for ConflictResolution {
-    fn default() -> Self {
-        Self::LastWriteWins
-    }
-}
-
 /// Replication policy for a namespace
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct ReplicationPolicy {

crates/distributed-storage/src/store.rs

@@ -66,7 +66,7 @@ impl StorageKey {
     /// Compute SHA256 hash of the key (for DHT routing)
     pub fn hash(&self) -> [u8; 32] {
         let mut hasher = Sha256::new();
-        hasher.update(&self.to_bytes());
+        hasher.update(self.to_bytes());
         hasher.finalize().into()
     }
 

crates/distributed-storage/src/submission.rs

@@ -329,7 +329,7 @@ impl AggregatedEvaluations {
         scores.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal));
 
         let len = scores.len();
-        let median = if len % 2 == 0 {
+        let median = if len.is_multiple_of(2) {
             (scores[len / 2 - 1] + scores[len / 2]) / 2.0
         } else {
             scores[len / 2]