fix: resolve contract and SOAR correctness issues

.github/workflows/ci.yml

@@ -37,7 +37,7 @@ jobs:
           cache-dependency-path: net-guardia-frontend/package-lock.json
 
       - name: Install frontend dependencies
-        run: npm install
+        run: npm ci
         working-directory: net-guardia-frontend
 
       - name: Install Rust stable toolchain
@@ -72,14 +72,25 @@ jobs:
           ls -la "$HOME/.cargo/bin/bpf-linker"
         timeout-minutes: 45
 
-      - name: cargo check
-        run: cargo check --package net-guardia
+      - name: cargo check default workspace members
+        run: cargo check
 
-      - name: cargo test
-        run: cargo test --package net-guardia
+      - name: cargo test default workspace members
+        run: cargo test
 
-      - name: cargo clippy
-        run: cargo clippy --package net-guardia -- -D warnings
+      - name: cargo clippy default workspace members
+        run: cargo clippy -- -D warnings
+
+      - name: Frontend build
+        run: npm run build
+        working-directory: net-guardia-frontend
+
+      - name: Frontend tests
+        run: npm test
+        working-directory: net-guardia-frontend
+
+      - name: Trainer Python compile check
+        run: python3 -m compileall -q net-guardia-trainer/src
 
   integration-test:
     name: Integration Test (placeholder)

cli/src/main.rs

@@ -1,3 +1,7 @@
+use std::fs;
+use std::io::Write;
+#[cfg(unix)]
+use std::os::unix::io::AsRawFd;
 use std::path::PathBuf;
 use std::time::Duration;
 
@@ -5,6 +9,8 @@ use clap::{Parser, Subcommand};
 use reqwest::Client;
 use serde_json::Value;
 
+const CSRF_HEADER: &str = "X-CSRF-Token";
+
 /// NetGuardia CLI management tool.
 #[derive(Parser)]
 #[command(name = "ng", about = "NetGuardia CLI", version)]
@@ -94,14 +100,14 @@ impl ApiClient {
     }
 
     fn load_token(&self) -> Option<String> {
-        std::fs::read_to_string(&self.token_path).ok()
+        fs::read_to_string(&self.token_path).ok()
     }
 
-    fn save_token(&self, token: &str) {
+    fn save_token(&self, token: &str) -> Result<(), String> {
         if let Some(parent) = self.token_path.parent() {
-            let _ = std::fs::create_dir_all(parent);
+            fs::create_dir_all(parent).map_err(|e| format!("Failed to create token directory: {}", e))?;
         }
-        let _ = std::fs::write(&self.token_path, token);
+        fs::write(&self.token_path, token).map_err(|e| format!("Failed to save token: {}", e))
     }
 
     async fn get(&self, path: &str) -> Result<Value, String> {
@@ -127,10 +133,14 @@ impl ApiClient {
 
     async fn request(&self, method: reqwest::Method, path: &str, body: Option<Value>) -> Result<Value, String> {
         let url = format!("{}{}", self.base_url, path);
+        let include_csrf = should_send_csrf(&method);
         let mut req = self.client.request(method, &url);
         if let Some(token) = self.load_token() {
             req = req.header("Authorization", format!("Bearer {}", token.trim()));
         }
+        if include_csrf {
+            req = req.header(CSRF_HEADER, "ng-cli");
+        }
         if let Some(b) = body {
             req = req.json(&b);
         }
@@ -187,11 +197,17 @@ fn print_json(data: &Value) {
     println!("{}", serde_json::to_string_pretty(data).unwrap_or_default());
 }
 
+fn should_send_csrf(method: &reqwest::Method) -> bool {
+    !matches!(
+        *method,
+        reqwest::Method::GET | reqwest::Method::HEAD | reqwest::Method::OPTIONS
+    )
+}
+
 fn read_password() -> String {
     // Disable echo for password input
     #[cfg(unix)]
     {
-        use std::os::unix::io::AsRawFd;
         let fd = std::io::stdin().as_raw_fd();
         let mut termios = unsafe { std::mem::zeroed::<libc::termios>() };
         unsafe { libc::tcgetattr(fd, &mut termios) };
@@ -285,21 +301,24 @@ async fn main() {
         },
         Commands::Login => {
             print!("Username: ");
-            std::io::Write::flush(&mut std::io::stdout()).unwrap();
+            let mut stdout = std::io::stdout();
+            stdout.flush().unwrap();
             let mut username = String::new();
             std::io::stdin().read_line(&mut username).unwrap();
             let username = username.trim();
 
             print!("Password: ");
-            std::io::Write::flush(&mut std::io::stdout()).unwrap();
+            stdout.flush().unwrap();
             let password = read_password();
 
             match api.login(username, &password).await {
-                Ok(token) => {
-                    api.save_token(&token);
-                    println!("Login successful. Token saved to ~/.ng/token");
-                    Ok(())
-                }
+                Ok(token) => match api.save_token(&token) {
+                    Ok(()) => {
+                        println!("Login successful. Token saved to ~/.ng/token");
+                        Ok(())
+                    }
+                    Err(e) => Err(e),
+                },
                 Err(e) => Err(e),
             }
         }
@@ -361,3 +380,18 @@ async fn main() {
         std::process::exit(1);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn csrf_header_is_only_needed_for_state_changing_methods() {
+        assert!(!should_send_csrf(&reqwest::Method::GET));
+        assert!(!should_send_csrf(&reqwest::Method::HEAD));
+        assert!(!should_send_csrf(&reqwest::Method::OPTIONS));
+        assert!(should_send_csrf(&reqwest::Method::POST));
+        assert!(should_send_csrf(&reqwest::Method::PUT));
+        assert!(should_send_csrf(&reqwest::Method::DELETE));
+    }
+}

net-guardia-frontend

@@ -1 +1 @@
-Subproject commit 9f1621ca8f2c9fe4e3a5e315abbc0b6c5f16498d
+Subproject commit dd0e9eae657a1240964a9f5e3c9781674885d623

net-guardia/src/adapter/http/data_plane/filter.rs

@@ -157,7 +157,9 @@ async fn remove_ssh_service(
 }
 
 async fn is_ssh_white_list_enable(service: web::Data<dyn ProtocolFilterPort>) -> impl Responder {
-    HttpResponse::Ok().json(service.is_ssh_white_list_enable())
+    HttpResponse::Ok().json(serde_json::json!({
+        "enabled": service.is_ssh_white_list_enable(),
+    }))
 }
 
 async fn enable_ssh_white_list(service: web::Data<dyn ProtocolFilterPort>) -> impl Responder {

net-guardia/src/adapter/http/setup.rs

@@ -144,14 +144,10 @@ async fn complete_setup(
     match password::hash_password(&body.admin_password) {
         Ok(hash) => {
             // Find admin user and update password
-            if let Ok(Some(user)) = db.find_user(DEFAULT_ADMIN_USERNAME).await {
-                if let Err(e) = db.update_user_password(user.id, &hash).await {
-                    log!(SystemError::SetupPasswordUpdateFailed(e));
-                }
-                // Clear force_password_change since setup wizard set the password
-                if let Err(e) = db.reset_user_password(user.id, &hash).await {
-                    log!(SystemError::SetupPasswordUpdateFailed(e));
-                }
+            if let Ok(Some(user)) = db.find_user(DEFAULT_ADMIN_USERNAME).await
+                && let Err(e) = db.update_user_password(user.id, &hash).await
+            {
+                log!(SystemError::SetupPasswordUpdateFailed(e));
             }
         }
         Err(e) => {

net-guardia/src/adapter/persistence/soar_block.rs

@@ -159,19 +159,29 @@ impl Database {
                 let existing_acl_count: i64 = tx.query_row(
                     "SELECT COUNT(*) FROM acl_rules
                      WHERE ip_version = ?1 AND direction = ?2 AND list_type = ?3 AND ip_address = ?4 AND port = ?5",
-                    params![ip_version, "source", "blacklist", source_ip, 0i64],
+                    params![ip_version, "source", "blacklist", source_ip.as_str(), 0i64],
                     |row| row.get(0),
                 )?;
-                let created_acl_rule = existing_acl_count == 0;
+                let active_soar_owned_count: i64 = tx.query_row(
+                    "SELECT COUNT(*) FROM soar_block_rules
+                     WHERE source_ip = ?1
+                       AND unblocked_at IS NULL
+                       AND expires_at > datetime('now')
+                       AND created_acl_rule = 1
+                       AND preserve_acl_on_unblock = 0",
+                    params![source_ip.as_str()],
+                    |row| row.get(0),
+                )?;
+                let created_acl_rule = existing_acl_count == 0 || active_soar_owned_count > 0;
                 tx.execute(
                     "INSERT INTO soar_block_rules (source_ip, playbook_id, expires_at, created_acl_rule)
                      VALUES (?1, ?2, ?3, ?4)",
-                    params![source_ip, playbook_id, expires_at, created_acl_rule as i64],
+                    params![source_ip.as_str(), playbook_id, expires_at, created_acl_rule as i64],
                 )?;
                 let soar_block_id = tx.last_insert_rowid();
                 tx.execute(
                     "INSERT OR IGNORE INTO acl_rules (ip_version, direction, list_type, ip_address, port) VALUES (?1, ?2, ?3, ?4, ?5)",
-                    params![ip_version, "source", "blacklist", source_ip, 0i64],
+                    params![ip_version, "source", "blacklist", source_ip.as_str(), 0i64],
                 )?;
                 tx.commit()?;
                 Ok(soar_block_id)
@@ -190,16 +200,24 @@ impl Database {
             .conn_mut_and_then(move |conn| {
                 let tx = conn.transaction()?;
                 let should_delete_acl: bool = tx.query_row(
-                    "SELECT created_acl_rule = 1 AND preserve_acl_on_unblock = 0
+                    "SELECT created_acl_rule = 1
+                            AND preserve_acl_on_unblock = 0
+                            AND NOT EXISTS (
+                                SELECT 1 FROM soar_block_rules
+                                WHERE id != ?1
+                                  AND source_ip = ?2
+                                  AND unblocked_at IS NULL
+                                  AND expires_at > datetime('now')
+                            )
                      FROM soar_block_rules WHERE id = ?1",
-                    params![soar_block_id],
+                    params![soar_block_id, source_ip.as_str()],
                     |row| row.get(0),
                 )?;
                 if should_delete_acl {
                     tx.execute(
                         "DELETE FROM acl_rules
                          WHERE ip_version = ?1 AND direction = ?2 AND list_type = ?3 AND ip_address = ?4 AND port = ?5",
-                        params![ip_version, "source", "blacklist", source_ip, 0i64],
+                        params![ip_version, "source", "blacklist", source_ip.as_str(), 0i64],
                     )?;
                 }
                 tx.execute(
@@ -236,3 +254,69 @@ impl DbAdminRepo for Database {
             .await
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const SOURCE_IP: &str = "198.51.100.42";
+    const ACTIVE_UNTIL: &str = "2999-01-01 00:00:00";
+
+    fn acl_contains(rules: &[AclRuleViewForTest], ip: &str) -> bool {
+        rules
+            .iter()
+            .any(|rule| rule.ip_address == ip && rule.direction == "source" && rule.list_type == "blacklist")
+    }
+
+    type AclRuleViewForTest = crate::domain::data_plane::acl_rule::AclRuleView;
+
+    #[tokio::test]
+    async fn overlapping_soar_blocks_keep_acl_until_last_block_unblocks() {
+        let db = Database::new(":memory:").await.expect("database");
+        let first = db
+            .commit_soar_block_to_db(SOURCE_IP, 4, 1, ACTIVE_UNTIL)
+            .await
+            .expect("first block");
+        let second = db
+            .commit_soar_block_to_db(SOURCE_IP, 4, 2, ACTIVE_UNTIL)
+            .await
+            .expect("second block");
+
+        db.commit_soar_unblock_to_db(first, 4, SOURCE_IP)
+            .await
+            .expect("first unblock");
+        assert!(
+            acl_contains(&db.list_acl_rules().await.expect("acl after first unblock"), SOURCE_IP),
+            "ACL row must stay while an overlapping SOAR block is active"
+        );
+
+        db.commit_soar_unblock_to_db(second, 4, SOURCE_IP)
+            .await
+            .expect("second unblock");
+        assert!(
+            !acl_contains(&db.list_acl_rules().await.expect("acl after second unblock"), SOURCE_IP),
+            "last SOAR-owned block should remove the SOAR-owned ACL row"
+        );
+    }
+
+    #[tokio::test]
+    async fn manual_acl_existing_before_soar_block_is_preserved_on_unblock() {
+        let db = Database::new(":memory:").await.expect("database");
+        db.insert_acl_rule(4, "source", "blacklist", SOURCE_IP, 0)
+            .await
+            .expect("manual acl");
+        let block = db
+            .commit_soar_block_to_db(SOURCE_IP, 4, 1, ACTIVE_UNTIL)
+            .await
+            .expect("soar block");
+
+        db.commit_soar_unblock_to_db(block, 4, SOURCE_IP)
+            .await
+            .expect("unblock");
+
+        assert!(
+            acl_contains(&db.list_acl_rules().await.expect("acl rules"), SOURCE_IP),
+            "pre-existing manual ACL row must remain after SOAR unblock"
+        );
+    }
+}