Allow dots within filepaths if not trailing.

This commit is contained in:
Julian Stirling 2026-06-02 14:43:03 +01:00
parent 67560a21f8
commit fe409ccefb
2 changed files with 87 additions and 72 deletions

View file

@ -9,6 +9,7 @@ import tomllib
from datetime import datetime from datetime import datetime
from functools import wraps from functools import wraps
from importlib.metadata import version from importlib.metadata import version
from pathlib import PurePath
from typing import ( from typing import (
Any, Any,
Callable, Callable,
@ -80,6 +81,10 @@ def requires_lock(
_WINDOWS_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-:/\ \\]") _WINDOWS_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-:/\ \\]")
# Matches anything that isn't a-z, A-Z, 0-9, _, ., -, \, space # Matches anything that isn't a-z, A-Z, 0-9, _, ., -, \, space
_POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/ ]") _POSIX_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-/ ]")
UNSAFE_PATTERN = (
_WINDOWS_UNSAFE_PATTERN if sys.platform.startswith("win") else _POSIX_UNSAFE_PATTERN
)
# Matches anything that isn't a-z, A-Z, 0-9, _, ., - # Matches anything that isn't a-z, A-Z, 0-9, _, ., -
_NAME_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-]") _NAME_UNSAFE_PATTERN = re.compile(r"[^a-zA-Z0-9_.\-]")
@ -121,86 +126,83 @@ def is_path_safe(unsafe_path_string: str) -> bool:
:returns: A boolean indicating if any unsafe features were found. :returns: A boolean indicating if any unsafe features were found.
""" """
# Split by separators first to sanitise components independently return all(
[
components = re.split(r"([/\\])", unsafe_path_string) _path_normalised(unsafe_path_string),
_no_trailing_dots_or_whitespaces(unsafe_path_string),
unsafe_character_pattern = ( _no_unsafe_characters(unsafe_path_string),
_WINDOWS_UNSAFE_PATTERN _no_reserved_name(unsafe_path_string),
if sys.platform.startswith("win") ]
else _POSIX_UNSAFE_PATTERN
) )
# Ensure each warning only gets logged once per path
unsafe_relative_path = False
trailing_dots = False
trailing_whitespace = False
unsafe_characters = False
reserved_word = False
for i, component in enumerate(components): def _path_normalised(unsafe_path_string: str) -> bool:
# Skip separators and empty strings """Check that the path starts normalised.
if component in ("/", "\\") or not component:
Warn and return false if . or .. is not at the start of the path.
"""
# Ensure correct path separators before normalisation:
if sys.platform.startswith("win"):
unsafe_path_string = unsafe_path_string.replace("/", "\\")
normalised = os.path.normpath(unsafe_path_string)
# Allow starting with ./ (or .\ on Windows)
if unsafe_path_string.startswith("." + os.path.sep):
normalised = "." + os.path.sep + normalised
# Allow trailing path separator
if unsafe_path_string.endswith(os.path.sep):
normalised += os.path.sep
if normalised != unsafe_path_string:
LOGGER.warning(
f"File path {unsafe_path_string} may be unsafe due to unexpected relative "
"navigation."
)
return False
return True
def _no_trailing_dots_or_whitespaces(unsafe_path_string: str) -> bool:
"""Disallow trailing dots or whitespace."""
for component in PurePath(unsafe_path_string).parts:
if component == "..":
continue continue
bad_trailing_chars = ". " if sys.platform.startswith("win") else "."
# 1. Check for relative paths in the wrong place if component.rstrip(bad_trailing_chars) != component:
if component in (".", "..") and not unsafe_relative_path:
is_first = i == 0
is_second = (
i == 2
and components[i - 1] in ("/", "\\")
and components[i - 2] == ".."
)
if not (is_first or is_second):
LOGGER.warning(
f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation."
)
unsafe_relative_path = True
continue
# 2. Check for trailing dots in path
if "." in component and i != len(components) - 1 and not trailing_dots:
# Check for trailing dots in the file path before the file name
# e.g.: foo/bar./file is bad, but foo/bar.file.py is fine
LOGGER.warning( LOGGER.warning(
f"File path {unsafe_path_string} may be unsafe due to trailing dots." f"File path {unsafe_path_string} may be unsafe due to trailing dots "
"or whitespace."
) )
trailing_dots = True return False
# All passed
return True
# Check for trailing spaces - Windows specific
if (
sys.platform.startswith("win")
and component.endswith(" ")
and not trailing_whitespace
):
LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.")
trailing_whitespace = True
# 3. Check for unsafe characters (Regex) def _no_unsafe_characters(unsafe_path_string: str) -> bool:
if unsafe_character_pattern.search(component) and not unsafe_characters: """Disallow unsafe characters."""
for component in PurePath(unsafe_path_string).parts:
if UNSAFE_PATTERN.search(component):
LOGGER.warning( LOGGER.warning(
f"{unsafe_path_string} contains characters that may be unsafe on this platform." f"{unsafe_path_string} contains characters that may be unsafe on this "
"platform."
) )
unsafe_characters = True return False
# All passed
return True
# 4. Check for Reserved Names (e.g., CON, PRN, LPT1)
# Assuming _sanitise_reserved returns a different string if it's a reserved name def _no_reserved_name(unsafe_path_string: str) -> bool:
if ( """Disallow use of Windows reserved names (e.g., CON, PRN, LPT1)."""
_sanitise_reserved(component, is_filename=False) != component for component in PurePath(unsafe_path_string).parts:
and not reserved_word if _sanitise_reserved(component, is_filename=False) != component:
):
LOGGER.warning( LOGGER.warning(
f"{unsafe_path_string} contains a reserved system name and may cause issues." f"{unsafe_path_string} contains a reserved system name and may cause "
"issues."
) )
reserved_word = True return False
# All passed
return not ( return True
unsafe_relative_path
or trailing_dots
or trailing_whitespace
or unsafe_characters
or reserved_word
)
def make_name_safe(unsafe_name_string: str) -> str: def make_name_safe(unsafe_name_string: str) -> str:

View file

@ -11,7 +11,9 @@ from openflexure_microscope_server.utilities import (
make_name_safe, make_name_safe,
) )
PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."} BAD_PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."}
GOOD_PATHS_WITH_DOTS = {"a.a/b/.c", "path.to/file.file.file", "./path.to/file.files"}
PATHS_WITH_SPACES = {"path /to /file ", "path/to /file", "path/a long path name /file"} PATHS_WITH_SPACES = {"path /to /file ", "path/to /file", "path/a long path name /file"}
@ -130,7 +132,10 @@ def test_is_path_safe_trailing_space_in_components(caplog, path):
if sys.platform.startswith("win"): if sys.platform.startswith("win"):
assert not is_path_safe(path) assert not is_path_safe(path)
assert len(caplog.records) == 1 assert len(caplog.records) == 1
assert f"{path} contains unsafe trailing spaces." in caplog.messages assert (
f"File path {path} may be unsafe due to trailing dots or whitespace."
in caplog.messages
)
else: else:
# Trailing spaces allows in POSIX systems # Trailing spaces allows in POSIX systems
assert is_path_safe(path) assert is_path_safe(path)
@ -158,12 +163,20 @@ def test_is_path_safe_relative(caplog):
) )
@pytest.mark.parametrize("filepath", PATHS_WITH_DOTS) @pytest.mark.parametrize("filepath", BAD_PATHS_WITH_DOTS)
def test_is_path_safe_dots_warning(caplog, filepath): def test_is_path_safe_with_trailing_dots_warning(caplog, filepath):
"""Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged.""" """Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged."""
with caplog.at_level(logging.WARNING): with caplog.at_level(logging.WARNING):
assert not is_path_safe(filepath) assert not is_path_safe(filepath)
assert ( assert (
f"File path {filepath} may be unsafe due to trailing dots." f"File path {filepath} may be unsafe due to trailing dots or whitespace."
in caplog.messages in caplog.messages
) )
@pytest.mark.parametrize("filepath", GOOD_PATHS_WITH_DOTS)
def test_is_path_safe_allowed_dots_warning(caplog, filepath):
"""Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged."""
with caplog.at_level(logging.WARNING):
assert is_path_safe(filepath)
assert len(caplog.records) == 0