Commit Graph

16 Commits

Author SHA1 Message Date
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 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 cb97a86c9f fix: drop net48-incompatible TimeoutException filter in timeout catch
The when filter (ex.InnerException is TimeoutException) only matches on
.NET 5+. On .NET Framework 4.8 — which CI exercises via the test project's
net48 target — HttpClient.Timeout throws a bare TaskCanceledException
with no inner exception, so CI failed:

  Expected: typeof(PSProxmoxVE.Core.Exceptions.PveApiException)
  Actual:   typeof(System.Threading.Tasks.TaskCanceledException)
  ---- System.Threading.Tasks.TaskCanceledException : A task was canceled.

PveHttpClient.SendAsync never passes a CancellationToken to the inner
HttpClient.SendAsync, so the only way a TaskCanceledException can reach
this catch is HttpClient.Timeout firing — true on net48, .NET Core, and
.NET 5+. Drop the filter and wrap unconditionally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:22:47 -05:00
Clint Branham a1d9550d83 fix: surface HttpClient timeouts as PveApiException(RequestTimeout)
When HttpClient.Timeout elapses, .NET throws TaskCanceledException — not
HttpRequestException — so the existing catch in PveHttpClient.SendAsync
missed it and callers got a raw stack trace. With -TimeoutSeconds now
configurable and documented, this gap became user-visible.

In .NET 5+ HttpClient surfaces transport timeouts as TaskCanceledException
with a TimeoutException inner; user-driven token cancellation does not.
Catch by that inner-type signature and rethrow as PveApiException with
HttpStatusCode.RequestTimeout, the resource path, and a message that
reports the configured timeout.

Adds SendAsync_TimeoutFires_ThrowsPveApiExceptionWithRequestTimeout which
swaps in a delaying HttpMessageHandler with a 50ms timeout to exercise
the path deterministically. Drops the redundant
DefaultSessionTimeoutIs100Seconds test (covered by
PveSessionTests.Timeout_DefaultIs100Seconds and the existing flow-through
test).

Addresses PR #61 review feedback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:17:31 -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 598e9edef2 fix: replace FormUrlEncodedContent with minimal form encoding
.NET's FormUrlEncodedContent over-encodes characters like : and !
(%3A, %21) in form values. PVE's internal API consumers (specifically
the cluster join process) do not URL-decode these values before using
them, causing fingerprint comparison failures and password mismatches.

Replace with BuildFormContent/EncodeFormValue that only encodes
characters that break form parsing (&, =, +, space, %). This matches
curl's -d behavior and fixes cluster join "Cluster join aborted!" errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -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 ec8f3a86e2 fix: use specific exception types in catch blocks
Replaced 5 bare catch blocks with specific exception types:
- PveHttpClient: catch JsonException for JSON parse fallback
- PveCmdletBase: catch Exception with filter excluding OOM/SOE for polling
- VmService/ContainerService: catch PveApiException|HttpRequestException
  for offline node handling
- GetPveVmCmdlet: catch PveApiException for VM enrichment failures

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-22 08:14:51 -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 fb4a21a4f9 fix: use cryptographic RNG for multipart boundary generation
Replace `new Random()` with `RandomNumberGenerator` in
PveHttpClient.GenerateBoundary(). Uses the static Fill() method on
.NET Core and the disposable Create() pattern on net48/netstandard2.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 12:31:40 -05:00
Clint Branham 14cbff9877 docs: add XML doc comments to Core public API and remove CS1591 suppression
- Add missing XML doc comments to all public types and members in
  PSProxmoxVE.Core (Authentication, Exceptions, Models, Client)
- Remove CS1591 suppression from PSProxmoxVE.Core.csproj
- Build succeeds with 0 warnings

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-20 12:21:54 -05:00
Clint Branham b8af36a7fc fix(build): add netstandard2.0 target to eliminate System.Runtime version mismatch
The net9.0 DLL references System.Runtime 9.0.0.0 which is unavailable
on GitHub runners where pwsh ships with .NET 8.x. Adding netstandard2.0
as a target produces a DLL compatible with any .NET version (PS 5.1 on
.NET Framework through PS 7.x on .NET 8/9).

- Add netstandard2.0 to TargetFrameworks in Core and module csproj
- Update #if NET48 conditionals to include NETSTANDARD2_0
- Switch CI workflows from net9.0 to netstandard2.0 for Pester tests
- Update _TestHelper.ps1 framework search order

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-19 09:34:40 -05:00
Clint Branham 0c70e11fe8 fix: resolve all C# compiler warnings (CS8604, CS8618, CS8603, CS1573, xUnit1012)
- CS8604: Use cmdlet's own Node property instead of nullable task.Node
  in WaitForTask calls across all VM cmdlets
- CS8618: Make PveSession.ServerVersion nullable (PveVersion?) since it
  is set post-construction by PveAuthenticator
- CS8603: Add null-forgiving operator where IsNullOrWhiteSpace guards
  guarantee non-null in PveHttpClient
- CS1573: Add XML param documentation to all public service methods
- xUnit1012: Make test parameters nullable (string?) for null test cases

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-18 18:11:49 -05:00
Clint Branham 8ea983c0fc fix: resolve all integration test failures against live PVE
- BaseUrl trailing slash and resource path cleanup: PveSession.BaseUrl now
  ends with '/', all cmdlet resource strings stripped of leading slashes and
  /api2/json/ prefixes so URLs compose correctly
- PveNodeStatus: rename Name -> Node to match JSON field and test expectations
- NewPveVmCmdlet: auto-allocate vmid via GET cluster/nextid when -VmId omitted
- UploadFileAsync (BZ 7389 workarounds):
  * Unquoted multipart boundary in Content-Type header
  * Quoted name= values in Content-Disposition (embedded double-quotes)
  * Content-Disposition set before Content-Type on file part — PVE's parser
    closes the connection if Content-Type appears first
- SendPveIsoCmdlet: run upload in Task.Run, track progress atomically with
  Interlocked, poll and call WriteProgress only from pipeline thread to avoid
  InvalidOperationException from PSCmdlet thread-affinity requirements
- Add debug/Capture-UploadDiff.ps1: mitmproxy capture script used to isolate
  the header-ordering bug by diffing raw multipart bytes from PS vs C# module

Integration test result: 11 passed, 0 failed, 4 skipped (expected — no
template/stopped VMs on bare nested PVE test node).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-18 12:57:21 -05:00
Clint Branham 5b0c81a1a8 fix(auth): use TryAddWithoutValidation for Authorization header
.NET's HttpHeaders.Add() validates the Authorization header value
against the HTTP spec, rejecting PVE's custom PVEAPIToken scheme
that contains @, !, and = in the token string.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-17 19:14:03 -05:00
Clint Branham 2c7e90d185 feat(client): implement HTTP client with manual multipart ISO upload (bugzilla 7389 workaround)
PveHttpClient handles auth headers, error parsing, and both sync/async
request methods. ISO upload constructs raw multipart body manually to
avoid .NET MultipartFormDataContent sub-headers that Proxmox pveproxy
mishandles. Streams files in 4MB chunks with progress callback support.

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