diff --git a/libs/arcade-cli/arcade_cli/deployment.py b/libs/arcade-cli/arcade_cli/deployment.py index 342c8e96..41b44c80 100644 --- a/libs/arcade-cli/arcade_cli/deployment.py +++ b/libs/arcade-cli/arcade_cli/deployment.py @@ -9,6 +9,8 @@ from pathlib import Path from typing import Any import toml +from arcade_core import Toolkit +from arcade_core.toolkit import valid_path from arcadepy import Arcade, NotFoundError from httpx import Client, ConnectError, HTTPStatusError, TimeoutException from packaging.requirements import Requirement @@ -228,18 +230,7 @@ class Worker(BaseModel): def exclude_filter(tarinfo: tarfile.TarInfo) -> tarfile.TarInfo | None: """Filter for files/directories to exclude from the compressed package""" - basename = os.path.basename(tarinfo.name) - - # Exclude all hidden directories/files - if basename.startswith("."): - return None - - # Exclude specific directories/files - if basename in {"dist", "build", "__pycache__", "venv", "coverage.xml"}: - return None - - # Exclude lock files - if basename.endswith(".lock"): + if not valid_path(tarinfo.name): return None return tarinfo @@ -262,6 +253,9 @@ class Worker(BaseModel): f"package '{package_path}' must contain a pyproject.toml or setup.py file" ) + # Validate that we are able to load the package + Toolkit.tools_from_directory(package_dir=package_path, package_name=package_path.name) + # Compress the package into a byte stream and tar byte_stream = io.BytesIO() with tarfile.open(fileobj=byte_stream, mode="w:gz") as tar: diff --git a/libs/arcade-core/arcade_core/toolkit.py b/libs/arcade-core/arcade_core/toolkit.py index 0212d6be..2e2c3933 100644 --- a/libs/arcade-core/arcade_core/toolkit.py +++ b/libs/arcade-core/arcade_core/toolkit.py @@ -4,7 +4,7 @@ import logging import os import types from collections import defaultdict -from pathlib import Path +from pathlib import Path, PurePosixPath, PureWindowsPath from pydantic import BaseModel, ConfigDict, field_validator @@ -87,14 +87,6 @@ class Toolkit(BaseModel): except (ImportError, AttributeError) as e: raise ToolkitLoadError(f"Failed to locate package directory for '{package}'.") from e - # Get all python files in the package directory - try: - modules = [f for f in package_dir.glob("**/*.py") if f.is_file()] - except OSError as e: - raise ToolkitLoadError( - f"Failed to locate Python files in package directory for '{package}'." - ) from e - toolkit = cls( name=name, package_name=package_name, @@ -105,14 +97,7 @@ class Toolkit(BaseModel): repository=repo, ) - for module_path in modules: - relative_path = module_path.relative_to(package_dir) - import_path = ".".join(relative_path.with_suffix("").parts) - import_path = f"{package_name}.{import_path}" - toolkit.tools[import_path] = get_tools_from_file(str(module_path)) - - if not toolkit.tools: - raise ToolkitLoadError(f"No tools found in package {package}") + toolkit.tools = cls.tools_from_directory(package_dir, package_name) return toolkit @@ -229,6 +214,61 @@ class Toolkit(BaseModel): return all_toolkits + @classmethod + def tools_from_directory(cls, package_dir: Path, package_name: str) -> dict[str, list[str]]: + """ + Load a Toolkit from a directory. + """ + # Get all python files in the package directory + try: + modules = [f for f in package_dir.glob("**/*.py") if f.is_file() and valid_path(f)] + except OSError as e: + raise ToolkitLoadError( + f"Failed to locate Python files in package directory for '{package_name}'." + ) from e + + tools: dict[str, list[str]] = {} + + for module_path in modules: + relative_path = module_path.relative_to(package_dir) + cls.validate_file(module_path) + import_path = ".".join(relative_path.with_suffix("").parts) + import_path = f"{package_name}.{import_path}" + tools[import_path] = get_tools_from_file(str(module_path)) + + if not tools: + raise ToolkitLoadError(f"No tools found in package {package_name}") + + return tools + + @classmethod + def validate_file(cls, file_path: str | Path) -> None: + """ + Validate that the Python code in the given file is syntactically correct. + + Args: + file_path: Path to the Python file to validate + """ + # Convert string path to Path object if needed + path = Path(file_path) if isinstance(file_path, str) else file_path + + # Check if file exists + if not path.exists(): + raise ValueError(f"❌ File not found: {path}") + + # Check if it's a Python file + if not path.suffix == ".py": + raise ValueError(f"❌ Not a Python file: {path}") + + try: + # Try to compile the code to check for syntax errors + with open(path, encoding="utf-8") as f: + source = f.read() + + compile(source, str(path), "exec") + except Exception as e: + raise SyntaxError(f"{path}: {e}") + def get_package_directory(package_name: str) -> str: """ @@ -247,3 +287,25 @@ def get_package_directory(package_name: str) -> str: return spec.submodule_search_locations[0] else: raise ImportError(f"Package {package_name} does not have a file path associated with it") + + +def valid_path(path: str) -> bool: + """ + Validate if a path is valid to be served or deployed. + """ + # Check both POSIX and Windows interpretations + posix_path = PurePosixPath(path) + windows_path = PureWindowsPath(path) + + # Get all possible parts from both interpretations + all_parts = set(posix_path.parts) | set(windows_path.parts) + + for part in all_parts: + if part.startswith("."): + return False + if part in {"dist", "build", "__pycache__", "venv", "coverage.xml"}: + return False + if part.endswith(".lock"): + return False + + return True diff --git a/libs/arcade-core/pyproject.toml b/libs/arcade-core/pyproject.toml index 2c98e974..d1cb9d5d 100644 --- a/libs/arcade-core/pyproject.toml +++ b/libs/arcade-core/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "arcade-core" -version = "2.2.0" +version = "2.2.1" description = "Arcade Core - Core library for Arcade platform" readme = "README.md" license = {text = "MIT"} diff --git a/libs/tests/core/test_toolkit.py b/libs/tests/core/test_toolkit.py index ce23b507..91e0c70a 100644 --- a/libs/tests/core/test_toolkit.py +++ b/libs/tests/core/test_toolkit.py @@ -1,8 +1,9 @@ +from pathlib import Path from unittest.mock import MagicMock, patch import pytest from arcade_core.errors import ToolkitLoadError -from arcade_core.toolkit import Toolkit +from arcade_core.toolkit import Toolkit, valid_path class TestToolkit: @@ -342,3 +343,224 @@ class TestToolkitIntegration: assert names == {"custom", "utils", "legacy"} utils_toolkit = next(t for t in toolkits if t.name == "utils") assert utils_toolkit.version == "2.0.0" + + +@pytest.fixture +def cleanup_test_files(): + test_files = ["valid.py", "invalid.py", "test.txt"] + + # Run the test + yield + + # Clean up test files after the test + for file in test_files: + try: # noqa: SIM105 + Path(file).unlink(missing_ok=True) + except Exception: # noqa: S110 + pass + + +class TestValidateFile: + @pytest.mark.usefixtures("cleanup_test_files") + def test_validate_file(self): + """Test validation of Python files with valid syntax.""" + # Create a temporary valid Python file + valid_file = Path("valid.py") + valid_file.write_text("def test(): return True") + + # Should not raise any exceptions + Toolkit.validate_file(valid_file) + # Test with string path + Toolkit.validate_file(str(valid_file)) + + def test_validate_tools_nonexistent_file(self): + """Test validation with non-existent file.""" + nonexistent = Path("nonexistent.py") + + with pytest.raises(ValueError, match="File not found"): + Toolkit.validate_file(nonexistent) + + @pytest.mark.usefixtures("cleanup_test_files") + def test_validate_tools_non_python_file(self): + """Test validation with non-Python file.""" + txt_file = Path("test.txt") + txt_file.write_text("Not a Python file") + + with pytest.raises(ValueError, match="Not a Python file"): + Toolkit.validate_file(txt_file) + + @pytest.mark.usefixtures("cleanup_test_files") + def test_validate_tools_syntax_error(self): + """Test validation with Python file containing syntax errors.""" + invalid_file = Path("invalid.py") + invalid_file.write_text("def test(): return True:") # Invalid syntax + + with pytest.raises(SyntaxError): + Toolkit.validate_file(invalid_file) + + +class TestValidPath: + """Test the valid_path function for path validation during deployment and serving.""" + + @pytest.mark.parametrize( + "path_input", + [ + # Simple valid paths + "file.py", + "module.py", + "utils.py", + "main.py", + "README.md", + "config.json", + # Valid nested paths + "src/main.py", + "lib/utils/helper.py", + "package/subpackage/module.py", + "tools/scripts/deploy.py", + "docs/api/reference.md", + "tests/unit/test_module.py", + # Deep nested paths + "very/deep/nested/directory/structure/file.py", + "a/b/c/d/e/f/g/module.py", + # Edge cases + "", # Path("") creates current directory path "." + "a", + "1", + # Files containing but not matching restricted patterns + "my_dist_file.py", # contains "dist" but doesn't match exactly + "build_utils.py", # contains "build" but doesn't match exactly + "lockfile.py", # contains "lock" but doesn't end with .lock + "unlock.py", # contains "lock" but doesn't end with .lock + # Case sensitivity - these should be valid because case doesn't match exactly + "DIST/file.py", # "DIST" != "dist" + "Build/file.py", # "Build" != "build" + "VENV/file.py", # "VENV" != "venv" + "Package.LOCK", # ".LOCK" != ".lock" + # Windows-style paths + "src\\module.py", + "lib\\utils\\helper.py", + # Absolute paths that should be valid + "/home/user/project/file.py", + # Unicode and special characters + "файл.py", # Cyrillic + "文件.py", # Chinese + "módulo.py", # Accented characters + "file with spaces.py", + "file-with-dashes.py", + "file_with_underscores.py", + # Path objects + Path("file.py"), + Path("src/module.py"), + ], + ids=[ + "simple_file_py", + "simple_module_py", + "simple_utils_py", + "simple_main_py", + "simple_readme_md", + "simple_config_json", + "nested_src_main", + "nested_lib_utils", + "nested_package_subpackage", + "nested_tools_scripts", + "nested_docs_api", + "nested_tests_unit", + "deep_nested_very_deep", + "deep_nested_a_b_c", + "edge_case_empty_string", + "edge_case_single_a", + "edge_case_single_1", + "contains_dist_but_valid", + "contains_build_but_valid", + "contains_lock_but_valid", + "contains_unlock_valid", + "case_sensitive_DIST", + "case_sensitive_Build", + "case_sensitive_VENV", + "case_sensitive_LOCK", + "windows_src_module", + "windows_lib_utils", + "absolute_home_user", + "unicode_cyrillic", + "unicode_chinese", + "unicode_accented", + "spaces_in_filename", + "dashes_in_filename", + "underscores_in_filename", + "path_object_file", + "path_object_nested", + ], + ) + def test_valid_paths(self, path_input): + """Test that valid paths are accepted.""" + assert valid_path(path_input) is True + + @pytest.mark.parametrize( + "path_input", + [ + # Hidden files (starting with .) + ".hidden", + ".gitignore", + ".env", + ".DS_Store", + ".vscode", + ".pytest_cache", + # Paths containing hidden directories + ".hidden/file.py", + "src/.cache/data.py", + "lib/.git/config", + ".vscode/settings.json", + "deep/.hidden/nested/file.py", + "normal/.DS_Store", + # Excluded directories (exact matches) + "dist", + "build", + "__pycache__", + "venv", + "coverage.xml", + # Paths containing excluded directories + "dist/bundle.js", + "build/output/file.py", + "src/__pycache__/module.cpython-39.pyc", + "venv/lib/python3.9/site-packages/module.py", + "project/build/artifacts/file.py", + "lib/dist/package/module.py", + "tools/venv/bin/python", + # Lock files (ending with .lock) + "package.lock", + "poetry.lock", + "requirements.lock", + "Pipfile.lock", + "yarn.lock", + "npm.lock", + "custom.lock", + # Lock files in nested paths + "src/package.lock", + "frontend/yarn.lock", + "backend/poetry.lock", + "deep/nested/path/file.lock", + # Multiple violations + ".hidden/dist/file.py", # hidden + excluded dir + "build/.cache/file.py", # excluded dir + hidden + "venv/package.lock", # excluded dir + lock file + ".git/hooks/file.lock", # hidden + lock file + # Windows-style paths that should be invalid + "dist\\bundle.js", + ".hidden\\file.py", + "src\\package.lock", + # Absolute paths that should be invalid + "/home/user/project/.hidden/file.py", + "/home/user/project/dist/file.py", + "/opt/project/package.lock", + # Unicode with exclusion patterns + ".файл", # Hidden Cyrillic file + "文件.lock", # Chinese lock file + # Path objects + Path(".hidden/file.py"), + Path("dist/bundle.js"), + Path("package.lock"), + ], + ) + def test_invalid_paths(self, path_input): + """Test that invalid paths are rejected.""" + assert valid_path(path_input) is False diff --git a/pyproject.toml b/pyproject.toml index ccdfa03a..0280963a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ requires-python = ">=3.10" dependencies = [ # CLI dependencies - "arcade-core>=2.0.0,<3.0.0", + "arcade-core>=2.2.1,<3.0.0", "typer==0.10.0", "rich==13.9.4", "Jinja2==3.1.6",