From cdad648f2071b245979f76fece2b617c44cea42e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 25 Apr 2026 14:14:24 -0500 Subject: [PATCH] fix(image): sanitize HTML error pages in container registry responses When a container registry returns an HTTP error with an HTML body (e.g., CDN 403 page), the raw HTML was dumped into user-facing error messages. Add SanitizeRegistryError() that detects transport.Error with HTML in the response body and replaces it with a clean status code message. Applied at all 4 remote.Image() call sites. --- pkg/devcontainer/buildkit/remote.go | 2 +- pkg/devcontainer/feature/features.go | 2 + pkg/image/error.go | 42 +++++++++++++ pkg/image/error_test.go | 92 ++++++++++++++++++++++++++++ pkg/image/image.go | 4 +- 5 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 pkg/image/error.go create mode 100644 pkg/image/error_test.go diff --git a/pkg/devcontainer/buildkit/remote.go b/pkg/devcontainer/buildkit/remote.go index 8f19380fd..336491b1f 100644 --- a/pkg/devcontainer/buildkit/remote.go +++ b/pkg/devcontainer/buildkit/remote.go @@ -444,7 +444,7 @@ func getImageDetails( remote.WithContext(ctx), ) if err != nil { - return nil, err + return nil, image.SanitizeRegistryError(err) } imageConfig, err := remoteImage.ConfigFile() if err != nil { diff --git a/pkg/devcontainer/feature/features.go b/pkg/devcontainer/feature/features.go index 2edb67846..2668e1314 100644 --- a/pkg/devcontainer/feature/features.go +++ b/pkg/devcontainer/feature/features.go @@ -16,6 +16,7 @@ import ( "github.com/devsy-org/devsy/pkg/extract" "github.com/devsy-org/devsy/pkg/hash" devsyhttp "github.com/devsy-org/devsy/pkg/http" + "github.com/devsy-org/devsy/pkg/image" "github.com/devsy-org/devsy/pkg/log" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" @@ -169,6 +170,7 @@ func processOCIFeature(id string) (string, error) { log.Debugf("fetching OCI image: reference=%s", ref.String()) img, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { + err = image.SanitizeRegistryError(err) log.Errorf("failed to fetch OCI image: error=%v, reference=%s", err, ref.String()) return "", err } diff --git a/pkg/image/error.go b/pkg/image/error.go new file mode 100644 index 000000000..450f6733d --- /dev/null +++ b/pkg/image/error.go @@ -0,0 +1,42 @@ +package image + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/google/go-containerregistry/pkg/v1/remote/transport" +) + +var htmlMarkers = []string{"

403 Forbidden

` + inner := makeTransportErrorWithBody(http.StatusForbidden, htmlBody) + wrapped := fmt.Errorf("retrieve image mcr.microsoft.com/devcontainers/base:ubuntu: %w", inner) + + sanitized := SanitizeRegistryError(wrapped) + if sanitized == wrapped { + t.Fatal("expected error to be sanitized, got original back") + } + want := "unexpected status code 403 Forbidden" + if sanitized.Error() != want { + t.Errorf("got %q, want %q", sanitized.Error(), want) + } +} + +func TestSanitizeRegistryError_HTMLWithSmallTag(t *testing.T) { + htmlBody := `ErrorService Unavailable` + inner := makeTransportErrorWithBody(http.StatusServiceUnavailable, htmlBody) + + sanitized := SanitizeRegistryError(inner) + want := "unexpected status code 503 Service Unavailable" + if sanitized.Error() != want { + t.Errorf("got %q, want %q", sanitized.Error(), want) + } +} + +func TestSanitizeRegistryError_JSONBodyPassesThrough(t *testing.T) { + jsonBody := `{"errors":[{"code":"DENIED","message":"access denied"}]}` + inner := makeTransportErrorWithBody(http.StatusForbidden, jsonBody) + wrapped := fmt.Errorf("retrieve image example.com/foo:latest: %w", inner) + + sanitized := SanitizeRegistryError(wrapped) + if sanitized != wrapped { + t.Error("expected JSON-body error to pass through unchanged") + } +} + +func TestSanitizeRegistryError_PlainTextBodyPassesThrough(t *testing.T) { + inner := makeTransportErrorWithBody(http.StatusInternalServerError, "internal server error") + wrapped := fmt.Errorf("retrieve image example.com/foo:latest: %w", inner) + + sanitized := SanitizeRegistryError(wrapped) + if sanitized != wrapped { + t.Error("expected plain text error to pass through unchanged") + } +} + +func TestSanitizeRegistryError_NonTransportError(t *testing.T) { + err := errors.New("network timeout") + sanitized := SanitizeRegistryError(err) + if sanitized != err { + t.Error("expected non-transport error to pass through unchanged") + } +} + +func TestSanitizeRegistryError_Nil(t *testing.T) { + sanitized := SanitizeRegistryError(nil) + if sanitized != nil { + t.Error("expected nil to pass through as nil") + } +} + +func TestSanitizeRegistryError_EmptyBodyPassesThrough(t *testing.T) { + inner := makeTransportErrorWithBody(http.StatusForbidden, "") + sanitized := SanitizeRegistryError(inner) + if sanitized != inner { + t.Error("expected empty-body error to pass through unchanged") + } +} diff --git a/pkg/image/image.go b/pkg/image/image.go index 9b22076b1..ee51c7ece 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -30,7 +30,7 @@ func GetImage(ctx context.Context, image string) (v1.Image, error) { img, err := remote.Image(ref, remote.WithAuthFromKeychain(keychain)) if err != nil { - return nil, fmt.Errorf("retrieve image %s: %w", image, err) + return nil, fmt.Errorf("retrieve image %s: %w", image, SanitizeRegistryError(err)) } return img, err @@ -54,7 +54,7 @@ func GetImageForArch(ctx context.Context, image, arch string) (v1.Image, error) img, err := remote.Image(ref, remoteOptions...) if err != nil { - return nil, fmt.Errorf("retrieve image %s: %w", image, err) + return nil, fmt.Errorf("retrieve image %s: %w", image, SanitizeRegistryError(err)) } return img, err