diff --git a/spp_cel_domain/README.rst b/spp_cel_domain/README.rst index 9a72fa2b4..44ebf3975 100644 --- a/spp_cel_domain/README.rst +++ b/spp_cel_domain/README.rst @@ -142,6 +142,15 @@ Dependencies Changelog ========= +19.0.2.1.1 +~~~~~~~~~~ + +- fix(security): metric cache lookups are keyed strictly by the + requested params — a parameterized metric request no longer falls back + to unparameterized (or differently-parameterized) cache rows, which + could return values computed for other parameters. ``evaluate()`` also + guards the ``params`` kwarg against legacy signatures. + 19.0.2.1.0 ~~~~~~~~~~ diff --git a/spp_cel_domain/__manifest__.py b/spp_cel_domain/__manifest__.py index 550c3d0f3..d53b3c3a2 100644 --- a/spp_cel_domain/__manifest__.py +++ b/spp_cel_domain/__manifest__.py @@ -2,7 +2,7 @@ { "name": "CEL Domain Query Builder", "summary": "Write simple CEL-like expressions to filter records (OpenSPP/OpenG2P friendly)", - "version": "19.0.2.1.0", + "version": "19.0.2.1.1", "license": "LGPL-3", "development_status": "Production/Stable", "author": "OpenSPP.org, OpenSPP Community", diff --git a/spp_cel_domain/models/cel_executor.py b/spp_cel_domain/models/cel_executor.py index b9fb36003..a0241e02e 100644 --- a/spp_cel_domain/models/cel_executor.py +++ b/spp_cel_domain/models/cel_executor.py @@ -1,5 +1,6 @@ from __future__ import annotations +import inspect import logging from collections.abc import Iterable, Iterator from typing import Any @@ -1218,7 +1219,9 @@ def _exec_metric( if not batch_ids: continue total_requested += len(batch_ids) - batch_values, batch_stats = svc.evaluate(p.metric, subject_model, batch_ids, period_key, mode=eval_mode) + batch_values, batch_stats = self._svc_evaluate_batch( # pragma: no cover + svc, p, subject_model, batch_ids, period_key, eval_mode + ) aggregated_values.update(batch_values) if batch_stats: stats_total["cache_hits"] += int(batch_stats.get("cache_hits") or 0) @@ -1463,6 +1466,39 @@ def _feature_value_subquery( ) return SQL("(%s)", SQL(sql, *args)) + @staticmethod + def _evaluate_accepts_params(svc) -> bool: + """Whether ``svc.evaluate`` accepts a ``params`` keyword argument. + + The evaluation service (``spp.indicator``) is provided by a legacy/external + module whose signature we do not control and which may predate the + ``params`` kwarg. Returns True when ``evaluate`` declares an explicit + ``params`` parameter or a ``**kwargs`` catch-all; False otherwise (so the + caller degrades to an unparameterized call instead of raising ``TypeError``). + """ + try: + sig = inspect.signature(svc.evaluate) + except (TypeError, ValueError): + return False + return any(prm.name == "params" or prm.kind is inspect.Parameter.VAR_KEYWORD for prm in sig.parameters.values()) + + def _svc_evaluate_batch(self, svc, p, subject_model, batch_ids, period_key, eval_mode): # pragma: no cover + """Call the legacy/external evaluation service for one batch. + + Threads the metric's params through so parameterized refreshes are computed + with the right params — but only when ``evaluate`` accepts a ``params`` kwarg + (see ``_evaluate_accepts_params``), degrading gracefully on older services. + + Not covered by tests: ``spp.indicator`` is not in this repo's dependency + closure, so this path is unreachable here; the params-compat decision is + unit-tested via ``_evaluate_accepts_params``. + """ + eval_kwargs = {"mode": eval_mode} + metric_params = getattr(p, "params", None) + if metric_params and self._evaluate_accepts_params(svc): + eval_kwargs["params"] = metric_params + return svc.evaluate(p.metric, subject_model, batch_ids, period_key, **eval_kwargs) + def _provider_clause(self, provider: str, params_hash: str, allow_any_provider: bool) -> tuple[str, list[Any]]: provider = provider or "" params_hash = params_hash or "" @@ -1470,10 +1506,12 @@ def _provider_clause(self, provider: str, params_hash: str, allow_any_provider: (provider, params_hash), ] if provider: + # Relax the provider (a routing/registry detail) but keep the requested + # params_hash. Params are a semantic filter, not a provider detail: a + # non-empty params_hash must never fall back to params_hash "" rows, or a + # parameterized metric would match unparameterized/legacy cache rows. When + # params_hash == "" this combo already covers the unparameterized rows. combos.append(("", params_hash)) - if params_hash: - combos.append((provider, "")) - combos.append(("", "")) # Deduplicate while preserving order seen = set() uniq_combos: list[tuple[str, str]] = [] diff --git a/spp_cel_domain/readme/HISTORY.md b/spp_cel_domain/readme/HISTORY.md index 1b4fd10d3..a8f2a1c02 100644 --- a/spp_cel_domain/readme/HISTORY.md +++ b/spp_cel_domain/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.2.1.1 + +- fix(security): metric cache lookups are keyed strictly by the requested params — a parameterized metric request no longer falls back to unparameterized (or differently-parameterized) cache rows, which could return values computed for other parameters. `evaluate()` also guards the `params` kwarg against legacy signatures. + ### 19.0.2.1.0 - feat(sql): compile CEL ternary expressions to SQL CASE via `to_sql_case`, with `case_when`/`comparison` builders and a right-associative ternary parsing fix diff --git a/spp_cel_domain/static/description/index.html b/spp_cel_domain/static/description/index.html index f280ce2a1..99156805f 100644 --- a/spp_cel_domain/static/description/index.html +++ b/spp_cel_domain/static/description/index.html @@ -522,6 +522,16 @@

Changelog

+

19.0.2.1.1

+ +
+

19.0.2.1.0

-
+

19.0.2.0.0