image name validation

Author: sjmiller609

cmd/api/api/images.go

@@ -31,6 +31,11 @@ func (s *ApiService) CreateImage(ctx context.Context, request oapi.CreateImageRe
 	img, err := s.ImageManager.CreateImage(ctx, *request.Body)
 	if err != nil {
 		switch {
+		case images.IsInvalidNameError(err):
+			return oapi.CreateImage400JSONResponse{
+				Code:    "invalid_name",
+				Message: err.Error(),
+			}, nil
 		case errors.Is(err, images.ErrAlreadyExists):
 			return oapi.CreateImage400JSONResponse{
 				Code:    "already_exists",
@@ -53,7 +58,7 @@ func (s *ApiService) GetImage(ctx context.Context, request oapi.GetImageRequestO
 	img, err := s.ImageManager.GetImage(ctx, request.Name)
 	if err != nil {
 		switch {
-		case errors.Is(err, images.ErrNotFound):
+		case images.IsInvalidNameError(err), errors.Is(err, images.ErrNotFound):
 			return oapi.GetImage404JSONResponse{
 				Code:    "not_found",
 				Message: "image not found",
@@ -75,7 +80,7 @@ func (s *ApiService) DeleteImage(ctx context.Context, request oapi.DeleteImageRe
 	err := s.ImageManager.DeleteImage(ctx, request.Name)
 	if err != nil {
 		switch {
-		case errors.Is(err, images.ErrNotFound):
+		case images.IsInvalidNameError(err), errors.Is(err, images.ErrNotFound):
 			return oapi.DeleteImage404JSONResponse{
 				Code:    "not_found",
 				Message: "image not found",

cmd/api/api/images_test.go

@@ -169,6 +169,30 @@ func TestCreateImage_InvalidTag(t *testing.T) {
 	t.Fatal("Build did not fail within timeout")
 }
 
+func TestCreateImage_InvalidName(t *testing.T) {
+	svc := newTestService(t)
+	ctx := ctx()
+
+	invalidNames := []string{
+		"invalid::",
+		"has spaces",
+		"",
+	}
+
+	for _, name := range invalidNames {
+		t.Run(name, func(t *testing.T) {
+			createResp, err := svc.CreateImage(ctx, oapi.CreateImageRequestObject{
+				Body: &oapi.CreateImageRequest{Name: name},
+			})
+			require.NoError(t, err)
+
+			badReq, ok := createResp.(oapi.CreateImage400JSONResponse)
+			require.True(t, ok, "expected 400 bad request for invalid name: %s", name)
+			require.Equal(t, "invalid_name", badReq.Code)
+		})
+	}
+}
+
 func getQueuePos(pos *int) int {
 	if pos == nil {
 		return 0

lib/images/errors.go

@@ -1,11 +1,20 @@
 package images
 
-import "errors"
+import (
+	"errors"
+	"strings"
+)
 
 var (
-	ErrNotFound         = errors.New("image not found")
-	ErrAlreadyExists    = errors.New("image already exists")
-	ErrInvalidImage     = errors.New("invalid image reference")
-	ErrConversionFailed = errors.New("failed to convert image")
+	ErrNotFound      = errors.New("image not found")
+	ErrAlreadyExists = errors.New("image already exists")
 )
 
+// IsInvalidNameError checks if an error is due to invalid image name
+func IsInvalidNameError(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(err.Error(), "invalid image name") ||
+		strings.Contains(err.Error(), "invalid reference format")
+}

lib/images/manager.go

@@ -8,6 +8,7 @@ import (
 	"sort"
 	"time"
 
+	"github.com/distribution/reference"
 	"github.com/onkernel/hypeman/lib/oapi"
 )
 
@@ -60,23 +61,28 @@ func (m *manager) ListImages(ctx context.Context) ([]oapi.Image, error) {
 }
 
 func (m *manager) CreateImage(ctx context.Context, req oapi.CreateImageRequest) (*oapi.Image, error) {
-	if imageExists(m.dataDir, req.Name) {
+	normalizedName, err := normalizeImageName(req.Name)
+	if err != nil {
+		return nil, fmt.Errorf("invalid image name: %w", err)
+	}
+
+	if imageExists(m.dataDir, normalizedName) {
 		return nil, ErrAlreadyExists
 	}
 
 	meta := &imageMetadata{
-		Name:      req.Name,
+		Name:      normalizedName,
 		Status:    StatusPending,
 		Request:   &req,
 		CreatedAt: time.Now(),
 	}
 
-	if err := writeMetadata(m.dataDir, req.Name, meta); err != nil {
+	if err := writeMetadata(m.dataDir, normalizedName, meta); err != nil {
 		return nil, fmt.Errorf("write initial metadata: %w", err)
 	}
 
-	queuePos := m.queue.Enqueue(req.Name, req, func() {
-		m.buildImage(context.Background(), req.Name, req)
+	queuePos := m.queue.Enqueue(normalizedName, req, func() {
+		m.buildImage(context.Background(), normalizedName, req)
 	})
 
 	img := meta.toOAPI()
@@ -179,22 +185,45 @@ func (m *manager) RecoverInterruptedBuilds() {
 }
 
 func (m *manager) GetImage(ctx context.Context, name string) (*oapi.Image, error) {
-	meta, err := readMetadata(m.dataDir, name)
+	normalizedName, err := normalizeImageName(name)
+	if err != nil {
+		return nil, fmt.Errorf("invalid image name: %w", err)
+	}
+
+	meta, err := readMetadata(m.dataDir, normalizedName)
 	if err != nil {
 		return nil, err
 	}
 
 	img := meta.toOAPI()
 
 	if meta.Status == StatusPending {
-		img.QueuePosition = m.queue.GetPosition(name)
+		img.QueuePosition = m.queue.GetPosition(normalizedName)
 	}
 
 	return img, nil
 }
 
 func (m *manager) DeleteImage(ctx context.Context, name string) error {
-	return deleteImage(m.dataDir, name)
+	normalizedName, err := normalizeImageName(name)
+	if err != nil {
+		return fmt.Errorf("invalid image name: %w", err)
+	}
+	return deleteImage(m.dataDir, normalizedName)
+}
+
+// normalizeImageName validates and normalizes an OCI image reference
+// Examples: alpine → docker.io/library/alpine:latest
+//           nginx:1.0 → docker.io/library/nginx:1.0
+func normalizeImageName(name string) (string, error) {
+	named, err := reference.ParseNormalizedNamed(name)
+	if err != nil {
+		return "", err
+	}
+	
+	// Ensure it has a tag (add :latest if missing)
+	tagged := reference.TagNameOnly(named)
+	return tagged.String(), nil
 }
 
 

lib/images/validation_test.go

@@ -0,0 +1,48 @@
+package images
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNormalizeImageName(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+		wantErr  bool
+	}{
+		// Valid images with full reference
+		{"docker.io/library/alpine:latest", "docker.io/library/alpine:latest", false},
+		{"ghcr.io/myorg/myapp:v1.0.0", "ghcr.io/myorg/myapp:v1.0.0", false},
+		
+		// Shorthand (gets expanded)
+		{"alpine", "docker.io/library/alpine:latest", false},
+		{"alpine:3.18", "docker.io/library/alpine:3.18", false},
+		{"nginx", "docker.io/library/nginx:latest", false},
+		{"nginx:alpine", "docker.io/library/nginx:alpine", false},
+		
+		// Without tag (gets :latest added)
+		{"docker.io/library/alpine", "docker.io/library/alpine:latest", false},
+		{"ubuntu", "docker.io/library/ubuntu:latest", false},
+		
+		// Invalid
+		{"", "", true},
+		{"invalid::", "", true},
+		{"has spaces", "", true},
+		{"UPPERCASE", "", true}, // Repository names must be lowercase
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			result, err := normalizeImageName(tt.input)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+