Merge pull request #283 from jakub-nt/CFE-4592

CFE-4592: Fix local modules in subdirectories failing validation

Author: olehermanse

cfbs/module.py

@@ -8,8 +8,9 @@ def is_module_added_manually(added_by: str):
 
 
 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 `./`
+    """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("./")
 
 

cfbs/utils.py

@@ -10,7 +10,7 @@
 import urllib.error
 from collections import OrderedDict
 from shutil import rmtree
-from typing import Union
+from typing import Iterable, Union
 
 from cfbs.pretty import pretty
 
@@ -180,29 +180,29 @@ def item_index(iterable, item, extra_at_end=True):
             return -1
 
 
-def strip_right(string, ending):
+def strip_right(string: str, ending) -> str:
     # can be replaced with str.removesuffix from Python 3.9 onwards
     if not string.endswith(ending):
         return string
     return string[0 : -len(ending)]
 
 
-def strip_left(string, beginning):
+def strip_left(string: str, beginning) -> str:
     # can be replaced with str.removeprefix from Python 3.9 onwards
     if not string.startswith(beginning):
         return string
     return string[len(beginning) :]
 
 
-def strip_right_any(string, suffixes):
+def strip_right_any(string: str, suffixes: Iterable[str]) -> str:
     for suffix in suffixes:
         if string.endswith(suffix):
             return string[0 : -len(suffix)]
 
     return string
 
 
-def strip_left_any(string, prefixes):
+def strip_left_any(string: str, prefixes: Iterable[str]) -> str:
     for prefix in prefixes:
         if string.startswith(prefix):
             return string[len(prefix) :]

cfbs/validate.py

@@ -196,6 +196,9 @@ def validate_module_name_content(name):
         proper_name = strip_left(proper_name, "./")
         proper_name = strip_right_any(proper_name, ("/", ".cf", ".json"))
 
+        # only validate the local module's name, not the entire path to it
+        proper_name = proper_name.split("/")[-1]
+
         # allow underscores, only for local modules
         proper_name = proper_name.replace("_", "-")
 
@@ -205,7 +208,7 @@ def validate_module_name_content(name):
             "Module name contains illegal characters (only lowercase ASCII alphanumeric characters are legal)",
         )
 
-    log.debug("Validated name of module %s" % name)
+    log.debug("Successfully validated name of module %s" % name)
 
 
 def validate_config_raise_exceptions(config, empty_build_list_ok=False):

tests/test_validate.py

@@ -0,0 +1,32 @@
+import pytest
+
+from cfbs.utils import CFBSValidationError
+from cfbs.validate import validate_module_name_content
+
+
+def test_validate_module_name_content():
+    validate_module_name_content("regular-name")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("Uppercase-name")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("underscore_but_not_local")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("name with spaces")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("-leading-hyphen")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content(
+            "module-name-too-longggggggggggggggggggggggggggggggggggggggggggggg"
+        )
+
+    validate_module_name_content("./local_module.cf")
+    validate_module_name_content("./local_module_directory/")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("not_local_module.cf")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("./_leading_underscore/")
+    validate_module_name_content("./good-extension.json")
+    with pytest.raises(CFBSValidationError):
+        validate_module_name_content("./bad-extension.zip")
+
+    validate_module_name_content("./123 Illeg@l!/legal-name.cf")