remotes: surface OCI error body on HEAD 403 via GET fallback

When a registry returns 403 Forbidden on a HEAD request (e.g., manifest
resolve or push existence check), the diagnostic error body is lost
because HEAD responses carry no body per HTTP spec. This leaves users
with an opaque "403 Forbidden" message and no actionable guidance.

Add a follow-up GET on HEAD 403 to retrieve the registry's OCI error
body. The fallback lives in a shared withGETErrorBody helper used by both
the pusher and resolver: it only enriches when the GET also returns 403,
and preserves the original HEAD request's method and status while
borrowing just the body, so the resulting error's status and body stay
consistent.

Scoped to 403 only because it is rare (CMK key disabled, IP firewall,
RBAC misconfiguration) and its body is highly diagnostic, while other
status codes either already use GET or have bodies that add no value.

Backport of #13547 to release/2.0, hand-adapted for this branch: it
predates the shared unexpectedResponseErr helper, so withGETErrorBody
decodes the registry error body inline (mirroring that helper's
decode-and-join) and uses the older doWithRetries signature.

Fixes #8969

Signed-off-by: Andrew Au <cshung@gmail.com>
(cherry picked from commit 5c66703ee3)
This commit is contained in:
cshung
2026-07-08 20:41:52 +00:00
parent 080e7eabe1
commit 71a73c8a0f
4 changed files with 350 additions and 1 deletions

View File

@@ -18,6 +18,7 @@ package docker
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
@@ -150,6 +151,18 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
} else if resp.StatusCode != http.StatusNotFound {
err := remoteserrors.NewUnexpectedStatusErr(resp)
log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
// 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, nil)
})
}
resp.Body.Close()
return nil, err
}
@@ -550,6 +563,52 @@ 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
retErr := remoteserrors.NewUnexpectedStatusErr(&enriched)
// Decode the registry error body inline and join it to the status error:
// the joined message surfaces the diagnostic details while the body stays
// out of the status error's own message.
if rerr, ok := retErr.(remoteserrors.ErrUnexpectedStatus); ok && len(rerr.Body) > 0 {
var registryErr Errors
if jerr := json.Unmarshal(rerr.Body, &registryErr); jerr == nil && registryErr.Len() > 0 {
retErr = errors.Join(rerr, registryErr)
}
}
return retErr
}
func requestWithMountFrom(req *request, mount, from string) *request {
creq := *req

View File

@@ -708,3 +708,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")
}

View File

@@ -340,7 +340,23 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
}
if resp.StatusCode > 399 {
if firstErrPriority < 3 {
firstErr = remoteerrors.NewUnexpectedStatusErr(resp)
err := remoteerrors.NewUnexpectedStatusErr(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 {
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, nil)
})
}
firstErr = err
firstErrPriority = 3
}
log.G(ctx).Infof("%s after status: %s", nextHostOrFail(i), resp.Status)

View File

@@ -1225,3 +1225,202 @@ func (srv *refreshTokenServer) BasicTestFunc() func(h http.Handler) (string, Res
return base, options, close
}
}
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())
}
}