From 56dcf22ceac4ad3779004d6b83c19cf90917e09e Mon Sep 17 00:00:00 2001
From: Clint Branham <goodolclint@gmail.com>
Date: Fri, 22 May 2026 14:38:54 -0500
Subject: [PATCH] fix: address PR #69 review feedback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- VmService.ExecuteGuestCommand: guard against null elements in -Args.
  The old JSON-serialization tolerated nulls (as "null"); the repeated-key
  path would NRE in EncodeFormValue. Throw a clear ArgumentException instead.
- findings.json: refresh the stale counters block (untouched since F085) to
  the actual ledger state — next_id 92, resolved 83 — and bump last_updated
  to 2026-05-22. last_scan_date stays 2026-03-26 (F086–F091 came from issue
  triage, not a formal review scan).
- VmServiceTests: add empty-array (single command entry) and null-element
  (throws) cases.

Note: F091 is the correct next ID — F086–F090 already exist from prior
merged PRs (#60/#61/#66/#67); only the counters were lagging.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 docs/review/findings.json                     |  8 ++---
 src/PSProxmoxVE.Core/Services/VmService.cs    |  4 +++
 .../Services/VmServiceTests.cs                | 32 +++++++++++++++++++
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/docs/review/findings.json b/docs/review/findings.json
index d783826..df95a91 100644
--- a/docs/review/findings.json
+++ b/docs/review/findings.json
@@ -1,15 +1,15 @@
 {
   "_schema_version": "1.0",
   "_description": "PSProxmoxVE stable findings ledger. IDs are permanent (F001, F002...). Resolved findings are never deleted — they are marked resolved with evidence. If a finding reappears, it is marked regressed and retains its original ID.",
-  "last_updated": "2026-03-26",
+  "last_updated": "2026-05-22",
   "last_scan_date": "2026-03-26",
   "counters": {
-    "next_id": 86,
+    "next_id": 92,
     "total_open": 7,
-    "total_resolved": 77,
+    "total_resolved": 83,
     "total_regressed": 0,
     "open": 7,
-    "resolved": 77,
+    "resolved": 83,
     "regressed": 0,
     "wont_fix": 1
   },
diff --git a/src/PSProxmoxVE.Core/Services/VmService.cs b/src/PSProxmoxVE.Core/Services/VmService.cs
index 8e1d5e6..5b7764a 100644
--- a/src/PSProxmoxVE.Core/Services/VmService.cs
+++ b/src/PSProxmoxVE.Core/Services/VmService.cs
@@ -610,7 +610,11 @@ namespace PSProxmoxVE.Core.Services
                 if (args != null)
                 {
                     foreach (var arg in args)
+                    {
+                        if (arg == null)
+                            throw new ArgumentException("Args elements must not be null.", nameof(args));
                         data.Add(new KeyValuePair<string, string>("command", arg));
+                    }
                 }
 
                 var response = client.PostAsync($"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}/agent/exec", data)
diff --git a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
index 74826e5..136b064 100644
--- a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
+++ b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Moq;
@@ -77,5 +78,36 @@ namespace PSProxmoxVE.Core.Tests.Services
             Assert.Equal("command", only.Key);
             Assert.Equal("whoami", only.Value);
         }
+
+        [Fact]
+        public void ExecuteGuestCommand_EmptyArgs_SendsSingleCommandEntry()
+        {
+            List<KeyValuePair<string, string>>? captured = null;
+            var mockClient = new Mock<IPveHttpClient>();
+            mockClient
+                .Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<IEnumerable<KeyValuePair<string, string>>>()))
+                .Callback<string, IEnumerable<KeyValuePair<string, string>>>((_, data) => captured = data.ToList())
+                .ReturnsAsync("{\"data\":{\"pid\":9}}");
+
+            var service = new VmService(mockClient.Object);
+            service.ExecuteGuestCommand(CreateSession(), TestNode, TestVmId, "whoami", new string[0]);
+
+            Assert.NotNull(captured);
+            var only = Assert.Single(captured!);
+            Assert.Equal("command", only.Key);
+            Assert.Equal("whoami", only.Value);
+        }
+
+        [Fact]
+        public void ExecuteGuestCommand_NullArgElement_ThrowsArgumentException()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            var service = new VmService(mockClient.Object);
+
+            var ex = Assert.Throws<ArgumentException>(() =>
+                service.ExecuteGuestCommand(CreateSession(), TestNode, TestVmId,
+                    "cmd.exe", new[] { "/c", null!, "echo" }));
+            Assert.Equal("args", ex.ParamName);
+        }
     }
 }