fix(concerto-core): validate metamodel AST bounds and required structures

packages/concerto-core/src/basemodelmanager.ts

@@ -294,10 +294,10 @@ class BaseModelManager {
         } catch (err: unknown) {
             const error = err as Error;
             throw new MetamodelException(error.message);
-        }
-
-        if (!alreadyHasMetamodel) {
-            this.deleteModelFile(MetaModelNamespace);
+        } finally {
+            if (!alreadyHasMetamodel) {
+                this.deleteModelFile(MetaModelNamespace);
+            }
         }
     }
 
@@ -854,7 +854,13 @@ class BaseModelManager {
      */
     fromAst(ast: IModels, options?: { disableValidation?: boolean }) {
         this.clearModelFiles();
+        if (!ast || !Array.isArray(ast.models)) {
+            throw new IllegalModelException('Models AST must contain a models array');
+        }
         ast.models.forEach( (model: IModel) => {
+            if (!model) {
+                throw new IllegalModelException('Model cannot be null or undefined');
+            }
             if(!EXCLUDE_NS.includes(model.namespace)) { // excludes the internal namespaces, already added
                 const modelFile = new ModelFile( this, model );
                 this.addModelFile( modelFile, null, null, true );

packages/concerto-core/src/introspect/classdeclaration.ts

@@ -108,6 +108,14 @@ class ClassDeclaration extends Declaration {
         for (let n = 0; n < this.ast.properties.length; n++) {
             let thing = this.ast.properties[n];
 
+            if (!thing) {
+                throw new IllegalModelException(
+                    `Property cannot be null or undefined, for class ${this.name}`,
+                    this.modelFile,
+                    this.ast.location
+                );
+            }
+
             if(ModelUtil.isSystemProperty(thing.name)) {
                 throw new IllegalModelException(`Invalid field name '${thing.name}'`, this.modelFile, this.ast.location);
             }

packages/concerto-core/src/introspect/collectionsizevalidator.ts

@@ -43,12 +43,15 @@ class CollectionSizeValidator extends Validator {
      */
     constructor(field: ValidatedElement, validator: ICollectionSizeValidator) {
         super(field, validator);
-        this.minSize = validator.minSize ?? null;
-        this.maxSize = validator.maxSize ?? null;
+        this.minSize = !isNull(validator?.minSize) ? validator.minSize : null;
+        this.maxSize = !isNull(validator?.maxSize) ? validator.maxSize : null;
 
         if (isNull(this.minSize) && isNull(this.maxSize)) {
             this.reportModelError('Invalid collection size, minSize and/or maxSize must be specified.');
-        } else if ((this.minSize ?? 0) < 0 || (this.maxSize ?? 0) < 0) {
+        } else if (
+            (this.minSize !== null && (!Number.isInteger(this.minSize) || this.minSize < 0)) ||
+            (this.maxSize !== null && (!Number.isInteger(this.maxSize) || this.maxSize < 0))
+        ) {
             this.reportModelError('minSize and/or maxSize must be positive integers.');
         } else if (isNull(this.minSize) || isNull(this.maxSize)) {
             // this is fine and means that we don't need to check whether minSize > maxSize

packages/concerto-core/src/introspect/mapkeytype.ts

@@ -96,6 +96,13 @@ class MapKeyType extends Decorated {
             this.type = 'String';
             break;
         case `${MetaModelNamespace}.ObjectMapKeyType`:
+            if (!this.ast.type || !this.ast.type.name) {
+                throw new IllegalModelException(
+                    `ObjectMapKeyType must contain property 'type', for MapDeclaration named ${this.parent.name}`,
+                    this.modelFile,
+                    this.ast.location
+                );
+            }
             this.type = String(this.ast.type.name);
             break;
         }

packages/concerto-core/src/introspect/mapvaluetype.ts

@@ -108,17 +108,17 @@ class MapValueType extends Decorated {
         case `${MetaModelNamespace}.RelationshipMapValueType`:
 
             // ObjectMapValueType must have TypeIdentifier.
-            if (!('type' in ast)) {
+            if (!('type' in ast) || !ast.type) {
                 throw new IllegalModelException(`ObjectMapValueType must contain property 'type', for MapDeclaration named ${this.parent.name}`);
             }
 
             // ObjectMapValueType TypeIdentifier must be properly formed.
-            if (!('$class' in ast.type) || !('name' in ast.type)) {
+            if (typeof ast.type !== 'object' || !('$class' in ast.type) || !('name' in ast.type)) {
                 throw new IllegalModelException(`ObjectMapValueType type must contain property '$class' and property 'name', for MapDeclaration named ${this.parent.name}`);
             }
 
             // And the $class must be valid.
-            if (ast.type.$class !== 'concerto.metamodel@1.0.0.TypeIdentifier') {
+            if (ast.type.$class !== `${MetaModelNamespace}.TypeIdentifier`) {
                 throw new IllegalModelException(`ObjectMapValueType type $class must be of TypeIdentifier for MapDeclaration named ${this.parent.name}`);
             }
 

packages/concerto-core/src/introspect/modelfile.ts

@@ -766,6 +766,10 @@ class ModelFile extends Decorated {
             throw new Error(`Cannot create a ModelFile with an unversioned namespace: ${ast.namespace}. All models must specify a version (e.g., @1.0.0).`);
         }
 
+        if (ast.imports && !Array.isArray(ast.imports)) {
+            throw new IllegalModelException('Model imports must be an array', this, this.ast.location);
+        }
+
         // Make sure to clone imports since we will add built-in imports
         const imports = ast.imports ? ast.imports.concat([]) : [];
 
@@ -781,6 +785,9 @@ class ModelFile extends Decorated {
 
         this.imports = imports;
         this.imports.forEach((imp) => {
+            if (!imp) {
+                throw new IllegalModelException('Import cannot be null or undefined', this, this.ast.location);
+            }
             this.enforceImportVersioning(imp);
             switch(imp.$class) {
             case `${MetaModelNamespace}.ImportAll`:
@@ -820,9 +827,17 @@ class ModelFile extends Decorated {
             return;
         }
 
+        if (!Array.isArray(ast.declarations)) {
+            throw new IllegalModelException('Model declarations must be an array', this, this.ast.location);
+        }
+
         for(let n=0; n < ast.declarations.length; n++) {
             let thing = ast.declarations[n];
 
+            if (!thing) {
+                throw new IllegalModelException('Declaration cannot be null or undefined', this, this.ast.location);
+            }
+
             switch(thing.$class) {
             case `${MetaModelNamespace}.AssetDeclaration`:
                 // Default super type for asset

packages/concerto-core/src/introspect/numbervalidator.ts

@@ -48,25 +48,21 @@ class NumberValidator extends Validator{
     constructor(field: ValidatedElement, ast: NumberDomainValidatorAst) {
         super(field, ast);
 
-        this.lowerBound = null;
-        this.upperBound = null;
+        this.lowerBound = !isNull(ast?.lower) ? (ast.lower as number) : null;
+        this.upperBound = !isNull(ast?.upper) ? (ast.upper as number) : null;
 
-        // the hasOwnProperty guards establish that the bound is present
-        if(Object.prototype.hasOwnProperty.call(ast, 'lower')) {
-            this.lowerBound = ast.lower as number;
-        }
-
-        if(Object.prototype.hasOwnProperty.call(ast, 'upper')) {
-            this.upperBound = ast.upper as number;
-        }
-
-        if(this.lowerBound === null && this.upperBound === null) {
+        if (this.lowerBound === null && this.upperBound === null) {
             // can't specify no upper and lower value
             this.reportModelError('Invalid range, lower and-or upper bound must be specified.');
+        } else if (
+            (this.lowerBound !== null && (typeof this.lowerBound !== 'number' || Number.isNaN(this.lowerBound))) ||
+            (this.upperBound !== null && (typeof this.upperBound !== 'number' || Number.isNaN(this.upperBound)))
+        ) {
+            this.reportModelError('lower and-or upper bound must be numbers.');
         } else if (this.lowerBound === null || this.upperBound === null) {
             // this is fine and means that we don't need to check whether upper > lower
         } else {
-            if(this.lowerBound > this.upperBound) {
+            if (this.lowerBound > this.upperBound) {
                 this.reportModelError('Lower bound must be less than or equal to upper bound.');
             }
         }

packages/concerto-core/src/introspect/property.ts

@@ -151,6 +151,14 @@ class Property extends Decorated {
     validate(classDecl: ClassDeclaration) {
         super.validate();
 
+        if (this.ast.$class === `${MetaModelNamespace}.ObjectProperty` && (!this.ast.type || !this.ast.type.name)) {
+            throw new IllegalModelException(
+                `ObjectProperty must contain property 'type', for property ${this.ast.name}`,
+                this.getModelFile(),
+                this.ast.location
+            );
+        }
+
         if(this.type) {
             classDecl.getModelFile().resolveType( 'property ' + this.getFullyQualifiedName(), this.type);
         }
@@ -160,7 +168,7 @@ class Property extends Decorated {
             if(this.type && !this.isPrimitive()) {
                 try {
                     const resolvedType = classDecl.getModelFile().getType(this.type);
-                    isMapType = resolvedType.isMapDeclaration?.() === true;
+                    isMapType = (resolvedType as any)?.isMapDeclaration?.() === true;
                 } catch(e) {
                     // type resolution failed — will be caught by other validation
                 }

packages/concerto-core/src/introspect/stringvalidator.ts

@@ -57,35 +57,44 @@ class StringValidator extends Validator{
         this.regex = null;
 
         if (lengthValidator) {
-            this.minLength = lengthValidator?.minLength;
-            this.maxLength = lengthValidator?.maxLength;
+            this.minLength = !isNull(lengthValidator.minLength) ? lengthValidator.minLength : null;
+            this.maxLength = !isNull(lengthValidator.maxLength) ? lengthValidator.maxLength : null;
 
-            if ((this.minLength === null || this.minLength === undefined) &&
-                (this.maxLength === null || this.maxLength === undefined)) {
+            if (this.minLength === null && this.maxLength === null) {
                 // can't specify no upper and lower value
                 this.reportModelError('Invalid string length, minLength and-or maxLength must be specified.');
-            } else if ((this.minLength ?? 0) < 0 || (this.maxLength ?? 0) < 0) {
+            } else if (
+                (this.minLength !== null && (!Number.isInteger(this.minLength) || this.minLength < 0)) ||
+                (this.maxLength !== null && (!Number.isInteger(this.maxLength) || this.maxLength < 0))
+            ) {
                 this.reportModelError('minLength and-or maxLength must be positive integers.');
             } else if (this.minLength === null || this.maxLength === null) {
                 // this is fine and means that we don't need to check whether minLength > maxLength
-            } else if (this.minLength !== undefined && this.maxLength !== undefined && this.minLength > this.maxLength) {
+            } else if (this.minLength > this.maxLength) {
                 this.reportModelError('minLength must be less than or equal to maxLength.');
             }
         }
 
         if (validator) {
-            try {
-                // ScalarDeclarations have no parent, so the custom RegExp option
-                // is only picked up for properties
-                const parent = 'getParent' in field ? field.getParent() : undefined;
-                const CustomRegExp = (parent?.getModelFile()?.getModelManager()?.options?.regExp || RegExp) as typeof RegExp;
-                this.regex = new CustomRegExp(validator.pattern, validator.flags);
-            }
-            catch (exception) {
+            if (isNull(validator.pattern) || typeof validator.pattern !== 'string' || validator.pattern.length === 0) {
                 this.reportModelError(
-                    (exception as Error).message,
+                    'Regex pattern must be specified.',
                     ErrorCodes.REGEX_VALIDATOR_EXCEPTION
                 );
+            } else {
+                try {
+                    // ScalarDeclarations have no parent, so the custom RegExp option
+                    // is only picked up for properties
+                    const parent = 'getParent' in field ? field.getParent() : undefined;
+                    const CustomRegExp = (parent?.getModelFile()?.getModelManager()?.options?.regExp || RegExp) as typeof RegExp;
+                    this.regex = new CustomRegExp(validator.pattern, validator.flags || undefined);
+                }
+                catch (exception) {
+                    this.reportModelError(
+                        (exception as Error).message,
+                        ErrorCodes.REGEX_VALIDATOR_EXCEPTION
+                    );
+                }
             }
         }