diff --git a/core/remotes/docker/pusher.go b/core/remotes/docker/pusher.go index 900389a0fb143..94ef661d90d85 100644 --- a/core/remotes/docker/pusher.go +++ b/core/remotes/docker/pusher.go @@ -158,6 +158,18 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str } } else if resp.StatusCode != http.StatusNotFound { err := unexpectedResponseErr(resp) + // A HEAD 403 carries no body, so issue a follow-up GET to the + // same URL to surface the registry's error details for diagnostics. + if resp.StatusCode == http.StatusForbidden && req.method == http.MethodHead { + err = withGETErrorBody(ctx, err, resp, func() (*http.Response, error) { + getReq := p.request(host, http.MethodGet, existCheck...) + getReq.header.Set("Accept", strings.Join([]string{desc.MediaType, `*/*`}, ", ")) + if addErr := getReq.addNamespace(p.refspec.Hostname()); addErr != nil { + return nil, addErr + } + return getReq.doWithRetries(ctx, false) + }) + } log.G(ctx).WithError(err).Debug("unexpected response") resp.Body.Close() return nil, err @@ -574,6 +586,41 @@ func (pw *pushWriter) Truncate(size int64) error { return errors.New("cannot truncate remote upload") } +// withGETErrorBody enriches originalErr, produced from a bodyless HEAD +// response, with the error body from a follow-up GET to the same URL. HEAD +// responses carry no body, so a 403 only surfaces its status code; a GET +// returns the registry's error details (e.g. "key vault access denied", IP +// restrictions) that explain the failure. +// +// The GET body is only used when the GET also returns 403, so the enriched +// error's status and body stay consistent. In that case the original HEAD +// request's method and status are preserved and only the body is borrowed from +// the GET; any other outcome (GET failed, or returned a different status) +// leaves originalErr untouched. +func withGETErrorBody(ctx context.Context, originalErr error, headResp *http.Response, doGET func() (*http.Response, error)) error { + getResp, err := doGET() + if err != nil { + log.G(ctx).WithError(err).Debug("failed to retrieve error body with GET fallback") + return originalErr + } + defer getResp.Body.Close() + + if getResp.StatusCode != http.StatusForbidden { + log.G(ctx).WithFields(log.Fields{ + "head_status": headResp.Status, + "get_status": getResp.Status, + }).Debug("ignoring GET fallback response with different status") + return originalErr + } + + // Preserve the original HEAD request's method and status and borrow only + // the body from the GET, so the error still reflects the request the caller + // actually made. + enriched := *headResp + enriched.Body = getResp.Body + return unexpectedResponseErr(&enriched) +} + func requestWithMountFrom(req *request, mount, from string) *request { creq := *req diff --git a/core/remotes/docker/pusher_test.go b/core/remotes/docker/pusher_test.go index 9a68f93c31894..2e2a8e807099c 100644 --- a/core/remotes/docker/pusher_test.go +++ b/core/remotes/docker/pusher_test.go @@ -739,3 +739,78 @@ func Test_dockerPusher_push(t *testing.T) { }) } } + +func TestPusherForbiddenGETFallbackForErrorBody(t *testing.T) { + // When the existence-check HEAD returns 403, the pusher should issue + // a follow-up GET to retrieve the registry's error body for diagnostics. + const errorMessage = "encryption key is disabled" + + p, reg, _, done := samplePusher(t) + defer done() + + reg.defaultHandlerFunc = func(w http.ResponseWriter, r *http.Request) bool { + if strings.Contains(r.URL.Path, "/manifests/") || strings.Contains(r.URL.Path, "/blobs/sha256:") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + if r.Method == http.MethodGet { + w.Write([]byte(fmt.Sprintf(`{"errors":[{"code":"DENIED","message":"%s"}]}`, errorMessage))) + } + // HEAD: no body + return true + } + return false + } + + ctx := context.Background() + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromString("test-manifest"), + Size: 100, + } + + _, err := p.push(ctx, desc, "test-ref", true) + require.Error(t, err) + assert.Contains(t, err.Error(), errorMessage, + "expected registry error body from GET fallback") + assert.Contains(t, err.Error(), "403") +} + +func TestPusherForbiddenGETFallbackProxy(t *testing.T) { + // When the pusher is configured as a proxy (Host != image hostname), + // the GET fallback must include the ?ns= query parameter so the proxy + // can route the request to the correct upstream registry. + const errorMessage = "encryption key is disabled" + + var gotNsParam string + + p, reg, _, done := samplePusher(t) + defer done() + + reg.defaultHandlerFunc = func(w http.ResponseWriter, r *http.Request) bool { + if strings.Contains(r.URL.Path, "/manifests/") || strings.Contains(r.URL.Path, "/blobs/sha256:") { + if r.Method == http.MethodGet { + gotNsParam = r.URL.Query().Get("ns") + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + if r.Method == http.MethodGet { + w.Write([]byte(fmt.Sprintf(`{"errors":[{"code":"DENIED","message":"%s"}]}`, errorMessage))) + } + return true + } + return false + } + + ctx := context.Background() + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromString("test-manifest"), + Size: 100, + } + + _, err := p.push(ctx, desc, "test-ref", true) + require.Error(t, err) + assert.Contains(t, err.Error(), errorMessage) + assert.Equal(t, samplePusherHostname, gotNsParam, + "GET fallback on proxy must include ?ns= query parameter") +} diff --git a/core/remotes/docker/resolver.go b/core/remotes/docker/resolver.go index dd89921562a87..45ce4dd59ddf9 100644 --- a/core/remotes/docker/resolver.go +++ b/core/remotes/docker/resolver.go @@ -341,8 +341,25 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp continue } if resp.StatusCode > 399 { + err := unexpectedResponseErr(resp) + // A HEAD 403 carries no body, so issue a follow-up GET to + // the same URL to surface the registry's error details + // (e.g. "key vault access denied", IP restrictions) for + // diagnostics. + if resp.StatusCode == http.StatusForbidden && req.method == http.MethodHead { + err = withGETErrorBody(ctx, err, resp, func() (*http.Response, error) { + getReq := base.request(host, http.MethodGet, u...) + if addErr := getReq.addNamespace(base.refspec.Hostname()); addErr != nil { + return nil, addErr + } + for key, value := range r.resolveHeader { + getReq.header[key] = append(getReq.header[key], value...) + } + return getReq.doWithRetries(ctx, false) + }) + } if firstErrPriority < 3 { - firstErr = unexpectedResponseErr(resp) + firstErr = err firstErrPriority = 3 } log.G(ctx).Infof("%s after status: %s", nextHostOrFail(i), resp.Status) diff --git a/core/remotes/docker/resolver_test.go b/core/remotes/docker/resolver_test.go index 1bd65a9eb3737..c691949f7283c 100644 --- a/core/remotes/docker/resolver_test.go +++ b/core/remotes/docker/resolver_test.go @@ -1602,3 +1602,202 @@ func TestResolverErrorStatusCodeOnFetch(t *testing.T) { }) } } + +func TestResolveForbiddenGETFallbackForErrorBody(t *testing.T) { + // When HEAD returns 403 with no body, the resolver should issue a + // follow-up GET to retrieve the registry's error details. + const ( + name = "test/repo" + tag = "latest" + errorBody = `{"errors":[{"code":"DENIED","message":"encryption key is disabled"}]}` + ) + + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/token") { + rw.Header().Set("Content-Type", "application/json") + rw.Write([]byte(`{"access_token":"test"}`)) + return + } + if strings.Contains(r.URL.Path, "/manifests/") { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusForbidden) + if r.Method == http.MethodGet { + rw.Write([]byte(errorBody)) + } + return + } + if r.URL.Path == "/v2/" { + rw.WriteHeader(http.StatusOK) + return + } + rw.WriteHeader(http.StatusNotFound) + }) + + base, options, close := tlsServer(handler) + defer close() + + options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client)) + resolver := NewResolver(options) + ref := fmt.Sprintf("%s/%s:%s", base, name, tag) + + _, _, err := resolver.Resolve(context.Background(), ref) + if err == nil { + t.Fatal("expected error from resolve, got nil") + } + + errMsg := err.Error() + if !strings.Contains(errMsg, "encryption key is disabled") { + t.Errorf("expected error to contain registry error body, got: %s", errMsg) + } + if !strings.Contains(errMsg, "403") { + t.Errorf("expected error to contain status code 403, got: %s", errMsg) + } +} + +func TestResolveForbiddenNoFallbackOnGET(t *testing.T) { + const ( + name = "test/repo" + tag = "latest" + ) + + var getCount int + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/token") { + rw.Header().Set("Content-Type", "application/json") + rw.Write([]byte(`{"access_token":"test"}`)) + return + } + if strings.Contains(r.URL.Path, "/manifests/") { + if r.Method == http.MethodGet { + getCount++ + } + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusForbidden) + rw.Write([]byte(`{"errors":[{"code":"DENIED","message":"forbidden"}]}`)) + return + } + if r.URL.Path == "/v2/" { + rw.WriteHeader(http.StatusOK) + return + } + rw.WriteHeader(http.StatusNotFound) + }) + + base, options, close := tlsServer(handler) + defer close() + + options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client)) + resolver := NewResolver(options) + ref := fmt.Sprintf("%s/%s:%s", base, name, tag) + + _, _, err := resolver.Resolve(context.Background(), ref) + if err == nil { + t.Fatal("expected error from resolve, got nil") + } + + // The resolver should issue exactly 1 GET (the fallback from HEAD 403). + // It must NOT issue a second fallback GET when the first GET also returns 403. + if getCount != 1 { + t.Errorf("expected exactly 1 GET fallback request for manifests, got %d", getCount) + } +} + +func TestResolve404NoFallbackGET(t *testing.T) { + const ( + name = "test/repo" + tag = "latest" + ) + + var getCount int + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/token") { + rw.Header().Set("Content-Type", "application/json") + rw.Write([]byte(`{"access_token":"test"}`)) + return + } + if strings.Contains(r.URL.Path, "/manifests/") { + if r.Method == http.MethodGet { + getCount++ + } + rw.WriteHeader(http.StatusNotFound) + return + } + if r.URL.Path == "/v2/" { + rw.WriteHeader(http.StatusOK) + return + } + rw.WriteHeader(http.StatusNotFound) + }) + + base, options, close := tlsServer(handler) + defer close() + + options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client)) + resolver := NewResolver(options) + ref := fmt.Sprintf("%s/%s:%s", base, name, tag) + + _, _, err := resolver.Resolve(context.Background(), ref) + if err == nil { + t.Fatal("expected error from resolve, got nil") + } + + if getCount != 0 { + t.Errorf("expected 0 GET requests for 404, got %d", getCount) + } +} + +func TestResolveForbiddenGETFallbackNetworkError(t *testing.T) { + const ( + name = "test/repo" + tag = "latest" + ) + + var requestCount int + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/token") { + rw.Header().Set("Content-Type", "application/json") + rw.Write([]byte(`{"access_token":"test"}`)) + return + } + if strings.Contains(r.URL.Path, "/manifests/") { + requestCount++ + if r.Method == http.MethodHead { + rw.WriteHeader(http.StatusForbidden) + return + } + hj, ok := rw.(http.Hijacker) + if !ok { + rw.WriteHeader(http.StatusForbidden) + return + } + conn, _, err := hj.Hijack() + if err != nil { + rw.WriteHeader(http.StatusInternalServerError) + return + } + conn.Close() + return + } + if r.URL.Path == "/v2/" { + rw.WriteHeader(http.StatusOK) + return + } + rw.WriteHeader(http.StatusNotFound) + }) + + base, options, close := tlsServer(handler) + defer close() + + options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client)) + resolver := NewResolver(options) + ref := fmt.Sprintf("%s/%s:%s", base, name, tag) + + _, _, err := resolver.Resolve(context.Background(), ref) + if err == nil { + t.Fatal("expected error from resolve, got nil") + } + + if !strings.Contains(err.Error(), "403") { + t.Errorf("expected 403 in error, got: %s", err.Error()) + } +}