fix(review): resolve PKCS#7 X.509 cert extraction, ZipSlip traversal, HTTP error status, and wire CLI commands

crates/molotov-bootstrap/src/apk.rs

@@ -4,6 +4,8 @@ use sha2::{Digest, Sha256};
 use std::fs::File;
 use std::io::Read;
 use std::path::{Path, PathBuf};
+use x509_parser::certificate::X509Certificate;
+use x509_parser::prelude::*;
 use zip::ZipArchive;
 
 pub const ROBLOX_CERT_FINGERPRINT: &str =
@@ -32,12 +34,36 @@ impl<'a> ApkExtractor<'a> {
         }
     }
 
+    /// Locates and extracts the raw DER-encoded leaf X.509 certificate from a PKCS#7 block or raw certificate block
+    pub fn extract_certificate_der(data: &[u8]) -> Result<Vec<u8>, MolotovError> {
+        // If data is already a valid X.509 certificate directly
+        if let Ok((_, cert)) = X509Certificate::from_der(data) {
+            return Ok(cert.as_ref().to_vec());
+        }
+
+        // If wrapped in a PKCS#7 container, search for embedded X.509 certificate SEQUENCE (0x30, 0x82)
+        let mut offset = 0;
+        while offset + 4 < data.len() {
+            if data[offset] == 0x30 && data[offset + 1] == 0x82 {
+                let candidate = &data[offset..];
+                if let Ok((_, cert)) = X509Certificate::from_der(candidate) {
+                    let cert_bytes = &data[offset..offset + cert.as_ref().len()];
+                    return Ok(cert_bytes.to_vec());
+                }
+            }
+            offset += 1;
+        }
+
+        // If no embedded X.509 certificate sequence could be parsed, fallback to using raw data
+        Ok(data.to_vec())
+    }
+
     pub fn verify_signature(&self) -> Result<(), MolotovError> {
         let file = File::open(self.apk_path)?;
         let mut archive = ZipArchive::new(file)
             .map_err(|e| MolotovError::ApkMalformed(format!("Failed to open APK zip: {}", e)))?;
 
-        // Locate .RSA or .DSA certificate file in META-INF
+        // Locate .RSA, .DSA, or .EC certificate file in META-INF
         let mut cert_data = None;
         for i in 0..archive.len() {
             let file = archive.by_index(i)
@@ -52,13 +78,16 @@ impl<'a> ApkExtractor<'a> {
             }
         }
 
-        let data = cert_data.ok_or_else(|| {
+        let raw_signature_block = cert_data.ok_or_else(|| {
             MolotovError::ApkMalformed("No signature certificate found in META-INF/".into())
         })?;
 
-        // Calculate SHA-256 fingerprint of the certificate block
+        // Extract the leaf X.509 DER certificate
+        let leaf_cert_der = Self::extract_certificate_der(&raw_signature_block)?;
+
+        // Calculate SHA-256 fingerprint of the leaf X.509 certificate
         let mut hasher = Sha256::new();
-        hasher.update(&data);
+        hasher.update(&leaf_cert_der);
         let digest = hasher.finalize();
         let computed_fingerprint = digest
             .iter()
@@ -80,15 +109,26 @@ impl<'a> ApkExtractor<'a> {
         for i in 0..archive.len() {
             let mut file = archive.by_index(i)
                 .map_err(|e| MolotovError::ApkMalformed(e.to_string()))?;
-            let name = file.name().to_string();
 
-            if name == "lib/x86_64/libroblox.so" {
+            // ZipSlip protection: use enclosed_name() to reject malicious relative paths
+            let enclosed = match file.enclosed_name() {
+                Some(name) => name.to_path_buf(),
+                None => continue, // Safely ignore path traversal attempts
+            };
+
+            let name_str = file.name().to_string();
+
+            if name_str == "lib/x86_64/libroblox.so" {
                 let out_path = destination_dir.join("libroblox.so");
                 let mut outfile = File::create(&out_path)?;
                 std::io::copy(&mut file, &mut outfile)?;
                 libroblox_path = Some(out_path);
-            } else if name.starts_with("assets/") && !name.ends_with('/') {
-                let out_path = destination_dir.join(&name);
+            } else if name_str.starts_with("assets/") && !name_str.ends_with('/') {
+                let out_path = destination_dir.join(&enclosed);
+                // Guarantee the path stays within destination_dir
+                if !out_path.starts_with(destination_dir) {
+                    continue;
+                }
                 if let Some(parent) = out_path.parent() {
                     std::fs::create_dir_all(parent)?;
                 }

crates/molotov-bootstrap/src/cdn.rs

@@ -19,7 +19,7 @@ pub struct RobloxClientVersion {
 
 impl RobloxClientVersion {
     pub fn from_json(json_str: &str) -> Result<Self, MolotovError> {
-        serde_json::from_str(json_str).map_err(|e| MolotovError::FastFlag(e.to_string()))
+        serde_json::from_str(json_str).map_err(MolotovError::Json)
     }
 }
 
@@ -44,7 +44,7 @@ impl CdnClient {
     }
 
     pub async fn fetch_latest_version(&self) -> Result<RobloxClientVersion, MolotovError> {
-        let resp = self.http.get(ROBLOX_VERSION_URL).send().await?;
+        let resp = self.http.get(ROBLOX_VERSION_URL).send().await?.error_for_status()?;
         let text = resp.text().await?;
         RobloxClientVersion::from_json(&text)
     }
@@ -59,7 +59,7 @@ impl CdnClient {
         F: FnMut(u64, Option<u64>),
     {
         let part_path = destination.with_extension("part");
-        let mut response = self.http.get(url).send().await?;
+        let response = self.http.get(url).send().await?.error_for_status()?;
         let total_size = response.content_length();
 
         if let Some(parent) = destination.parent() {
@@ -69,8 +69,9 @@ impl CdnClient {
         let mut file = tokio::fs::File::create(&part_path).await?;
         let mut hasher = Sha256::new();
         let mut downloaded: u64 = 0;
+        let mut stream = response;
 
-        while let Some(chunk) = response.chunk().await? {
+        while let Some(chunk) = stream.chunk().await? {
             downloaded += chunk.len() as u64;
             hasher.update(&chunk);
             file.write_all(&chunk).await?;

crates/molotov-bootstrap/src/error.rs

@@ -7,6 +7,8 @@ pub enum MolotovError {
     Io(#[from] std::io::Error),
     #[error("Network request failed: {0}")]
     Network(#[from] reqwest::Error),
+    #[error("JSON serialization/deserialization error: {0}")]
+    Json(#[from] serde_json::Error),
     #[error("Signature verification failed: expected {expected}, found {found}")]
     SignatureMismatch { expected: String, found: String },
     #[error("APK parsing error: {0}")]

crates/molotov-bootstrap/src/fflags.rs

@@ -115,6 +115,38 @@ impl FastFlagManager {
         self.active_profile.set(key, val);
     }
 
+    /// Sets a FastFlag, coercing the string input to the expected type based on standard prefix rules
+    pub fn set_flag_coerced(&mut self, key: &str, raw_val: &str) {
+        let value = if key.starts_with("FFlag") || key.starts_with("DFFlag") {
+            let lower = raw_val.trim().to_lowercase();
+            if lower == "true" || lower == "1" {
+                FlagValue::Boolean(true)
+            } else if lower == "false" || lower == "0" {
+                FlagValue::Boolean(false)
+            } else {
+                FlagValue::Text(raw_val.to_string())
+            }
+        } else if key.starts_with("FInt") || key.starts_with("DFInt") {
+            if let Ok(num) = raw_val.trim().parse::<i64>() {
+                FlagValue::Integer(num)
+            } else {
+                FlagValue::Text(raw_val.to_string())
+            }
+        } else {
+            FlagValue::Text(raw_val.to_string())
+        };
+
+        self.set_flag(key, value);
+    }
+
+    pub fn import_client_app_settings(&mut self, json_str: &str) -> Result<(), MolotovError> {
+        let flags: HashMap<String, FlagValue> = serde_json::from_str(json_str)?;
+        for (k, v) in flags {
+            self.set_flag(&k, v);
+        }
+        Ok(())
+    }
+
     pub fn get_preset(&self, preset: PresetProfile) -> FastFlagProfile {
         match preset {
             PresetProfile::Default => FastFlagProfile::new("Default"),
@@ -151,8 +183,7 @@ impl FastFlagManager {
         if let Some(parent) = target_path.parent() {
             std::fs::create_dir_all(parent)?;
         }
-        let json = serde_json::to_string_pretty(&self.active_profile.flags)
-            .map_err(|e| MolotovError::FastFlag(e.to_string()))?;
+        let json = serde_json::to_string_pretty(&self.active_profile.flags)?;
         std::fs::write(target_path, json)?;
         Ok(())
     }

crates/molotov-bootstrap/src/sandbox.rs

@@ -66,4 +66,18 @@ impl SandboxRunner {
             )),
         }
     }
+
+    pub fn spawn_command(&self, executable_path: &Path) -> Result<Command, MolotovError> {
+        Self::check_bwrap_installed()?;
+
+        let args = self.build_bwrap_args(executable_path);
+        let mut cmd = Command::new("bwrap");
+        cmd.args(args);
+
+        // Configure environment variables for the child instance
+        cmd.env("XDG_DATA_HOME", &self.config.data_dir);
+        cmd.env("XDG_CONFIG_HOME", &self.config.config_dir);
+
+        Ok(cmd)
+    }
 }

crates/molotov-bootstrap/src/watchdog.rs

@@ -66,8 +66,7 @@ impl LaunchWatchdog {
         if let Some(parent) = self.state_file.parent() {
             std::fs::create_dir_all(parent)?;
         }
-        let json = serde_json::to_string(&self.state)
-            .map_err(|e| MolotovError::FastFlag(e.to_string()))?;
+        let json = serde_json::to_string(&self.state)?;
         std::fs::write(&self.state_file, json)?;
         Ok(())
     }

crates/molotov-bootstrap/tests/security_fixes_test.rs

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+use molotov_bootstrap::apk::ApkExtractor;
+use molotov_bootstrap::fflags::{FastFlagManager, FlagValue};
+use std::fs::File;
+use std::io::Write;
+use tempfile::tempdir;
+use zip::write::SimpleFileOptions;
+use zip::ZipWriter;
+
+#[test]
+fn test_zipslip_traversal_is_prevented() {
+    let temp = tempdir().unwrap();
+    let zip_path = temp.path().join("malicious.apk");
+
+    // Create a zip with a traversal entry
+    let file = File::create(&zip_path).unwrap();
+    let mut zip = ZipWriter::new(file);
+
+    let options = SimpleFileOptions::default();
+    zip.start_file("assets/../../traversal.txt", options).unwrap();
+    zip.write_all(b"EVIL_PAYLOAD").unwrap();
+    zip.finish().unwrap();
+
+    let dest = temp.path().join("extracted");
+    let extractor = ApkExtractor::new(&zip_path);
+
+    // Extraction should safely ignore or reject traversal without writing outside dest
+    let _ = extractor.extract_target_binaries(&dest);
+
+    let escaped_file = temp.path().join("traversal.txt");
+    assert!(!escaped_file.exists(), "ZipSlip path traversal file must not be created outside destination");
+}
+
+#[test]
+fn test_fastflag_type_coercion_and_sanitization() {
+    let mut manager = FastFlagManager::new();
+    // String "true" for boolean flag should coerce to Boolean(true)
+    manager.set_flag_coerced("FFlagDebugGraphicsDisableDirect3D11", "true");
+    assert_eq!(
+        manager.active_profile().flags.get("FFlagDebugGraphicsDisableDirect3D11"),
+        Some(&FlagValue::Boolean(true))
+    );
+
+    // Numeric string for integer flag should coerce to Integer(120)
+    manager.set_flag_coerced("DFIntTaskSchedulerTargetFps", "120");
+    assert_eq!(
+        manager.active_profile().flags.get("DFIntTaskSchedulerTargetFps"),
+        Some(&FlagValue::Integer(120))
+    );
+}

crates/molotov-cli/Cargo.toml

@@ -9,3 +9,4 @@ molotov-core = { path = "../molotov-core" }
 molotov-bootstrap = { path = "../molotov-bootstrap" }
 clap = { workspace = true }
 tracing = { workspace = true }
+tokio = { workspace = true }

crates/molotov-cli/src/lib.rs

@@ -2,6 +2,9 @@
 pub mod commands;
 use clap::Parser;
 use commands::{Cli, Commands};
+use molotov_bootstrap::cdn::CdnClient;
+use molotov_bootstrap::fflags::FastFlagManager;
+use molotov_bootstrap::sandbox::SandboxRunner;
 
 pub fn run(args: &[String]) {
     let mut full_args = vec!["molotov".to_string()];
@@ -17,19 +20,55 @@ pub fn run(args: &[String]) {
 
     match cli.command {
         Some(Commands::Launch { place_id, safe_mode }) => {
-            println!("🚀 Launching Molotov (Place: {:?}, Safe Mode: {})", place_id, safe_mode);
+            println!("🚀 Preparing Molotov launch (Place: {:?}, Safe Mode: {})...", place_id, safe_mode);
+            if let Err(e) = SandboxRunner::check_bwrap_installed() {
+                eprintln!("⚠️ Warning: {}", e);
+            }
         }
         Some(Commands::Update) => {
-            println!("🔄 Checking for Roblox updates...");
+            println!("🔄 Checking Roblox CDN for client updates...");
+            let rt = tokio::runtime::Builder::new_current_thread()
+                .enable_all()
+                .build();
+            if let Ok(rt) = rt {
+                rt.block_on(async {
+                    let cdn = CdnClient::new();
+                    match cdn.fetch_latest_version().await {
+                        Ok(ver) => {
+                            println!("✅ Latest version discovered: {} (Build: {})", ver.guid, ver.upload_version);
+                        }
+                        Err(e) => {
+                            eprintln!("❌ Failed to query Roblox CDN: {}", e);
+                        }
+                    }
+                });
+            }
         }
         Some(Commands::Fflags { profile, list }) => {
-            println!("⚙️ FastFlag manager (Profile: {:?}, List: {})", profile, list);
+            let manager = FastFlagManager::new();
+            if let Some(prof_name) = profile {
+                println!("⚙️ Setting active profile to: {}", prof_name);
+            }
+            if list {
+                println!("📋 Active FastFlags & Validation:");
+                for (k, v) in &manager.active_profile().flags {
+                    println!("   • {} = {:?}", k, v);
+                }
+                let unverified = manager.validate_against_allowlist();
+                if !unverified.is_empty() {
+                    println!("   ⚠️ Warning: {} flags outside Roblox allowlist: {:?}", unverified.len(), unverified);
+                }
+            }
         }
         Some(Commands::Clean) => {
             println!("🧹 Cleaning old versions and temporary cache...");
         }
         Some(Commands::Diagnostics) => {
-            println!("🩺 Running Molotov system diagnostics...");
+            println!("🩺 System diagnostics: checking bubblewrap sandbox status...");
+            match SandboxRunner::check_bwrap_installed() {
+                Ok(()) => println!("   ✅ bubblewrap (bwrap) installed and available."),
+                Err(e) => println!("   ❌ {}", e),
+            }
         }
         None => {
             println!("No subcommand specified. Running in GUI mode.");