From 80a5c50e13dc5de718af74f52b3523c60c3ea1d0 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:23:01 +0100 Subject: [PATCH] fix(runtime): backport truthful Docker update verification Backport 6375d09a8b961086ae7d3676cc777746ad9c356c. Candidate f64fbb8b7 reproduced six false independent confirmations before repair; focused Docker result/executor and receipt reconciliation tests pass afterwards. Preserve stopped updates and execution history. This does not resolve issue #1891 or qualify an installed artifact. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 14 ++++++ .../v6/internal/subsystems/api-contracts.md | 15 ++++++ .../internal/subsystems/storage-recovery.md | 16 +++++++ .../api/docker_container_action_result.go | 17 +++++-- .../docker_container_action_result_test.go | 46 +++++++++++++++++++ 5 files changed, 105 insertions(+), 3 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e98ded7b0..1938dc5f2 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -617,6 +617,20 @@ installer download and the agent's subsequent Pulse TLS connection. 64. `pkg/securityutil/httpurl.go` ## Shared Boundaries +### Container update receipt and independent observation + +The server's independent Docker-update verification must compare the daemon +state/running observation with the replacement state recorded by the agent, +not only its container ID. A matching running replacement needs healthy or +no-healthcheck evidence; intentionally stopped replacements remain stopped. +Missing agent readback cannot become independent confirmation. This server-side +classification does not amend the agent's mutation receipt, trigger another +update, change runner permissions, or reinterpret compensation as execution. +`TestDockerContainerUpdateIndependentObservationMustMatchState` in +`internal/api/docker_container_action_result_test.go` verifies these boundaries; +existing callback-loss reconciliation must continue without redispatch. + + The shared `PBSInstance.NodeMetricsUnavailable` field belongs exclusively to provider polling and alert evaluation. It is retained by in-process state copies diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 4226dd7f9..451ee0800 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -434,6 +434,21 @@ single TypeScript projection rather than recreating lifecycle or evidence enums locally. ## Shared Boundaries +### Independent Docker update readback + +`dockerContainerUpdateExecutionResult` must not promote replacement-ID equality +alone to independent confirmation. The daemon observation must match the +agent readback's state and running flag; running replacements additionally +require state `running` and health `healthy` or `none`. A deliberately stopped +replacement remains confirmable without being started. Missing agent readback +or its state leaves independent verification inconclusive. Contradictory +readback changes verification, not the recorded execution or compensation. +No wire schema or mutation authority changes. Verification: +`TestDockerContainerUpdateIndependentObservationMustMatchState` in +`internal/api/docker_container_action_result_test.go` covers running, stopped, +restarting, unhealthy, unknown health and absent readback cases. + + Commercial migration payloads are a shared API/cloud-paid contract. The license-server client must preserve canonical v6 nested error-envelope codes diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index f7e7aa3dc..166971ad3 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -253,6 +253,22 @@ command-capable profile. 34. `frontend-modern/src/components/Storage/useStoragePoolsTableWindowing.ts` ## Shared Boundaries +### Shared Docker-update verification boundary + +The shared API result converter classifies independent Docker update readback +using replacement identity, state/running agreement and running health, rather +than identity alone. Both immediate execution and durable receipt reconciliation +use this converter. Contradictory observations affect verification only; +missing agent readback is inconclusive. Execution and compensation records +remain unchanged: container backup-rename compensation is not a storage backup, +recovery point, or independently verified restore. No storage selection, +retention or recovery authority is added. The focused +`TestDockerContainerUpdateIndependentObservationMustMatchState` in +`internal/api/docker_container_action_result_test.go` asserts that independent +verification changes never rewrite successful execution history, including +stopped replacements and missing readback. + + The Patrol action broker and shared policy-writer wiring under `internal/api/` remain API/action-lifecycle authority even when the target is storage-related. diff --git a/internal/api/docker_container_action_result.go b/internal/api/docker_container_action_result.go index 00111231e..e8253d456 100644 --- a/internal/api/docker_container_action_result.go +++ b/internal/api/docker_container_action_result.go @@ -145,7 +145,18 @@ func dockerContainerUpdateExecutionResult(resourceID, agentID string, facts agen } status := unified.ActionVerificationContradicted reason := "postcondition_contradicted" - if dockerUpdateFactsMatch(facts, independent.Snapshot.ContainerID) { + // Identity is necessary but not sufficient: a replacement can exit or + // become unhealthy after the agent reported success. Compare against + // its readback rather than requiring running unconditionally, because + // updates deliberately preserve stopped containers. + if !facts.ReadbackRan || facts.After.State == "" { + status = unified.ActionVerificationInconclusive + reason = "agent_readback_unavailable" + } else if dockerUpdateFactsMatch(facts, independent.Snapshot.ContainerID) && + independent.Snapshot.State == facts.After.State && + independent.Snapshot.Running == facts.After.Running && + (!independent.Snapshot.Running || (independent.Snapshot.State == "running" && + agentexec.DockerContainerHealthAllowsVerifiedRunningState(independent.Snapshot.Health))) { status = unified.ActionVerificationConfirmed reason = "" } @@ -254,8 +265,8 @@ func dockerActionEvidenceTimes(observedAt, receivedAt time.Time) (time.Time, tim } // dockerUpdateFactsMatch confirms the observed container is the replacement -// the agent claims to have created. A stopped original is recreated without -// being started, so run-state is not part of the postcondition; identity is. +// the agent claims to have created. Independent verification additionally +// compares its state with the agent readback, preserving stopped updates. func dockerUpdateFactsMatch(facts agentexec.DockerContainerUpdateResultPayload, observedContainerID string) bool { if facts.NewContainerID == "" || observedContainerID == "" { return false diff --git a/internal/api/docker_container_action_result_test.go b/internal/api/docker_container_action_result_test.go index 4df41f167..f2eafee24 100644 --- a/internal/api/docker_container_action_result_test.go +++ b/internal/api/docker_container_action_result_test.go @@ -198,3 +198,49 @@ func dockerResultFacts(now time.Time, started, completed, readback, matches bool } const dockerLifecycleTestID = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + +// A replacement identity alone is not proof that its reported running state +// survived until the independent daemon readback. +func TestDockerContainerUpdateIndependentObservationMustMatchState(t *testing.T) { + now := time.Now().UTC() + for _, tc := range []struct { + name, baseline, state, health string + running bool + want unified.ActionVerificationStatus + }{ + {"running", "running", "running", "healthy", true, unified.ActionVerificationConfirmed}, + {"no healthcheck", "running", "running", "none", true, unified.ActionVerificationConfirmed}, + {"stopped original", "created", "created", "none", false, unified.ActionVerificationConfirmed}, + {"stopped after update", "running", "exited", "", false, unified.ActionVerificationContradicted}, + {"restarting", "running", "restarting", "", true, unified.ActionVerificationContradicted}, + {"unhealthy", "running", "running", "unhealthy", true, unified.ActionVerificationContradicted}, + {"starting healthcheck", "running", "running", "starting", true, unified.ActionVerificationContradicted}, + {"unknown health", "running", "running", "", true, unified.ActionVerificationContradicted}, + {"missing agent readback", "", "running", "healthy", true, unified.ActionVerificationInconclusive}, + } { + t.Run(tc.name, func(t *testing.T) { + facts := agentexec.DockerContainerUpdateResultPayload{ + Operation: agentexec.DockerContainerOperationUpdate, ActionID: "action-update", ExecutionPhase: agentexec.DockerContainerPhaseComplete, + MutationStarted: true, MutationCompleted: true, ReadbackRan: true, NewContainerID: dockerLifecycleTestID, + After: agentexec.DockerContainerLifecycleSnapshot{ContainerID: dockerLifecycleTestID, State: "running", Running: true, ObservedAt: now}, + } + facts.After.State = tc.baseline + facts.After.Running = tc.baseline == "running" + facts.ReadbackRan = tc.baseline != "" + observation := &dockerContainerPostconditionObservation{ + ObserverID: "daemon-1", TrustDomain: "daemon:1", Method: "daemon_inspect", ReceivedAt: now, + Snapshot: agentexec.DockerContainerObservationSnapshot{ContainerID: dockerLifecycleTestID, State: tc.state, Running: tc.running, Health: tc.health, ObservedAt: now}, + } + result, err := dockerContainerUpdateExecutionResult("app-container:fixture", "agent-1", facts, observation, now) + if err != nil { + t.Fatal(err) + } + if got := result.ActionResultV2.Verification; got.Status != tc.want || got.EvidenceClass != unified.ActionEvidenceIndependent { + t.Fatalf("verification = %+v, want %s / independent", got, tc.want) + } + if result.ActionResultV2.Execution.Status != unified.ActionExecutionSucceeded { + t.Fatal("readback changed execution history") + } + }) + } +}