From 379e6d2ada3393458a60b069a1aea3c0947ac718 Mon Sep 17 00:00:00 2001 From: Clint Branham Date: Sun, 22 Mar 2026 08:14:39 -0500 Subject: [PATCH] fix: use SecureString for guest password, validate Pool/CephPool conflict Set-PveVmGuestPassword now accepts SecureString instead of plain string for the -Password parameter, matching the established pattern in Set-PvePassword. The password is converted via Marshal and zeroed from memory after use. New-PveStorage now validates that -Pool and -CephPool cannot both be specified, since both mapped to the same API key and CephPool silently overwrote Pool. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Cmdlets/Storage/NewPveStorageCmdlet.cs | 10 +++++++++- .../Cmdlets/Vms/SetPveVmGuestPasswordCmdlet.cs | 18 ++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/PSProxmoxVE/Cmdlets/Storage/NewPveStorageCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Storage/NewPveStorageCmdlet.cs index 8e321f1..d4797a9 100644 --- a/src/PSProxmoxVE/Cmdlets/Storage/NewPveStorageCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Storage/NewPveStorageCmdlet.cs @@ -15,7 +15,7 @@ namespace PSProxmoxVE.Cmdlets.Storage /// [Cmdlet(VerbsCommon.New, "PveStorage", SupportsShouldProcess = true)] [OutputType(typeof(PveStorage))] - public class NewPveStorageCmdlet : PveCmdletBase + public sealed class NewPveStorageCmdlet : PveCmdletBase { /// The unique storage identifier/name. [Parameter(Mandatory = true, Position = 0, HelpMessage = "The storage pool name.")] @@ -80,6 +80,14 @@ namespace PSProxmoxVE.Cmdlets.Storage if (!ShouldProcess(Storage, "Create PVE Storage")) return; + if (!string.IsNullOrEmpty(Pool) && !string.IsNullOrEmpty(CephPool)) + { + ThrowTerminatingError(new ErrorRecord( + new PSArgumentException("Pool and CephPool cannot both be specified. Use Pool for ZFS pools or CephPool for Ceph/RBD pools."), + "PoolConflict", ErrorCategory.InvalidArgument, null)); + return; + } + var session = GetSession(); using var client = new PveHttpClient(session); diff --git a/src/PSProxmoxVE/Cmdlets/Vms/SetPveVmGuestPasswordCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/SetPveVmGuestPasswordCmdlet.cs index f3104ed..656eada 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/SetPveVmGuestPasswordCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/SetPveVmGuestPasswordCmdlet.cs @@ -1,4 +1,6 @@ using System.Management.Automation; +using System.Runtime.InteropServices; +using System.Security; using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Cmdlets.Vms @@ -12,6 +14,7 @@ namespace PSProxmoxVE.Cmdlets.Vms /// /// [Cmdlet(VerbsCommon.Set, "PveVmGuestPassword", SupportsShouldProcess = true)] + [OutputType(typeof(void))] public sealed class SetPveVmGuestPasswordCmdlet : PveCmdletBase { /// The Proxmox VE node name. @@ -33,7 +36,7 @@ namespace PSProxmoxVE.Cmdlets.Vms /// The new password for the user. /// [Parameter(Mandatory = true, HelpMessage = "The new password.")] - public string Password { get; set; } = string.Empty; + public SecureString Password { get; set; } = new SecureString(); /// /// When specified, indicates the password is already in crypted/hashed format. @@ -48,9 +51,20 @@ namespace PSProxmoxVE.Cmdlets.Vms var session = GetSession(); + var ptr = Marshal.SecureStringToGlobalAllocUnicode(Password); + string plainPassword; + try + { + plainPassword = Marshal.PtrToStringUni(ptr) ?? string.Empty; + } + finally + { + Marshal.ZeroFreeGlobalAllocUnicode(ptr); + } + WriteVerbose($"Setting password for user '{Username}' on VM {VmId} via guest agent..."); var service = new VmService(); - service.SetGuestPassword(session, Node, VmId, Username, Password, Crypted.IsPresent); + service.SetGuestPassword(session, Node, VmId, Username, plainPassword, Crypted.IsPresent); } } }