fix(project): keep new vehicle names within the selected base directory

Author: yashhzd

ardupilot_methodic_configurator/backend_filesystem_program_settings.py

@@ -24,11 +24,9 @@
 from ntpath import normpath as ntpath_normpath
 from os import makedirs as os_makedirs
 from os import path as os_path
-from os import sep as os_sep
 from pathlib import Path
 from platform import system as platform_system
 from posixpath import normpath as posixpath_normpath
-from re import escape as re_escape
 from re import match as re_match
 from typing import Any, Optional, Union
 
@@ -40,6 +38,16 @@
 
 # Platform detection constant to avoid repeated system calls
 IS_WINDOWS = platform_system() == "Windows"
+WINDOWS_RESERVED_FILENAMES = frozenset(
+    {
+        "CON",
+        "PRN",
+        "AUX",
+        "NUL",
+        *(f"COM{i}" for i in range(1, 10)),
+        *(f"LPT{i}" for i in range(1, 10)),
+    }
+)
 
 
 @dataclass(frozen=True)
@@ -248,11 +256,13 @@ def create_new_vehicle_dir(new_vehicle_dir: str) -> str:
     @staticmethod
     def valid_directory_name(dir_name: str) -> bool:
         """
-        Check if a name contains only alphanumeric characters, underscores, hyphens, dots and the OS directory separator.
+        Check if a vehicle directory name is a single safe path segment.
 
-        This function is designed to ensure that the directory name does not contain characters that are
-        invalid for directory names in many operating systems. It does not guarantee that the name
-        is valid in all contexts or operating systems, as directory name validity can vary.
+        This function is designed to validate a user-provided vehicle name, not an
+        arbitrary path. Path separators and traversal segments are rejected so project
+        creation always remains under the selected base directory. Dots within a name
+        are allowed, but `.` and `..` are rejected as standalone traversal segments.
+        On Windows, trailing dots and reserved device names are also rejected.
 
         Args:
           dir_name (str): The directory name to check.
@@ -261,8 +271,16 @@ def valid_directory_name(dir_name: str) -> bool:
           bool: True if the directory name matches the allowed pattern, False otherwise.
 
         """
-        # Include os.sep and dot in the pattern
-        pattern = r"^[\w" + re_escape(os_sep) + ".-]+$"
+        if not dir_name or dir_name in {".", ".."}:
+            return False
+        if "/" in dir_name or "\\" in dir_name:
+            return False
+        if IS_WINDOWS:
+            if dir_name.endswith("."):
+                return False
+            if dir_name.split(".", maxsplit=1)[0].upper() in WINDOWS_RESERVED_FILENAMES:
+                return False
+        pattern = r"^[\w.-]+$"
         return re_match(pattern, dir_name) is not None
 
     @staticmethod

tests/test_backend_filesystem_program_settings.py

@@ -239,25 +239,39 @@ def test_user_can_validate_directory_names_correctly(self) -> None:
         GIVEN: A user needs to validate potential directory names
         WHEN: User checks various directory name formats
         THEN: Validation should correctly identify valid and invalid names
-        AND: Platform-specific path separators should be handled appropriately
+        AND: Path separators and traversal segments should be rejected
         """
         # Arrange & Act & Assert: Test various directory name patterns
 
         # Valid names should pass validation
         assert ProgramSettings.valid_directory_name("valid_dir_name-123") is True
         assert ProgramSettings.valid_directory_name("valid_dir_name") is True
 
-        # Platform-specific path separators
-        forward_slash_valid = ProgramSettings.valid_directory_name("valid_dir_name/")
-        assert forward_slash_valid is (platform.system() != "Windows")
-
-        backward_slash_valid = ProgramSettings.valid_directory_name("valid_dir_name\\")
-        assert backward_slash_valid is (platform.system() == "Windows")
+        # Path separators and traversal are not valid in a vehicle name
+        assert ProgramSettings.valid_directory_name("valid_dir_name/child") is False
+        assert ProgramSettings.valid_directory_name("valid_dir_name\\child") is False
+        assert ProgramSettings.valid_directory_name("../escape") is False
+        assert ProgramSettings.valid_directory_name("..") is False
+        assert ProgramSettings.valid_directory_name(".") is False
 
         # Invalid characters should fail validation
         assert ProgramSettings.valid_directory_name("invalid<dir>name") is False
         assert ProgramSettings.valid_directory_name("invalid*dir?name") is False
 
+    def test_windows_specific_vehicle_names_are_rejected(self) -> None:
+        """
+        User receives consistent validation for Windows-incompatible vehicle names.
+
+        GIVEN: AMC is validating a new vehicle name for a Windows environment
+        WHEN: The candidate name maps to a reserved device file or ends with a trailing dot
+        THEN: Validation should fail before AMC attempts to create the directory
+        """
+        with patch("ardupilot_methodic_configurator.backend_filesystem_program_settings.IS_WINDOWS", new=True):
+            assert ProgramSettings.valid_directory_name("CON") is False
+            assert ProgramSettings.valid_directory_name("Lpt1.param") is False
+            assert ProgramSettings.valid_directory_name("vehicle.") is False
+            assert ProgramSettings.valid_directory_name("vehicle.v1") is True
+
 
 class TestUserConfigurationAccess:
     """Test user configuration directory access and settings management."""

tests/test_data_model_vehicle_project_creator.py

@@ -202,16 +202,16 @@ def test_user_cannot_create_project_with_nonexistent_template_directory(self, pr
         new_base_dir = "/valid/base/dir"
         new_vehicle_name = "valid_name"
 
-        with patch.object(LocalFilesystem, "directory_exists", return_value=False):
+        with (
+            patch.object(LocalFilesystem, "directory_exists", return_value=False),
+            pytest.raises(VehicleProjectCreationError) as exc_info,
+        ):
             # Act & Assert: Should raise validation error
-            with pytest.raises(VehicleProjectCreationError) as exc_info:
-                project_creator.create_new_vehicle_from_template(
-                    template_dir, new_base_dir, new_vehicle_name, default_settings
-                )
+            project_creator.create_new_vehicle_from_template(template_dir, new_base_dir, new_vehicle_name, default_settings)
 
-            assert "Vehicle template directory" in exc_info.value.title
-            assert "does not exist" in exc_info.value.message
-            assert template_dir in exc_info.value.message
+        assert "Vehicle template directory" in exc_info.value.title
+        assert "does not exist" in exc_info.value.message
+        assert template_dir in exc_info.value.message
 
     def test_user_cannot_create_project_with_empty_vehicle_name(self, project_creator, default_settings) -> None:
         """
@@ -226,15 +226,15 @@ def test_user_cannot_create_project_with_empty_vehicle_name(self, project_creato
         new_base_dir = "/valid/base/dir"
         new_vehicle_name = ""
 
-        with patch.object(LocalFilesystem, "directory_exists", return_value=True):
+        with (
+            patch.object(LocalFilesystem, "directory_exists", return_value=True),
+            pytest.raises(VehicleProjectCreationError) as exc_info,
+        ):
             # Act & Assert: Should raise validation error
-            with pytest.raises(VehicleProjectCreationError) as exc_info:
-                project_creator.create_new_vehicle_from_template(
-                    template_dir, new_base_dir, new_vehicle_name, default_settings
-                )
+            project_creator.create_new_vehicle_from_template(template_dir, new_base_dir, new_vehicle_name, default_settings)
 
-            assert "New vehicle directory" in exc_info.value.title
-            assert "must not be empty" in exc_info.value.message
+        assert "New vehicle directory" in exc_info.value.title
+        assert "must not be empty" in exc_info.value.message
 
     def test_user_cannot_create_project_with_invalid_vehicle_name(self, project_creator, default_settings) -> None:
         """
@@ -263,6 +263,28 @@ def test_user_cannot_create_project_with_invalid_vehicle_name(self, project_crea
             assert "invalid characters" in exc_info.value.message
             assert new_vehicle_name in exc_info.value.message
 
+    def test_user_cannot_create_project_with_traversal_style_vehicle_name(self, project_creator, default_settings) -> None:
+        """
+        User receives validation error when vehicle name contains path traversal syntax.
+
+        GIVEN: A user attempts to create a vehicle project
+        WHEN: They provide a vehicle name that looks like a relative path
+        THEN: AMC rejects it before any filesystem changes are attempted
+        """
+        template_dir = "/valid/template/dir"
+        new_base_dir = "/valid/base/dir"
+        new_vehicle_name = "../escape"
+
+        with (
+            patch.object(LocalFilesystem, "directory_exists", return_value=True),
+            pytest.raises(VehicleProjectCreationError) as exc_info,
+        ):
+            project_creator.create_new_vehicle_from_template(template_dir, new_base_dir, new_vehicle_name, default_settings)
+
+        assert "New vehicle directory" in exc_info.value.title
+        assert "invalid characters" in exc_info.value.message
+        project_creator.local_filesystem.create_new_vehicle_dir.assert_not_called()
+
 
 class TestVehicleProjectCreationWorkflow:
     """Test complete vehicle project creation workflows."""