Repository navigation
fix(parallel-analysis): validate controls before native discovery #923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
c0e5b2b
3e59acc
65c9cca
ed95eeb
dea9dcb
2d3a126
73a425f
88db84b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Parallel-analysis control trust hardening | ||
|
|
||
| ## Security | ||
|
|
||
| - Validate `n_iterations`, `centile`, and `seed` before native-core discovery, accepting only exact built-in integers and supported concrete NumPy integer scalars while rejecting caller-defined subclasses and conversion providers without executing their callbacks. | ||
| - Preserve the existing positive-iteration, centile `0..99`, Rust `u64` seed, and 128 MiB random-benchmark workspace limits without changing Rust-owned Horn/Glorfeld factor-retention arithmetic. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # Parallel-analysis control trust boundary | ||
|
|
||
| ## Decision | ||
|
|
||
| `parallel_analysis()` treats `n_iterations`, `centile`, and `seed` as inert configuration data. The Python boundary admits only exact built-in `int` values and exact package-supported concrete NumPy integer scalar identities, validates their established domains, and does so before native-core discovery. Caller-defined integer subclasses and arbitrary numeric-conversion providers are rejected without invoking conversion or representation hooks. | ||
|
|
||
| The validation change is intentionally non-numerical. Horn/Glorfeld factor-retention arithmetic, the deterministic random stream, correlation/eigenvalue computation, random-eigenvalue adjustment, retention scanning, and all result-affecting production mathematics remain Rust-owned. | ||
|
|
||
| ## Security basis | ||
|
|
||
| This is an accept-known-good type-validation boundary consistent with CWE-1287, which recommends validating the expected type and rejecting inputs outside the accepted specification. OWASP ASVS 5.0.0 remains the current stable ASVS release and is used as the application-security validation baseline. NIST SP 800-218 SSDF 1.1 remains final; SP 800-218 Rev. 1 / SSDF 1.2 is still an Initial Public Draft and is tracked as informative draft material rather than represented as final authority. | ||
|
|
||
| ## Regression evidence | ||
|
|
||
| `tests/test_parallel_analysis_control_bounds.py` requires rejected scalar values, Python/NumPy integer subclasses, arbitrary conversion providers, and out-of-domain controls to fail before native-core discovery. It also proves supported concrete NumPy integer scalars normalize to exact Python integers at the Rust boundary while preserving the existing iteration, centile, seed, and random-workspace limits. | ||
|
|
||
| ## References | ||
|
|
||
| MITRE. (2026). *CWE-1287: Improper validation of specified type of input* (CWE 4.20). https://cwe.mitre.org/data/definitions/1287.html | ||
|
|
||
| National Institute of Standards and Technology. (2022). *Secure software development framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). https://doi.org/10.6028/NIST.SP.800-218 | ||
|
|
||
| Booth, H., Ogata, M., Kent, K., Souppaya, M., & Dodson, D. (2025). *Secure software development framework (SSDF) version 1.2: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218 Rev. 1, Initial Public Draft). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218r1.ipd | ||
|
|
||
| OWASP Foundation. (2025). *OWASP application security verification standard 5.0.0*. https://owasp.org/www-project-application-security-verification-standard/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,8 @@ def __init__(self) -> None: | |
| self.calls: list[tuple[object, ...]] = [] | ||
|
|
||
| def parallel_analysis(self, *args): | ||
| controls = args[-3:] | ||
| assert all(type(value) is int for value in controls) | ||
| self.calls.append(args) | ||
| return { | ||
| "retained": 1, | ||
|
|
@@ -50,6 +52,20 @@ def _install_core(monkeypatch: pytest.MonkeyPatch, core: object) -> None: | |
| monkeypatch.setattr(fitstats, "_core_module", lambda: core) | ||
|
|
||
|
|
||
| def _reject_core_discovery(monkeypatch: pytest.MonkeyPatch) -> list[bool]: | ||
| """Make native-core discovery itself observable and forbidden.""" | ||
| import fast_mlsirm.fitstats as fitstats | ||
|
|
||
| calls: list[bool] = [] | ||
|
|
||
| def discover_core() -> object: | ||
| calls.append(True) | ||
| raise AssertionError("native core discovered before control validation") | ||
|
|
||
| monkeypatch.setattr(fitstats, "_core_module", discover_core) | ||
| return calls | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("name", "bad_value"), | ||
| [ | ||
|
|
@@ -64,30 +80,109 @@ def _install_core(monkeypatch: pytest.MonkeyPatch, core: object) -> None: | |
| ("seed", "1"), | ||
| ], | ||
| ) | ||
| def test_noninteger_controls_fail_before_rust_dispatch( | ||
| def test_noninteger_controls_fail_before_core_discovery( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| name: str, | ||
| bad_value: object, | ||
| ) -> None: | ||
| """Booleans, floats, and strings cannot be silently coerced to controls.""" | ||
| _install_core(monkeypatch, _TrapCore()) | ||
| """Rejected scalar controls cannot cross the native-loader boundary.""" | ||
| discovery_calls = _reject_core_discovery(monkeypatch) | ||
| kwargs: dict[str, object] = {"n_iterations": 2, "centile": 0, "seed": 1} | ||
| kwargs[name] = bad_value | ||
|
|
||
| with pytest.raises(ValueError, match=rf"^{name} "): | ||
| parallel_analysis(_DATA, **kwargs) | ||
|
|
||
| assert discovery_calls == [] | ||
|
|
||
| def test_hostile_integer_conversion_is_not_executed(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Control validation must use admitted types rather than caller conversion hooks.""" | ||
| _install_core(monkeypatch, _TrapCore()) | ||
|
|
||
| class HostileInt: | ||
| @pytest.mark.parametrize("kind", ["python", "numpy"]) | ||
| def test_integer_subclasses_fail_without_callbacks_or_core_discovery( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| kind: str, | ||
| ) -> None: | ||
| """Caller-defined integer subclasses cannot authorize executable coercion.""" | ||
| discovery_calls = _reject_core_discovery(monkeypatch) | ||
| callbacks: list[str] = [] | ||
|
|
||
| class HostilePythonInt(int): | ||
| def __int__(self) -> int: | ||
| callbacks.append("int") | ||
| raise AssertionError("caller-controlled __int__ executed") | ||
|
|
||
| def __repr__(self) -> str: | ||
| callbacks.append("repr") | ||
| raise AssertionError("caller-controlled __repr__ executed") | ||
|
|
||
| class HostileNumpyInt(np.int64): | ||
| def __int__(self) -> int: | ||
| callbacks.append("int") | ||
| raise AssertionError("caller-controlled NumPy __int__ executed") | ||
|
|
||
| def __repr__(self) -> str: | ||
| callbacks.append("repr") | ||
| raise AssertionError("caller-controlled NumPy __repr__ executed") | ||
|
|
||
| value = HostilePythonInt(2) if kind == "python" else HostileNumpyInt(2) | ||
|
|
||
| with pytest.raises(ValueError, match=r"^n_iterations must be an integer$"): | ||
| parallel_analysis(_DATA, n_iterations=value) | ||
|
|
||
| assert callbacks == [] | ||
| assert discovery_calls == [] | ||
|
|
||
|
|
||
| def test_arbitrary_integer_protocol_is_not_executed( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """An arbitrary integer protocol provider is data, not control authority.""" | ||
| discovery_calls = _reject_core_discovery(monkeypatch) | ||
| callbacks: list[str] = [] | ||
|
|
||
| class IntegerProvider: | ||
| def __int__(self) -> int: | ||
| raise RuntimeError("caller-controlled conversion executed") | ||
| callbacks.append("int") | ||
| raise AssertionError("arbitrary __int__ executed") | ||
|
|
||
| def __index__(self) -> int: | ||
| callbacks.append("index") | ||
| raise AssertionError("arbitrary __index__ executed") | ||
|
|
||
| def __repr__(self) -> str: | ||
| callbacks.append("repr") | ||
| raise AssertionError("arbitrary __repr__ executed") | ||
|
|
||
| with pytest.raises(ValueError, match=r"^n_iterations must be an integer$"): | ||
| parallel_analysis(_DATA, n_iterations=IntegerProvider()) | ||
|
|
||
| assert callbacks == [] | ||
| assert discovery_calls == [] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("updates", "message"), | ||
| [ | ||
| ({"n_iterations": 0}, r"^n_iterations must be >= 1$"), | ||
| ({"centile": -1}, r"^centile must be >= 0$"), | ||
| ({"centile": 100}, r"^centile must be <= 99$"), | ||
| ({"seed": -1}, r"^seed must be >= 0$"), | ||
| ({"seed": 2**64}, r"^seed must be <= 18446744073709551615$"), | ||
| ], | ||
| ) | ||
| def test_invalid_domains_fail_before_core_discovery( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| updates: dict[str, int], | ||
| message: str, | ||
| ) -> None: | ||
| """Established control domains are enforced before native discovery.""" | ||
| discovery_calls = _reject_core_discovery(monkeypatch) | ||
| kwargs: dict[str, int] = {"n_iterations": 2, "centile": 0, "seed": 1} | ||
| kwargs.update(updates) | ||
|
|
||
| with pytest.raises(ValueError, match=r"^n_iterations "): | ||
| parallel_analysis(_DATA, n_iterations=HostileInt()) # type: ignore[arg-type] | ||
| with pytest.raises(ValueError, match=message): | ||
| parallel_analysis(_DATA, **kwargs) | ||
|
|
||
| assert discovery_calls == [] | ||
|
|
||
|
|
||
| def test_oversized_random_benchmark_workspace_fails_before_rust_dispatch( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
@@ -108,16 +203,32 @@ def test_seed_must_fit_rust_u64_before_dispatch(monkeypatch: pytest.MonkeyPatch) | |
| parallel_analysis(_DATA, n_iterations=2, seed=2**64) | ||
|
|
||
|
|
||
| def test_numpy_integer_controls_remain_accepted(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Exact NumPy integer scalars retain the documented public contract.""" | ||
| @pytest.mark.parametrize( | ||
| "scalar_type", | ||
| [ | ||
| np.int8, | ||
| np.int16, | ||
| np.int32, | ||
| np.int64, | ||
| np.uint8, | ||
| np.uint16, | ||
| np.uint32, | ||
| np.uint64, | ||
| ], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On current NumPy, |
||
| ) | ||
| def test_numpy_integer_controls_remain_accepted( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| scalar_type: type[np.integer], | ||
| ) -> None: | ||
| """Exact supported NumPy integers normalize to built-ins at the Rust boundary.""" | ||
| core = _RecordingCore() | ||
| _install_core(monkeypatch, core) | ||
|
|
||
| result = parallel_analysis( | ||
| _DATA, | ||
| n_iterations=np.int64(2), | ||
| centile=np.int64(50), | ||
| seed=np.int64(7), | ||
| n_iterations=scalar_type(2), | ||
| centile=scalar_type(50), | ||
| seed=scalar_type(7), | ||
| ) | ||
|
|
||
| assert result.retained == 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
np.bool_(True)next toTruefor each control. Sibling control suites already do this; without it,n_iterations=np.bool_(True)becoming1would not be caught here. The successor locks this in #938.