Merge pull request #2049 from rcourtman/maintainer/20260911T093431Z

Stop registry error pages from appearing as Docker image updates

internal/api/resourceapi/resources_registry_propagation_test.go

@@ -0,0 +1,66 @@
+package resourceapi
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/config"
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+	unified "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
+)
+
+// This is the final leg of collector -> ApplyDockerReport -> /api/resources.
+func TestResourcesRegistryResponseQualification(t *testing.T) {
+	path := os.Getenv("PULSE_REGISTRY_SNAPSHOT_PROOF")
+	if path == "" {
+		t.Skip("set PULSE_REGISTRY_SNAPSHOT_PROOF to ingestion qualification output")
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var snapshots []models.StateSnapshot
+	if err := json.Unmarshal(data, &snapshots); err != nil {
+		t.Fatal(err)
+	}
+	if len(snapshots) != 2 {
+		t.Fatal("expected failure and recovery snapshots")
+	}
+	for phase, snapshot := range snapshots {
+		registry := unified.NewRegistry(nil)
+		registry.IngestSnapshot(snapshot)
+		h := NewQueryService(&config.Config{DataPath: t.TempDir()})
+		h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: snapshot, resources: registry.ListByType(unified.ResourceTypeAppContainer)})
+		rec := httptest.NewRecorder()
+		h.HandleListResources(rec, httptest.NewRequest(http.MethodGet, "/api/resources?type=app-container&limit=100", nil))
+		if rec.Code != http.StatusOK {
+			t.Fatalf("HTTP %d: %s", rec.Code, rec.Body.String())
+		}
+		var response ResourcesResponse
+		if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil {
+			t.Fatal(err)
+		}
+		if len(response.Data) != 4 {
+			t.Fatalf("expected four resources: %s", rec.Body.String())
+		}
+		for _, r := range response.Data {
+			var want *models.DockerContainerUpdateStatus
+			for _, c := range snapshot.DockerHosts[0].Containers {
+				if c.Name == r.Name {
+					want = c.UpdateStatus
+				}
+			}
+			if want == nil || r.Docker == nil || r.Docker.UpdateStatus == nil {
+				t.Fatalf("lost identity/status: %+v", r)
+			}
+			got := r.Docker.UpdateStatus
+			if got.CurrentDigest != want.CurrentDigest || got.LatestDigest != want.LatestDigest || got.Error != want.Error || got.UpdateAvailable != want.UpdateAvailable {
+				t.Fatalf("projection changed status: %+v vs %+v", got, want)
+			}
+		}
+		t.Logf("phase %d endpoint=%s", phase, rec.Body.String())
+	}
+}

internal/dockeragent/collect_registry_response_test.go

@@ -0,0 +1,114 @@
+package dockeragent
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	containertypes "github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/api/types/image"
+	agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker"
+	"github.com/rs/zerolog"
+)
+
+// Optional output feeds the ingest and API qualification probes without
+// substituting hand-written update statuses for collector output.
+func TestCollectRegistryResponseIsolation(t *testing.T) {
+	refs := []string{"redis:8-alpine", "postgres:16-alpine", "ghcr.io/searxng/searxng:latest", "ghcr.io/paperless-ngx/paperless-ngx:latest"}
+	var reports []agentsdocker.Report
+	for _, phase := range []string{"invalid", "valid"} {
+		checker := NewRegistryChecker(zerolog.Nop())
+		calls := map[string]int{}
+		checker.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
+			if req.URL.Host == "auth.docker.io" || strings.HasSuffix(req.URL.Path, "/token") {
+				return newStringResponse(200, nil, `{"token":"synthetic"}`), nil
+			}
+			key := req.URL.Host + req.URL.Path
+			calls[key]++
+			var i int
+			for i = 0; i < len(refs); i++ {
+				reg, repo, tag := parseImageReference(refs[i])
+				if key == reg+"/v2/"+repo+"/manifests/"+tag {
+					break
+				}
+			}
+			if i == len(refs) {
+				return nil, fmt.Errorf("unexpected reference %s", key)
+			}
+			if phase == "invalid" && i < 3 {
+				if req.Method == http.MethodHead {
+					return newStringResponse(200, nil, ""), nil
+				}
+				return newStringResponse(200, nil, `<html>registry unavailable</html>`), nil
+			}
+			if req.Method == http.MethodHead {
+				return newStringResponse(200, map[string]string{"Content-Type": "application/vnd.oci.image.index.v1+json", "Docker-Content-Digest": fmt.Sprintf("sha256:index-%d", i)}, ""), nil
+			}
+			return newStringResponse(200, nil, fmt.Sprintf(`{"schemaVersion":2,"manifests":[{"digest":"sha256:platform-%d","platform":{"architecture":"amd64","os":"linux"}},{"digest":"sha256:arm-%d","platform":{"architecture":"arm64","os":"linux"}}]}`, i, i)), nil
+		})}
+		report := agentsdocker.Report{Timestamp: time.Now().UTC(), Agent: agentsdocker.AgentInfo{ID: "registry-proof", IntervalSeconds: 30}, Host: agentsdocker.HostInfo{Hostname: "registry-proof"}}
+		for i, ref := range refs {
+			inspect := baseInspect()
+			inspect.Config.Image = ref
+			reg, repo, _ := parseImageReference(ref)
+			current := fmt.Sprintf("sha256:index-%d", i)
+			if i == 2 {
+				current = fmt.Sprintf("sha256:platform-%d", i)
+			}
+			a := &Agent{logger: zerolog.Nop(), runtime: RuntimeDocker, prevContainerCPU: make(map[string]cpuSample), registryChecker: checker, docker: &fakeDockerClient{
+				containerInspectWithRawFn: func(context.Context, string, bool) (containertypes.InspectResponse, []byte, error) {
+					return inspect, nil, nil
+				},
+				containerStatsOneShotFn: func(context.Context, string) (dockerStatsResponseReader, error) {
+					return statsReader(t, containertypes.StatsResponse{}), nil
+				},
+				imageInspectWithRawFn: func(context.Context, string) (image.InspectResponse, []byte, error) {
+					return image.InspectResponse{RepoDigests: []string{reg + "/" + repo + "@" + current}, Architecture: "amd64", Os: "linux"}, nil, nil
+				},
+			}}
+			for attempt := 0; attempt < 2; attempt++ {
+				got, err := a.collectContainer(context.Background(), containertypes.Summary{ID: fmt.Sprintf("%064x", i+1), Names: []string{fmt.Sprintf("/container-%d", i)}, Image: ref, ImageID: fmt.Sprintf("image-%d", i), State: "running"})
+				if err != nil {
+					t.Fatal(err)
+				}
+				s := got.UpdateStatus
+				if s == nil {
+					t.Fatal("missing status")
+				}
+				if s.CurrentDigest != current || s.UpdateAvailable {
+					t.Errorf("%s %s attempt %d: %+v", phase, ref, attempt, s)
+				}
+				if phase == "invalid" && i < 3 {
+					if s.Error == "" || s.LatestDigest != "" {
+						t.Errorf("non-manifest became update: %+v", s)
+					}
+				} else if s.Error != "" || s.LatestDigest != fmt.Sprintf("sha256:index-%d", i) {
+					t.Errorf("reference/platform mismatch: %+v", s)
+				}
+				if attempt == 1 {
+					report.Containers = append(report.Containers, got)
+				}
+			}
+		}
+		for key, n := range calls {
+			if n != 2 {
+				t.Errorf("%s requests=%d, expected one HEAD/GET with cache reuse", key, n)
+			}
+		}
+		reports = append(reports, report)
+	}
+	data, err := json.MarshalIndent(reports, "", "  ")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if path := os.Getenv("PULSE_REGISTRY_REPORT_PROOF"); path != "" {
+		if err := os.WriteFile(path, data, 0600); err != nil {
+			t.Fatal(err)
+		}
+	}
+}

internal/dockeragent/registry.go

@@ -433,6 +433,24 @@ func (r *RegistryChecker) fetchManifest(ctx context.Context, manifestURL, author
 	if err != nil {
 		return "", "", nil, fmt.Errorf("read manifest body: %w", err)
 	}
+	if len(body) == 0 {
+		return "", "", nil, fmt.Errorf("no digest in response")
+	}
+
+	// Only hash or trust digest metadata for an actual image manifest. A
+	// proxy/login/error page with HTTP 200 otherwise becomes a plausible but
+	// unrelated sha256 value, shared by every reference receiving that page.
+	var manifest struct {
+		SchemaVersion int `json:"schemaVersion"`
+		Config        struct {
+			Digest string `json:"digest"`
+		} `json:"config"`
+		Manifests []json.RawMessage `json:"manifests"`
+	}
+	if err := json.Unmarshal(body, &manifest); err != nil || manifest.SchemaVersion != 2 ||
+		(manifest.Config.Digest == "" && manifest.Manifests == nil) {
+		return "", "", nil, fmt.Errorf("registry returned a non-manifest response")
+	}
 	digest := strings.Trim(resp.Header.Get("Docker-Content-Digest"), `"`)
 	if digest == "" {
 		digest = strings.Trim(resp.Header.Get("Etag"), `"`)

internal/dockeragent/registry_coverage_test.go

@@ -382,7 +382,7 @@ func TestRegistryChecker_FetchDigest_DigestHeaders(t *testing.T) {
 	})
 
 	t.Run("GET fallback resolves manifest list", func(t *testing.T) {
-		manifestBody := `{"manifests":[{"digest":"sha256:amd64","platform":{"architecture":"amd64","os":"linux"}}]}`
+		manifestBody := `{"schemaVersion":2,"manifests":[{"digest":"sha256:amd64","platform":{"architecture":"amd64","os":"linux"}}]}`
 		checker := &RegistryChecker{
 			httpClient: &http.Client{
 				Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
@@ -407,7 +407,7 @@ func TestRegistryChecker_FetchDigest_DigestHeaders(t *testing.T) {
 	})
 
 	t.Run("GET fallback preserves index digest when HEAD identifies a manifest list", func(t *testing.T) {
-		manifestBody := `{"manifests":[{"digest":"sha256:amd64","platform":{"architecture":"amd64","os":"linux"}}]}`
+		manifestBody := `{"schemaVersion":2,"manifests":[{"digest":"sha256:amd64","platform":{"architecture":"amd64","os":"linux"}}]}`
 		var methods []string
 		checker := &RegistryChecker{
 			httpClient: &http.Client{

internal/dockeragent/registry_manifest_test.go

@@ -15,6 +15,7 @@ func TestRegistryChecker_ResolveManifestList(t *testing.T) {
 	logger := zerolog.Nop()
 	t.Run("resolve manifest list", func(t *testing.T) {
 		manifestListBody := `{
+                    "schemaVersion": 2,
                     "manifests": [
                         {
                             "digest": "sha256:armv7",

internal/dockeragent/registry_response_validation_test.go

@@ -0,0 +1,37 @@
+package dockeragent
+
+import (
+	"context"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/rs/zerolog"
+)
+
+// A successful HTTP status is not proof that a fallback body is a manifest:
+// proxies and registry frontends can return the same HTML/JSON error for unrelated tags.
+func TestRegistryCheckerRejectsNonManifestFallback(t *testing.T) {
+	for _, body := range []string{`<html>registry unavailable</html>`, `{"errors":[{"code":"UNAVAILABLE"}]}`, `{}`, `null`, `{"schemaVersion":2}`} {
+		t.Run(body, func(t *testing.T) {
+			checker := NewRegistryChecker(zerolog.Nop())
+			checker.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
+				if req.URL.Host == "auth.docker.io" || strings.HasSuffix(req.URL.Path, "/token") {
+					return newStringResponse(200, nil, `{"token":"synthetic"}`), nil
+				}
+				if req.Method == http.MethodHead {
+					return newStringResponse(200, nil, ""), nil
+				}
+				return newStringResponse(200, nil, body), nil
+			})}
+			for _, ref := range []string{"redis:8-alpine", "postgres:16-alpine", "ghcr.io/searxng/searxng:latest"} {
+				for attempt := 0; attempt < 2; attempt++ {
+					got := checker.CheckImageUpdate(context.Background(), ref, "sha256:current", "amd64", "linux", "")
+					if got.Error == "" || got.LatestDigest != "" || got.UpdateAvailable {
+						t.Errorf("%s attempt %d accepted non-manifest response: %+v", ref, attempt, got)
+					}
+				}
+			}
+		})
+	}
+}

internal/monitoring/docker_registry_propagation_test.go

@@ -0,0 +1,61 @@
+package monitoring
+
+import (
+	"encoding/json"
+	"os"
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/mock"
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+	agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker"
+)
+
+// Consumes the real collector's synthetic registry response qualification output.
+func TestDockerRegistryReportQualification(t *testing.T) {
+	path := os.Getenv("PULSE_REGISTRY_REPORT_PROOF")
+	if path == "" {
+		t.Skip("set PULSE_REGISTRY_REPORT_PROOF to collector qualification output")
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var reports []agentsdocker.Report
+	if err := json.Unmarshal(data, &reports); err != nil {
+		t.Fatal(err)
+	}
+	if len(reports) != 2 {
+		t.Fatal("expected failure and recovery reports")
+	}
+	previous := mock.IsMockEnabled()
+	mustSetMockEnabled(t, false)
+	t.Cleanup(func() { mustSetMockEnabled(t, previous) })
+	m := newTestMonitor(t)
+	var snapshots []models.StateSnapshot
+	for _, report := range reports {
+		host, err := m.ApplyDockerReport(report, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(host.Containers) != len(report.Containers) {
+			t.Fatal("container inventory changed")
+		}
+		for i, c := range host.Containers {
+			want := report.Containers[i].UpdateStatus
+			got := c.UpdateStatus
+			if got == nil || got.CurrentDigest != want.CurrentDigest || got.LatestDigest != want.LatestDigest || got.Error != want.Error || got.UpdateAvailable != want.UpdateAvailable {
+				t.Fatalf("ingest altered status: %+v vs %+v", got, want)
+			}
+		}
+		snapshots = append(snapshots, models.StateSnapshot{DockerHosts: []models.DockerHost{host}, LastUpdate: report.Timestamp})
+	}
+	if path := os.Getenv("PULSE_REGISTRY_SNAPSHOT_PROOF"); path != "" {
+		data, err := json.MarshalIndent(snapshots, "", "  ")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, data, 0600); err != nil {
+			t.Fatal(err)
+		}
+	}
+}

tests/qualification/registry-response/README.md

@@ -0,0 +1,50 @@
+# Registry fallback response qualification
+
+This synthetic proof covers the Docker agent's registry response fallback,
+container collection, report ingestion and `GET /api/resources`. It does not
+contact a registry, run Docker, update a container or establish an affected
+installation's cause. Registry fixtures use the reported references
+`redis:8-alpine`, `postgres:16-alpine`, `ghcr.io/searxng/searxng:latest` and
+`ghcr.io/paperless-ngx/paperless-ngx:latest` with synthetic digest values.
+
+The HTTP fixture first returns the same headerless HTTP 200 error page for
+three references while the fourth is healthy. Those responses must produce
+errors, no latest digest and no update indication, including cached checks.
+A fresh checker then models agent restart with valid independent indexes;
+matching index **or** platform digests must suppress updates. Exact request
+paths and one HEAD/GET pair per reference establish reference isolation and
+cache reuse. The production collector, not a hand-written status fixture,
+produces the reports consumed by the next two probes.
+
+Run from the repository root, using a fresh private temporary directory:
+
+```sh
+proof=$(mktemp -d)
+export PULSE_REGISTRY_REPORT_PROOF="$proof/reports.json"
+export PULSE_REGISTRY_SNAPSHOT_PROOF="$proof/snapshots.json"
+go test -race ./internal/dockeragent -run 'TestCollectRegistryResponseIsolation|TestRegistryChecker' -count=1
+go test -race ./internal/monitoring -run '^TestDockerRegistryReportQualification$' -count=1
+go test -race ./internal/api/resourceapi -run '^TestResourcesRegistryResponseQualification$' -count=1 -v
+```
+
+The latter two probes skip without explicit input paths; they fail for missing
+or malformed input when configured. Preserve both JSON files and command logs.
+The endpoint probe calls the resource query handler directly, not the HTTP
+authentication middleware. No credential or authorization acceptance is claimed.
+
+`fetchManifest` requires a schema-2 manifest envelope (config digest or index
+manifest array) before using a GET fallback body or its digest metadata. This
+is a minimal error-page discriminator, not full descriptor/cryptographic
+validation. See the [OCI manifest specification](https://github.com/opencontainers/image-spec/blob/147f9c13cedb47a0c4d9a11a222961073d585877/manifest.md)
+and [image index specification](https://github.com/opencontainers/image-spec/blob/147f9c13cedb47a0c4d9a11a222961073d585877/image-index.md),
+both pinned to OCI Image Specification v1.1.1.
+Existing HEAD digest handling, credentials, TLS, cache intervals and HTTP
+error handling remain unchanged. A failed check is not proof of an up-to-date
+image; consumers must retain the error field. No new UI state is introduced.
+
+Before attributing an incident to this mechanism, obtain safe response evidence
+from the affected agent's network path: status, content type, digest headers,
+and locally computed body hash, excluding authorization headers, cookies and
+private response content. A matching synthetic symptom alone does not prove
+that the reporter received the same response. Installed acceptance requires an
+updated agent and verified error/recovery status on the same workload.