From 002df90e7dfeb8c9bbd6f10afcd34b9f38073a97 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 3 Jul 2025 17:31:39 +0200 Subject: [PATCH 1/3] Minor code quality improvements to strip utility functions Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/utils.py | 12 +++++------- tests/test_utils.py | 20 ++++++-------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/cfbs/utils.py b/cfbs/utils.py index 045eee2c..38b8e55c 100644 --- a/cfbs/utils.py +++ b/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: diff --git a/tests/test_utils.py b/tests/test_utils.py index 042c436b..69aeaaab 100644 --- a/tests/test_utils.py +++ b/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(): From 9961cee72a9f6a6d0a0b3675508ca5b239192abd Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 4 Jul 2025 15:09:01 +0200 Subject: [PATCH 2/3] Add validation of module name content rules Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- JSON.md | 2 ++ cfbs/module.py | 6 ++++++ cfbs/validate.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/JSON.md b/JSON.md index c15e1a0a..81d89cf5 100644 --- a/JSON.md +++ b/JSON.md @@ -174,6 +174,8 @@ 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. - `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`. diff --git a/cfbs/module.py b/cfbs/module.py index b25781aa..a2a55ed9 100644 --- a/cfbs/module.py +++ b/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""" diff --git a/cfbs/validate.py b/cfbs/validate.py index 2f1855bc..96250e44 100644 --- a/cfbs/validate.py +++ b/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,43 @@ 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")) + + 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 +224,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 +373,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 +389,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 +687,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""" From 476e7832465b70f59940609709a633b482a25efc Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 4 Jul 2025 15:26:00 +0200 Subject: [PATCH 3/3] Allow underscores in local module names Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- JSON.md | 1 + cfbs/validate.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/JSON.md b/JSON.md index 81d89cf5..ca7704f7 100644 --- a/JSON.md +++ b/JSON.md @@ -176,6 +176,7 @@ The modules inside `build`, `provides`, and `index` use these fields: 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`. diff --git a/cfbs/validate.py b/cfbs/validate.py index 96250e44..cb82979f 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -198,6 +198,9 @@ def validate_module_name_content(name): 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,