25 Commits

Author SHA1 Message Date
Clint Branham 56dcf22cea fix: address PR #69 review feedback
- 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>
2026-05-22 14:38:54 -05:00
Clint Branham bc71ed4a12 fix: deliver Invoke-PveVmGuestExec -Args to the guest as argv
ExecuteGuestCommand JSON-serialized the args array into the agent/exec
'input-data' field — which is the process's STDIN, not its arguments. So
guest commands ran with no argv: cmd.exe started interactively and the
JSON blob ['/c','echo',...] arrived at its prompt.

PVE's agent/exec 'command' parameter is itself an array (element 0 = the
executable, the rest = argv) sent as repeated form keys. The low-level
client couldn't express repeated keys (Dictionary<string,string> only),
so:

- Add PostAsync(string, IEnumerable<KeyValuePair<string,string>>) to
  IPveHttpClient/PveHttpClient; BuildFormContent now emits one key=value
  field per pair, so a key may repeat.
- ExecuteGuestCommand builds command = [exe] + args as repeated 'command'
  fields and no longer touches input-data.

Tests: form-encoder repeated-key + per-value encoding cases; VmService
tests asserting the command array, order, and absence of input-data; an
integration regression guard that echoes an arg and checks it round-trips
as stdout.

Tracked as F091. Closes #68.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:32:04 -05:00
Clint Branham 63ee16a9e7 Merge remote-tracking branch 'origin/main' into feat/new-pvevm-disk-options
# Conflicts:
#	docs/review/findings.json
2026-05-22 14:15:20 -05:00
Clint Branham c1714048b4 feat: disk controller/IO options on New-PveVm + surface all Get-PveVmConfig keys
New-PveVm (F089):
- Add -DiskBus (virtio/scsi/sata/ide, default virtio), -ScsiHardware (scsihw),
  -DiskIoThread, -DiskAio, -DiskSsd, -DiskDiscard, -DiskCache so a tuned disk
  (e.g. virtio-scsi-single + scsi0,iothread=1,aio=native,ssd=1,discard=on) can
  be created in one call instead of diskless + a hand-built Set-PveVmConfig string.
- Disk spec built via BuildDiskSpec; ValidateDiskOptions runs before ShouldProcess
  and rejects ssd on virtio and iothread on sata/ide or scsi-without-virtio-scsi-single
  with clear errors, instead of letting PVE fail at VM start.

Get-PveVmConfig (F090):
- PveVmConfig was a fixed allow-list, silently dropping keys like scsihw, efidisk0,
  tpmstate0, hostpci0. Add typed scsihw/efidisk0/tpmstate0 plus a [JsonExtensionData]
  catch-all exposed as AdditionalProperties (native types via JsonHelper.ToNative,
  per D013 — no JToken leakage). Makes the disk tuning above verifiable by reading
  the config back.

Closes #65.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:10:14 -05:00
Clint Branham 3fd770d84c fix: percent-encode ';' in form values
PveHttpClient.EncodeFormValue encoded &, =, +, space, and % but left ';'
literal. PVE's application/x-www-form-urlencoded parser treats a raw ';'
as a field separator (the historical alternative to '&'), so a value like

    boot=order=scsi0;ide2

was split into 'boot=order=scsi0' plus an empty 'ide2' field, and PVE
rejected the PUT with "ide2: unable to parse drive options". This broke
any multi-device boot order set via Set-PveVmConfig -AdditionalConfig,
and any other value containing ';'.

Encode ';' as %3B. Safe under the existing minimal-encoding policy that
keeps ':' and '!' literal for cluster-join: cluster-join payloads never
contain ';', and PVE url-decodes config form values.

Tracked as F088. Closes #64.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:04:31 -05:00
Clint Branham ea2bcdc336 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>
2026-05-20 18:17:20 -05:00
Clint Branham f3b06171b2 fix: add -TimeoutSeconds for long-running HTTP calls
PveHttpClient was constructed without setting HttpClient.Timeout, so
.NET's 100s default applied to every request. Multi-GB ISO uploads via
Send-PveFile on a real LAN reliably tripped this with TaskCanceledException
after 100 seconds, and there was no way to override it.

- PveSession gains a Timeout (TimeSpan) property, defaulting to 100s.
- PveHttpClient accepts an optional per-instance timeout override that
  takes precedence over the session timeout.
- 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 default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan.

Tracked as F087. Closes #59.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:08:26 -05:00
Clint Branham fa361a3691 fix: address PR #60 review feedback
- SizeParser: wrap TB-suffix overflow in try/catch so callers get
  ArgumentException with the parameter name rather than OverflowException.
- New-PveVm/New-PveContainer: validate -DiskSize/-RootFsSize before
  ShouldProcess so typos like "512M" are rejected even with -WhatIf and
  even when the matching -DiskStorage/-RootFsStorage is omitted.
- Add Pester tests for the new DiskSize and RootFsSize validation paths,
  including a new New-PveContainer.Tests.ps1.
- Add SizeParserTests coverage for the TB overflow path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:06:13 -05:00
Clint Branham 6181c8ce77 fix: normalize -DiskSize and -RootFsSize before sending to PVE
Disk and rootfs size strings were interpolated directly into the disk
spec as "<storage>:<size>", so "60G" produced "local-lvm:60G". On
LVM/LVM-thin storages PVE parses the value after the colon as a volume
name unless it is a bare integer, returning "unable to parse lvm volume
name '60G'". File-backed storages mask this by accepting either form.

SizeParser.NormalizeToGibibytes() now strips G/GB/T/TB suffixes and
returns a bare GiB integer string, so the documented "32G" call shape
works on every storage type. Sub-GB units are rejected with a clear
error rather than being silently truncated.

Tracked as F086. Closes #58.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 17:59:21 -05:00
Clint Branham 197e206123 chore: scan-9 review findings + fix publish secret name
- Resolve F039 (bare catch regression), F053 (HA coverage), F060
  (cluster config coverage), F084 (PveSession secret hiding)
- Update findings.json: 11 open → 7 open, 0 regressions
- Update REVIEW_REPORT.md with scan-9 results
- Fix publish.yml to use NUGET_API_KEY secret (matches other projects)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 08:58:35 -05:00
copilot-swe-agent[bot] 4a63055c10 chore: remove pre-correction D013-compliance.md and fix JsonHelper XML doc comment nullability
Co-authored-by: GoodOlClint <151449+GoodOlClint@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoodOlClint/PSProxmoxVE/sessions/2feecd30-2ba1-40bc-b1b5-c1fcc485a319
2026-03-25 16:42:29 +00:00
Clint Branham 10c277af10 fix: resolve F085 — PveClusterJoinInfo Nodelist/Totem D013 violations
D013 compliance scan found 2 remaining Newtonsoft type exposures:
- PveClusterJoinInfo.Nodelist: JArray → List<Dictionary<string, object?>>
- PveClusterJoinInfo.Totem: JObject → Dictionary<string, object?>

Both now use NativeListConverter/NativeDictionaryConverter for
deserialization. Full D013 compliance report added.

Updated findings.json: F085 status open with scan evidence.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 11:31:05 -05:00
Clint Branham f311c738b4 docs: review scan-8 findings update
Scan-8 full re-scan of PSProxmoxVE module (2026-03-24).

New findings:
- F084 [medium/security]: PveSession exposes auth secrets in pipeline output

Regressed:
- F039 [medium]: Bare catch blocks in VmService.PingGuestAgent and
  ImportPveOvaCmdlet (D004 violation)

Resolved:
- F059: VM/CT-level firewall was already supported via Level parameter

Findings DB: F001-F084, 10 open + 1 regressed + 72 resolved + 1 wont_fix

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 18:15:46 -05:00
Clint Branham b4754c72a0 docs: update README, CHANGELOG, SECURITY.md, and review artifacts
- README: move Set-PveStorage to Storage section, Set-PveApiToken to
  Access section (were miscategorized under "Access — Groups & Domains")
- CHANGELOG: add Changed section with 13 architectural improvements,
  expand Fixed section with all ConfirmImpact and credential fixes
- SECURITY.md: add SecureString detail, Input Validation section,
  Dependabot mention, SkipCertificateCheck warning note
- CLAUDE.md: update review system reference to scan-7
- findings.json: F050 resolved (URL encoding), F031 resolved (credentials)
- REVIEW_REPORT.md: scan-7 report

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 17:41:19 -05:00
Clint Branham f6a2f843db docs: update cmdlet help via platyPS merge, fix generate-help.ps1
- generate-help.ps1: rewritten to use Update-MarkdownHelp (merge) by
  default, preserving hand-written descriptions and examples. Only
  New-MarkdownHelp on genuinely new cmdlets. -Force flag for full regen.
- docs/cmdlets: parameter metadata updated via platyPS merge, CRLF
  normalized to LF
- Removed stale Send-PveIso.md (cmdlet renamed to Send-PveFile)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 17:41:04 -05:00
Clint Branham 053fd4c06a fix(security): add Uri.EscapeDataString to all URL path parameters
20 instances across 5 service files were missing URL encoding on
dynamic path segments (node, iface, zone, vnet). Fixes D003 regression.

- NodeService: 7 methods
- NetworkService: 7 methods (5 node-network + 2 SDN remove)
- CloudInitService: 3 methods
- TaskService: 2 methods (GetTask, GetTaskLog)
- TemplateService: 1 method (CreateTemplate)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 17:40:36 -05:00
Clint Branham a424dd18ac refactor(ci): parallel provisioning, file caching, provision/test/cleanup split
Restructures the integration test workflow from a monolithic sequential
job into separate provision → test → cleanup stages:

- provision: creates ALL nested PVE VMs in a single terraform apply
  (parallel), waits for APIs, creates tokens, passes outputs to tests
- test: matrix [pve9, pve8] consumes provision outputs, no provisioning
- cleanup: always runs, API-only teardown for all VMs

Terraform refactored to for_each with pve_instances map variable,
enabling parallel ISO upload and VM creation.

New caching scripts reduce redundant downloads:
- ensure-base-iso.sh: downloads PVE ISOs to /opt/pve-isos if missing
- ensure-cloud-images.sh: caches cloud image + OVA with 7-day TTL
- prepare-auto-iso.sh: --cache-dir flag with hash-based skip

Runner no longer needs manual ISO provisioning (zero-touch setup).
cleanup-images bumped to min-versions-to-keep: 3 to survive overlapping
runs. All jobs gated against dependabot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 16:05:20 -05:00
Clint Branham 89bead7a0e fix: add -Confirm:$false to Restart-PveContainer integration test (F080, F081)
Restart-PveContainer gained ConfirmImpact.High in the F062 fix, which
correctly prompts for confirmation on destructive operations. The
integration test was missing -Confirm:$false, causing a
NullReferenceException in non-interactive CI. The cascading failure also
broke the Copy-PveContainer test (F081).

Also updates CONTRIBUTING.md to reference .NET SDK 10.0+ (was 9.0+) to
match all CI workflows and test project TFM (F083).

Includes scan-6 review report and findings database update (F001-F083).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 14:48:11 -05:00
Clint Branham 68dadbfdc0 fix: remediate findings F045, F047, F048, F064, F070, F071, F076-F079
Phase 1 — Trivial fixes:
- F071: Add Uri.EscapeDataString to GetPveTemplateCmdlet node path
- F077: Add ValidateRange(100, 999999999) to GetPveTaskListCmdlet.VmId
- F076: Create .github/dependabot.yml (nuget + github-actions, weekly)
- F079: Fix unit-tests.yml dotnet SDK from 9.0.x to 10.0.x
- F048: Mark wont_fix — sync-over-async accepted for PS 5.1 compat

Phase 2 — Framework targeting (D009 compliance):
- F047: Reduce publishable csproj to netstandard2.0 only, remove all
  #if NET48/NETSTANDARD2_0 conditionals from PveHttpClient.cs,
  restructure build.yml for netstandard2.0 publish + net10.0/net48 tests
- F064: Resolved by F047 — SMA 7.5.0 ItemGroup removed with net10.0 TFM
- F070: Add PS 5.1 smoke-test job to publish.yml (windows-latest)

Phase 3 — IPveHttpClient interface extraction (F045):
- Extract IPveHttpClient interface from PveHttpClient
- Add constructor injection to all 14 service classes
- Services use injected client when available, create+dispose when not

Phase 4 — Service unit tests (F078):
- 196 new xUnit tests across 10 service test files
- All services tested via Moq-mocked IPveHttpClient
- Total test count: 382 (was 186)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 13:51:37 -05:00
Clint Branham 94424367bf fix: remediate scan-4 findings F058, F062, F063, F071, F072, F073, F074, F075
F058 (critical): Replace while(true) infinite-loop task polling with
TaskService.WaitForTask in 5 container snapshot and storage cmdlets.

F073+F047 (high): Migrate net9.0 → net10.0 across both source .csproj
files, build.yml, publish.yml, and test helper.

F071 (medium): Add Uri.EscapeDataString() to all inline URL path
segments in ~16 cmdlets that bypass service classes (D003).

F062+F063 (medium): Add ConfirmImpact.High to Restart-PveContainer
and Suspend-PveContainer (D006).

F075 (medium): Generate markdown help docs for 89 cmdlets that were
missing documentation (170 total, up from 81).

F072 (low): Remove unused System.Text.Json dependency from Core.csproj.
F074 (low): Raise publish smoke-test threshold from 60 to 150.
F065 (low): Add .github/ISSUE_TEMPLATE/config.yml.
F066 (low): Add CODEOWNERS.

Also fix _TestHelper.ps1 net9.0 → net10.0 framework reference.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 09:54:28 -05:00
Clint Branham 0fa92d689f docs(review): scan-4 review report and findings update
Add scan-4 full review: 1 resolved (F040), 6 new findings (F071-F076).
27 open findings, 49 resolved. No regressions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 09:07:51 -05:00
Clint Branham df4cae01bc feat: close API coverage gaps — 51 new cmdlets across 6 areas
Phase 1 — Missing Update/Set operations (10 cmdlets):
- SDN: Set-PveSdnZone/Vnet/Subnet/Controller/Ipam/Dns + Invoke-PveSdnApply
- Set-PveRole, Set-PveStorage, Set-PveApiToken

Phase 2 — Cluster operations (8 cmdlets):
- Get-PveClusterResource (cluster-wide inventory)
- Get-PveTaskList, Stop-PveTask
- Get/New/Set/Remove-PvePool
- Get-PveBackupInfo (unprotected VMs)

Phase 3 — VM & container gaps (14 cmdlets):
- Move-PveVmDisk, Remove-PveVmDisk
- Guest agent: Get-PveVmGuestOsInfo/FsInfo, Read/Write-PveVmGuestFile,
  Set-PveVmGuestPassword, Invoke-PveVmGuestFsTrim
- Suspend/Resume-PveContainer, Resize-PveContainerDisk,
  New-PveContainerTemplate, Move-PveContainerVolume,
  Get-PveContainerInterface

Phase 4 — Storage content management (4 cmdlets):
- Get-PveStorageStatus, Remove/Set-PveStorageContent, New-PveStorageDisk

Phase 5 — Node operations (6 cmdlets):
- Get/Set-PveNodeConfig, Get/Set-PveNodeDns, Start/Stop-PveNodeVms

Phase 6 — Access management (9 cmdlets):
- Get/New/Set/Remove-PveGroup, Get/New/Set/Remove-PveDomain, Set-PvePassword

Total: 169 cmdlets (was 118). Includes models, services, format views,
Pester unit tests, manifest updates, README, CHANGELOG, API coverage docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 09:55:19 -05:00
Clint Branham a72642d203 feat: add firewall, backup, and SDN IPAM/DNS/controller cmdlets
Add 35 new cmdlets bringing the total to 118:
- Firewall (21): rules, groups, aliases, IP sets, options at
  cluster/node/VM/container levels
- Backup (5): ad-hoc vzdump and scheduled backup job CRUD
- SDN IPAM/DNS/Controller (9): plugin management for SDN subsystem

Also includes:
- Fix: Remove-PveRole now has ConfirmImpact.High
- Fix: URL-encode snapshot names in API paths
- Refactor: extract auth header strings to constants in PveHttpClient
- Add PSGallery version badge to README
- Full test coverage: 11 JSON fixtures, xUnit model tests, Pester
  unit tests, and integration tests for firewall/backup/OVA import
- Updated manifest, format file, CHANGELOG, README, API coverage docs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 07:51:49 -05:00
Clint Branham 088221a5f1 feat: add Import-PveVmDisk cmdlet for disk image import
Imports disk images (qcow2, raw, vmdk) and OVA archives into VMs
via the PVE API using POST config with import-from syntax.

- VmService.ImportDisk() method using POST /nodes/{node}/qemu/{vmid}/config
- Import-PveVmDisk cmdlet with -Source, -Disk, -TargetStorage, -Format, -Wait
- Pester unit tests
- prepare-test-vm.sh updated to use Import-PveVmDisk instead of SSH qm importdisk
- Set-PveNetwork: add required Type parameter (fixes PVE API 400 error)
- Integration test fix: pass -Type bridge to Set-PveNetwork

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 12:55:36 -05:00
Clint Branham 4f04391e73 docs: add platyPS help generation and MAML XML
- generate-help.ps1 script to build markdown stubs and MAML XML
- 81 cmdlet markdown help files in docs/cmdlets/
- PSProxmoxVE.dll-Help.xml (898 KB) for Get-Help support
- PVE_API_COVERAGE.md documenting implemented and missing API areas
- .csproj updated to include help XML in build output
- HelpInfoUri added to module manifest

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 12:32:03 -05:00