fix(tmpdir): address full-pr-review findings for suffix robustness and security

- Add LocalMavenLocation/LocalPypiLocation to __extract_location_provenance
- Change unknown location type fallthrough to log warning and return "unknown"
- Apply _SUFFIX_MAX_LEN cap to full suffix string (prefix + uri), not just uri
- Replace consecutive dots (..) with _ to prevent path traversal via mkdir
- Add containment guard in get_suffix_path (resolve-based, is_relative_to)
- Redact Git URL userinfo (credentials) before using as location_uri
- Extract _SUFFIX_MAX_LEN and _UNSAFE_PATH_CHARS as module-level constants
- Deduplicate test capturing closure into shared _capture_suffix_calls()
- Strengthen test assertions: URI fragment presence, safe-char regex, length guard

Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>

Author: jimisola

src/reqstool/common/utils.py

@@ -213,6 +213,9 @@ def get_path(self) -> Path:
 
     def get_suffix_path(self, suffix: str) -> Path:
         new_path = Path(self._tmpdir.name) / str(self._count) / suffix
+        root = Path(self._tmpdir.name).resolve()
+        if not new_path.resolve().is_relative_to(root):
+            raise ValueError(f"suffix {suffix!r} would escape the managed temp directory")
         new_path.mkdir(parents=True, exist_ok=True)
         self._count += 1
         return new_path

src/reqstool/model_generators/combined_raw_datasets_generator.py

@@ -4,6 +4,9 @@
 import os
 import re
 from collections import defaultdict
+
+_SUFFIX_MAX_LEN = 80
+_UNSAFE_PATH_CHARS = re.compile(r"[^a-zA-Z0-9._-]")
 from typing import Dict, List, Optional, Set, Tuple
 
 from reqstool_python_decorators.decorators.decorators import Requirements
@@ -255,7 +258,9 @@ def __parse_source(self, current_location_handler: LocationResolver) -> RawDatas
         automated_tests = None
 
         location_type, location_uri = self.__extract_location_provenance(current_location_handler.current)
-        safe_suffix = f"{location_type}_" + re.sub(r"[^a-zA-Z0-9._-]", "_", location_uri or "")[:80]
+        type_prefix = location_type or "unknown"
+        sanitized_uri = re.sub(r"\.{2,}", "_", _UNSAFE_PATH_CHARS.sub("_", location_uri or ""))
+        safe_suffix = f"{type_prefix}_{sanitized_uri}"[:_SUFFIX_MAX_LEN]
         tmp_path = self._tmpdir_manager.get_suffix_path(safe_suffix).absolute()
 
         actual_tmp_path = current_location_handler.make_available_on_localdisk(dst_path=tmp_path)
@@ -302,19 +307,31 @@ def __parse_source(self, current_location_handler: LocationResolver) -> RawDatas
     @staticmethod
     def __extract_location_provenance(location: LocationInterface) -> tuple:
         """Extract location_type and location_uri from a resolved location."""
+        from urllib.parse import urlparse, urlunparse
+
         from reqstool.locations.git_location import GitLocation
+        from reqstool.locations.local_maven_location import LocalMavenLocation
+        from reqstool.locations.local_pypi_location import LocalPypiLocation
         from reqstool.locations.maven_location import MavenLocation
         from reqstool.locations.pypi_location import PypiLocation
 
         if isinstance(location, LocalLocation):
             return "local", f"file://{os.path.abspath(location.path)}"
         elif isinstance(location, GitLocation):
-            return "git", location.url
+            parsed = urlparse(location.url)
+            if parsed.username or parsed.password:
+                parsed = parsed._replace(netloc=parsed.hostname + (f":{parsed.port}" if parsed.port else ""))
+            return "git", urlunparse(parsed)
         elif isinstance(location, MavenLocation):
             return "maven", f"{location.group_id}:{location.artifact_id}:{location.version}"
         elif isinstance(location, PypiLocation):
             return "pypi", f"{location.package}=={location.version}"
-        return None, None
+        elif isinstance(location, LocalMavenLocation):
+            return "local_maven", f"file://{os.path.abspath(location.path)}"
+        elif isinstance(location, LocalPypiLocation):
+            return "local_pypi", f"file://{os.path.abspath(location.path)}"
+        logging.warning("Unknown location type %s; using 'unknown' prefix for tmp dir", type(location).__name__)
+        return "unknown", None
 
     @staticmethod
     def __extract_source_paths(location: LocationInterface, requirements_indata: RequirementsIndata) -> Dict[str, str]:

tests/unit/reqstool/model_generators/test_combined_raw_datasets_generator.py

@@ -1,5 +1,7 @@
 # Copyright © LFV
 
+import contextlib
+import re
 from unittest.mock import patch
 
 import pytest
@@ -141,50 +143,54 @@ def test_implementation_traversal_recursive(local_testdata_resources_rootdir_w_p
     assert ("lib-c", "implementation") in crd.parsing_graph["lib-b"]
 
 
-@SVCs("SVC_020")
-def test_tmpdir_suffix_local_uses_local_prefix():
-    captured_suffixes = []
+@contextlib.contextmanager
+def _capture_suffix_calls():
+    captured = []
     original = TempDirectoryManager.get_suffix_path
 
-    def capturing(self, suffix):
-        captured_suffixes.append(suffix)
+    def _capturing(self, suffix):
+        captured.append(suffix)
         return original(self, suffix)
 
-    with patch.object(TempDirectoryManager, "get_suffix_path", capturing):
+    with patch.object(TempDirectoryManager, "get_suffix_path", _capturing):
+        yield captured
+
+
+def _assert_safe_suffix(suffix, expected_prefix, uri_fragment):
+    assert len(suffix) >= 1, "get_suffix_path was never called"
+    assert suffix.startswith(expected_prefix)
+    assert uri_fragment in suffix
+    assert re.match(r"^[a-zA-Z0-9._-]+$", suffix), f"Suffix contains unsafe chars: {suffix!r}"
+
+
+@SVCs("SVC_020")
+def test_tmpdir_suffix_local_uses_local_prefix():
+    with _capture_suffix_calls() as captured_suffixes:
         with pytest.raises(MissingRequirementsFileError):
             CombinedRawDatasetsGenerator(
                 initial_location=LocalLocation(path="/nonexistent/path"),
                 semantic_validator=SemanticValidator(validation_error_holder=ValidationErrorHolder()),
             )
-
-    assert captured_suffixes[0].startswith("local_")
-    assert "can_we_use_urn_here" not in captured_suffixes[0]
+    assert len(captured_suffixes) >= 1, "get_suffix_path was never called"
+    _assert_safe_suffix(captured_suffixes[0], "local_", "nonexistent")
 
 
 @SVCs("SVC_020")
 @pytest.mark.parametrize(
-    "location,expected_prefix",
+    "location,expected_prefix,uri_fragment",
     [
-        (GitLocation(url="https://github.com/org/repo", branch="main"), "git_"),
-        (MavenLocation(group_id="com.example", artifact_id="my-artifact", version="1.0.0"), "maven_"),
-        (PypiLocation(package="my-package", version="1.0.0"), "pypi_"),
+        (GitLocation(url="https://github.com/org/repo", branch="main"), "git_", "github.com"),
+        (MavenLocation(group_id="com.example", artifact_id="my-artifact", version="1.0.0"), "maven_", "my-artifact"),
+        (PypiLocation(package="my-package", version="1.0.0"), "pypi_", "my-package"),
     ],
 )
-def test_tmpdir_suffix_remote_uses_location_type_prefix(tmp_path, location, expected_prefix):
-    captured_suffixes = []
-    original = TempDirectoryManager.get_suffix_path
-
-    def capturing(self, suffix):
-        captured_suffixes.append(suffix)
-        return original(self, suffix)
-
-    with patch.object(TempDirectoryManager, "get_suffix_path", capturing):
+def test_tmpdir_suffix_remote_uses_location_type_prefix(tmp_path, location, expected_prefix, uri_fragment):
+    with _capture_suffix_calls() as captured_suffixes:
         with patch.object(LocationResolver, "make_available_on_localdisk", return_value=str(tmp_path)):
             with pytest.raises(MissingRequirementsFileError):
                 CombinedRawDatasetsGenerator(
                     initial_location=location,
                     semantic_validator=SemanticValidator(validation_error_holder=ValidationErrorHolder()),
                 )
-
-    assert captured_suffixes[0].startswith(expected_prefix)
-    assert "can_we_use_urn_here" not in captured_suffixes[0]
+    assert len(captured_suffixes) >= 1, "get_suffix_path was never called"
+    _assert_safe_suffix(captured_suffixes[0], expected_prefix, uri_fragment)