From 5fbff576afc135c698fdbd6622b4bcf22438bec0 Mon Sep 17 00:00:00 2001
From: "Joseph.Rawlings" <root0091@gmail.com>
Date: Thu, 18 Dec 2025 08:52:53 -0500
Subject: [PATCH] fix: enhance firewall rule handling with improved payload
 structure and error logging

---
 .../Servers/Proxmox/Livewire/Firewall.php     | 86 +++++++++++++------
 1 file changed, 61 insertions(+), 25 deletions(-)

diff --git a/extensions/Servers/Proxmox/Livewire/Firewall.php b/extensions/Servers/Proxmox/Livewire/Firewall.php
index 9a04101bf..933b2145a 100644
--- a/extensions/Servers/Proxmox/Livewire/Firewall.php
+++ b/extensions/Servers/Proxmox/Livewire/Firewall.php
@@ -660,74 +660,110 @@ class Firewall extends Component
         $this->loading = true;
         
         try {
+            // Build the payload according to Proxmox API requirements
             $payload = [
-                'action' => $this->ruleAction,
-                'type' => $this->ruleType,
+                'action' => strtoupper($this->ruleAction), // Ensure uppercase (ACCEPT, REJECT, DROP)
+                'type' => $this->ruleType, // 'in' or 'out'
                 'enable' => $this->ruleEnable ? 1 : 0,
             ];
 
-            // Note: ruleMacro is only used for UI convenience to auto-populate fields
-            // It is NOT sent to Proxmox API - only the actual proto/dport values are sent
-            
-            if (!empty(trim($this->ruleIface))) {
+            // Add optional fields only if they have values
+            // Proxmox API is strict about empty values
+            if (!empty($this->ruleIface)) {
                 $payload['iface'] = trim($this->ruleIface);
             }
             
-            if (!empty(trim($this->ruleSource))) {
+            if (!empty($this->ruleSource)) {
                 $payload['source'] = trim($this->ruleSource);
             }
             
-            if (!empty(trim($this->ruleDest))) {
+            if (!empty($this->ruleDest)) {
                 $payload['dest'] = trim($this->ruleDest);
             }
             
-            if (!empty(trim($this->ruleProto))) {
-                $payload['proto'] = trim($this->ruleProto);
+            // Protocol must be lowercase for Proxmox API
+            if (!empty($this->ruleProto)) {
+                $payload['proto'] = strtolower(trim($this->ruleProto));
             }
             
-            if (!empty(trim($this->ruleDport))) {
+            if (!empty($this->ruleDport)) {
                 $payload['dport'] = trim($this->ruleDport);
             }
             
-            if (!empty(trim($this->ruleSport))) {
+            if (!empty($this->ruleSport)) {
                 $payload['sport'] = trim($this->ruleSport);
             }
             
-            if (!empty(trim($this->ruleComment))) {
+            if (!empty($this->ruleComment)) {
                 $payload['comment'] = trim($this->ruleComment);
             }
             
-            if (!empty(trim($this->ruleLog)) && $this->ruleLog !== 'nolog') {
+            if (!empty($this->ruleLog) && $this->ruleLog !== 'nolog') {
                 $payload['log'] = $this->ruleLog;
             }
 
+            // Determine if we're editing an existing rule or creating a new one
             if ($this->editingRuleIndex !== null && isset($this->rules[$this->editingRuleIndex])) {
+                // EDIT EXISTING RULE
                 $pos = $this->rules[$this->editingRuleIndex]['pos'];
-                $response = $this->firewallRequest(
-                    '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos,
-                    'put',
-                    $payload
-                );
+                $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos;
+                
+                Log::info('Updating firewall rule', [
+                    'url' => $url,
+                    'payload' => $payload,
+                    'pos' => $pos
+                ]);
+                
+                $response = $this->firewallRequest($url, 'put', $payload);
             } else {
-                $response = $this->firewallRequest(
-                    '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules',
-                    'post',
-                    $payload
-                );
+                // CREATE NEW RULE
+                $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules';
+                
+                Log::info('Creating firewall rule', [
+                    'url' => $url,
+                    'payload' => $payload
+                ]);
+                
+                $response = $this->firewallRequest($url, 'post', $payload);
             }
 
+            // Log the response for debugging
+            Log::info('Firewall rule API response', [
+                'status' => $response->status(),
+                'successful' => $response->successful(),
+                'body' => $response->body()
+            ]);
+
             if ($response->successful()) {
                 $this->dispatch('notify', 'Rule ' . ($this->editingRuleIndex !== null ? 'updated' : 'created') . ' successfully', 'success');
+                
+                // Reset the form
                 $this->resetRuleForm();
                 $this->editingRuleIndex = null;
                 $this->showRuleForm = false;
+                
+                // Reload rules from Proxmox to get the updated list
                 $this->loadFirewallRules();
             } else {
-                $message = $response->json()['errors'][0]['message'] ?? 'Unknown error';
+                // Extract error message from Proxmox API response
+                $responseData = $response->json();
+                $message = $responseData['errors'][0]['message'] ?? $responseData['message'] ?? $response->body();
+                
+                Log::error('Failed to save firewall rule', [
+                    'status' => $response->status(),
+                    'response' => $responseData,
+                    'payload' => $payload
+                ]);
+                
                 $this->error = 'Failed to save rule: ' . $message;
                 $this->showErrorModal = true;
             }
         } catch (\Exception $e) {
+            Log::error('Exception saving firewall rule', [
+                'message' => $e->getMessage(),
+                'trace' => $e->getTraceAsString()
+            ]);
+            
             $this->error = 'Failed to save rule: ' . $e->getMessage();
             $this->showErrorModal = true;
         } finally {