Add forbidden windows commands to stitching and utilities to check for file name sanitation. Unit tests also added

src/openflexure_microscope_server/stitching.py

@@ -48,18 +48,46 @@ class StitcherValidationError(RuntimeError):
     """The stitcher received values that it deems unsafe to create a command from."""
 
 
+# A list of commands that are forbidden in any part of a generated CLI command.
+# This provides defense-in-depth against trying to execute arbitrary shells
+# or elevation tools.
+_FORBIDDEN_COMMANDS = {
+    "sudo",
+    "sh",
+    "bash",
+    "python",
+    "python3",
+    "perl",
+    "ruby",
+    "php",
+    "nc",
+    "netcat",
+    "curl",
+    "wget",
+}
+
+
 def validate_command(cmd: list[str]) -> None:
-    """Validate that the command only characters that are allowed in a path.
+    """Validate that the command only contains characters that are allowed in a path.
 
     The values in the commands should be numbers, commandline flags, paths, and
     executables. All of these should be allowed by ``make_path_safe``.
 
+    This also checks against a blacklist of forbidden commands for defense-in-depth.
+
     :raises StitcherValidationError: if any element in the command is not safe.
     """
     for element in cmd:
+        # Check against forbidden commands (case-insensitive)
+        if element.lower() in _FORBIDDEN_COMMANDS:
+            raise StitcherValidationError(
+                f"Invalid stiching command: Forbidden element '{element}' detected."
+            )
+
+        # Ensure characters are safe for a path component
         if element != make_path_safe(element):
             raise StitcherValidationError(
-                "Invalid stiching command: Contains unsafe characters."
+                f"Invalid stiching command: Element '{element}' contains unsafe characters."
             )
 
 

src/openflexure_microscope_server/utilities.py

@@ -83,6 +83,32 @@ _POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/]")
 # Matches anything that isn't a-z, A-Z, 0-9, _, ., -
 _NAME_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-]")
 
+# Windows reserved names (case-insensitive)
+_WINDOWS_RESERVED_NAMES = {
+    "CON",
+    "PRN",
+    "AUX",
+    "NUL",
+    "COM1",
+    "COM2",
+    "COM3",
+    "COM4",
+    "COM5",
+    "COM6",
+    "COM7",
+    "COM8",
+    "COM9",
+    "LPT1",
+    "LPT2",
+    "LPT3",
+    "LPT4",
+    "LPT5",
+    "LPT6",
+    "LPT7",
+    "LPT8",
+    "LPT9",
+}
+
 
 def make_path_safe(unsafe_path_string: str) -> str:
     """Replace unsafe characters in a file path with underscores, preserving separators.
@@ -94,16 +120,41 @@ def make_path_safe(unsafe_path_string: str) -> str:
     including path separators (forward slash on POSIX, and forward
     slash/backslash/colon on Windows).
 
+    Additionally, it sanitises each path component for Windows reserved names and
+    trailing dots/spaces to ensure maximum cross-platform compatibility.
+
     :param unsafe_path_string: The original path string to sanitise.
 
     :returns: A version of the input string safe to use as a file path.
     """
+    # Split by separators first to sanitise components independently
+    components = re.split(r"([/\\])", unsafe_path_string)
+    sanitised_components = []
+
     unsafe_character_pattern = (
         _WINDOWS_UNSAFE_PATTERN
         if sys.platform.startswith("win")
         else _POSIX_UNSAFE_PATTERN
     )
-    return unsafe_character_pattern.sub("_", unsafe_path_string)
+
+    for component in components:
+        if component in ("/", "\\"):
+            sanitised_components.append(component)
+        elif component:
+            # 1. Strip trailing dots and spaces
+            # 2. Apply unsafe character regex
+            # 3. Check for reserved names
+            sanitised = component.rstrip(". ")
+            if not sanitised:
+                sanitised = "_"
+            else:
+                sanitised = unsafe_character_pattern.sub("_", sanitised)
+
+            sanitised_components.append(_sanitise_reserved(sanitised))
+        else:
+            sanitised_components.append(component)
+
+    return "".join(sanitised_components)
 
 
 def make_name_safe(unsafe_name_string: str) -> str:
@@ -112,11 +163,37 @@ def make_name_safe(unsafe_name_string: str) -> str:
     This excludes all path separators, ensuring the result is safe to use as a
     standalone filename or identifier component.
 
+    This also handles Windows reserved names and trailing dots/spaces.
+
     :param unsafe_name_string: The original name string to sanitise.
 
     :returns: A version of the input string safe to use as a file name or identifier.
     """
-    return _NAME_UNSAFE_PATTERN.sub("_", unsafe_name_string)
+    # 1. Strip trailing dots and spaces
+    # 2. Apply unsafe character regex
+    # 3. Check for reserved names
+    name = unsafe_name_string.rstrip(". ")
+    if not name:
+        return "_"
+
+    name = _NAME_UNSAFE_PATTERN.sub("_", name)
+    return _sanitise_reserved(name)
+
+
+def _sanitise_reserved(name: str) -> str:
+    """Check a name against Windows reserved names.
+
+    :param name: The component to check.
+    :returns: The sanitised component.
+    """
+    # Check for Windows reserved names.
+    # These are reserved even with an extension (e.g. NUL.txt).
+    # We check the part before the first dot.
+    base_name = name.split(".", maxsplit=1)[0].upper()
+    if base_name in _WINDOWS_RESERVED_NAMES:
+        return f"{name}_"
+
+    return name
 
 
 class VersionData(BaseModel):

tests/unit_tests/test_stitching_validation.py

@@ -0,0 +1,49 @@
+"""Unit tests for stitching command validation."""
+
+import pytest
+
+from openflexure_microscope_server.stitching import (
+    StitcherValidationError,
+    validate_command,
+)
+
+
+def test_validate_command_success():
+    """Test valid commands pass validation."""
+    validate_command(["openflexure-stitch", "--stitching_mode", "all", "path/to/scan"])
+    validate_command(["--resize", "0.5", "8192"])
+
+
+def test_validate_command_forbidden():
+    """Test forbidden commands raise error."""
+    with pytest.raises(
+        StitcherValidationError, match="Forbidden element 'sudo' detected"
+    ):
+        validate_command(["sudo", "rm", "-rf", "/"])
+
+    with pytest.raises(
+        StitcherValidationError, match="Forbidden element 'python' detected"
+    ):
+        validate_command(["python", "-c", "print('hello')"])
+
+    with pytest.raises(
+        StitcherValidationError, match="Forbidden element 'sh' detected"
+    ):
+        validate_command(["sh", "-c", "whoami"])
+
+
+def test_validate_command_unsafe_chars():
+    """Test elements with unsafe characters raise error."""
+    with pytest.raises(StitcherValidationError, match="contains unsafe characters"):
+        validate_command(["openflexure-stitch", "path;rm -rf /"])
+
+    with pytest.raises(StitcherValidationError, match="contains unsafe characters"):
+        validate_command(["openflexure-stitch", "path&echo hello"])
+
+
+def test_validate_command_case_insensitive():
+    """Test forbidden command check is case-insensitive."""
+    with pytest.raises(
+        StitcherValidationError, match="Forbidden element 'SUDO' detected"
+    ):
+        validate_command(["SUDO", "ls"])

tests/unit_tests/test_utilities.py

@@ -0,0 +1,71 @@
+"""Unit tests for utility functions."""
+
+import sys
+
+from openflexure_microscope_server.utilities import make_name_safe, make_path_safe
+
+
+def test_make_name_safe_basic():
+    """Test basic functionality of make_name_safe."""
+    assert make_name_safe("normal_name") == "normal_name"
+    assert make_name_safe("name with spaces") == "name_with_spaces"
+    assert make_name_safe("name/with/slashes") == "name_with_slashes"
+    assert make_name_safe("name\\with\\backslashes") == "name_with_backslashes"
+
+
+def test_make_name_safe_trailing_chars():
+    """Test that trailing dots and spaces are removed."""
+    assert make_name_safe("name.") == "name"
+    assert make_name_safe("name ") == "name"
+    assert make_name_safe("name. ") == "name"
+    assert make_name_safe("name .") == "name"
+    assert make_name_safe(".") == "_"
+    assert make_name_safe(" ") == "_"
+    assert make_name_safe(". ") == "_"
+
+
+def test_make_name_safe_reserved_names():
+    """Test Windows reserved names."""
+    assert make_name_safe("CON") == "CON_"
+    assert make_name_safe("con") == "con_"
+    assert make_name_safe("NUL.txt") == "NUL.txt_"
+    assert make_name_safe("COM1") == "COM1_"
+    assert make_name_safe("LPT9.tar.gz") == "LPT9.tar.gz_"
+    # Check that names STARTING with reserved names but not followed by . or end are safe
+    assert make_name_safe("CONSTANT") == "CONSTANT"
+    assert make_name_safe("CON2") == "CON2"
+
+
+def test_make_path_safe_basic():
+    """Test basic functionality of make_path_safe."""
+    # Note: behavior depends on platform for separators
+    if sys.platform.startswith("win"):
+        assert make_path_safe("C:\\path\\to/file") == "C:\\path\\to/file"
+    else:
+        # On POSIX, \ is not a separator and might be replaced by _ if not in allowed pattern
+        # The regex for POSIX is [^a-zA-Z0-9_.\-/]
+        # And : is also not allowed
+        assert make_path_safe("path/to/file") == "path/to/file"
+
+
+def test_make_path_safe_reserved_in_components():
+    """Test reserved names within path components."""
+    assert make_path_safe("path/CON/file") == "path/CON_/file"
+    assert make_path_safe("path/NUL.txt/file") == "path/NUL.txt_/file"
+
+    if sys.platform.startswith("win"):
+        assert make_path_safe("C:/AUX/test") == "C:/AUX_/test"
+    else:
+        assert make_path_safe("C:/AUX/test") == "C_/AUX_/test"
+
+
+def test_make_path_safe_trailing_in_components():
+    """Test trailing chars in path components."""
+    assert make_path_safe("path /to. /file ") == "path/to/file"
+    assert make_path_safe("path./to /file.") == "path/to/file"
+
+
+def test_make_path_safe_separators():
+    """Test that separators are preserved and components sanitised."""
+    assert make_path_safe("a/b\\c") == "a/b\\c"
+    assert make_path_safe("a./b /c.") == "a/b/c"