From 671ee5162600d2026fa591d56fd63a080b5ce02e Mon Sep 17 00:00:00 2001 From: Shaun Arman Date: Sun, 21 Jun 2026 15:28:24 -0500 Subject: [PATCH] fix(firewall): correct PVE API field names for rule listing and creation PVE firewall rules use pos (not rule_num), proto (not protocol), and enable as 0/1 integer (not enabled bool). Fix list_firewall_rules and add_rule in firewall.rs to use correct field names. Normalize Tauri command response to include rule/status fields that FirewallRuleList expects. add_rule now sends required type: in field and only includes optional fields when non-empty. --- src-tauri/src/commands/proxmox.rs | 17 ++++- src-tauri/src/proxmox/firewall.rs | 121 ++++++++++++++++++------------ 2 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src-tauri/src/commands/proxmox.rs b/src-tauri/src/commands/proxmox.rs index 4ee9df66..012d822a 100644 --- a/src-tauri/src/commands/proxmox.rs +++ b/src-tauri/src/commands/proxmox.rs @@ -1423,10 +1423,23 @@ pub async fn list_firewall_rules( .await .map_err(|e| format!("Failed to list firewall rules: {}", e))?; + // Normalize to match what FirewallRuleList component expects: + // rule (position number), action, protocol, source, destination, port, status let json_rules: Vec = rules .into_iter() - .map(|r| serde_json::to_value(r).map_err(|e| format!("Failed to serialize rule: {}", e))) - .collect::, _>>()?; + .map(|r| { + serde_json::json!({ + "id": r.rule_num.to_string(), + "rule": r.rule_num, + "action": r.action, + "protocol": r.protocol, + "source": r.source, + "destination": r.destination, + "port": r.port, + "status": if r.enabled { "enabled" } else { "disabled" }, + }) + }) + .collect(); Ok(json_rules) } diff --git a/src-tauri/src/proxmox/firewall.rs b/src-tauri/src/proxmox/firewall.rs index 3c1f6f89..086d1e11 100644 --- a/src-tauri/src/proxmox/firewall.rs +++ b/src-tauri/src/proxmox/firewall.rs @@ -23,7 +23,8 @@ pub struct FirewallStatus { pub rule_count: u32, } -/// List firewall rules +/// List firewall rules — returns normalized Vec using correct PVE field names. +/// PVE uses: pos (position), proto (protocol), enable (0/1 integer), src (source), dest (destination). pub async fn list_firewall_rules( client: &crate::proxmox::client::ProxmoxClient, node: &str, @@ -35,44 +36,50 @@ pub async fn list_firewall_rules( .await .map_err(|e| format!("Failed to list firewall rules: {}", e))?; - if let Some(rules) = response.as_array() { - let rule_list: Vec = rules - .iter() - .filter_map(|rule| { - let rule_num = rule.get("rule_num")?.as_u64()? as u32; - let action = rule.get("action")?.as_str()?.to_string(); - let protocol = rule.get("protocol")?.as_str().unwrap_or("").to_string(); - let source = rule.get("source")?.as_str().unwrap_or("").to_string(); - let destination = rule.get("dest")?.as_str().unwrap_or("").to_string(); - let port = rule - .get("dport") - .or(rule.get("sport")) - .and_then(|p| p.as_str()) - .map(|s| s.to_string()); - let enabled = rule - .get("enabled") - .and_then(|e| e.as_bool()) - .unwrap_or(true); + let rules = response.as_array().ok_or("Invalid response format")?; - Some(FirewallRule { - rule_num, - action, - protocol, - source, - destination, - port, - enabled, + let rule_list: Vec = rules + .iter() + .filter_map(|rule| { + // PVE uses "pos" for the rule position number + let rule_num = rule.get("pos").and_then(|v| v.as_u64()).unwrap_or(0) as u32; + let action = rule.get("action").and_then(|v| v.as_str())?.to_string(); + // PVE uses "proto" not "protocol" + let protocol = rule.get("proto").and_then(|v| v.as_str()).unwrap_or("").to_string(); + // source and dest are optional fields + let source = rule.get("source").and_then(|v| v.as_str()).unwrap_or("").to_string(); + let destination = rule.get("dest").and_then(|v| v.as_str()).unwrap_or("").to_string(); + let port = rule + .get("dport") + .or_else(|| rule.get("sport")) + .and_then(|p| p.as_str()) + .map(|s| s.to_string()); + // PVE uses "enable" as integer (1=enabled, 0=disabled), not "enabled" bool + let enabled = rule + .get("enable") + .and_then(|e| { + e.as_i64() + .map(|n| n != 0) + .or_else(|| e.as_bool()) }) - }) - .collect(); + .unwrap_or(true); - Ok(rule_list) - } else { - Err("Invalid response format".to_string()) - } + Some(FirewallRule { + rule_num, + action, + protocol, + source, + destination, + port, + enabled, + }) + }) + .collect(); + + Ok(rule_list) } -/// Add firewall rule +/// Add firewall rule — uses correct PVE API field names (proto, enable, dest). pub async fn add_rule( client: &crate::proxmox::client::ProxmoxClient, node: &str, @@ -80,15 +87,29 @@ pub async fn add_rule( ticket: &str, ) -> Result<(), String> { let path = format!("nodes/{}/firewall/rules", node); - let config = serde_json::json!({ + + let mut config = serde_json::json!({ "action": rule.action, - "protocol": rule.protocol, - "source": rule.source, - "dest": rule.destination, - "dport": rule.port, - "enabled": rule.enabled + "type": "in", + "enable": if rule.enabled { 1 } else { 0 } }); + // Only include optional fields when non-empty + if !rule.protocol.is_empty() { + config["proto"] = serde_json::Value::String(rule.protocol.clone()); + } + if !rule.source.is_empty() { + config["source"] = serde_json::Value::String(rule.source.clone()); + } + if !rule.destination.is_empty() { + config["dest"] = serde_json::Value::String(rule.destination.clone()); + } + if let Some(ref port) = rule.port { + if !port.is_empty() { + config["dport"] = serde_json::Value::String(port.clone()); + } + } + let _response: serde_json::Value = client .post(&path, &config, Some(ticket)) .await @@ -200,19 +221,23 @@ pub async fn get_firewall_status( .unwrap_or(&Vec::new()) .iter() .filter_map(|rule| { - let rule_num = rule.get("rule_num")?.as_u64()? as u32; - let action = rule.get("action")?.as_str()?.to_string(); - let protocol = rule.get("protocol")?.as_str().unwrap_or("").to_string(); - let source = rule.get("source")?.as_str().unwrap_or("").to_string(); - let destination = rule.get("dest")?.as_str().unwrap_or("").to_string(); + let rule_num = rule.get("pos").and_then(|v| v.as_u64()).unwrap_or(0) as u32; + let action = rule.get("action").and_then(|v| v.as_str())?.to_string(); + let protocol = rule.get("proto").and_then(|v| v.as_str()).unwrap_or("").to_string(); + let source = rule.get("source").and_then(|v| v.as_str()).unwrap_or("").to_string(); + let destination = rule.get("dest").and_then(|v| v.as_str()).unwrap_or("").to_string(); let port = rule .get("dport") - .or(rule.get("sport")) + .or_else(|| rule.get("sport")) .and_then(|p| p.as_str()) .map(|s| s.to_string()); let enabled = rule - .get("enabled") - .and_then(|e| e.as_bool()) + .get("enable") + .and_then(|e| { + e.as_i64() + .map(|n| n != 0) + .or_else(|| e.as_bool()) + }) .unwrap_or(true); Some(FirewallRule {