fix(spp_alerts): evaluate alert rules as their owner, not the elevated cron

spp_alerts/README.rst

@@ -771,6 +771,19 @@ Only test this section if multi-company is enabled.
 Changelog
 =========
 
+19.0.2.0.1
+~~~~~~~~~~
+
+- fix(security): evaluate each alert rule's monitored search as the user
+  who configured what the rule targets (new system-managed
+  ``eval_as_user_id``, re-bound to the editor whenever a targeting field
+  changes) instead of the elevated cron/superuser identity, so record
+  rules bound what a rule can surface to that user's own visibility. A
+  non-admin Alerts Manager can no longer author — or repoint an
+  admin-authored rule — to leak records they are not allowed to see via
+  the alerts the cron creates. The search now also runs in that user's
+  own company scope rather than the triggering cron's default company.
+
 19.0.2.0.0
 ~~~~~~~~~~
 

spp_alerts/__manifest__.py

@@ -4,7 +4,7 @@
     "summary": "Generic alert engine for threshold monitoring, expiry tracking, "
     "and deadline management across OpenSPP modules.",
     "category": "OpenSPP/Infrastructure",
-    "version": "19.0.2.0.0",
+    "version": "19.0.2.0.1",
     "sequence": 1,
     "author": "OpenSPP.org",
     "website": "https://github.com/OpenSPP/OpenSPP2",

spp_alerts/migrations/19.0.2.0.1/post-migration.py

@@ -0,0 +1,17 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Backfill eval_as_user_id for alert rules that predate 19.0.2.0.1.
+
+The evaluation identity that bounds a rule's monitored search to its
+configurer's record-rule visibility is new in this version. For rules created
+before the upgrade it defaults to whoever created the rule.
+"""
+
+
+def migrate(cr, version):
+    if not version:
+        return
+    # Every existing row predates eval_as_user_id, so set it authoritatively from
+    # create_uid. This is unconditional (not `WHERE ... IS NULL`) because the field
+    # carries no Python default: nothing else populates the column at upgrade time,
+    # and an IS NULL guard would be defeated if Odoo's _init_column ever pre-filled it.
+    cr.execute("UPDATE spp_alert_rule SET eval_as_user_id = create_uid")

spp_alerts/models/alert_rule.py

@@ -173,6 +173,64 @@ class AlertRule(models.Model):
         help="Number of alerts created by this rule",
     )
 
+    eval_as_user_id = fields.Many2one(
+        "res.users",
+        string="Evaluated As",
+        readonly=True,
+        help="User whose record-rule visibility bounds this rule's monitored search. "
+        "Set to whoever last defined what the rule targets, so evaluation can never "
+        "surface records the configurer cannot see. System-managed; not editable.",
+    )
+    # No Python `default` on purpose: a default would make Odoo's _init_column
+    # backfill existing rows with the *upgrade* user on module update (before the
+    # migration runs), and would let a client forge the value through a
+    # `default_eval_as_user_id` context key via default_get. The identity is set
+    # explicitly in create() instead, and the migration backfills existing rows.
+
+    # Fields that define what a rule reads or which records it surfaces. Changing
+    # any of them re-binds the evaluation identity to the editor (see write), so a
+    # rule can never be repointed — by model, domain, field, type, threshold, or by
+    # (re)activation — to surface records its editor is not allowed to see.
+    _EVAL_TARGETING_FIELDS = (
+        "model_id",
+        "domain_filter",
+        "monitored_field_id",
+        "date_field_id",
+        "rule_type",
+        "comparison",
+        "threshold_value",
+        "days_before",
+        "active",
+    )
+
+    @api.model_create_multi
+    def create(self, vals_list):
+        """Force the evaluation identity to the creator; it is never client-supplied.
+
+        Setting the key explicitly (rather than popping it) keeps the field present
+        in vals so Odoo's default_get — which honours a client `default_eval_as_user_id`
+        context key — is never consulted for it.
+        """
+        vals_list = [dict(vals, eval_as_user_id=self.env.uid) for vals in vals_list]
+        return super().create(vals_list)
+
+    def write(self, vals):
+        """Re-bind the evaluation identity to the editor when targeting changes.
+
+        eval_as_user_id is never client-writable directly; it tracks whoever last
+        defined what the rule targets, so record rules bound the monitored search
+        to that user's visibility regardless of the elevated cron that runs it. Note
+        the identity is `self.env.uid` (the acting user, preserved even under
+        `sudo()`); an explicit `with_user(<elevated>)` write would re-widen scope,
+        which is why only trusted internal callers should do that.
+        """
+        if "eval_as_user_id" in vals or any(field in vals for field in self._EVAL_TARGETING_FIELDS):
+            vals = dict(vals)
+            vals.pop("eval_as_user_id", None)
+            if any(field in vals for field in self._EVAL_TARGETING_FIELDS):
+                vals["eval_as_user_id"] = self.env.uid
+        return super().write(vals)
+
     def _compute_alert_count(self):
         """Compute the number of alerts associated with each rule."""
         alert_data = self.env["spp.alert"].read_group(
@@ -294,6 +352,31 @@ def _evaluate_rule(self):
             _logger.warning("Alert rule '%s' (ID: %d): model '%s' not found, skipping.", self.name, self.id, model_name)
             return 0
 
+        # Evaluate the monitored search as the user who configured what the rule
+        # targets (eval_as_user_id), not the elevated cron/superuser identity that
+        # may be triggering the run. Record rules are then enforced against the
+        # configurer, so a non-admin cannot surface — and leak, via alerts readable
+        # by all managers — records they are not allowed to see. eval_as_user_id is
+        # system-managed (re-bound to the editor on any targeting change) and never
+        # client-writable, so it cannot be forged to escalate. create_uid is the
+        # fallback for rows predating this field.
+        eval_user = self.eval_as_user_id or self.create_uid
+        if not eval_user:
+            # Fail closed: without a resolvable configurer we must not fall back to
+            # the elevated cron identity, which would search with record rules bypassed.
+            _logger.warning(
+                "Alert rule '%s' (ID: %d): no evaluation user resolved; skipping to avoid an elevated search.",
+                self.name,
+                self.id,
+            )
+            return 0
+        # Bind to the configurer AND their own company scope, so multi-company record
+        # rules apply as they would for that user — not as the triggering cron's
+        # default company. eval_user is a system-managed field, not client input.
+        Model = Model.with_user(eval_user.id).with_context(  # nosemgrep: odoo-with-user-unvalidated
+            allowed_company_ids=eval_user.company_ids.ids or eval_user.company_id.ids
+        )
+
         # Parse domain filter
         try:
             domain = safe_eval.safe_eval(  # nosemgrep: odoo-unsafe-safe-eval
@@ -453,9 +536,9 @@ def _prepare_alert_vals(self, record, model_name, current_value=None, days_until
     # Cron
     # -------------------------------------------------------------------------
 
-    # Cron runs as superuser (OdooBot). Rule evaluation searches monitored models with
-    # full access, bypassing record rules. This is intentional — only managers can create
-    # rules, so the monitored scope is admin-controlled.
+    # Cron runs as superuser (OdooBot), but each rule's monitored search is evaluated as
+    # the rule's owner (see _evaluate_rule), so record rules still bound what a rule can
+    # surface to whoever configured it — the elevated cron identity does not widen scope.
     @api.model
     def _cron_evaluate_rules(self):
         """Scheduled action to evaluate all active, configured rules."""

spp_alerts/readme/HISTORY.md

@@ -1,3 +1,14 @@
+### 19.0.2.0.1
+
+- fix(security): evaluate each alert rule's monitored search as the user who
+  configured what the rule targets (new system-managed `eval_as_user_id`, re-bound
+  to the editor whenever a targeting field changes) instead of the elevated
+  cron/superuser identity, so record rules bound what a rule can surface to that
+  user's own visibility. A non-admin Alerts Manager can no longer author — or
+  repoint an admin-authored rule — to leak records they are not allowed to see via
+  the alerts the cron creates. The search now also runs in that user's own company
+  scope rather than the triggering cron's default company.
+
 ### 19.0.2.0.0
 
 - Initial migration to OpenSPP2

spp_alerts/static/description/index.html

@@ -1164,6 +1164,20 @@ <h2><a class="toc-backref" href="#toc-entry-17">Changelog</a></h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.0.1</h1>
+<ul class="simple">
+<li>fix(security): evaluate each alert rule’s monitored search as the user
+who configured what the rule targets (new system-managed
+<tt class="docutils literal">eval_as_user_id</tt>, re-bound to the editor whenever a targeting field
+changes) instead of the elevated cron/superuser identity, so record
+rules bound what a rule can surface to that user’s own visibility. A
+non-admin Alerts Manager can no longer author — or repoint an
+admin-authored rule — to leak records they are not allowed to see via
+the alerts the cron creates. The search now also runs in that user’s
+own company scope rather than the triggering cron’s default company.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_alerts/tests/__init__.py

@@ -3,3 +3,4 @@
 from . import test_alert
 from . import test_alert_rule
 from . import test_rule_evaluation
+from . import test_rule_evaluation_access