From 69ba1c3fad644fedb5585608ebd0f667e3d3b49f Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Wed, 10 Jan 2024 00:07:31 +0000 Subject: [PATCH] MinetestCheck: Validate supported_games syntax --- app/tasks/importtasks.py | 7 +++--- app/tasks/minetestcheck/tree.py | 42 ++++++++++++++++++++------------- app/utils/models.py | 6 +++-- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/app/tasks/importtasks.py b/app/tasks/importtasks.py index 01632405..76898c5a 100644 --- a/app/tasks/importtasks.py +++ b/app/tasks/importtasks.py @@ -32,7 +32,8 @@ from app.models import AuditSeverity, db, NotificationType, PackageRelease, Meta MinetestRelease, Package, PackageState, PackageScreenshot, PackageUpdateTrigger, PackageUpdateConfig, \ PackageGameSupport from app.tasks import celery, TaskError -from app.utils import random_string, post_bot_message, add_system_notification, add_system_audit_log, get_games_from_csv +from app.utils import random_string, post_bot_message, add_system_notification, add_system_audit_log, \ + get_games_from_list from app.utils.git import clone_repo, get_latest_tag, get_latest_commit, get_temp_dir from .minetestcheck import build_tree, MinetestCheckError, ContentType from app import app @@ -163,7 +164,7 @@ def post_release_check_update(self, release: PackageRelease, path): game_is_supported = {} if "supported_games" in tree.meta: - for game in get_games_from_csv(db.session, tree.meta["supported_games"]): + for game in get_games_from_list(db.session, tree.meta["supported_games"]): game_is_supported[game.id] = True has_star = any(map(lambda x: x.strip() == "*", tree.meta["supported_games"].split(","))) @@ -175,7 +176,7 @@ def post_release_check_update(self, release: PackageRelease, path): package.supports_all_games = True if "unsupported_games" in tree.meta: - for game in get_games_from_csv(db.session, tree.meta["unsupported_games"]): + for game in get_games_from_list(db.session, tree.meta["unsupported_games"]): game_is_supported[game.id] = False resolver.set_supported(package, game_is_supported, 10) diff --git a/app/tasks/minetestcheck/tree.py b/app/tasks/minetestcheck/tree.py index 582e88a3..4f3790dd 100644 --- a/app/tasks/minetestcheck/tree.py +++ b/app/tasks/minetestcheck/tree.py @@ -23,6 +23,7 @@ from . import MinetestCheckError, ContentType from .config import parse_conf basenamePattern = re.compile("^([a-z0-9_]+)$") +licensePattern = re.compile("^(licen[sc]e|copying)(.[^/\n]+)?$", re.IGNORECASE) DISALLOWED_NAMES = { "core", "minetest", "group", "table", "string", "lua", "luajit", "assert", "debug", @@ -64,6 +65,19 @@ def get_csv_line(line): return [x.strip() for x in line.split(",") if x.strip() != ""] +def check_name_list(key: str, value: list[str], relative: str, allow_star: bool = False): + for dep in value: + if not basenamePattern.match(dep): + if dep == "*" and allow_star: + continue + elif " " in dep: + raise MinetestCheckError( + f"Invalid {key} name '{dep}' at {relative}, did you forget a comma?") + else: + raise MinetestCheckError( + f"Invalid {key} name '{dep}' at {relative}, names must only contain a-z0-9_.") + + class PackageTreeNode: def __init__(self, base_dir, relative, author=None, repo=None, name=None): self.baseDir = base_dir @@ -94,10 +108,9 @@ class PackageTreeNode: self.add_children_from_mod_dir(None) def find_license_file(self) -> Optional[str]: - names = ["LICENSE", "LICENSE.md", "LICENSE.txt", "COPYING"] - for name in names: + for name in os.listdir(self.baseDir): path = os.path.join(self.baseDir, name) - if os.path.isfile(path): + if os.path.isfile(path) and licensePattern.match(name): return path return None @@ -182,20 +195,17 @@ class PackageTreeNode: result["depends"] = [] result["optional_depends"] = [] - def check_dependencies(deps): - for dep in deps: - if not basenamePattern.match(dep): - if " " in dep: - raise MinetestCheckError("Invalid dependency name '{}' for mod at {}, did you forget a comma?" \ - .format(dep, self.relative)) - else: - raise MinetestCheckError( - "Invalid dependency name '{}' for mod at {}, names must only contain a-z0-9_." \ - .format(dep, self.relative)) + # Read supported games + result["supported_games"] = get_csv_line(result.get("supported_games", "")) + result["unsupported_games"] = get_csv_line(result.get("unsupported_games", "")) # Check dependencies - check_dependencies(result["depends"]) - check_dependencies(result["optional_depends"]) + check_name_list("depends", result["depends"], self.relative) + check_name_list("optional_depends", result["optional_depends"], self.relative) + + # Check supported games + check_name_list("supported_games", result["supported_games"], self.relative, True) + check_name_list("unsupported_games", result["unsupported_games"], self.relative) # Fix games using "name" as "title" if self.type == ContentType.GAME and "name" in result: @@ -203,7 +213,7 @@ class PackageTreeNode: del result["name"] # Calculate Title - if "name" in result and not "title" in result: + if "name" in result and "title" not in result: result["title"] = result["name"].replace("_", " ").title() # Calculate short description diff --git a/app/utils/models.py b/app/utils/models.py index 9b63743a..0029726c 100644 --- a/app/utils/models.py +++ b/app/utils/models.py @@ -140,10 +140,12 @@ def post_bot_message(package: Package, title: str, message: str): def get_games_from_csv(session: sqlalchemy.orm.Session, csv: str) -> List[Package]: + return get_games_from_list(session, [name.strip() for name in csv.split(",")]) + + +def get_games_from_list(session: sqlalchemy.orm.Session, supported_games_raw: list[str]) -> List[Package]: retval = [] - supported_games_raw = csv.split(",") for game_name in supported_games_raw: - game_name = game_name.strip() if game_name.endswith("_game"): game_name = game_name[:-5] games = session.query(Package).filter(and_(Package.state==PackageState.APPROVED, Package.type==PackageType.GAME,