diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index d7d9f033fb..6fb3b0261e 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -3423,6 +3423,10 @@ merged `Fingerprint`, so a pin still verifies the peer. Regression tests and `TestHandleUpdateNodePreservesDisabledVerifySSLThroughConsolidation` cover the merge and save paths. +The persisted `VerifySSLExplicit` marker is internal node configuration. It is +not added to the auto-register request or response, nor to the node API +response, so existing clients see no payload change. + ### Manual update freshness Server update-check freshness is owned by internal/updates and its API adapter. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index e5fd6fe223..327a84dd83 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -4550,6 +4550,13 @@ replaced by the registration's capture state. New nodes continue to take the captured value. This is a behavioural clarification of the existing endpoint, not a new field or contract delta. +The node add and update payloads keep `verifySSL` as an optional pointer, so an +explicit `false` remains distinguishable from an omitted field. An explicit +choice is recorded as operator-owned and is not overridden by automatic PVE +consolidation, which runs on save, load and monitor reconciliation. The response +shape is unchanged: the persisted explicit marker is internal and is not exposed +through the node API. + ### Manual update freshness GET /api/updates/check accepts an optional boolean force query alongside the diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 512b7b5501..edd92e93d8 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -951,6 +951,20 @@ tokens, and path-normalization variants. ## Current State +### Node TLS verification preference is operator-owned + +A PVE connection's `VerifySSL` value is security-relevant, so the stored +operator choice must not be changed by automatic reconciliation. The persisted +`VerifySSLExplicit` field records that a caller supplied `verifySSL` through the +node add or update API, distinguishing a deliberate opt-out from the zero +value. Automatic PVE consolidation (`ConsolidatePVEInstances`) and canonical +auto-registration never override an explicit choice; the historical promotion +is retained only when neither side recorded one, and a merged certificate +fingerprint still pins the peer. A record written before the field existed +loads as non-explicit and is unchanged until the operator saves a choice. +`TestPVEInstanceVerifySSLExplicitRoundTrips` in +`internal/config/config_load_test.go` pins the persisted default and round-trip. + ### Manual update availability feedback The settings state owner has a dedicated proof route to diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 69a99cd6bf..ee92aea499 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2618,8 +2618,11 @@ preference when the host agent re-registers the node after a disconnect, including when the stored fingerprint was cleared. This prevents a rotating or self-signed certificate from silently re-enabling verification and breaking the monitoring path used for backup and recovery evidence. New nodes still take the -captured registration value. See the agent-lifecycle contract for the runtime -mechanism. +captured registration value. The same preference is preserved when automatic +PVE consolidation folds a duplicate cluster or an overlapping standalone into +the canonical connection on save, load or monitor reconciliation; a merged +certificate fingerprint still pins the peer. See the agent-lifecycle contract +for the runtime mechanism. ### Manual update freshness diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 87f851bca0..d293e613c1 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -1616,6 +1616,32 @@ func TestContract_NodeConfigUpdateTracksOptionalConnectionFieldsAndRedactedSecre } } +// TestContract_NodeConfigUpdateVerifySSLIsOptionalExplicitField pins the node +// config update payload contract the TLS-preference fix relies on: verifySSL is +// an optional pointer, so an explicit false is distinguishable from an omitted +// value. The update handler records an explicit choice from the non-nil pointer +// and must never treat an omitted field as an opt-out (#2140). +func TestContract_NodeConfigUpdateVerifySSLIsOptionalExplicitField(t *testing.T) { + var disabled NodeConfigRequest + if err := json.Unmarshal([]byte(`{"verifySSL":false}`), &disabled); err != nil { + t.Fatalf("decode explicit disable request: %v", err) + } + if disabled.VerifySSL == nil { + t.Fatal("explicit verifySSL:false must decode to a non-nil pointer") + } + if *disabled.VerifySSL { + t.Fatal("explicit verifySSL:false must decode to false") + } + + var omitted NodeConfigRequest + if err := json.Unmarshal([]byte(`{"name":"cluster"}`), &omitted); err != nil { + t.Fatalf("decode request without verifySSL: %v", err) + } + if omitted.VerifySSL != nil { + t.Fatal("omitted verifySSL must stay nil so an existing TLS choice is preserved") + } +} + func TestContract_HostedMagicLinkStablePrincipalProof(t *testing.T) { source, err := os.ReadFile(filepath.Clean("magic_link_handlers.go")) if err != nil { diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 33ccf9f310..f2b8c53cae 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -3,6 +3,7 @@ package config import ( "bytes" "encoding/base64" + "encoding/json" "errors" "os" "path/filepath" @@ -663,3 +664,29 @@ func TestLoadSystemSettingsWithRetry(t *testing.T) { assert.Equal(t, 1, calls) }) } + +// TestPVEInstanceVerifySSLExplicitRoundTrips pins the persisted TLS-choice +// field: an operator's explicit selection must survive serialization so +// automatic consolidation can distinguish it from the zero value, while a +// record written before the field existed loads as the non-explicit default +// (#2140). +func TestPVEInstanceVerifySSLExplicitRoundTrips(t *testing.T) { + original := PVEInstance{ + Name: "homelab", + Host: "https://pve.local:8006", + VerifySSL: false, + VerifySSLExplicit: true, + } + encoded, err := json.Marshal(original) + require.NoError(t, err) + + var decoded PVEInstance + require.NoError(t, json.Unmarshal(encoded, &decoded)) + assert.False(t, decoded.VerifySSL) + assert.True(t, decoded.VerifySSLExplicit, "an explicit TLS choice must survive persistence") + + var legacy PVEInstance + require.NoError(t, json.Unmarshal([]byte(`{"Name":"legacy","Host":"https://pve.local:8006"}`), &legacy)) + assert.False(t, legacy.VerifySSL) + assert.False(t, legacy.VerifySSLExplicit, "a record without the field is not an explicit choice") +}