Skip to content

fix(orchestrator): sanitize OCI pull errors #3096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dobrac merged 2 commits into from
Jun 26, 2026
Merged

fix(orchestrator): sanitize OCI pull errors #3096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27e1525
fix(orchestrator): sanitize OCI pull errors
dobrac Jun 26, 2026
33a4b50
test(orchestrator): use testify error assertions
dobrac Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions packages/orchestrator/pkg/template/build/core/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,13 @@ func DefaultPlatform() containerregistry.Platform {
}

// wrapImagePullError converts technical Docker registry errors into user-friendly messages.
func wrapImagePullError(err error, imageRef string) error {
func wrapImagePullError(ctx context.Context, err error, imageRef string) error {
if err == nil {
return nil
}

logger.L().Warn(ctx, "failed to pull image", zap.String("image_ref", imageRef), zap.Error(err))

// Check for transport errors with specific error codes from the registry API
var transportErr *transport.Error
if errors.As(err, &transportErr) {
Expand All @@ -87,9 +89,13 @@ func wrapImagePullError(err error, imageRef string) error {
return fmt.Errorf("access denied to '%s': you don't have permission to pull this image", imageRef)
}
}

if transportErr.StatusCode != 0 {
return fmt.Errorf("failed to pull image '%s': registry returned status code %d", imageRef, transportErr.StatusCode)
}
}

return fmt.Errorf("failed to pull image '%s': %w", imageRef, err)
return fmt.Errorf("failed to pull image '%s': unable to retrieve image from registry", imageRef)
}

func GetPublicImage(ctx context.Context, dockerhubRepository dockerhub.RemoteRepository, tag string, authProvider auth.RegistryAuthProvider) (containerregistry.Image, error) {
Expand All @@ -108,12 +114,12 @@ func GetPublicImage(ctx context.Context, dockerhubRepository dockerhub.RemoteRep
if authProvider == nil && ref.Context().RegistryStr() == name.DefaultRegistry {
img, err := dockerhubRepository.GetImage(ctx, tag, platform)
if err != nil {
return nil, wrapImagePullError(err, tag)
return nil, wrapImagePullError(ctx, err, tag)
}

telemetry.ReportEvent(ctx, "pulled public image through docker remote repository proxy")

err = verifyImagePlatform(img, platform)
err = verifyImagePlatform(ctx, img, platform, tag)
if err != nil {
return nil, err
}
Expand All @@ -137,12 +143,12 @@ func GetPublicImage(ctx context.Context, dockerhubRepository dockerhub.RemoteRep

img, err := remote.Image(ref, opts...)
if err != nil {
return nil, wrapImagePullError(err, tag)
return nil, wrapImagePullError(ctx, err, tag)
}

telemetry.ReportEvent(ctx, "pulled public image")

err = verifyImagePlatform(img, platform)
err = verifyImagePlatform(ctx, img, platform, tag)
if err != nil {
return nil, err
}
Expand All @@ -158,12 +164,14 @@ func GetImage(ctx context.Context, artifactRegistry artifactsregistry.ArtifactsR

img, err := artifactRegistry.GetImage(childCtx, templateId, buildId, platform)
if err != nil {
return nil, fmt.Errorf("error pulling image: %w", err)
logger.L().Warn(childCtx, "failed to pull build image", logger.WithTemplateID(templateId), logger.WithBuildID(buildId), zap.Error(err))

return nil, errors.New("failed to pull build image from registry")
}

telemetry.ReportEvent(childCtx, "pulled image")

err = verifyImagePlatform(img, platform)
err = verifyImagePlatform(childCtx, img, platform, fmt.Sprintf("%s/%s", templateId, buildId))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -475,10 +483,12 @@ func getDirSize(ctx context.Context, dir string) (int64, error) {
return size, nil
}

func verifyImagePlatform(img containerregistry.Image, platform containerregistry.Platform) error {
func verifyImagePlatform(ctx context.Context, img containerregistry.Image, platform containerregistry.Platform, imageRef string) error {
config, err := img.ConfigFile()
Comment thread
dobrac marked this conversation as resolved.
if err != nil {
return fmt.Errorf("error getting image config file: %w", err)
logger.L().Warn(ctx, "failed to get image config file", zap.String("image_ref", imageRef), zap.Error(err))

return fmt.Errorf("failed to inspect image '%s': unable to retrieve image metadata from registry", imageRef)
}
if config.Architecture != platform.Architecture {
return fmt.Errorf("image architecture %q does not match expected %q", config.Architecture, platform.Architecture)
Expand Down
45 changes: 45 additions & 0 deletions packages/orchestrator/pkg/template/build/core/oci/oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/google/go-containerregistry/pkg/v1/tarball"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -364,3 +365,47 @@ func TestGetPublicImageWithGeneralAuth(t *testing.T) {
assert.Len(t, layers, 1)
})
}

func TestWrapImagePullErrorSanitizesOriginalError(t *testing.T) {
t.Parallel()
ctx := t.Context()
imageRef := "registry.example.com/test/image:latest"

remoteErr := &transport.Error{
StatusCode: http.StatusTeapot,
Errors: []transport.Diagnostic{
{
Code: transport.UnknownErrorCode,
Message: "registry-controlled message",
Detail: "registry-controlled detail",
},
},
}

err := wrapImagePullError(ctx, remoteErr, imageRef)
require.EqualError(t, err, "failed to pull image 'registry.example.com/test/image:latest': registry returned status code 418")
assert.NotContains(t, err.Error(), "registry-controlled message")
assert.NotContains(t, err.Error(), "registry-controlled detail")
assert.NotErrorIs(t, err, remoteErr)
}

func TestWrapImagePullErrorUsesPredefinedRegistryCodeMessage(t *testing.T) {
t.Parallel()
ctx := t.Context()
imageRef := "registry.example.com/test/image:latest"

remoteErr := &transport.Error{
StatusCode: http.StatusUnauthorized,
Errors: []transport.Diagnostic{
{
Code: transport.UnauthorizedErrorCode,
Message: "registry-controlled message",
},
},
}

err := wrapImagePullError(ctx, remoteErr, imageRef)
require.EqualError(t, err, "access denied to 'registry.example.com/test/image:latest': authentication required or insufficient permissions")
assert.NotContains(t, err.Error(), "registry-controlled message")
assert.NotErrorIs(t, err, remoteErr)
}
Loading