fix(grpc): sanitize container image refs before containerd (#1071)

* fix(grpc): sanitize container image refs before containerd

* chore: allow sanitized spelling in lint config

* .golangci.yml: use ignore-words for sanitized

* test(grpc): fix sanitizer request factory

* test(grpc): align sanitizer expected id

Author: andreineamtu

.golangci.yml

@@ -20,6 +20,8 @@ linters-settings:
     line-length: 120
   misspell:
     locale: UK
+    ignore-words:
+      - sanitized
   goimports:
     local-prefixes: github.com/liquidmetal-dev/flintlock
   nolintlint:

infrastructure/grpc/sanitize.go

@@ -0,0 +1,89 @@
+package grpc
+
+import (
+	"fmt"
+	"strings"
+	"unicode"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/liquidmetal-dev/flintlock/api/types"
+)
+
+func sanitizeMicroVMImageReferences(logger *logrus.Entry, spec *types.MicroVMSpec) {
+	if spec == nil {
+		return
+	}
+
+	if spec.Kernel != nil {
+		sanitizedValue, changed := cleanContainerImageReference(spec.Kernel.Image)
+		if changed {
+			logSanitizedField(logger, "kernel.image", spec.Kernel.Image, sanitizedValue)
+			spec.Kernel.Image = sanitizedValue
+		}
+	}
+
+	if spec.Initrd != nil {
+		sanitizedValue, changed := cleanContainerImageReference(spec.Initrd.Image)
+		if changed {
+			logSanitizedField(logger, "initrd.image", spec.Initrd.Image, sanitizedValue)
+			spec.Initrd.Image = sanitizedValue
+		}
+	}
+
+	sanitizeVolumeImage(logger, "rootVolume", spec.RootVolume)
+
+	for index, volume := range spec.AdditionalVolumes {
+		fieldName := fmt.Sprintf("additionalVolumes[%d]", index)
+		sanitizeVolumeImage(logger, fieldName, volume)
+	}
+}
+
+func sanitizeVolumeImage(logger *logrus.Entry, fieldName string, volume *types.Volume) {
+	if volume == nil || volume.Source == nil || volume.Source.ContainerSource == nil {
+		return
+	}
+
+	originalValue := *volume.Source.ContainerSource
+	sanitizedValue, changed := cleanContainerImageReference(originalValue)
+	if !changed {
+		return
+	}
+
+	logSanitizedField(logger, fieldName+".containerSource", originalValue, sanitizedValue)
+	volume.Source.ContainerSource = &sanitizedValue
+}
+
+func cleanContainerImageReference(raw string) (string, bool) {
+	if raw == "" {
+		return raw, false
+	}
+
+	trimmed := strings.TrimSpace(raw)
+
+	cleaned := strings.Map(func(r rune) rune {
+		if r < 0x20 || r == 0x7f {
+			return -1
+		}
+
+		if unicode.IsControl(r) {
+			return -1
+		}
+
+		return r
+	}, trimmed)
+
+	if cleaned == raw {
+		return cleaned, false
+	}
+
+	return cleaned, true
+}
+
+func logSanitizedField(logger *logrus.Entry, field string, originalValue, sanitizedValue string) {
+	logger.WithFields(logrus.Fields{
+		"field":          field,
+		"originalImage":  originalValue,
+		"sanitizedImage": sanitizedValue,
+	}).Warn("sanitized container image reference before Flintlock processing")
+}

infrastructure/grpc/server.go

@@ -41,6 +41,8 @@ func (s *server) CreateMicroVM(
 		//nolint:wrapcheck // don't wrap grpc errors when using the status package
 		return nil, status.Error(codes.InvalidArgument, "invalid create microvm request: MicroVMSpec required")
 	}
+
+	sanitizeMicroVMImageReferences(logger, req.Microvm)
 	modelSpec, err := convertMicroVMToModel(req.Microvm)
 	if err != nil {
 		return nil, fmt.Errorf("converting request: %w", err)

infrastructure/grpc/server_test.go

@@ -22,6 +22,8 @@ func TestServer_CreateMicroVM(t *testing.T) {
 		createReq   *mvm1.CreateMicroVMRequest
 		expectError bool
 		expect      func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder)
+		expectedID  string
+		expectedNS  string
 	}{
 		{
 			name:        "nil request should fail with error",
@@ -58,6 +60,8 @@ func TestServer_CreateMicroVM(t *testing.T) {
 			name:        "valid spec should not fail",
 			createReq:   createTestCreateRequest("mvm1", "default"),
 			expectError: false,
+			expectedID:  "mvm1",
+			expectedNS:  "default",
 			expect: func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder) {
 				vmid, _ := models.NewVMID("mvm1", "default", "uid")
 
@@ -77,6 +81,80 @@ func TestServer_CreateMicroVM(t *testing.T) {
 				)
 			},
 		},
+		{
+			name: "sanitizes control characters before invoking usecase",
+			createReq: (func() *mvm1.CreateMicroVMRequest {
+				filename := "kernel"
+				mac := "AA:FF:00:00:00:01"
+				rootSource := "\n ghcr.io/test/root:latest\r"
+				additionalSource := "\tghcr.io/test/additional:latest\n"
+				initrdImage := "\n ghcr.io/test/initrd:latest\n"
+
+				return &mvm1.CreateMicroVMRequest{
+					Microvm: &types.MicroVMSpec{
+						Id:         "mvm-sanitize",
+						Namespace:  "default",
+						Vcpu:       2,
+						MemoryInMb: 1024,
+						Kernel: &types.Kernel{
+							Image:    "\n ghcr.io/test/kernel:latest\t",
+							Filename: &filename,
+						},
+						Initrd: &types.Initrd{
+							Image: initrdImage,
+						},
+						Interfaces: []*types.NetworkInterface{
+							{
+								DeviceId: "eth0",
+								GuestMac: &mac,
+								Type:     types.NetworkInterface_MACVTAP,
+							},
+						},
+						RootVolume: &types.Volume{
+							Id: "root",
+							Source: &types.VolumeSource{
+								ContainerSource: &rootSource,
+							},
+						},
+						AdditionalVolumes: []*types.Volume{
+							{
+								Id: "extra",
+								Source: &types.VolumeSource{
+									ContainerSource: &additionalSource,
+								},
+							},
+						},
+					},
+				}
+			})(),
+			expectError: false,
+			expectedID:  "mvm-sanitize",
+			expectedNS:  "default",
+			expect: func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder) {
+				vmid, _ := models.NewVMID("mvm-sanitize", "default", "uid")
+				cm.CreateMicroVM(
+					gomock.AssignableToTypeOf(context.Background()),
+					gomock.AssignableToTypeOf(&models.MicroVM{}),
+				).DoAndReturn(func(_ context.Context, mvm *models.MicroVM) (*models.MicroVM, error) {
+					Expect(string(mvm.Spec.Kernel.Image)).To(Equal("ghcr.io/test/kernel:latest"))
+					Expect(mvm.Spec.Initrd).NotTo(BeNil())
+					Expect(string(mvm.Spec.Initrd.Image)).To(Equal("ghcr.io/test/initrd:latest"))
+					Expect(mvm.Spec.RootVolume.Source.Container).NotTo(BeNil())
+					Expect(string(mvm.Spec.RootVolume.Source.Container.Image)).To(Equal("ghcr.io/test/root:latest"))
+					Expect(len(mvm.Spec.AdditionalVolumes)).To(Equal(1))
+					Expect(string(mvm.Spec.AdditionalVolumes[0].Source.Container.Image)).To(Equal("ghcr.io/test/additional:latest"))
+
+					return &models.MicroVM{
+						ID:      *vmid,
+						Version: 0,
+						Spec:    mvm.Spec,
+						Status: models.MicroVMStatus{
+							State: models.CreatedState,
+						},
+					}, nil
+				})
+			},
+		},
 	}
 
 	for _, tc := range tt {
@@ -97,8 +175,12 @@ func TestServer_CreateMicroVM(t *testing.T) {
 				Expect(err).To(HaveOccurred())
 			} else {
 				Expect(err).NotTo(HaveOccurred())
-				Expect(resp.Microvm.Spec.Id).To(Equal("mvm1"))
-				Expect(resp.Microvm.Spec.Namespace).To(Equal("default"))
+				if tc.expectedID != "" {
+					Expect(resp.Microvm.Spec.Id).To(Equal(tc.expectedID))
+				}
+				if tc.expectedNS != "" {
+					Expect(resp.Microvm.Spec.Namespace).To(Equal(tc.expectedNS))
+				}
 			}
 		})
 	}