internal/master/registry/registry.go

@@ -4,11 +4,16 @@ package registry
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"net/http"
+	"regexp"
+	"strings"
 
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 
 	"github.com/shcizo/package-updater/internal/imageref"
 )
@@ -49,7 +54,33 @@ func (c *Client) Digest(ctx context.Context, ref string) (string, error) {
 		remote.WithAuthFromKeychain(c.keychain),
 		remote.WithUserAgent(c.userAgent))
 	if err != nil {
-		return "", fmt.Errorf("head %s: %w", r, err)
+		var terr *transport.Error
+		if errors.As(err, &terr) {
+			return "", fmt.Errorf("lookup %s: %s", r, describe(terr))
+		}
+		return "", fmt.Errorf("lookup %s: %w", r, err)
 	}
 	return desc.Digest.String(), nil
 }
+
+// errorCodePattern matches the registry error codes of the distribution
+// spec ("MANIFEST_UNKNOWN"). Anything else is dropped from error text.
+var errorCodePattern = regexp.MustCompile(`^[A-Z_]{1,40}$`)
+
+// describe returns the HTTP status and the structured error codes of a
+// registry error, without the messages, details or raw body that ggcr copies
+// into its error text. The registry host comes from an image name, so
+// whatever it answers must not end up in the UI.
+func describe(e *transport.Error) string {
+	msg := fmt.Sprintf("registry answered %d %s", e.StatusCode, http.StatusText(e.StatusCode))
+	var codes []string
+	for _, d := range e.Errors {
+		if errorCodePattern.MatchString(string(d.Code)) {
+			codes = append(codes, string(d.Code))
+		}
+	}
+	if len(codes) > 0 {
+		msg += " (" + strings.Join(codes, ", ") + ")"
+	}
+	return msg
+}

internal/master/registry/registry_test.go

@@ -249,3 +249,36 @@ func TestMissingDigestHeaderDoesNotFallBackToDownload(t *testing.T) {
 	assert.Error(t, err)
 	assert.Equal(t, []string{http.MethodHead}, methods)
 }
+
+// ggcr copies up to 64 KiB of the registry's response into its errors. The
+// registry host comes from an image name, so that text must not reach the
+// stored poll error.
+func TestRegistryErrorBodyDoesNotLeak(t *testing.T) {
+	dockerConfig(t, "")
+	tests := []struct {
+		name string
+		body string
+		want string
+	}{
+		{"raw body", "internal-secret-body", "registry answered 400 Bad Request"},
+		{"structured errors", `{"errors":[{"code":"MANIFEST_UNKNOWN","message":"internal-secret-body","detail":"internal-secret-body"},{"code":"internal-secret-body"}]}`,
+			"registry answered 400 Bad Request (MANIFEST_UNKNOWN)"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				// HEAD answers carry no body, so the leak is in the GET /v2/ ping.
+				w.WriteHeader(http.StatusBadRequest)
+				_, _ = w.Write([]byte(tt.body))
+			}))
+			t.Cleanup(srv.Close)
+			host := strings.TrimPrefix(srv.URL, "http://")
+
+			_, err := New("test").Digest(context.Background(), host+"/a/b:1")
+
+			require.Error(t, err)
+			assert.NotContains(t, err.Error(), "internal-secret-body")
+			assert.Equal(t, "lookup "+host+"/a/b:1: "+tt.want, strings.Replace(err.Error(), "index.docker.io/", "", 1))
+		})
+	}
+}