Merge branch 'dev' into fix/ghsa-download-integrity

Author: ericspod

.pre-commit-config.yaml

@@ -13,11 +13,13 @@ repos:
     hooks:
       - id: end-of-file-fixer
       - id: trailing-whitespace
+      - id: check-ast
       - id: check-yaml
       - id: check-docstring-first
       - id: check-executables-have-shebangs
       - id: check-toml
       - id: check-case-conflict
+      - id: check-illegal-windows-names
       - id: check-added-large-files
         args: ['--maxkb=1024']
       - id: detect-private-key
@@ -26,8 +28,9 @@ repos:
         args: ['--autofix', '--no-sort-keys', '--indent=4']
       - id: end-of-file-fixer
       - id: mixed-line-ending
+
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.15.20
+    rev: v0.16.5
     hooks:
     -   id: ruff-check
         args: ["--fix"]
@@ -37,8 +40,25 @@ repos:
               ^monai/_version.py
           )
 
-  - repo: https://github.com/hadialqattan/pycln
-    rev: v2.6.0
+  - repo: https://github.com/psf/black-pre-commit-mirror
+    rev: 26.5.1  # Black version, keep synced with MONAI requirements
     hooks:
-      - id: pycln
-        args: [--config=pyproject.toml]
+      - id: black
+        language_version: python3
+        # black will be given individual file names and so will ignore the excludes in pyproject.toml
+        exclude: |
+          (?x)(
+              ^versioneer.py|
+              ^monai/_version.py
+          )
+
+  - repo: https://github.com/pycqa/isort
+    rev: 9.0.1  # isort version, keep synced with MONAI requirements
+    hooks:
+      - id: isort
+        name: isort (python)
+        exclude: |
+          (?x)(
+              ^versioneer.py|
+              ^monai/_version.py
+          )

monai/auto3dseg/utils.py

@@ -493,6 +493,14 @@ def algo_from_json(filename: str, template_path: PathLike | None = None, **kwarg
             if state_template_path:
                 algo_config["template_path"] = state_template_path
 
+            warnings.warn(
+                f"Loading {filename}: the file's `_target_` value is resolved to an imported callable and "
+                "invoked, and template directories from the file may be added to `sys.path`; only load "
+                "algo_object.json files from a source you trust "
+                "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv).",
+                stacklevel=2,
+            )
+
             parser = ConfigParser(algo_config)
             algo = parser.get_parsed_content()
             used_template_path = path

monai/data/image_reader.py

@@ -347,6 +347,7 @@ def _get_affine(self, img, lps_to_ras: bool = True):
         affine: np.ndarray = np.eye(sr + 1)
         affine[:sr, :sr] = direction[:sr, :sr] @ np.diag(spacing[:sr])
         affine[:sr, -1] = origin[:sr]
+
         if lps_to_ras:
             affine = orientation_ras_lps(affine)
         return affine
@@ -752,13 +753,25 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
                 stacklevel=2,
             )
             return affine
+
+        def _raise_if_not_finite(values: Sequence[Any], tag: str) -> None:
+            if not np.isfinite(tuple(values)).all():
+                raise ValueError(
+                    f"PydicomReader: cannot derive affine matrix because DICOM tag {tag} "
+                    f"has a non-finite value: {values}."
+                )
+
         # "00200037" is the tag of `ImageOrientationPatient`
         rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"]
+        _raise_if_not_finite((rx, ry, rz, cx, cy, cz), "ImageOrientationPatient (0020,0037)")
         # "00200032" is the tag of `ImagePositionPatient`
         sx, sy, sz = metadata["00200032"]["Value"]
+        _raise_if_not_finite((sx, sy, sz), "ImagePositionPatient (0020,0032)")
         # "00280030" is the tag of `PixelSpacing`
         spacing = metadata["00280030"]["Value"] if "00280030" in metadata else (1.0, 1.0)
+        _raise_if_not_finite(tuple(spacing), "PixelSpacing (0028,0030)")
         dr, dc = metadata.get("spacing", spacing)[:2]
+        _raise_if_not_finite((dr, dc), "spacing")
         affine[0, 0] = cx * dr
         affine[0, 1] = rx * dc
         affine[0, 3] = sx
@@ -773,12 +786,16 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
         # 3d
         if "lastImagePositionPatient" in metadata:
             t1n, t2n, t3n = metadata["lastImagePositionPatient"]
+            _raise_if_not_finite((t1n, t2n, t3n), "lastImagePositionPatient")
             n = metadata[MetaKeys.SPATIAL_SHAPE][-1]
             if n > 1:
                 affine[0, 2] = (t1n - sx) / (n - 1)
                 affine[1, 2] = (t2n - sy) / (n - 1)
                 affine[2, 2] = (t3n - sz) / (n - 1)
 
+        if not np.isfinite(affine).all():
+            raise ValueError("PydicomReader: affine matrix not finite after composition.")
+
         if lps_to_ras:
             affine = orientation_ras_lps(affine)
         return affine

pyproject.toml

@@ -130,7 +130,8 @@ mlflow = ["mlflow>=3.15.2"]
 nibabel = ["nibabel"]
 nni = [
   "nni; platform_system == 'Linux' and 'arm' not in platform_machine and 'aarch' not in platform_machine",
-  "filelock<3.12.0"  # https://github.com/microsoft/nni/issues/5523
+  "filelock<3.12.0",  # https://github.com/microsoft/nni/issues/5523
+  "typeguard<3"  # https://github.com/microsoft/nni/issues/5457
 ]
 onnx = ["onnx>=1.13.0", "onnxruntime; python_version <= '3.10'", "onnx_graphsurgeon", "onnxscript"]
 openslide = ["openslide-python", "openslide-bin"]
@@ -159,9 +160,9 @@ transformers = ["transformers>=5.5.0"]  # 5.x needs the transchex BertLayer/Bert
 zarr = ["zarr"]
 # these dependencies are for testing/building only, they aren't needed for regular use so don't appear in "all"
 testing = [
-  "black>=26.3.1",
+  "black>=26.5.1",
   "coverage>=5.5",
-  "isort>=5.1,<6,!=6.0.0",
+  "isort>9.0.0",
   "mccabe",
   "packaging",
   "parameterized",
@@ -170,9 +171,8 @@ testing = [
   "pycodestyle",
   "pyflakes",
   "pyrefly>=1.0.0",
-  "ruff>=0.14.11,<0.15",
+  "ruff>=0.16.5",
   "tomli",  # used in print_dependencies.py for Python<3.11
-  "typeguard<3",  # https://github.com/microsoft/nni/issues/5457
   "types-PyYAML",
   "types-setuptools"
 ]
@@ -289,39 +289,35 @@ exclude = '''
 )
 '''
 
-[tool.pycln]
-all = true
-exclude = "monai/bundle/__main__.py"
-
 [tool.ruff]
 line-length = 120
 target-version = "py310"
 
 [tool.ruff.lint]
 select = [
-  "B",  # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b
-  "C90",  # mccabe (complexity) - https://docs.astral.sh/ruff/rules/#mccabe-c90
-  "E",  # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
-  "F",  # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f
-  "N",  # pep8-naming - https://docs.astral.sh/ruff/rules/#pep8-naming-n
-  "PIE",  # flake8-pie - https://docs.astral.sh/ruff/rules/#flake8-pie-pie
-  "TID", # flake8-tidy-imports - https://docs.astral.sh/ruff/rules/#flake8-tidy-imports-tid
-  "W",  # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w
-  "NPY",         # NumPy specific rules
-  "UP",          # pyupgrade
-  "RUF100",      # aka yesqa
+  "B",       # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b
+  "C90",     # mccabe (complexity) - https://docs.astral.sh/ruff/rules/#mccabe-c90
+  "E",       # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
+  "F",       # pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f
+  "N",       # pep8-naming - https://docs.astral.sh/ruff/rules/#pep8-naming-n
+  "PIE",     # flake8-pie - https://docs.astral.sh/ruff/rules/#flake8-pie-pie
+  "TID",     # flake8-tidy-imports - https://docs.astral.sh/ruff/rules/#flake8-tidy-imports-tid
+  "W",       # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w
+  "NPY",     # NumPy specific rules - https://docs.astral.sh/ruff/rules/#numpy-specific-rules-npy
+  "UP",      # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up
+  "RUF100",  # aka yesqa - https://docs.astral.sh/ruff/rules/unused-noqa/
+  "F401",    # unused imports - https://docs.astral.sh/ruff/rules/unused-import/
 ]
 extend-ignore = [
-  "E741", # ambiguous variable name
-  "F401", # unused import
+  "E741",   # ambiguous variable name
   "NPY002", # numpy-legacy-random
-  "E203", # whitespace before ':' (pycodestyle)
-  "E501", # line too long (pycodestyle)
-  "C408", # unnecessary collection call (flake8-comprehensions)
-  "N812", # lowercase imported as non lowercase (pep8-naming)
-  "B023", # function uses loop variable (flake8-bugbear)
-  "B905", # zip() without an explicit strict= parameter (flake8-bugbear)
-  "B028", # no explicit stacklevel keyword argument found (flake8-bugbear)
+  "E203",   # whitespace before ':' (pycodestyle)
+  "E501",   # line too long (pycodestyle)
+  "C408",   # unnecessary collection call (flake8-comprehensions)
+  "N812",   # lowercase imported as non lowercase (pep8-naming)
+  "B023",   # function uses loop variable (flake8-bugbear)
+  "B905",   # zip() without an explicit strict= parameter (flake8-bugbear)
+  "B028",   # no explicit stacklevel keyword argument found (flake8-bugbear)
 ]
 
 [tool.ruff.lint.per-file-ignores]
@@ -334,6 +330,8 @@ extend-ignore = [
 "monai/apps/detection/utils/ATSS_matcher.py" = [
   "N999"
 ]
+"__init__.py" = ["F401"]  # TODO: change importation in __init__.py files to suit F401
+"monai/bundle/__main__.py" = ["F401"]
 
 [tool.ruff.lint.mccabe]
 max-complexity = 50  # todo lower this treshold when yesqa id replaced with Ruff's RUF100

runtests.sh

@@ -221,7 +221,7 @@ function print_style_fail_msg() {
     echo "${red}Check failed!${noColor}"
     if [ "$homedir" = "$currentdir" ]
     then
-        echo "Please run auto style fixes: ${green}./runtests.sh --autofix${noColor}"
+        echo "Please run auto style fixes if necessary: ${green}./runtests.sh --autofix${noColor}"
     else :
     fi
 }

tests/apps/test_auto3dseg.py

@@ -11,9 +11,11 @@
 
 from __future__ import annotations
 
+import json
 import os
 import tempfile
 import unittest
+import warnings
 from copy import deepcopy
 from numbers import Number
 
@@ -36,6 +38,7 @@
     SampleOperations,
     SegSummarizer,
     SummaryOperations,
+    algo_from_json,
     datafold_read,
     verify_report_format,
 )
@@ -177,6 +180,20 @@ def __call__(self, data):
         return d
 
 
+class _DummyAlgo:
+    """Minimal stand-in for an Auto3DSeg Algo object used in warning tests."""
+
+    def __init__(self) -> None:
+        self.template_path: str | None = None
+        self.output_path = os.getcwd()
+
+    def load_state_dict(self, state: dict) -> None:
+        pass
+
+    def get_output_path(self) -> str:
+        return self.output_path
+
+
 class TestDataAnalyzer(unittest.TestCase):
     def setUp(self):
         self.test_dir = tempfile.TemporaryDirectory()
@@ -619,5 +636,23 @@ def tearDown(self) -> None:
         self.test_dir.cleanup()
 
 
+class TestAlgoFromJsonSecurityWarning(unittest.TestCase):
+    def test_warns_about_untrusted_target(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            algo_file = os.path.join(tmpdir, "algo_object.json")
+            with open(algo_file, "w", encoding="utf-8") as f:
+                json.dump({"_target_": f"{__name__}._DummyAlgo"}, f)
+
+            with warnings.catch_warnings(record=True) as caught:
+                warnings.simplefilter("always")
+                algo_from_json(algo_file)
+
+            messages = [str(w.message) for w in caught]
+            self.assertTrue(
+                any("algo_object.json" in msg and "trust" in msg for msg in messages),
+                f"Keywords 'algo_object.json' and 'trust' not found in warning messages: {messages}",
+            )
+
+
 if __name__ == "__main__":
     unittest.main()

tests/data/test_pydicom_reader.py

@@ -16,6 +16,7 @@
 import numpy as np
 
 from monai.data import PydicomReader
+from monai.utils import MetaKeys
 from tests.test_utils import SkipIfNoModule
 
 
@@ -39,6 +40,73 @@ def test_partial_orientation_tags_warns(self):
             affine = reader._get_affine(metadata)
         np.testing.assert_array_equal(affine, np.eye(4))
 
+    def test_non_finite_pixel_spacing_raises(self):
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
+            "00200032": {"Value": [0.0, 0.0, 0.0]},
+            "00280030": {"Value": [np.nan, 1.0]},
+        }
+        with self.assertRaisesRegex(ValueError, "PixelSpacing"):
+            reader._get_affine(metadata, lps_to_ras=False)
+
+    def test_non_finite_image_position_raises(self):
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
+            "00200032": {"Value": [np.inf, 0.0, 0.0]},
+            "00280030": {"Value": [1.0, 1.0]},
+        }
+        with self.assertRaisesRegex(ValueError, "ImagePositionPatient"):
+            reader._get_affine(metadata, lps_to_ras=False)
+
+    def test_finite_values_return_affine(self):
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
+            "00200032": {"Value": [10.0, 20.0, 30.0]},
+            "00280030": {"Value": [0.5, 0.25]},
+        }
+        affine = reader._get_affine(metadata, lps_to_ras=False)
+        self.assertEqual(affine.shape, (4, 4))
+        self.assertTrue(np.all(np.isfinite(affine)))
+        np.testing.assert_allclose(affine[0, 3], 10.0)
+        np.testing.assert_allclose(affine[1, 3], 20.0)
+        np.testing.assert_allclose(affine[2, 3], 30.0)
+
+    def test_non_finite_orientation_raises(self):
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [np.nan, 0.0, 0.0, 0.0, 1.0, 0.0]},
+            "00200032": {"Value": [0.0, 0.0, 0.0]},
+            "00280030": {"Value": [1.0, 1.0]},
+        }
+        with self.assertRaisesRegex(ValueError, "ImageOrientationPatient"):
+            reader._get_affine(metadata, lps_to_ras=False)
+
+    def test_non_finite_last_image_position_raises(self):
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
+            "00200032": {"Value": [0.0, 0.0, 0.0]},
+            "00280030": {"Value": [1.0, 1.0]},
+            "lastImagePositionPatient": [0.0, 0.0, np.inf],
+            MetaKeys.SPATIAL_SHAPE: [1, 1, 2],
+        }
+        with self.assertRaisesRegex(ValueError, "lastImagePositionPatient"):
+            reader._get_affine(metadata, lps_to_ras=False)
+
+    def test_overflow_from_finite_inputs_raises(self):
+        # Finite inputs whose product overflows produce a non-finite affine.
+        reader = PydicomReader()
+        metadata = {
+            "00200037": {"Value": [1e308, 0.0, 0.0, 1e308, 0.0, 0.0]},
+            "00200032": {"Value": [0.0, 0.0, 0.0]},
+            "00280030": {"Value": [1e308, 1e308]},
+        }
+        with self.assertRaisesRegex(ValueError, "not finite"):
+            reader._get_affine(metadata, lps_to_ras=False)
+
 
 if __name__ == "__main__":
     unittest.main()