fix(model-comparison): harden control callback boundary

docs/changelog.d/877-model-comparison-control-callbacks.md

@@ -0,0 +1,5 @@
+# Model-comparison callback-boundary hardening
+
+## Security
+
+- Harden parameter-count, audit-label, and real-valued model-comparison controls so caller-defined integer/string/NumPy subclasses and arbitrary integer-protocol providers are rejected before conversion or normalization callbacks execute, while preserving genuine NumPy scalar compatibility and Rust-owned Vuong arithmetic.

docs/doctoring/model_comparison_control_validation.md

@@ -2,22 +2,26 @@
 
 ## Decision
 
-Public model-comparison semantic controls are validated as bounded package contracts before any caller-defined representation or numeric conversion hook can execute. `relation` accepts only `ModelRelation` or a built-in `str`; `alpha` and `omega_tol` accept the intended built-in/NumPy real-scalar domain and reject arbitrary coercible objects. Stable package-owned validation messages remain non-reflective.
+Public model-comparison semantic controls are validated as bounded package contracts before any caller-defined representation or numeric conversion hook can execute. `relation` accepts only `ModelRelation` or a built-in `str`; `model_a` and `model_b` accept exact built-in strings before normalization; `k_a` and `k_b` accept exact built-in integers or genuine supported NumPy integer scalar classes; and `alpha` and `omega_tol` accept exact built-in real values or genuine supported NumPy integer/floating scalar classes. Caller-defined Python/NumPy subclasses and arbitrary integer-protocol providers are rejected before `__index__`, `__int__`, `__float__`, string-normalization, or representation callbacks can execute. Stable package-owned validation messages remain non-reflective.
 
 This hardening changes only the validation boundary. Rust-backed Vuong arithmetic, relation-safe routing, accepted relation identities, thresholds, result fields, and scientific interpretation remain unchanged.
 
 ## Security rationale
 
-Python's data model specifies that `str(object)` invokes `object.__str__()`. The built-in `float()` conversion for a general object delegates to numeric conversion methods such as `__float__()`. Performing those conversions on untrusted controls therefore executes caller-defined behavior before a finite vocabulary/range contract has been established. Validation should first prove that the runtime value belongs to the package's accepted scalar/type domain, then normalize trusted values only.
+Python's data model permits general integer conversion protocols such as `__index__` and numeric conversions such as `__float__`; string subclasses may also override normalization methods. Invoking these protocols before the package establishes a trusted scalar or label identity executes caller-controlled behavior inside a validation boundary. Module-name or inheritance metadata is not sufficient evidence that a scalar instance is one of the genuine NumPy scalar classes intended by the public contract.
 
-The fail-first regressions use hostile objects whose `__str__`, `__repr__`, or `__float__` raise. Acceptance requires the existing package-owned `ValueError` surfaces without invoking those callbacks and without changing the Rust numerical path.
+Validation therefore admits only exact built-in control types and an explicit finite set of genuine NumPy scalar classes, then normalizes those already-trusted values. This keeps user-defined subclasses and arbitrary protocol providers outside the trusted conversion boundary without changing the Rust numerical owner.
+
+The fail-first regressions use hostile `__index__`, integer-subclass, string-subclass, and spoofed NumPy floating-subclass controls. Acceptance requires the existing package-owned `ValueError` surfaces with zero hostile callback executions while ordinary supported NumPy scalars continue to normalize successfully.
 
 ## Verification contract
 
 - preserve every accepted `ModelRelation` identity and built-in-string relation value;
-- preserve accepted built-in and NumPy real scalar semantics for `alpha` and `omega_tol`;
-- preserve Boolean, non-finite, probability-range, and non-negative tolerance rejection;
-- reject arbitrary caller objects before `str()`, `repr()`, or `float()` hooks execute;
+- preserve exact built-in model labels while rejecting caller-defined string subclasses before normalization;
+- preserve exact built-in integers and genuine supported NumPy integer scalars for non-negative parameter counts;
+- preserve accepted exact built-in and genuine NumPy real-scalar semantics for `alpha` and `omega_tol`;
+- preserve Boolean, non-finite, probability-range, non-negative count, and non-negative tolerance rejection;
+- reject arbitrary integer-protocol providers and caller-defined Python/NumPy scalar subclasses before conversion or representation hooks execute;
 - preserve Rust-owned model-comparison arithmetic and scientific routing;
 - require focused regression coverage, full repository CI, Security Scan, SAST, and current-head review before integration.
 

python/fast_mlsirm/model_comparison.py

@@ -12,14 +12,36 @@
 from dataclasses import dataclass
 from enum import Enum
 import math
-import operator
 from typing import Any
 
+import numpy as np
+
 from .fitstats import vuong_nonnested
 
 MAX_CASEWISE_VALUES = 1_000_000
 MAX_MODEL_LABEL_CHARS = 128
 
+_NUMPY_INTEGER_SCALAR_TYPES = (
+    np.int8,
+    np.int16,
+    np.int32,
+    np.int64,
+    np.intp,
+    np.longlong,
+    np.uint8,
+    np.uint16,
+    np.uint32,
+    np.uint64,
+    np.uintp,
+    np.ulonglong,
+)
+_NUMPY_FLOAT_SCALAR_TYPES = (
+    np.float16,
+    np.float32,
+    np.float64,
+    np.longdouble,
+)
+
 
 class ModelRelation(str, Enum):
     """Declared mathematical relationship between two candidate models."""
@@ -92,8 +114,8 @@ def _is_boolean_like(value: Any) -> bool:
 
 
 def _model_label(value: str, name: str) -> str:
-    """Return a bounded printable model label suitable for audit output."""
-    if not isinstance(value, str):
+    """Return a bounded printable exact-string label suitable for audit output."""
+    if type(value) is not str:
         raise ValueError(f"{name} must be a non-empty string")
     normalized = value.strip()
     if not normalized:
@@ -121,36 +143,34 @@ def _relation(value: ModelRelation | str) -> ModelRelation:
 
 
 def _parameter_count(value: Any, name: str) -> int:
-    """Return a non-negative integer parameter count while rejecting booleans."""
-    if _is_boolean_like(value):
+    """Return an exact trusted non-negative integer parameter count."""
+    value_type = type(value)
+    if value_type is int:
+        normalized = value
+    elif any(value_type is trusted for trusted in _NUMPY_INTEGER_SCALAR_TYPES):
+        normalized = int(value)
+    else:
         raise ValueError(f"{name} must be a non-negative integer")
-    try:
-        normalized = operator.index(value)
-    except MemoryError:
-        raise
-    except Exception:
-        raise ValueError(f"{name} must be a non-negative integer") from None
     if normalized < 0:
         raise ValueError(f"{name} must be a non-negative integer")
-    return int(normalized)
+    return normalized
 
 
 def _trusted_real_scalar(value: Any, message: str) -> float:
-    """Return a built-in or NumPy real scalar without custom coercion hooks."""
+    """Return an exact built-in or genuine NumPy real scalar."""
     value_type = type(value)
-    if _is_boolean_like(value):
+    trusted = (
+        value_type is int
+        or value_type is float
+        or any(value_type is candidate for candidate in _NUMPY_INTEGER_SCALAR_TYPES)
+        or any(value_type is candidate for candidate in _NUMPY_FLOAT_SCALAR_TYPES)
+    )
+    if not trusted:
         raise ValueError(message)
-    if value_type is int or value_type is float:
+    try:
         return float(value)
-    if value_type.__module__.startswith("numpy"):
-        mro = value_type.__mro__
-        if any(
-            base.__module__.startswith("numpy")
-            and base.__name__ in {"integer", "floating"}
-            for base in mro
-        ):
-            return float(value)
-    raise ValueError(message)
+    except OverflowError:
+        raise ValueError(message) from None
 
 
 def _alpha_value(value: Any) -> float:

tests/test_model_comparison_control_callback_safety.py

@@ -0,0 +1,159 @@
+"""Callback-safety regressions for model-comparison control metadata."""
+
+from __future__ import annotations
+
+import numpy as np
+import pytest
+
+from fast_mlsirm.model_comparison import (
+    ComparisonStatus,
+    ModelRelation,
+    compare_nonnested_models,
+)
+
+
+_CASEWISE_A = (0.0, 0.1)
+_CASEWISE_B = (0.0, 0.1)
+
+
+class _HostileIndex:
+    """Arbitrary integer protocol provider whose callback must stay inert."""
+
+    calls = 0
+
+    def __index__(self) -> int:
+        """Record forbidden integer coercion."""
+        type(self).calls += 1
+        return 1
+
+
+class _HostileInt(int):
+    """Caller-defined integer subclass outside the trusted control boundary."""
+
+    calls = 0
+
+    def __index__(self) -> int:
+        """Record forbidden integer coercion if dispatched."""
+        type(self).calls += 1
+        return int.__index__(self)
+
+
+class _HostileStr(str):
+    """Caller-defined string subclass whose normalization must not execute."""
+
+    calls = 0
+
+    def strip(self, *args: object, **kwargs: object) -> str:
+        """Record forbidden label normalization."""
+        type(self).calls += 1
+        return str.strip(self, *args, **kwargs)
+
+
+class _HostileNumpyFloat(np.float64):
+    """NumPy-scalar subclass spoofing trusted-looking module metadata."""
+
+    __module__ = "numpy.user_controlled"
+    calls = 0
+
+    def __float__(self) -> float:
+        """Record forbidden real-scalar normalization."""
+        type(self).calls += 1
+        return np.float64.__float__(self)
+
+
+def _compare(**overrides: object):
+    """Call the public comparison boundary without requiring native dispatch."""
+    kwargs: dict[str, object] = {
+        "k_a": 1,
+        "k_b": 1,
+        "model_a": "A",
+        "model_b": "B",
+        "relation": ModelRelation.UNKNOWN,
+        "alpha": 0.05,
+        "omega_tol": 1e-12,
+    }
+    kwargs.update(overrides)
+    return compare_nonnested_models(
+        _CASEWISE_A,
+        _CASEWISE_B,
+        **kwargs,  # type: ignore[arg-type]
+    )
+
+
+@pytest.mark.parametrize(
+    ("field", "value", "owner"),
+    [
+        ("k_a", _HostileIndex(), _HostileIndex),
+        ("k_b", _HostileInt(1), _HostileInt),
+    ],
+)
+def test_parameter_controls_reject_untrusted_integer_types_without_callbacks(
+    field: str,
+    value: object,
+    owner: type,
+) -> None:
+    """Parameter counts fail closed before integer-protocol dispatch."""
+    owner.calls = 0
+    with pytest.raises(ValueError, match=rf"{field} must be a non-negative integer"):
+        _compare(**{field: value})
+    assert owner.calls == 0
+
+
+def test_model_label_rejects_string_subclass_without_normalization_callback() -> None:
+    """Audit labels require exact strings before any caller method can run."""
+    _HostileStr.calls = 0
+    with pytest.raises(ValueError, match="model_a must be a non-empty string"):
+        _compare(model_a=_HostileStr("A"))
+    assert _HostileStr.calls == 0
+
+
+@pytest.mark.parametrize(
+    ("field", "value", "message"),
+    [
+        ("alpha", _HostileNumpyFloat(0.05), "alpha must be finite and in \\(0, 1\\)"),
+        (
+            "omega_tol",
+            _HostileNumpyFloat(1e-12),
+            "omega_tol must be finite and non-negative",
+        ),
+    ],
+)
+def test_real_controls_reject_numpy_subclasses_without_float_callback(
+    field: str,
+    value: object,
+    message: str,
+) -> None:
+    """Real-valued controls reject caller NumPy subclasses before coercion."""
+    _HostileNumpyFloat.calls = 0
+    with pytest.raises(ValueError, match=message):
+        _compare(**{field: value})
+    assert _HostileNumpyFloat.calls == 0
+
+
+@pytest.mark.parametrize(
+    ("field", "message"),
+    [
+        ("alpha", "alpha must be finite and in \\(0, 1\\)"),
+        ("omega_tol", "omega_tol must be finite and non-negative"),
+    ],
+)
+def test_real_controls_normalize_builtin_integer_overflow_to_value_error(
+    field: str,
+    message: str,
+) -> None:
+    """Huge trusted integers retain the public field-specific error contract."""
+    with pytest.raises(ValueError, match=message):
+        _compare(**{field: 10**10000})
+
+
+def test_genuine_numpy_scalars_remain_supported() -> None:
+    """Trusted NumPy scalar compatibility remains part of the public contract."""
+    result = _compare(
+        k_a=np.int64(1),
+        k_b=np.uint32(1),
+        alpha=np.float64(0.05),
+        omega_tol=np.float32(1e-6),
+    )
+    assert result.status is ComparisonStatus.UNKNOWN_RELATION
+    assert result.k_a == 1
+    assert result.k_b == 1

tests/test_model_comparison_input_error_boundary.py

@@ -66,10 +66,13 @@ def __float__(self) -> float:
 
 
 class _MemoryIndex:
-    """Raise resource exhaustion during caller-controlled index coercion."""
+    """Probe an untrusted integer protocol that must never be dispatched."""
+
+    calls = 0
 
     def __index__(self) -> int:
-        """Preserve process-level resource exhaustion from index conversion."""
+        """Record forbidden index conversion before simulating exhaustion."""
+        type(self).calls += 1
         raise MemoryError("index allocation exhausted")
 
 
@@ -146,12 +149,22 @@ def test_parameter_count_conversion_failure_is_redacted() -> None:
         lambda: compare_nonnested_models(_MemoryIteratorFactory(), (0.1, 0.2), 2, 2),
         lambda: compare_nonnested_models(_MemoryIterable(), (0.1, 0.2), 2, 2),
         lambda: compare_nonnested_models((_MemoryFloat(), 0.2), (0.1, 0.2), 2, 2),
-        lambda: compare_nonnested_models(
-            (0.1, 0.2), (0.1, 0.2), _MemoryIndex(), 2  # type: ignore[arg-type]
-        ),
     ],
 )
 def test_memory_error_remains_explicit_resource_exhaustion(callable_) -> None:
-    """Resource exhaustion must not be downgraded into an input-validation error."""
+    """Resource exhaustion from accepted casewise callbacks remains explicit."""
     with pytest.raises(MemoryError):
         callable_()
+
+
+def test_untrusted_memory_index_is_rejected_without_callback() -> None:
+    """Parameter-count trust is established before arbitrary index protocols run."""
+    _MemoryIndex.calls = 0
+    with pytest.raises(ValueError, match="k_a must be a non-negative integer"):
+        compare_nonnested_models(
+            (0.1, 0.2),
+            (0.1, 0.2),
+            _MemoryIndex(),  # type: ignore[arg-type]
+            2,
+        )
+    assert _MemoryIndex.calls == 0