ENT-13089: Added validation of module names

JSON.md

@@ -174,6 +174,9 @@ The modules inside `build`, `provides`, and `index` use these fields:
   For `provides` and `index` dictionaries, this name must be the key of each entry (not a field inside).
   For the `build` array, it must be inside each module object (with `name` as the key).
   Local modules (files and folders in same directory as `cfbs.json`), must start with `./`, and end with `/` if it's a directory.
+  Module names should not be longer than 64 characters.
+  Module names (not including adfixes `./`, `/`, `.cf`, `.json` for local modules) should only contain lowercase ASCII alphanumeric characters possibly separated by dashes, and should start with a letter.
+  Local module names can contain underscores instead of dashes.
 - `description` (string): Human readable description of what this module does.
 - `tags` (array of strings): Mostly used for information / finding modules on [build.cfengine.com](https://build.cfengine.com).
   Some common examples include `supported`, `experimental`, `security`, `library`, `promise-type`.

cfbs/module.py

@@ -7,6 +7,12 @@ def is_module_added_manually(added_by: str):
     return added_by in ("cfbs add", "cfbs init")
 
 
+def is_module_local(name: str):
+    # a module might contain `"local"` in its `"tags"` but this is not required
+    # the source of truth for whether the module is local is whether it starts with `./`
+    return name.startswith("./")
+
+
 class Module:
     """Class representing a module in cfbs.json"""
 

cfbs/utils.py

@@ -130,23 +130,21 @@ def strip_left(string, beginning):
     return string[len(beginning) :]
 
 
-def strip_right_any(string, suffixes: tuple):
-    if not string.endswith(suffixes):
-        return string
-
+def strip_right_any(string, suffixes):
     for suffix in suffixes:
         if string.endswith(suffix):
             return string[0 : -len(suffix)]
 
+    return string
 
-def strip_left_any(string, prefixes: tuple):
-    if not string.startswith(prefixes):
-        return string
 
+def strip_left_any(string, prefixes):
     for prefix in prefixes:
         if string.startswith(prefix):
             return string[len(prefix) :]
 
+    return string
+
 
 def read_file(path):
     try:

cfbs/validate.py

@@ -20,13 +20,17 @@
 """
 
 import argparse
+import logging as log
 import sys
 import re
 from collections import OrderedDict
 from typing import List, Tuple
 
+from cfbs.module import is_module_local
 from cfbs.utils import (
     is_a_commit_hash,
+    strip_left,
+    strip_right_any,
     GenericExitError,
 )
 from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS
@@ -166,6 +170,46 @@ def _validate_top_level_keys(config):
             )
 
 
+def validate_module_name_content(name):
+    MAX_MODULE_NAME_LENGTH = 64
+
+    if len(name) > MAX_MODULE_NAME_LENGTH:
+        raise CFBSValidationError(
+            name,
+            "Module name is too long (over "
+            + str(MAX_MODULE_NAME_LENGTH)
+            + " characters)",
+        )
+
+    # lowercase ASCII alphanumericals, starting with a letter, and possible singular dashes in the middle
+    r = "[a-z][a-z0-9]*(-[a-z0-9]+)*"
+    proper_name = name
+
+    if is_module_local(name):
+        if not name.startswith("./"):
+            raise CFBSValidationError(name, "Local module names should begin with `./`")
+
+        if not name.endswith((".cf", ".json", "/")):
+            raise CFBSValidationError(
+                name,
+                "Local module names should end with `/` (for directories) or `.json` or `.cf` (for files).",
+            )
+
+        proper_name = strip_left(proper_name, "./")
+        proper_name = strip_right_any(proper_name, ("/", ".cf", ".json"))
+
+        # allow underscores, only for local modules
+        proper_name = proper_name.replace("_", "-")
+
+    if not re.fullmatch(r, proper_name):
+        raise CFBSValidationError(
+            name,
+            "Module name contains illegal characters (only lowercase ASCII alphanumeric characters are legal)",
+        )
+
+    log.debug("Validated name of module %s" % name)
+
+
 def _validate_config(config, empty_build_list_ok=False):
     # First validate the config i.e. the user's cfbs.json
     config.warn_about_unknown_keys()
@@ -183,6 +227,9 @@ def _validate_config(config, empty_build_list_ok=False):
         for name, module in raw_data["provides"].items():
             _validate_module_object("provides", name, module, config)
 
+    if config["type"] == "module":
+        validate_module_name_content(config["name"])
+
 
 def validate_config(config, empty_build_list_ok=False):
     try:
@@ -329,6 +376,7 @@ def validate_alias(name, module, context):
             raise CFBSValidationError(name, '"alias" must be of type string')
         if not module["alias"]:
             raise CFBSValidationError(name, '"alias" must be non-empty')
+        validate_module_name_content(name)
         if not config.can_reach_dependency(module["alias"], search_in):
             raise CFBSValidationError(
                 name, '"alias" must reference another module in the index'
@@ -344,6 +392,8 @@ def validate_name(name, module):
         if not module["name"]:
             raise CFBSValidationError(name, '"name" must be non-empty')
 
+        validate_module_name_content(name)
+
     def validate_description(name, module):
         assert "description" in module
         if type(module["description"]) != str:
@@ -640,6 +690,12 @@ def validate_module_input(name, module):
     if "input" in module:
         validate_module_input(name, module)
 
+    # Step 4 - Additional validation checks:
+
+    # Validate module name content also when there's no explicit "name" field (for "index" and "provides" project types)
+    if "name" not in module:
+        validate_module_name_content(name)
+
 
 def _validate_config_for_build_field(config, empty_build_list_ok=False):
     """Validate that neccessary fields are in the config for the build/download commands to work"""

tests/test_utils.py

@@ -58,25 +58,17 @@ def test_strip_left():
 
 
 def test_strip_right_any():
-    s = "a.b.c"
+    assert strip_right_any("a.b.c", (".b", ".c")) == "a.b"
+    assert strip_right_any("a.b.c", (".c", ".b")) == "a.b"
 
-    assert strip_right_any(s, (".b", ".c")) == "a.b"
-    assert strip_right_any(s, (".c", ".b")) == "a.b"
-
-    s = "a.b.b"
-
-    assert strip_right_any(s, (".b", ".b")) == "a.b"
+    assert strip_right_any("a.b.b", (".b", ".b")) == "a.b"
 
 
 def test_strip_left_any():
-    s = "a.b.c"
-
-    assert strip_left_any(s, ("a.", "b.")) == "b.c"
-    assert strip_left_any(s, ("b.", "a.")) == "b.c"
-
-    s = "a.a.b"
+    assert strip_left_any("a.b.c", ("a.", "b.")) == "b.c"
+    assert strip_left_any("a.b.c", ("b.", "a.")) == "b.c"
 
-    assert strip_left_any(s, ("a.", "a.")) == "a.b"
+    assert strip_left_any("a.a.b", ("a.", "a.")) == "a.b"
 
 
 def test_read_file():