diff --git a/internal/master/registry/registry.go b/internal/master/registry/registry.go index 69a4dd6..3017a47 100644 --- a/internal/master/registry/registry.go +++ b/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 +} diff --git a/internal/master/registry/registry_test.go b/internal/master/registry/registry_test.go index 888de7a..2f6859f 100644 --- a/internal/master/registry/registry_test.go +++ b/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)) + }) + } +}