security: staging batch 3 (#272 #342 #341 #262 #269 #325 #326 #335)

spp_cel_domain/README.rst

@@ -142,6 +142,31 @@ Dependencies
 Changelog
 =========
 
+19.0.2.1.2
+~~~~~~~~~~
+
+- fix: recognise ``me`` as a CEL context identifier. The resolver
+  rewrites cached variables (and the DCI override rewrites dotted
+  accessors) into ``metric('<accessor>', me)`` before identifiers are
+  extracted; because ``me`` was missing from
+  ``CEL_CONTEXT_IDENTIFIERS``, ``validate_expression`` /
+  ``validate_formula_expression`` wrongly reported valid expressions as
+  ``Undefined variables: me``. ``me`` is the individual record proxy in
+  the eval context, so it is now a recognised context identifier.
+
+19.0.2.1.1
+~~~~~~~~~~
+
+- fix(security): key metric cache lookups strictly by the requested
+  params. The provider clause used to fall back to param-agnostic cache
+  rows (``(provider, "")`` and ``("", "")``), so a parameterized
+  ``metric(..., arg=…)`` predicate could be satisfied by an
+  unparameterized/legacy cached value — silently selecting subjects by a
+  less-specific value in eligibility/targeting/DCI-search flows. Reads
+  are now keyed by the exact ``params_hash`` (both the freshness
+  preflight and the SQL fast path), and the compute/refresh path
+  re-caches under the correct params key.
+
 19.0.2.1.0
 ~~~~~~~~~~
 

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.2",
     "license": "LGPL-3",
     "development_status": "Production/Stable",
     "author": "OpenSPP.org, OpenSPP Community",

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,17 +1466,52 @@ 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 ""
         combos: list[tuple[str, str]] = [
             (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]] = []

spp_cel_domain/models/cel_variable_resolver.py

@@ -109,6 +109,7 @@ def _get_reserved_words(self):
                 "or",
                 "r",
                 "m",
+                "me",
                 "members",
                 "enrollments",
                 "entitlements",

spp_cel_domain/readme/HISTORY.md

@@ -1,3 +1,17 @@
+### 19.0.2.1.2
+
+- fix: recognise `me` as a CEL context identifier. The resolver rewrites cached
+  variables (and the DCI override rewrites dotted accessors) into
+  `metric('<accessor>', me)` before identifiers are extracted; because `me` was
+  missing from `CEL_CONTEXT_IDENTIFIERS`, `validate_expression` /
+  `validate_formula_expression` wrongly reported valid expressions as
+  `Undefined variables: me`. `me` is the individual record proxy in the eval
+  context, so it is now a recognised context identifier.
+
+### 19.0.2.1.1
+
+- fix(security): key metric cache lookups strictly by the requested params. The provider clause used to fall back to param-agnostic cache rows (`(provider, "")` and `("", "")`), so a parameterized `metric(..., arg=…)` predicate could be satisfied by an unparameterized/legacy cached value — silently selecting subjects by a less-specific value in eligibility/targeting/DCI-search flows. Reads are now keyed by the exact `params_hash` (both the freshness preflight and the SQL fast path), and the compute/refresh path re-caches under the correct params key.
+
 ### 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

spp_cel_domain/services/cel_parser.py

@@ -227,6 +227,7 @@ def __init__(self, kind: str, value: Any, pos: int):
 # ADR-008: Added 'r' as the standard prefix for current record access
 CEL_CONTEXT_IDENTIFIERS = {
     "m",
+    "me",
     "e",
     "r",
     "members",

spp_cel_domain/static/description/index.html

@@ -522,6 +522,33 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.1.2</h1>
+<ul class="simple">
+<li>fix: recognise <tt class="docutils literal">me</tt> as a CEL context identifier. The resolver
+rewrites cached variables (and the DCI override rewrites dotted
+accessors) into <tt class="docutils literal"><span class="pre">metric('&lt;accessor&gt;',</span> me)</tt> before identifiers are
+extracted; because <tt class="docutils literal">me</tt> was missing from
+<tt class="docutils literal">CEL_CONTEXT_IDENTIFIERS</tt>, <tt class="docutils literal">validate_expression</tt> /
+<tt class="docutils literal">validate_formula_expression</tt> wrongly reported valid expressions as
+<tt class="docutils literal">Undefined variables: me</tt>. <tt class="docutils literal">me</tt> is the individual record proxy in
+the eval context, so it is now a recognised context identifier.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
+<h1>19.0.2.1.1</h1>
+<ul class="simple">
+<li>fix(security): key metric cache lookups strictly by the requested
+params. The provider clause used to fall back to param-agnostic cache
+rows (<tt class="docutils literal">(provider, &quot;&quot;)</tt> and <tt class="docutils literal"><span class="pre">(&quot;&quot;,</span> &quot;&quot;)</tt>), so a parameterized
+<tt class="docutils literal"><span class="pre">metric(...,</span> <span class="pre">arg=…)</span></tt> predicate could be satisfied by an
+unparameterized/legacy cached value — silently selecting subjects by a
+less-specific value in eligibility/targeting/DCI-search flows. Reads
+are now keyed by the exact <tt class="docutils literal">params_hash</tt> (both the freshness
+preflight and the SQL fast path), and the compute/refresh path
+re-caches under the correct params key.</li>
+</ul>
+</div>
+<div class="section" id="section-3">
 <h1>19.0.2.1.0</h1>
 <ul class="simple">
 <li>feat(sql): compile CEL ternary expressions to SQL CASE via
@@ -533,7 +560,7 @@ <h1>19.0.2.1.0</h1>
 <li>test(translator): add coverage for the CEL translation cache helpers</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-4">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_cel_domain/tests/__init__.py

@@ -32,3 +32,5 @@
 from . import test_cel_relational_predicate
 from . import test_cel_smart_op_lookup
 from . import test_cel_translator_cache
+from . import test_cel_me_identifier
+from . import test_evaluate_accepts_params

spp_cel_domain/tests/test_cel_me_identifier.py

@@ -0,0 +1,46 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""`me` is a record-root context identifier and must not be flagged as an
+undefined variable during validation.
+
+The resolver rewrites cached variables into ``metric('<accessor>', me)`` and
+the DCI override rewrites dotted accessors the same way *before* the base
+resolver extracts identifiers. ``me`` then appears as a bare identifier in the
+scanned expression; unless it is a recognized context identifier,
+``validate_expression`` / ``validate_formula_expression`` wrongly report
+``Undefined variables: me``.
+"""
+
+from odoo.tests import TransactionCase, tagged
+
+
+@tagged("post_install", "-at_install")
+class TestMeContextIdentifier(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.resolver = cls.env["spp.cel.variable.resolver"]
+        cls.service = cls.env["spp.cel.service"]
+
+    def test_me_is_a_context_identifier(self):
+        from odoo.addons.spp_cel_domain.services.cel_parser import CEL_CONTEXT_IDENTIFIERS
+
+        self.assertIn("me", CEL_CONTEXT_IDENTIFIERS)
+
+    def test_expand_does_not_flag_me_as_missing(self):
+        result = self.resolver.expand_expression("metric('foo', me) == true")
+        self.assertNotIn("me", result["missing_variables"])
+
+    def test_validate_expression_accepts_bare_me(self):
+        result = self.resolver.validate_expression("metric('foo', me) == true")
+        self.assertTrue(
+            result["valid"],
+            f"expression with bare me should validate; errors: {result['errors']}",
+        )
+        self.assertNotIn(
+            "Undefined variables: me",
+            " ".join(result["errors"]),
+        )
+
+    def test_validate_formula_expression_accepts_bare_me(self):
+        result = self.service.validate_formula_expression("metric('foo', me)", "individual")
+        self.assertNotIn("Missing variables: me", result.get("error") or "")

spp_cel_domain/tests/test_evaluate_accepts_params.py

@@ -0,0 +1,51 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Unit tests for the params-compatibility guard used before calling the
+legacy/external evaluation service (spp.indicator.evaluate).
+
+The metric compute path passes the metric's params to svc.evaluate only when
+that method accepts a `params` kwarg, so a parameterized refresh is computed
+with the right params without risking a TypeError on an older service that
+predates the kwarg. See _evaluate_accepts_params / _svc_evaluate_batch in
+cel_executor.py.
+"""
+
+from odoo.tests.common import TransactionCase, tagged
+
+
+class _EvalWithParams:
+    def evaluate(self, metric, model, ids, period_key, mode="fallback", params=None):
+        return {}, {}
+
+
+class _EvalWithKwargs:
+    def evaluate(self, metric, model, ids, period_key, **kwargs):
+        return {}, {}
+
+
+class _EvalNoParams:
+    def evaluate(self, metric, model, ids, period_key, mode="fallback"):
+        return {}, {}
+
+
+class _NonCallableEvaluate:
+    evaluate = 42  # inspect.signature() raises TypeError -> degrade to False
+
+
+@tagged("post_install", "-at_install")
+class TestEvaluateAcceptsParams(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.executor = cls.env["spp.cel.executor"]
+
+    def test_explicit_params_arg_supported(self):
+        self.assertTrue(self.executor._evaluate_accepts_params(_EvalWithParams()))
+
+    def test_var_keyword_supported(self):
+        self.assertTrue(self.executor._evaluate_accepts_params(_EvalWithKwargs()))
+
+    def test_no_params_not_supported(self):
+        self.assertFalse(self.executor._evaluate_accepts_params(_EvalNoParams()))
+
+    def test_uninspectable_evaluate_degrades_to_false(self):
+        self.assertFalse(self.executor._evaluate_accepts_params(_NonCallableEvaluate()))

spp_dci_client/README.rst

@@ -140,6 +140,17 @@ Dependencies
 Changelog
 =========
 
+19.0.2.0.2
+~~~~~~~~~~
+
+- fix(security): make the OAuth2 token/header methods private so they
+  are no longer callable over RPC — a low-privilege internal user can no
+  longer mint a DCI access token or obtain a Bearer header via
+  ``get_oauth2_token()`` / ``get_headers()``. Restrict the token cache
+  fields (``_oauth2_access_token`` / ``_oauth2_token_expires_at``) to
+  system administrators, and require write access to run a connection
+  test.
+
 19.0.2.0.0
 ~~~~~~~~~~
 

spp_dci_client/__manifest__.py

@@ -2,7 +2,7 @@
 {
     "name": "OpenSPP DCI Client",
     "summary": "Base DCI client infrastructure with OAuth2 and data source management",
-    "version": "19.0.2.0.1",
+    "version": "19.0.2.0.2",
     "category": "OpenSPP/Integration",
     "author": "OpenSPP.org",
     "website": "https://github.com/OpenSPP/OpenSPP2",