Merge main into fix/http-client-timeout

Resolves findings.json conflict — F086 (from #60, merged into main) and
F087 (this branch) both append to the trailing findings array. Kept both
entries, in numeric order.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs/review/findings.json

@@ -2816,6 +2816,37 @@
         "verified_by": "dotnet build + dotnet test (382 passed)"
       }
     },
+    {
+      "id": "F086",
+      "title": "New-PveVm -DiskSize and New-PveContainer -RootFsSize pass unit suffix verbatim, LVM rejects 'NG'",
+      "category": "api_contract",
+      "severity": "high",
+      "status": "resolved",
+      "first_detected": "2026-05-20",
+      "github_issue": 58,
+      "files": [
+        "src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs",
+        "src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs"
+      ],
+      "description": "Disk and rootfs sizes are interpolated directly into the disk spec as '<storage>:<size>'. The parameter docstring advertises 'e.g. 32G' but on LVM/LVM-thin storages PVE parses the value after the colon as a volume name unless it is a bare integer, failing with 'unable to parse lvm volume name \"32G\"'. File-backed storages (NFS, directory) accept either form, masking the bug in mixed environments.",
+      "scan_history": [
+        {
+          "scan_date": "2026-05-20",
+          "local_id": null,
+          "status": "new"
+        },
+        {
+          "scan_date": "2026-05-20",
+          "local_id": null,
+          "status": "fixed"
+        }
+      ],
+      "resolution": {
+        "scan_date": "2026-05-20",
+        "evidence": "Added SizeParser.NormalizeToGibibytes() which strips G/GB/T/TB suffixes, rejects sub-GB units with a clear error, and converts TB overflows to ArgumentException. New-PveVm and New-PveContainer normalize -DiskSize and -RootFsSize before ShouldProcess so typos are caught with -WhatIf, regardless of whether the matching -DiskStorage/-RootFsStorage was supplied.",
+        "verified_by": "dotnet build + dotnet test (577 passed, 32 SizeParserTests) + Pester (39 passed, new DiskSize/RootFsSize validation contexts)"
+      }
+    },
     {
       "id": "F087",
       "title": "HttpClient uses 100s default timeout; Send-PveFile and other long-running calls fail on large payloads",
@@ -2832,7 +2863,7 @@
         "src/PSProxmoxVE/Cmdlets/Storage/InvokePveStorageDownloadCmdlet.cs",
         "src/PSProxmoxVE/Cmdlets/Connection/ConnectPveServerCmdlet.cs"
       ],
-      "description": "PveHttpClient constructs HttpClient without setting Timeout, so .NET's 100s default applies to every request. Send-PveFile, Invoke-PveStorageDownload, and Connect-PveServer expose no way to override it, so multi-GB ISO uploads on a real LAN reliably trip the 100s timeout with TaskCanceledException.",
+      "description": "PveHttpClient constructs HttpClient without setting Timeout, so .NET's 100s default applies to every request. Send-PveFile, Invoke-PveStorageDownload, and Connect-PveServer expose no way to override it, so multi-GB ISO uploads on a real LAN reliably trip the 100s timeout with TaskCanceledException. PveHttpClient.SendAsync also failed to surface the timeout as a PveApiException, leaking the raw TaskCanceledException to callers.",
       "scan_history": [
         {
           "scan_date": "2026-05-20",
@@ -2847,8 +2878,8 @@
       ],
       "resolution": {
         "scan_date": "2026-05-20",
-        "evidence": "Added PveSession.Timeout (default 100s) and a TimeSpan? override on PveHttpClient. Connect-PveServer exposes -TimeoutSeconds to set the session-default; Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with a 30-minute implicit default so large uploads/downloads do not trip the 100s HttpClient default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan.",
-        "verified_by": "dotnet build + dotnet test (550 passed, 5 new timeout tests)"
+        "evidence": "Added PveSession.Timeout (default 100s) and a TimeSpan? override on PveHttpClient. Connect-PveServer exposes -TimeoutSeconds to set the session-default; Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with a 30-minute implicit default so large uploads/downloads do not trip the 100s HttpClient default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan. PveHttpClient.SendAsync now catches the TimeoutException-wrapped TaskCanceledException and rethrows it as PveApiException(RequestTimeout) with the resource path.",
+        "verified_by": "dotnet build + dotnet test (passed including new SendAsync_TimeoutFires xUnit test) + Pester (-TimeoutSeconds coverage on all three cmdlets)"
       }
     }
   ]

src/PSProxmoxVE.Core/Utilities/SizeParser.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Globalization;
+using System.Text.RegularExpressions;
+
+namespace PSProxmoxVE.Core.Utilities
+{
+    /// <summary>
+    /// Parses storage size strings (e.g. "32G", "1T", "60") and normalizes them
+    /// to a bare integer count of gibibytes for use in PVE disk specs.
+    /// </summary>
+    /// <remarks>
+    /// PVE accepts a size suffix on file-backed storages (NFS, directory) but parses
+    /// the value after the colon as a volume name on LVM-backed storages — so
+    /// <c>local-lvm:32G</c> fails with "unable to parse lvm volume name '32G'" while
+    /// <c>local-lvm:32</c> works on every storage type. Cmdlets that build disk specs
+    /// must normalize size inputs through this helper before joining with the storage.
+    /// </remarks>
+    public static class SizeParser
+    {
+        private static readonly Regex Pattern = new Regex(
+            @"^\s*(?<num>\d+)\s*(?<unit>[A-Za-z]*)\s*$",
+            RegexOptions.Compiled);
+
+        /// <summary>
+        /// Parses a size string and returns the value as a bare integer count of GiB.
+        /// Accepts values like "60", "60G", "60GB" (= 60), "1T", "1TB" (= 1024).
+        /// Sub-GB units are rejected because PVE disk allocation is GB-granular.
+        /// </summary>
+        /// <param name="value">The size string supplied by the user.</param>
+        /// <param name="parameterName">Parameter name used in the error message.</param>
+        /// <returns>The size in whole GiB as a string, suitable for direct use in disk specs.</returns>
+        /// <exception cref="ArgumentException">The input cannot be parsed or uses an unsupported unit.</exception>
+        public static string NormalizeToGibibytes(string value, string parameterName = "size")
+        {
+            if (string.IsNullOrWhiteSpace(value))
+                throw new ArgumentException($"{parameterName} must not be null or empty.", parameterName);
+
+            var match = Pattern.Match(value);
+            if (!match.Success)
+                throw new ArgumentException(
+                    $"{parameterName} '{value}' is not a valid size. Expected a positive integer optionally suffixed with G, GB, T, or TB (e.g. '32G', '1T', '60').",
+                    parameterName);
+
+            if (!long.TryParse(match.Groups["num"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var num) || num <= 0)
+                throw new ArgumentException(
+                    $"{parameterName} '{value}' must be a positive integer.",
+                    parameterName);
+
+            var unit = match.Groups["unit"].Value.ToUpperInvariant();
+            long gib;
+            switch (unit)
+            {
+                case "":
+                case "G":
+                case "GB":
+                case "GIB":
+                    gib = num;
+                    break;
+                case "T":
+                case "TB":
+                case "TIB":
+                    try { gib = checked(num * 1024L); }
+                    catch (OverflowException)
+                    {
+                        throw new ArgumentException(
+                            $"{parameterName} '{value}' is too large to represent in GiB.",
+                            parameterName);
+                    }
+                    break;
+                default:
+                    throw new ArgumentException(
+                        $"{parameterName} '{value}' uses unsupported unit '{unit}'. Use G, GB, T, or TB. Sub-GB units (M, MB, K, KB) are not supported by PVE disk allocation.",
+                        parameterName);
+            }
+
+            return gib.ToString(CultureInfo.InvariantCulture);
+        }
+    }
+}

src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs

@@ -6,6 +6,7 @@ using Newtonsoft.Json.Linq;
 using PSProxmoxVE.Core.Client;
 using PSProxmoxVE.Core.Models.Vms;
 using PSProxmoxVE.Core.Services;
+using PSProxmoxVE.Core.Utilities;
 
 namespace PSProxmoxVE.Cmdlets.Containers
 {
@@ -59,9 +60,13 @@ namespace PSProxmoxVE.Cmdlets.Containers
         public int? Cores { get; set; }
 
         /// <summary>
-        /// <para type="description">Size of the root filesystem (e.g., "8G").</para>
+        /// <para type="description">
+        /// Size of the root filesystem. Accepts a bare integer in GiB ("8") or a value
+        /// suffixed with G/GB/T/TB (case-insensitive); the value is normalized to a
+        /// bare GiB count before being sent to the API.
+        /// </para>
         /// </summary>
-        [Parameter(Mandatory = false, HelpMessage = "Size of the root filesystem (e.g. 8G).")]
+        [Parameter(Mandatory = false, HelpMessage = "Size of the root filesystem in GiB (e.g. 8 or 8G).")]
         public string? RootFsSize { get; set; }
 
         /// <summary>
@@ -125,6 +130,13 @@ namespace PSProxmoxVE.Cmdlets.Containers
 
         protected override void ProcessRecord()
         {
+            // Validate -RootFsSize before ShouldProcess so typos like "512M" are rejected
+            // even with -WhatIf, and so the error is raised regardless of whether
+            // -RootFsStorage is also supplied.
+            string? rootFsSizeGib = null;
+            if (!string.IsNullOrEmpty(RootFsSize))
+                rootFsSizeGib = SizeParser.NormalizeToGibibytes(RootFsSize!, nameof(RootFsSize));
+
             if (!ShouldProcess($"Container on node '{Node}'", "New-PveContainer"))
                 return;
 
@@ -160,8 +172,8 @@ namespace PSProxmoxVE.Cmdlets.Containers
             if (!string.IsNullOrEmpty(RootFsStorage))
             {
                 var rootFsValue = RootFsStorage!;
-                if (!string.IsNullOrEmpty(RootFsSize))
-                    rootFsValue += $":{RootFsSize}";
+                if (rootFsSizeGib != null)
+                    rootFsValue += $":{rootFsSizeGib}";
                 config["rootfs"] = rootFsValue;
             }
 

src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs

@@ -4,6 +4,7 @@ using Newtonsoft.Json.Linq;
 using PSProxmoxVE.Core.Client;
 using PSProxmoxVE.Core.Models.Vms;
 using PSProxmoxVE.Core.Services;
+using PSProxmoxVE.Core.Utilities;
 
 namespace PSProxmoxVE.Cmdlets.Vms
 {
@@ -74,9 +75,13 @@ namespace PSProxmoxVE.Cmdlets.Vms
         public string? Machine { get; set; }
 
         /// <summary>
-        /// <para type="description">Size of the primary disk (e.g., "32G").</para>
+        /// <para type="description">
+        /// Size of the primary disk. Accepts a bare integer in GiB ("32") or a value
+        /// suffixed with G/GB/T/TB (case-insensitive); the value is normalized to a
+        /// bare GiB count before being sent to the API.
+        /// </para>
         /// </summary>
-        [Parameter(Mandatory = false, HelpMessage = "Size of the primary disk (e.g. 32G).")]
+        [Parameter(Mandatory = false, HelpMessage = "Size of the primary disk in GiB (e.g. 32 or 32G).")]
         public string? DiskSize { get; set; }
 
         /// <summary>
@@ -123,6 +128,13 @@ namespace PSProxmoxVE.Cmdlets.Vms
 
         protected override void ProcessRecord()
         {
+            // Validate -DiskSize before ShouldProcess so typos like "512M" are rejected
+            // even with -WhatIf, and so the error is raised regardless of whether
+            // -DiskStorage is also supplied.
+            string? diskSizeGib = null;
+            if (!string.IsNullOrEmpty(DiskSize))
+                diskSizeGib = SizeParser.NormalizeToGibibytes(DiskSize!, nameof(DiskSize));
+
             if (!ShouldProcess($"VM on node '{Node}'", "New-PveVm"))
                 return;
 
@@ -161,9 +173,9 @@ namespace PSProxmoxVE.Cmdlets.Vms
             if (!string.IsNullOrEmpty(OsType))
                 config["ostype"] = OsType!;
 
-            if (!string.IsNullOrEmpty(DiskStorage) && !string.IsNullOrEmpty(DiskSize))
+            if (!string.IsNullOrEmpty(DiskStorage) && diskSizeGib != null)
             {
-                var diskValue = $"{DiskStorage}:{DiskSize}";
+                var diskValue = $"{DiskStorage}:{diskSizeGib}";
                 if (!string.IsNullOrEmpty(DiskFormat))
                     diskValue += $",format={DiskFormat}";
                 config["virtio0"] = diskValue;

tests/PSProxmoxVE.Core.Tests/Utilities/SizeParserTests.cs

@@ -0,0 +1,90 @@
+using System;
+using PSProxmoxVE.Core.Utilities;
+using Xunit;
+
+namespace PSProxmoxVE.Core.Tests.Utilities
+{
+    public class SizeParserTests
+    {
+        [Theory]
+        [InlineData("32", "32")]
+        [InlineData("32G", "32")]
+        [InlineData("32g", "32")]
+        [InlineData("32GB", "32")]
+        [InlineData("32gb", "32")]
+        [InlineData("32GiB", "32")]
+        [InlineData("1T", "1024")]
+        [InlineData("1t", "1024")]
+        [InlineData("1TB", "1024")]
+        [InlineData("1TiB", "1024")]
+        [InlineData("2T", "2048")]
+        [InlineData("  60G  ", "60")]
+        [InlineData("60 G", "60")]
+        public void NormalizeToGibibytes_AcceptedInputs_ReturnsBareGibibyteString(string input, string expected)
+        {
+            var result = SizeParser.NormalizeToGibibytes(input);
+            Assert.Equal(expected, result);
+        }
+
+        [Theory]
+        [InlineData("512M")]
+        [InlineData("512MB")]
+        [InlineData("1024K")]
+        [InlineData("1024KB")]
+        [InlineData("100B")]
+        [InlineData("1P")]
+        [InlineData("1PB")]
+        public void NormalizeToGibibytes_UnsupportedUnit_Throws(string input)
+        {
+            var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
+            Assert.Contains("unsupported unit", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData("   ")]
+        [InlineData(null)]
+        public void NormalizeToGibibytes_EmptyOrWhitespace_Throws(string? input)
+        {
+            Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input!));
+        }
+
+        [Theory]
+        [InlineData("abc")]
+        [InlineData("G")]
+        [InlineData("-32")]
+        [InlineData("32.5G")]
+        [InlineData("32 G B")]
+        public void NormalizeToGibibytes_Malformed_Throws(string input)
+        {
+            Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
+        }
+
+        [Theory]
+        [InlineData("0")]
+        [InlineData("0G")]
+        public void NormalizeToGibibytes_Zero_Throws(string input)
+        {
+            var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
+            Assert.Contains("positive", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Fact]
+        public void NormalizeToGibibytes_UsesProvidedParameterNameInError()
+        {
+            var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes("512M", "DiskSize"));
+            Assert.Equal("DiskSize", ex.ParamName);
+            Assert.Contains("DiskSize", ex.Message);
+        }
+
+        [Fact]
+        public void NormalizeToGibibytes_TerabyteOverflow_ThrowsArgumentException()
+        {
+            // long.MaxValue with a T suffix overflows when multiplied by 1024.
+            var input = long.MaxValue.ToString(System.Globalization.CultureInfo.InvariantCulture) + "T";
+            var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input, "DiskSize"));
+            Assert.Equal("DiskSize", ex.ParamName);
+            Assert.Contains("too large", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+    }
+}

tests/PSProxmoxVE.Tests/Containers/New-PveContainer.Tests.ps1

@@ -0,0 +1,68 @@
+#Requires -Module Pester
+<#
+.SYNOPSIS
+    Pester 5 tests for New-PveContainer.
+    All tests are fully offline — no live Proxmox VE target is required.
+#>
+
+BeforeAll {
+    . $PSScriptRoot/../_TestHelper.ps1
+}
+
+Describe 'New-PveContainer' {
+
+    Context 'Command existence' {
+        It 'Should be available after module import' {
+            Get-Command 'New-PveContainer' -ErrorAction SilentlyContinue |
+                Should -Not -BeNullOrEmpty
+        }
+
+        It 'Should be a CmdletInfo (binary cmdlet)' {
+            (Get-Command 'New-PveContainer').CommandType | Should -Be 'Cmdlet'
+        }
+    }
+
+    Context 'ShouldProcess support' {
+        BeforeAll {
+            $script:Cmd = Get-Command 'New-PveContainer'
+        }
+
+        It 'Should support ShouldProcess (WhatIf parameter present)' {
+            $script:Cmd.Parameters.ContainsKey('WhatIf') | Should -BeTrue
+        }
+
+        It 'Should support ShouldProcess (Confirm parameter present)' {
+            $script:Cmd.Parameters.ContainsKey('Confirm') | Should -BeTrue
+        }
+    }
+
+    Context 'RootFsSize validation' {
+        # Validation runs before ShouldProcess so -WhatIf is enough to exercise it
+        # without an active session.
+
+        It 'Should reject sub-GB units (e.g. 512M)' {
+            { New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '512M' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*unsupported unit*'
+        }
+
+        It 'Should reject sub-GB units even when -RootFsStorage is omitted' {
+            { New-PveContainer -Node 'pve-node1' -RootFsSize '512M' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*unsupported unit*'
+        }
+
+        It 'Should reject malformed input (e.g. 8.5G)' {
+            { New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8.5G' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*not a valid size*'
+        }
+
+        It 'Should accept a bare integer with -WhatIf' {
+            { New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8' -WhatIf -ErrorAction Stop } |
+                Should -Not -Throw
+        }
+
+        It 'Should accept "8G" with -WhatIf' {
+            { New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8G' -WhatIf -ErrorAction Stop } |
+                Should -Not -Throw
+        }
+    }
+}

tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1

@@ -133,4 +133,39 @@ Describe 'New-PveVm' {
                 Should -Throw '*No active Proxmox VE session*'
         }
     }
+
+    Context 'DiskSize validation' {
+        # Validation runs before ShouldProcess so -WhatIf is enough to exercise it
+        # without an active session.
+
+        It 'Should reject sub-GB units (e.g. 512M)' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '512M' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*unsupported unit*'
+        }
+
+        It 'Should reject sub-GB units even when -DiskStorage is omitted' {
+            { New-PveVm -Node 'pve-node1' -DiskSize '512M' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*unsupported unit*'
+        }
+
+        It 'Should reject malformed input (e.g. 32.5G)' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32.5G' -WhatIf -ErrorAction Stop } |
+                Should -Throw '*not a valid size*'
+        }
+
+        It 'Should accept a bare integer with -WhatIf' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -WhatIf -ErrorAction Stop } |
+                Should -Not -Throw
+        }
+
+        It 'Should accept "32G" with -WhatIf' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32G' -WhatIf -ErrorAction Stop } |
+                Should -Not -Throw
+        }
+
+        It 'Should accept "1T" with -WhatIf' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '1T' -WhatIf -ErrorAction Stop } |
+                Should -Not -Throw
+        }
+    }
 }