Harden stochastic_overrides: cap NumClusters/NumSubPaths and reject non-finite floats - #698
Merged
Merged
Conversation
Close the two gaps reported in #697: 1. Cap NumClusters (<=50) and NumSubPaths (<=20) to match the frontend input caps. Previously any authenticated client could send huge values (e.g. NumClusters=99999999) and tie up a gthread worker for the full gunicorn timeout trying to allocate >14 GiB. 2. Uniformly validate every value in stochastic_overrides as a finite number, instead of maintaining a hand-curated allowlist of numeric keys. Float-valued overrides (KF_mu, AS_A_mu, ES_A_mu, SC_lambda, ...) previously bypassed the finite-check and crashed deep in the generator with a cryptic 500 ('float() argument must be a string or a real number, not NoneType') when NaN/Inf were sent - JSON.stringify(NaN) serialises to null on the frontend, which lands as None in Python. Co-authored-by: Robin Wydaeghe <rwydaegh@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
3 tasks
rwydaegh
added a commit
that referenced
this pull request
Apr 20, 2026
## Summary Install `StrictJSONProvider` on the Flask app so `NaN` / `Infinity` / `-Infinity` in JSON request bodies are rejected at decode time with a 400, instead of slipping past every scalar-float field's `math.isfinite` guard and either crashing downstream or silently propagating into numeric pipelines. Python's stdlib `json` accepts those literals by default as a non-standard RFC 8259 extension; overriding `parse_constant` at the provider level is a single choke point for the whole API surface. ## Why The QA/feature/code-review agents have been shipping one `isfinite`/`isnan` guard PR after another (#668, #670, #675, #685, #687, #693, #694, #698, #701, #707…), each patching a new scalar field on a new route. Whack-a-mole. This replaces the pond with the drain. ## Changes - `src/aegis/viewer/server/_app.py` — `StrictJSONProvider` subclass of `DefaultJSONProvider`, installed in `create_app`. Only `loads` is overridden so routes that intentionally emit NaN (masked spatial averaging) are unaffected on the output side. - `src/aegis/viewer/routes/_helpers.py` — `get_json_dict` now distinguishes empty body (returns `{}`) from parse failure (returns 400 with the parser's message), so provider rejections surface as 400 instead of silently falling through to route defaults. - `tests/test_viewer_strict_json_nonfinite.py` — 37-case regression matrix covering 12 routes × 3 literals (NaN / Infinity / -Infinity) + nested-NaN + happy-path sanity. - Two existing per-field NaN tests relaxed (`test_power_dbm_nan_rejected`, `test_freq_nan_rejected`) to accept either the legacy field-level message or the new parser-level message — the protection now fires earlier. ## Test plan - [x] `pytest tests/test_viewer_strict_json_nonfinite.py tests/test_viewer_non_dict_body_guards.py` — 68/68 pass - [x] `pytest tests/ -m "not slow" --deselect <network-flaky-overpass>` — 2842 pass, 32 skipped - [x] `ruff check` / `ruff format --check` on touched files — clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Close the two boundary gaps reported by QA in #697 at
POST /api/compute:Gap 1 - DoS via huge NumClusters/NumSubPaths: the frontend clamps these to
[1, 50]and[1, 20], but the API accepted any finite integer, letting a client send e.g.NumClusters=99999999and tie up a gthread worker for the full gunicorn 600 s timeout whileaegis.channel.generatortried to allocate >14 GiB. Now the validator caps both at the frontend maximums and returns 400 otherwise.Gap 2 - float overrides bypassed the finite-check: #694 only validated
NumClustersandNumSubPaths. The float-valued keys the panel exposes (KF_mu,AS_A_mu,ES_A_mu,SC_lambda, ...) were unguarded, soNaN/Inf(whichJSON.stringifyencodes asnull) landed in Python asNoneand crashed deep in the generator with a cryptic 500float() argument must be a string or a real number, not 'NoneType'.Rather than grow the hand-curated allowlist again (which has to keep chasing the QuaDRiGa
.confschema), this validates every value instochastic_overridesuniformly: must be a non-boolint/floatand must be finite. Per-key upper bounds are declared in a small_STOCHASTIC_OVERRIDE_MAXdict and applied in the same loop.Tests
Adds two parametrized test classes in
tests/viewer/test_compute_routes.py:test_stochastic_override_upper_bound- rejectsNumClusters > 50,NumSubPaths > 20test_invalid_stochastic_float_override_value- rejectsNone/NaN/Inf/ strings / lists acrossKF_mu,AS_A_mu,ES_A_mu,SC_lambda,DS_mu,SF_sigma,XPR_muExisting tests for
NumClusters/NumSubPathsstill pass (full-k stochasticsuite green, 56 tests).Fixes #697
Built by
claude-code-actionas agentqa-bot