Add missing module field name validation

compose/service/module.go

@@ -273,7 +273,7 @@ func (svc module) Create(ctx context.Context, new *types.Module) (*types.Module,
 		new.DeletedAt = nil
 
 		if new.Fields != nil {
-			_ = new.Fields.Walk(func(f *types.ModuleField) error {
+			err = new.Fields.Walk(func(f *types.ModuleField) error {
 				f.ID = nextID()
 				f.ModuleID = new.ID
 				f.NamespaceID = new.NamespaceID
@@ -287,8 +287,15 @@ func (svc module) Create(ctx context.Context, new *types.Module) (*types.Module,
 					f.Expressions.Validators[i] = v
 				}
 
+				if !handle.IsValid(f.Name) {
+					return ModuleErrInvalidHandle()
+				}
+
 				return nil
 			})
+			if err != nil {
+				return
+			}
 		}
 
 		aProps.setModule(new)
@@ -613,6 +620,12 @@ func updateModuleFields(ctx context.Context, s store.Storer, new, old *types.Mod
 			return ModuleErrFieldNameReserved()
 		}
 
+		// backward compatible; we didn't check for valid handle.
+		// if a field already existed and the handle is invalid we ignore the error.
+		if !handle.IsValid(f.Name) && old.Fields.FindByName(f.Name) == nil {
+			return ModuleErrInvalidHandle()
+		}
+
 		if f.ModuleID != new.ID {
 			return fmt.Errorf("module id of field %q does not match the module", f.Name)
 		}

tests/compose/module_test.go

@@ -193,6 +193,25 @@ func TestModuleCreate(t *testing.T) {
 		End()
 }
 
+func TestModuleCreateInvalidField(t *testing.T) {
+	h := newHelper(t)
+	h.clearModules()
+
+	helpers.AllowMe(h, types.NamespaceRbacResource(0), "read", "modules.search")
+	helpers.AllowMe(h, types.NamespaceRbacResource(0), "module.create")
+
+	ns := h.makeNamespace("some-namespace")
+
+	h.apiInit().
+		Post(fmt.Sprintf("/namespace/%d/module/", ns.ID)).
+		JSON(`{ "name": "mod", "fields": [{ "name": "a", "kind": "Number" }] }`).
+		Header("Accept", "application/json").
+		Expect(t).
+		Status(http.StatusOK).
+		Assert(helpers.AssertError("module.errors.invalidHandle")).
+		End()
+}
+
 func TestModuleUpdateForbidden(t *testing.T) {
 	h := newHelper(t)
 	h.clearModules()
@@ -267,6 +286,30 @@ func TestModuleFieldsUpdate(t *testing.T) {
 	h.a.Equal(m.Fields[1].Kind, "DateTime")
 }
 
+func TestModuleFieldsUpdate_invalidHandle(t *testing.T) {
+	h := newHelper(t)
+	h.clearModules()
+
+	helpers.AllowMe(h, types.NamespaceRbacResource(0), "read", "modules.search")
+	helpers.AllowMe(h, types.NamespaceRbacResource(0), "module.create")
+	helpers.AllowMe(h, types.ModuleRbacResource(0, 0), "update")
+
+	ns := h.makeNamespace("some-namespace")
+	mod := h.makeModule(ns, "mod", &types.ModuleField{
+		Name: "a",
+		Kind: "String",
+	})
+
+	h.apiInit().
+		Post(fmt.Sprintf("/namespace/%d/module/%d", ns.ID, mod.ID)).
+		JSON(`{ "name": "mod", "fields": [{ "name": "a", "kind": "String" }] }`).
+		Header("Accept", "application/json").
+		Expect(t).
+		Status(http.StatusOK).
+		Assert(helpers.AssertNoErrors).
+		End()
+}
+
 func TestModuleUpdateWithReservedFieldName(t *testing.T) {
 	h := newHelper(t)
 	h.clearModules()