fix: confine datastore write paths

.github/workflows/pythonapp.yml

@@ -32,7 +32,7 @@ jobs:
     strategy:
       matrix:
         os: [windows-latest, ubuntu-latest]
-        python-version: ["3.9", "3.10", "3.11", "3.12"]
+        python-version: ["3.10", "3.11", "3.12"]
     steps:
       - uses: actions/checkout@v6
       - name: Set up Python
@@ -54,7 +54,7 @@ jobs:
       MONAI_ZOO_AUTH_TOKEN: ${{ github.token }}
     strategy:
       matrix:
-        python-version: ["3.9", "3.10", "3.11", "3.12"]
+        python-version: ["3.10", "3.11", "3.12"]
     steps:
       - uses: actions/checkout@v6
       - name: Set up Python
@@ -71,6 +71,8 @@ jobs:
         run: |
           $(pwd)/runtests.sh --clean
       - name: PyType
+        # NOTE: pytype is currently a no-op in CI (see issue #1917). The echo is
+        # intentional until the pre-existing pytype errors are triaged.
         run: |
           echo $(pwd)/runtests.sh --pytype
       - name: Unit Tests
@@ -88,7 +90,7 @@ jobs:
       MONAI_ZOO_AUTH_TOKEN: ${{ github.token }}
     strategy:
       matrix:
-        python-version: ["3.9", "3.10", "3.11", "3.12"]
+        python-version: ["3.10", "3.11", "3.12"]
     steps:
       - uses: actions/checkout@v6
         with:

monailabel/datastore/local.py

@@ -228,6 +228,25 @@ def _to_id(self, file: str) -> Tuple[str, str]:
     def _filename(self, id: str, ext: str) -> str:
         return id + ext
 
+    @staticmethod
+    def _confine_path(dest: str, allowed_base: str) -> str:
+        """Resolve ``dest`` and require it to stay inside ``allowed_base``.
+
+        IDs and tags are concatenated into filesystem paths, so the resolved
+        destination is checked before writing. Raises :class:`ValueError` if
+        it is not inside the allowed directory.
+        """
+        dest = os.path.realpath(dest)
+        allowed_base = os.path.realpath(allowed_base)
+        try:
+            inside = os.path.commonpath([dest, allowed_base]) == allowed_base
+        except ValueError:
+            # e.g. Windows paths on different drives have no common path
+            inside = False
+        if not inside:
+            raise ValueError(f"Invalid path: {dest!r} is not inside {allowed_base!r}")
+        return dest
+
     def _to_bytes(self, file):
         return io.BytesIO(pathlib.Path(file).read_bytes())
 
@@ -438,7 +457,7 @@ def add_image(self, image_id: str, image_filename: str, image_info: Dict[str, An
 
         logger.info(f"Adding Image: {image_id} => {image_filename}")
         name = self._filename(image_id, image_ext)
-        dest = os.path.realpath(os.path.join(self._datastore.image_path(), name))
+        dest = self._confine_path(os.path.join(self._datastore.image_path(), name), self._datastore.image_path())
 
         with FileLock(self._lock_file):
             logger.debug("Acquired the lock!")
@@ -493,8 +512,12 @@ def save_label(self, image_id: str, label_filename: str, label_tag: str, label_i
 
         logger.info(f"Adding Label: {image_id} => {label_tag} => {label_filename}")
         label_path = self._datastore.label_path(label_tag)
+        # Confine the label directory itself as well as the final file path, so
+        # a crafted label tag cannot redirect the write outside the labels dir.
+        labels_root = self._datastore.label_path(None)
+        label_path = self._confine_path(label_path, labels_root)
         name = self._filename(image_id, label_ext)
-        dest = os.path.join(label_path, name)
+        dest = self._confine_path(os.path.join(label_path, name), label_path)
 
         with FileLock(self._lock_file):
             logger.debug("Acquired the lock!")

monailabel/endpoints/datastore.py

@@ -69,6 +69,12 @@ def add_image(
     file_ext = "".join(pathlib.Path(file.filename).suffixes) if file.filename else ".nii.gz"
 
     image_id = image if image else os.path.basename(file.filename).replace(file_ext, "")
+    # Normalize a caller-supplied image ID to a plain file name
+    # (no directory components) before it is used to build a path.
+    if image:
+        image_id = os.path.basename(image_id)
+        if not image_id or image_id in (".", "..") or image_id.startswith("."):
+            raise HTTPException(status_code=400, detail="Invalid image ID")
     image_file = tempfile.NamedTemporaryFile(suffix=file_ext).name
 
     with open(image_file, "wb") as buffer:

monailabel/main.py

@@ -171,7 +171,8 @@ def action_datasets(self, args):
             root_dir = os.path.dirname(os.path.realpath(dataset_dir))
             os.makedirs(root_dir, exist_ok=True)
             tarfile_name = f"{dataset_dir}.tar"
-            download_and_extract(resource[args.name], tarfile_name, root_dir, md5.get(args.name))
+            # DecathlonDataset publishes md5 digests; MONAI >= 1.6 defaults hash_type to sha256
+            download_and_extract(resource[args.name], tarfile_name, root_dir, md5.get(args.name), hash_type="md5")
 
             junk_files = pathlib.Path(dataset_dir).rglob("._*")
             for j in junk_files:

monailabel/transform/writer.py

@@ -30,7 +30,13 @@
 def write_nifti(data, filename, affine=None, original_affine=None, output_dtype=np.float32):
     writer = NibabelWriter(output_dtype=output_dtype)
     writer.set_data_array(data, channel_dim=None, spatial_ndim=None)
-    writer.set_metadata({"affine": affine, "original_affine": original_affine})
+    meta = {"affine": affine, "original_affine": original_affine}
+    if original_affine is None and np.ndim(data) == 2:
+        # Without a target affine the output keeps the input grid, so the spatial shape is
+        # the input shape. Passing it explicitly works around a MONAI 1.6 regression where
+        # 2D channel-less arrays fail to resample (spatial_size=None reaches fall_back_tuple).
+        meta["spatial_shape"] = np.asarray(np.shape(data))
+    writer.set_metadata(meta)
     writer.write(filename)
 
 

requirements-dev.txt

@@ -38,3 +38,5 @@ click>=8.1.2
 testresources
 pre-commit
 attrs>=22.1.0
+# optional at runtime (see requirements.txt); needed by the ITK DICOM SEG unit tests
+dcmqi

requirements.txt

@@ -28,6 +28,9 @@ pydicom==3.0.1
 highdicom==0.26.1
 pynetdicom==2.0.2
 pynrrd==1.1.3
+SimpleITK
+pyOpenSSL>=26.0.0
+huggingface_hub
 numpymaxflow==0.0.7; python_version < "3.13"
 setuptools>=61
 setuptools-scm<8.0.0

setup.cfg

@@ -54,6 +54,9 @@ install_requires =
     highdicom>=0.26.1
     pynetdicom>=2.0.2
     pynrrd>=1.1.3
+    SimpleITK
+    pyOpenSSL>=26.0.0
+    huggingface_hub
     girder-client>=3.2.3
     ninja>=1.11.1.1
     einops>=0.7.0

tests/unit/datastore/test_convert.py

@@ -264,8 +264,6 @@ def test_binary_to_image(self):
             count_diff_ratio < 0.01
         ), f"Nonzero count changed significantly: {original_nonzero_count} vs {result_nonzero_count}"
 
-        os.unlink(result)
-
     def _test_nifti_to_dicom_seg_with_label_info_impl(self, use_itk):
         """Helper: Test NIfTI to DICOM SEG conversion with custom label info."""
         series_dir, label = self._get_test_paths()

tests/unit/datastore/test_local_datastore_path_confinement.py

@@ -0,0 +1,71 @@
+# Copyright (c) MONAI Consortium
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#     http://www.apache.org/licenses/LICENSE-2.0
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+import shutil
+import tempfile
+import unittest
+
+from monailabel.datastore.local import LocalDatastore
+
+
+class TestLocalDatastorePathConfinement(unittest.TestCase):
+    """Image/label IDs and label tags must resolve inside the datastore image/labels directories."""
+
+    def setUp(self):
+        self.base = tempfile.mkdtemp(prefix="monailabel_ds_confine_")
+        self.studies = os.path.join(self.base, "studies")
+        os.makedirs(self.studies, exist_ok=True)
+        self.datastore = LocalDatastore(self.studies)
+
+        self.image_file = os.path.join(self.base, "image.nii.gz")
+        with open(self.image_file, "wb") as f:
+            f.write(b"image")
+
+        self.image_id = "image_1"
+        self.datastore.add_image(self.image_id, self.image_file, {})
+
+    def tearDown(self):
+        shutil.rmtree(self.base, ignore_errors=True)
+
+    def test_add_image_rejects_id_outside_image_dir(self):
+        outside = os.path.realpath(os.path.join(self.base, "other.nii.gz"))
+        with self.assertRaises(ValueError):
+            self.datastore.add_image("../../other", self.image_file, {})
+        self.assertFalse(os.path.exists(outside))
+
+    def test_add_image_accepts_plain_id(self):
+        image_id = self.datastore.add_image("image_2", self.image_file, {})
+        self.assertEqual(image_id, "image_2")
+        self.assertTrue(os.path.exists(os.path.join(self.studies, "image_2.nii.gz")))
+
+    def test_save_label_rejects_tag_outside_labels_dir(self):
+        outside = os.path.realpath(os.path.join(self.base, "labels", "final", "other.nii.gz"))
+        with self.assertRaises(ValueError):
+            self.datastore.save_label(self.image_id, self.image_file, "../../labels/final/other", {})
+        self.assertFalse(os.path.exists(outside))
+
+    def test_save_label_rejects_image_id_outside_labels_dir(self):
+        # an existing datastore entry whose ID does not resolve inside the labels directory
+        self.datastore._datastore.objects["../../other"] = self.datastore._datastore.objects[self.image_id]
+        outside = os.path.realpath(os.path.join(self.base, "other.nii.gz"))
+        with self.assertRaises(ValueError):
+            self.datastore.save_label("../../other", self.image_file, "final", {})
+        self.assertFalse(os.path.exists(outside))
+
+    def test_save_label_accepts_plain_input(self):
+        label_id = self.datastore.save_label(self.image_id, self.image_file, "final", {})
+        self.assertEqual(label_id, self.image_id)
+        self.assertTrue(os.path.exists(os.path.join(self.studies, "labels", "final", "image_1.nii.gz")))
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/scribbles/test_transforms_infer.py

@@ -12,8 +12,10 @@
 import unittest
 
 import numpy as np
+import pytest
+from monai import __version__ as monai_version
 from monai.transforms import LoadImage
-from monai.utils import set_determinism
+from monai.utils import set_determinism, version_geq
 from parameterized import parameterized
 
 from monailabel.scribbles.infer import HistogramBasedGraphCut
@@ -388,6 +390,12 @@ def test_soften_prob_softmax_transforms(self):
 
 class TestScribblesInferers(unittest.TestCase):
     @parameterized.expand(TEST_CASE_HISTOGRAM_GRAPHCUT)
+    @pytest.mark.xfail(
+        version_geq(monai_version, "1.6"),
+        reason="MONAI >= 1.6 Spacingd regression on '<key>_*' non-dict entries such as image_path "
+        "(Project-MONAI/MONAI#9165, fix in Project-MONAI/MONAI#9167)",
+        strict=False,
+    )
     @unittest.skipUnless(has_numpymaxflow, "numpymaxflow is not installed")
     def test_histogram_graphcut_inferer(self, test_input, expected_shape):
         test_input.update({"image_path": "fakepath.nii"})