-
Notifications
You must be signed in to change notification settings - Fork 1
fix(dif): harden delta-plot control boundary #944
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 all commits
be1cdf1
6c9fe69
54328a1
fa333b5
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,4 @@ | ||
| ### Security / trust boundary | ||
|
|
||
| - `delta_plot()` now establishes trusted selector, scalar, range, and iteration controls before materializing caller response/group data or discovering the compiled Rust core. Exact built-in strings and supported exact NumPy numeric scalar identities remain compatible; booleans, subclasses, and arbitrary conversion providers fail closed before caller callbacks. | ||
| - Normal-threshold `alpha` preserves the Rust `(0, 1)` domain, constraint ranges preserve `0 <= lo < hi <= 1`, fixed thresholds must be finite, additive adjustment counts stay positive, and `max_iter` is bounded by the package-wide `MAX_MAX_ITER` ceiling. Angoff Delta plot proportions, transforms, purification, thresholds, DIF flags, and result arithmetic remain Rust-owned. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # Delta-plot control trust boundary | ||
|
|
||
| Issue #942 hardens the Python-to-Rust control boundary of `delta_plot()` without moving Angoff Delta plot computation out of Rust. | ||
|
|
||
| ## Defect and boundary | ||
|
|
||
| Before this slice, the public wrapper materialized caller-owned response and group arrays before validating semantic controls. Selector checks could invoke string-subclass comparison behavior, while `alpha`, `fixed_threshold`, `const_range`, `nr_add`, and `max_iter` used generic Python conversion/comparison paths before a package-trusted scalar identity was established. Rejected controls therefore could execute caller callbacks or consume data-marshalling work before fail-closed rejection. | ||
|
|
||
| The correction performs allow-known-good admission before either data materialization or compiled-core discovery. Selector controls require exact built-in strings. Numeric controls accept exact built-in numeric identities and the package-supported concrete NumPy integer/floating scalar identities; booleans, scalar subclasses, and arbitrary conversion providers are rejected before coercion. Constraint ranges additionally require an exact built-in two-element tuple so tuple-subclass indexing cannot run at the trust boundary. | ||
|
|
||
| The normalized domains mirror the Rust core where applicable: normal-threshold `alpha` is finite in `(0, 1)`, constraint adjustment requires finite `0 <= lo < hi <= 1`, and additive adjustment requires `nr_add >= 1`. Fixed thresholds are finite because non-finite absolute thresholds cannot yield meaningful finite flag boundaries. `max_iter` is normalized to a built-in integer and bounded to `1..MAX_MAX_ITER` as a package resource-control ceiling. | ||
|
|
||
| After trusted controls are established, the existing response/group validation and PyO3 dispatch remain in place. Rust continues to own proportion calculation, extreme-proportion adjustment, Angoff delta transforms, covariance and major-axis calculation, threshold computation, iterative purification, convergence state, DIF flags, and all result-affecting arithmetic. Python performs validation and marshalling only. | ||
|
|
||
| ## Verification design | ||
|
|
||
| The RED regression commit `be1cdf12f903b3d63379e4bcc84cdb9a009d66de` introduces hostile string, integer, float, and tuple subclasses, together with data-materialization and compiled-core sentinels. GREEN requires rejected controls to produce zero caller callbacks and to fail before either sentinel. A fake native seam separately verifies genuine supported NumPy scalar controls are normalized to exact built-in values before PyO3 dispatch. Hosted exact-head CI, coverage, package, security, provenance, and formal review evidence remain required before lifecycle promotion. | ||
|
|
||
| This is trust-boundary and resource-control evidence, not new statistical-validity evidence. The existing Rust implementation and its pinned deltaPlotR/NumPy-oracle evidence continue to govern algorithmic parity and scientific behavior. | ||
|
|
||
| ## Standards trace | ||
|
|
||
| The engineering boundary follows CWE-1287's allow-known-good guidance for specified input types. OWASP ASVS 5.0.0 is the current released ASVS baseline. NIST SP 800-218 SSDF 1.1 remains the current final general SSDF baseline; SP 800-218 Rev. 1 / SSDF 1.2 is tracked as draft rather than represented as final authority. | ||
|
|
||
| ### References (APA 7th ed.) | ||
|
|
||
| CWE Content Team. (2026). *CWE-1287: Improper validation of specified type of input* (CWE Version 4.20). MITRE. https://cwe.mitre.org/data/definitions/1287.html | ||
|
|
||
| OWASP Foundation. (2025). *OWASP Application Security Verification Standard* (Version 5.0.0). https://owasp.org/www-project-application-security-verification-standard/ | ||
|
|
||
| Scarfone, K., Souppaya, M., & Dodson, D. (2022). *Secure Software Development Framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,140 @@ | |
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass | ||
| import math | ||
|
|
||
| import numpy as np | ||
|
|
||
| from .config import MAX_MAX_ITER | ||
|
|
||
|
|
||
| _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_FLOATING_SCALAR_TYPES = ( | ||
| np.float16, | ||
| np.float32, | ||
| np.float64, | ||
| np.longdouble, | ||
| ) | ||
|
|
||
|
|
||
| def _exact_integer(value: object, name: str) -> int: | ||
| """Return a trusted integer without invoking caller conversion hooks.""" | ||
| value_type = type(value) | ||
| if value_type is int: | ||
| return value | ||
| if any(value_type is scalar_type for scalar_type in _NUMPY_INTEGER_SCALAR_TYPES): | ||
| return int(value) | ||
| raise ValueError(f"{name} must be an integer") | ||
|
|
||
|
|
||
| def _exact_real(value: object, name: str) -> float: | ||
| """Return a trusted real scalar without invoking caller conversion hooks.""" | ||
| value_type = type(value) | ||
| if value_type is int or value_type is float: | ||
| try: | ||
| return float(value) | ||
| except OverflowError as exc: | ||
| raise ValueError(f"{name} must be a real number") from exc | ||
| if any(value_type is scalar_type for scalar_type in _NUMPY_INTEGER_SCALAR_TYPES): | ||
| try: | ||
| return float(value) | ||
| except OverflowError as exc: | ||
| raise ValueError(f"{name} must be a real number") from exc | ||
| if any(value_type is scalar_type for scalar_type in _NUMPY_FLOATING_SCALAR_TYPES): | ||
| return float(value) | ||
| raise ValueError(f"{name} must be a real number") | ||
|
|
||
|
|
||
| def _exact_selector(value: object, name: str) -> str: | ||
| """Return an exact built-in selector without invoking subclass callbacks.""" | ||
| if type(value) is not str: | ||
| raise ValueError(f"{name} must be an exact string") | ||
| return value | ||
|
|
||
|
|
||
| def _normalize_controls( | ||
| *, | ||
| threshold: object, | ||
| alpha: object, | ||
| fixed_threshold: object, | ||
| extreme: object, | ||
| const_range: object, | ||
| nr_add: object, | ||
| purify: object, | ||
| max_iter: object, | ||
| ) -> tuple[str, float, str, float, float, str | None, int]: | ||
| """Validate semantic controls before caller data or native discovery.""" | ||
| normalized_threshold = _exact_selector(threshold, "threshold") | ||
| if normalized_threshold not in ("norm", "fixed"): | ||
| raise ValueError("threshold must be 'norm' or 'fixed'") | ||
|
|
||
| normalized_extreme = _exact_selector(extreme, "extreme") | ||
| if normalized_extreme not in ("constraint", "add"): | ||
| raise ValueError("extreme must be 'constraint' or 'add'") | ||
|
|
||
| if purify is None: | ||
| normalized_purify = None | ||
| else: | ||
| if type(purify) is not str: | ||
| raise ValueError("purify must be None or an exact string") | ||
| normalized_purify = purify | ||
| if normalized_purify not in ("IPP1", "IPP2", "IPP3"): | ||
| raise ValueError("purify must be None, 'IPP1', 'IPP2', or 'IPP3'") | ||
|
|
||
| normalized_max_iter = _exact_integer(max_iter, "max_iter") | ||
| if not 1 <= normalized_max_iter <= MAX_MAX_ITER: | ||
| raise ValueError(f"max_iter must be between 1 and {MAX_MAX_ITER}") | ||
|
|
||
| if normalized_extreme == "constraint": | ||
| if type(const_range) is not tuple or len(const_range) != 2: | ||
| raise ValueError("const_range must be an exact 2-tuple") | ||
| lo = _exact_real(const_range[0], "const_range[0]") | ||
| hi = _exact_real(const_range[1], "const_range[1]") | ||
| if not ( | ||
| math.isfinite(lo) | ||
| and math.isfinite(hi) | ||
| and 0.0 <= lo < hi <= 1.0 | ||
| ): | ||
| raise ValueError("constraint range must satisfy 0 <= lo < hi <= 1") | ||
| ea, eb = lo, hi | ||
| else: | ||
| normalized_nr_add = _exact_integer(nr_add, "nr_add") | ||
| if normalized_nr_add < 1: | ||
| raise ValueError("nr_add must be a positive integer >= 1") | ||
| ea, eb = float(normalized_nr_add), 0.0 | ||
|
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. Bare |
||
|
|
||
| if normalized_threshold == "norm": | ||
| tv = _exact_real(alpha, "alpha") | ||
| if not math.isfinite(tv) or not 0.0 < tv < 1.0: | ||
| raise ValueError("alpha must be finite and in (0, 1)") | ||
| else: | ||
| tv = _exact_real(fixed_threshold, "fixed_threshold") | ||
| if not math.isfinite(tv): | ||
| raise ValueError("fixed_threshold must be finite") | ||
|
|
||
| return ( | ||
| normalized_threshold, | ||
| tv, | ||
| normalized_extreme, | ||
| ea, | ||
| eb, | ||
| normalized_purify, | ||
| normalized_max_iter, | ||
| ) | ||
|
Comment on lines
+104
to
+139
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. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Validate every control before selecting the active branch. When For example, Normalize and validate Proposed direction normalized_max_iter = _exact_integer(max_iter, "max_iter")
if not 1 <= normalized_max_iter <= MAX_MAX_ITER:
raise ValueError(f"max_iter must be between 1 and {MAX_MAX_ITER}")
+ normalized_alpha = _exact_real(alpha, "alpha")
+ if not math.isfinite(normalized_alpha) or not 0.0 < normalized_alpha < 1.0:
+ raise ValueError("alpha must be finite and in (0, 1)")
+
+ normalized_fixed_threshold = _exact_real(fixed_threshold, "fixed_threshold")
+ if not math.isfinite(normalized_fixed_threshold):
+ raise ValueError("fixed_threshold must be finite")
+
+ if type(const_range) is not tuple or len(const_range) != 2:
+ raise ValueError("const_range must be an exact 2-tuple")
+ lo = _exact_real(const_range[0], "const_range[0]")
+ hi = _exact_real(const_range[1], "const_range[1]")
+ if not (math.isfinite(lo) and math.isfinite(hi) and 0.0 <= lo < hi <= 1.0):
+ raise ValueError("constraint range must satisfy 0 <= lo < hi <= 1")
+
+ normalized_nr_add = _exact_integer(nr_add, "nr_add")
+ if normalized_nr_add < 1:
+ raise ValueError("nr_add must be a positive integer >= 1")
+
if normalized_extreme == "constraint":
- ...
ea, eb = lo, hi
else:
- ...
ea, eb = float(normalized_nr_add), 0.0🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @dataclass | ||
| class DeltaPlotResult: | ||
|
|
@@ -81,6 +212,25 @@ def delta_plot( | |
| https://doi.org/10.18637/jss.v059.c01 (package paper; the R sources | ||
| listed above were READ and ported.) | ||
| """ | ||
| ( | ||
| threshold, | ||
| tv, | ||
| extreme, | ||
| ea, | ||
| eb, | ||
| purify, | ||
| max_iter, | ||
| ) = _normalize_controls( | ||
| threshold=threshold, | ||
| alpha=alpha, | ||
| fixed_threshold=fixed_threshold, | ||
| extreme=extreme, | ||
| const_range=const_range, | ||
| nr_add=nr_add, | ||
| purify=purify, | ||
| max_iter=max_iter, | ||
| ) | ||
|
|
||
| xa = np.asarray(responses) | ||
| if xa.ndim != 2: | ||
| raise ValueError("responses must be a 2-D person-by-item matrix") | ||
|
|
@@ -111,21 +261,6 @@ def delta_plot( | |
| raise ValueError("group entries must be 0 (reference) or 1 (focal)") | ||
| gu = np.ascontiguousarray(gf, dtype=np.uint8) | ||
|
|
||
| if threshold not in ("norm", "fixed"): | ||
| raise ValueError("threshold must be 'norm' or 'fixed'") | ||
| if extreme not in ("constraint", "add"): | ||
| raise ValueError("extreme must be 'constraint' or 'add'") | ||
| if purify is not None and purify not in ("IPP1", "IPP2", "IPP3"): | ||
| raise ValueError("purify must be None, 'IPP1', 'IPP2', or 'IPP3'") | ||
| if extreme == "constraint": | ||
| lo, hi = float(const_range[0]), float(const_range[1]) | ||
| ea, eb = lo, hi | ||
| else: | ||
| if int(nr_add) != nr_add or nr_add < 1: | ||
| raise ValueError("nr_add must be a positive integer >= 1") | ||
| ea, eb = float(nr_add), 0.0 | ||
| tv = float(alpha) if threshold == "norm" else float(fixed_threshold) | ||
|
|
||
| from .fitstats import _core_module | ||
|
|
||
| core = _core_module() | ||
|
|
@@ -142,7 +277,7 @@ def delta_plot( | |
| threshold, | ||
| tv, | ||
| purify, | ||
| int(max_iter), | ||
| max_iter, | ||
| ) | ||
| n_iter = int(res["n_iter"]) | ||
| return DeltaPlotResult( | ||
|
|
||
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.
Issue #942 requires admitting all eight public controls before
np.asarray(responses|group). This block type-checks only the activeextreme/thresholdbranch, so a hosted options blob can still smuggle a hostile unused field into data materialization.Reproduced on this head:
delta_plot(DataSentinel(), DataSentinel(), extreme="add", nr_add=1, const_range="poison")hitsAssertionError: data must not be materializedinstead of a packageValueError.Type-admit every control first (exact string / exact 2-tuple /
_exact_integer/_exact_real), then apply the Rust domain only on the active branch. Keepalphain(0, 1)andconst_rangeat finite0 <= lo < hi <= 1to matchcrates/mlsirm-core/src/dif.rs.