From 78333f19a27fa523cb137617ba983a2858b8fd60 Mon Sep 17 00:00:00 2001
From: Ken Lewerentz <ken@openspp.org>
Date: Wed, 26 Aug 2026 13:47:23 +0700
Subject: [PATCH 1/4] fix(change_request): evaluate field-mapping transform
 expressions again
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

_eval_expression passed nocopy=True to safe_eval, which takes no such
argument in Odoo 19. Every transform expression therefore raised
TypeError, the blanket fallback swallowed it, and the untransformed value
was written to the registrant — so a configured Expression transform was
silently ignored, reported only as a warning in the log. It was the only
caller passing nocopy in the repository.

Failures now log at exception level with the offending expression: a
quiet warning is precisely how this went unnoticed. The fallback itself
is unchanged — a broken expression still writes the raw value rather
than failing the apply — which is arguably wrong for the same
fail-closed reason as rejecting an apply that can write nothing, but
that is a separate behavioural decision.

Tests cover a working transform, one referencing the registrant, one
whose result matches the stored value, the fallback plus its log, and
that env stays unreachable from an expression — that boundary matters
now that expressions actually execute.
---
 spp_change_request_v2/README.rst              |  15 +++
 spp_change_request_v2/__manifest__.py         |   2 +-
 spp_change_request_v2/readme/HISTORY.md       |   4 +
 .../static/description/index.html             |  38 ++++--
 .../strategies/field_mapping.py               |  16 ++-
 spp_change_request_v2/tests/__init__.py       |   1 +
 .../tests/test_field_mapping_transform.py     | 120 ++++++++++++++++++
 7 files changed, 180 insertions(+), 16 deletions(-)
 create mode 100644 spp_change_request_v2/tests/test_field_mapping_transform.py

diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst
index dc463b5be..d4f62b80d 100644
--- a/spp_change_request_v2/README.rst
+++ b/spp_change_request_v2/README.rst
@@ -853,6 +853,21 @@ Before declaring a new CR type complete:
 Changelog
 =========
 
+19.0.3.1.11
+~~~~~~~~~~~
+
+- fix(change_request): field-mapping transform expressions are evaluated
+  again. ``_eval_expression`` passed ``nocopy=True`` to ``safe_eval``,
+  which takes no such argument in Odoo 19, so every expression raised
+  ``TypeError``; the blanket fallback swallowed it and the
+  **untransformed** value was written to the registrant. A configured
+  transform was therefore ignored, reported only as a warning in the
+  log. Failures now log at exception level with the offending
+  expression, since a quiet warning is how this went unnoticed.
+  **Behaviour change:** request types that already have an Expression
+  transform configured will start transforming values on upgrade, having
+  silently passed the raw value through until now.
+
 19.0.3.1.2
 ~~~~~~~~~~
 
diff --git a/spp_change_request_v2/__manifest__.py b/spp_change_request_v2/__manifest__.py
index 181d8fec5..f34e3557e 100644
--- a/spp_change_request_v2/__manifest__.py
+++ b/spp_change_request_v2/__manifest__.py
@@ -1,6 +1,6 @@
 {
     "name": "OpenSPP Change Request V2",
-    "version": "19.0.3.1.2",
+    "version": "19.0.3.1.11",
     "sequence": 50,
     "category": "OpenSPP",
     "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",
diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md
index e6296543e..b53ea4e6e 100644
--- a/spp_change_request_v2/readme/HISTORY.md
+++ b/spp_change_request_v2/readme/HISTORY.md
@@ -1,3 +1,7 @@
+### 19.0.3.1.11
+
+- fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. Failures now log at exception level with the offending expression, since a quiet warning is how this went unnoticed. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now.
+
 ### 19.0.3.1.2
 
 - fix(change_request_v2): adding an ID now looks for a live one of that type rather than any row at all, so an ID that was removed through a change request no longer blocks adding a replacement of the same type (#1136)
diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html
index 1587cb85c..50ea6f78f 100644
--- a/spp_change_request_v2/static/description/index.html
+++ b/spp_change_request_v2/static/description/index.html
@@ -1339,6 +1339,22 @@ <h2>Changelog</h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.3.1.11</h1>
+<ul class="simple">
+<li>fix(change_request): field-mapping transform expressions are evaluated
+again. <tt class="docutils literal">_eval_expression</tt> passed <tt class="docutils literal">nocopy=True</tt> to <tt class="docutils literal">safe_eval</tt>,
+which takes no such argument in Odoo 19, so every expression raised
+<tt class="docutils literal">TypeError</tt>; the blanket fallback swallowed it and the
+<strong>untransformed</strong> value was written to the registrant. A configured
+transform was therefore ignored, reported only as a warning in the
+log. Failures now log at exception level with the offending
+expression, since a quiet warning is how this went unnoticed.
+<strong>Behaviour change:</strong> request types that already have an Expression
+transform configured will start transforming values on upgrade, having
+silently passed the raw value through until now.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.3.1.2</h1>
 <ul class="simple">
 <li>fix(change_request_v2): adding an ID now looks for a live one of that
@@ -1347,7 +1363,7 @@ <h1>19.0.3.1.2</h1>
 (#1136)</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.3.1.1</h1>
 <ul class="simple">
 <li>fix(change_request): enforce the <tt class="docutils literal">(cr_type_id, reason)</tt> uniqueness
@@ -1361,7 +1377,7 @@ <h1>19.0.3.1.1</h1>
 applied) so the constraint applies cleanly on upgrade.</li>
 </ul>
 </div>
-<div class="section" id="section-3">
+<div class="section" id="section-4">
 <h1>19.0.3.1.0</h1>
 <ul class="simple">
 <li>revert(change_request): restore the create-a-new-individual <strong>Add
@@ -1379,7 +1395,7 @@ <h1>19.0.3.1.0</h1>
 <strong>not</strong> restored here; reinstate separately if needed.</li>
 </ul>
 </div>
-<div class="section" id="section-4">
+<div class="section" id="section-5">
 <h1>19.0.3.0.0</h1>
 <ul class="simple">
 <li>feat(change_request): redesign the group/membership CR flows (#242) —
@@ -1401,7 +1417,7 @@ <h1>19.0.3.0.0</h1>
 must adapt (see #1133).</li>
 </ul>
 </div>
-<div class="section" id="section-5">
+<div class="section" id="section-6">
 <h1>19.0.2.0.8</h1>
 <ul class="simple">
 <li>fix(views): disable inline creation of CR document types on the Change
@@ -1412,7 +1428,7 @@ <h1>19.0.2.0.8</h1>
 Documents” modal (missing Name field) that blocked saving (#1125)</li>
 </ul>
 </div>
-<div class="section" id="section-6">
+<div class="section" id="section-7">
 <h1>19.0.2.0.7</h1>
 <ul class="simple">
 <li>fix(security): align CR Requestor / CR Local Validator / CR HQ
@@ -1424,7 +1440,7 @@ <h1>19.0.2.0.7</h1>
 dependencies.</li>
 </ul>
 </div>
-<div class="section" id="section-7">
+<div class="section" id="section-8">
 <h1>19.0.2.0.6</h1>
 <ul class="simple">
 <li>fix(views): route post-submit CRs (pending / approved / applied /
@@ -1439,7 +1455,7 @@ <h1>19.0.2.0.6</h1>
 list so row-click goes through the stage router.</li>
 </ul>
 </div>
-<div class="section" id="section-8">
+<div class="section" id="section-9">
 <h1>19.0.2.0.5</h1>
 <ul class="simple">
 <li>fix(security): add a global <tt class="docutils literal">ir.rule</tt> on <tt class="docutils literal">spp.change.request</tt> that
@@ -1452,27 +1468,27 @@ <h1>19.0.2.0.5</h1>
 roles).</li>
 </ul>
 </div>
-<div class="section" id="section-9">
+<div class="section" id="section-10">
 <h1>19.0.2.0.3</h1>
 <ul class="simple">
 <li>fix: add HTML escaping to all computed Html fields with
 <tt class="docutils literal">sanitize=False</tt> to prevent stored XSS (#50)</li>
 </ul>
 </div>
-<div class="section" id="section-10">
+<div class="section" id="section-11">
 <h1>19.0.2.0.2</h1>
 <ul class="simple">
 <li>fix: fix batch approval wizard line deletion (#130)</li>
 </ul>
 </div>
-<div class="section" id="section-11">
+<div class="section" id="section-12">
 <h1>19.0.2.0.1</h1>
 <ul class="simple">
 <li>fix: skip field types before getattr and isolate detail prefetch
 (#129)</li>
 </ul>
 </div>
-<div class="section" id="section-12">
+<div class="section" id="section-13">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>
diff --git a/spp_change_request_v2/strategies/field_mapping.py b/spp_change_request_v2/strategies/field_mapping.py
index a1c795e68..ff949737a 100644
--- a/spp_change_request_v2/strategies/field_mapping.py
+++ b/spp_change_request_v2/strategies/field_mapping.py
@@ -72,7 +72,13 @@ def apply(self, change_request):
         return True
 
     def _eval_expression(self, expr, value, detail, registrant):
-        """Safely evaluate transform expression."""
+        """Safely evaluate transform expression.
+
+        Note ``safe_eval`` takes no ``nocopy`` argument in Odoo 19 -- passing it
+        raised ``TypeError`` for every expression, which the fallback below
+        swallowed, so configured transforms were silently ignored and the
+        untransformed value was written instead.
+        """
         try:
             # Admin-defined field mapping expressions with restricted context (no env)
             return safe_eval(  # nosemgrep: odoo-unsafe-safe-eval
@@ -86,10 +92,12 @@ def _eval_expression(self, expr, value, detail, registrant):
                     "date": date,
                 },
                 mode="eval",
-                nocopy=True,
             )
-        except Exception as e:
-            _logger.warning("Expression eval failed: %s", e)
+        except Exception:
+            # Falls back to the untransformed value rather than failing the
+            # apply. Logged at exception level with the expression, because a
+            # silent warning is how the ``nocopy`` breakage went unnoticed.
+            _logger.exception("Field mapping transform expression failed, using the raw value: %s", expr)
             return value
 
     def _is_value_empty(self, value, record=None, field_name=None):
diff --git a/spp_change_request_v2/tests/__init__.py b/spp_change_request_v2/tests/__init__.py
index 51588fdeb..c35b6dd9d 100644
--- a/spp_change_request_v2/tests/__init__.py
+++ b/spp_change_request_v2/tests/__init__.py
@@ -26,3 +26,4 @@
 from . import test_html_escaping
 from . import test_wizard_html_escaping
 from . import test_reason_document_constraint
+from . import test_field_mapping_transform
diff --git a/spp_change_request_v2/tests/test_field_mapping_transform.py b/spp_change_request_v2/tests/test_field_mapping_transform.py
new file mode 100644
index 000000000..3bf9e46d3
--- /dev/null
+++ b/spp_change_request_v2/tests/test_field_mapping_transform.py
@@ -0,0 +1,120 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Field-mapping transform expressions must actually be evaluated.
+
+``_eval_expression`` passed ``nocopy=True`` to ``safe_eval``, which takes no
+such argument in Odoo 19. Every expression therefore raised ``TypeError``, the
+blanket fallback swallowed it, and the *untransformed* value was written to the
+registrant -- so a configured transform was silently ignored and only a warning
+in the log said so.
+"""
+
+from odoo.exceptions import UserError
+from odoo.tests import TransactionCase, tagged
+
+
+@tagged("post_install", "-at_install")
+class TestFieldMappingTransform(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.registrant = cls.env["res.partner"].create(
+            {
+                "name": "Transform Registrant",
+                "given_name": "john",
+                "family_name": "Fam",
+                "is_registrant": True,
+                "is_group": False,
+            }
+        )
+
+    def _type_with_transform(self, code, expression):
+        return self.env["spp.change.request.type"].create(
+            {
+                "code": code,
+                "name": code,
+                "target_type": "individual",
+                "detail_model": "spp.cr.detail.edit_individual",
+                "apply_strategy": "field_mapping",
+                "apply_mapping_ids": [
+                    (
+                        0,
+                        0,
+                        {
+                            "source_field": "given_name",
+                            "target_field": "given_name",
+                            "transform": "expression",
+                            "transform_expression": expression,
+                        },
+                    )
+                ],
+            }
+        )
+
+    def _apply(self, cr_type, detail_vals):
+        cr = self.env["spp.change.request"].create({"request_type_id": cr_type.id, "registrant_id": self.registrant.id})
+        cr.get_detail().write(detail_vals)
+        self.env["spp.cr.strategy.field_mapping"].apply(cr)
+        return cr
+
+    def test_transform_expression_is_applied(self):
+        cr_type = self._type_with_transform("tf_upper", "value.upper()")
+        self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(
+            self.registrant.given_name,
+            "JANE",
+            "the configured transform must be applied, not silently ignored",
+        )
+
+    def test_transform_can_reference_the_registrant(self):
+        cr_type = self._type_with_transform("tf_ref", "value + '-' + registrant.family_name")
+        self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(self.registrant.given_name, "jane-Fam")
+
+    def test_transform_result_is_what_gets_compared(self):
+        """A transform landing on the stored value means there is nothing to write."""
+        cr_type = self._type_with_transform("tf_noop", "'john'")
+        self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(self.registrant.given_name, "john")
+
+    def test_a_broken_expression_falls_back_to_the_raw_value(self):
+        cr_type = self._type_with_transform("tf_broken", "value.no_such_method()")
+        with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR") as logs:
+            self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(
+            self.registrant.given_name,
+            "jane",
+            "a failing expression falls back to the untransformed value",
+        )
+        self.assertTrue(
+            any("transform expression failed" in line for line in logs.output),
+            "the failure must be logged loudly enough to be diagnosable",
+        )
+
+    def test_env_is_not_reachable_from_an_expression(self):
+        """The context deliberately omits env; keep it that way."""
+        cr_type = self._type_with_transform("tf_env", "env['res.users'].search([])")
+        with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR"):
+            self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(self.registrant.given_name, "jane")
+
+    def test_direct_mappings_are_unaffected(self):
+        cr_type = self.env["spp.change.request.type"].create(
+            {
+                "code": "tf_direct",
+                "name": "tf_direct",
+                "target_type": "individual",
+                "detail_model": "spp.cr.detail.edit_individual",
+                "apply_strategy": "field_mapping",
+                "apply_mapping_ids": [(0, 0, {"source_field": "given_name", "target_field": "given_name"})],
+            }
+        )
+        self._apply(cr_type, {"given_name": "jane"})
+        self.assertEqual(self.registrant.given_name, "jane")
+
+    def test_unrelated_apply_still_raises_without_a_detail(self):
+        """Guard against the fallback masking a genuinely missing detail."""
+        cr_type = self._type_with_transform("tf_nodetail", "value.upper()")
+        cr = self.env["spp.change.request"].create({"request_type_id": cr_type.id, "registrant_id": self.registrant.id})
+        cr.write({"detail_res_id": False})
+        with self.assertRaises(UserError):
+            self.env["spp.cr.strategy.field_mapping"].apply(cr)

From 38e05b5ab5dca44d9daa87257b04c45b52b3fd6f Mon Sep 17 00:00:00 2001
From: Ken Lewerentz <ken@openspp.org>
Date: Thu, 27 Aug 2026 17:05:27 +0700
Subject: [PATCH 2/4] fix(security): sandbox and fail-close field-mapping
 transform expressions

The transform-expression fix restored evaluation of a context holding live
`detail`/`registrant` recordsets. safe_eval applies no attribute allowlist, so
those exposed env, sudo() and the cursor -- a change-request manager (not a
system admin) could reach superuser ORM and raw SQL. Harden the path:

- Pass attribute-readable snapshots (stored scalars only, m2o->id) instead of
  recordsets, so no ORM handle is reachable from an expression.
- Restrict transform_expression to base.group_system (ORM-enforced), and read
  it via sudo() in the detection path, which runs as the requester -- otherwise
  the field-groups guard would raise AccessError or silently skip the transform
  and put detection and apply back out of step.
- Fail closed: an unevaluable expression raises instead of writing the raw,
  requester-controlled value. Log error type + expression at ERROR (never the
  wrapped error, which embeds the PII field value), traceback at DEBUG only;
  the UserError message omits the underlying error since it is persisted to
  apply_error.

Tests assert the ORM escapes fail closed, the failure log carries no field
value, and a CR manager cannot author the expression.
---
 spp_change_request_v2/README.rst              | 61 ++++++++----
 .../models/change_request_type_mapping.py     | 10 +-
 spp_change_request_v2/readme/HISTORY.md       |  5 +-
 .../static/description/index.html             | 33 +++++--
 .../strategies/field_mapping.py               | 96 +++++++++++++++----
 .../tests/test_field_mapping_transform.py     | 84 ++++++++++++----
 6 files changed, 221 insertions(+), 68 deletions(-)

diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst
index d23894bb7..1c950c415 100644
--- a/spp_change_request_v2/README.rst
+++ b/spp_change_request_v2/README.rst
@@ -752,22 +752,22 @@ Methods available for override on detail models (all inherited from
 Related fields available on all detail models (from
 ``spp.cr.detail.base``):
 
-+--------------------------+-----------+------------------------------------------------------------+
-| Field                    | Type      | Source                                                     |
-+==========================+===========+============================================================+
-| ``change_request_id``    | Many2one  | Direct link to parent CR                                   |
-+--------------------------+-----------+------------------------------------------------------------+
-| ``registrant_id``        | Many2one  | ``change_request_id.registrant_id``                        |
-+--------------------------+-----------+------------------------------------------------------------+
-| ``approval_state``       | Selection | ``change_request_id.approval_state``                       |
-+--------------------------+-----------+------------------------------------------------------------+
-| ``is_applied``           | Boolean   | ``change_request_id.is_applied``                           |
-+--------------------------+-----------+------------------------------------------------------------+
-| ``use_dynamic_approval`` | Boolean   | ``change_request_id.request_type_id.use_dynamic_approval`` |
-+--------------------------+-----------+------------------------------------------------------------+
-| ``field_to_modify``      | Selection | Dynamic field selector (populated by                       |
-|                          |           | ``_get_field_to_modify_selection``)                        |
-+--------------------------+-----------+------------------------------------------------------------+
++----------------------------+-----------+------------------------------------------------------------+
+| Field                      | Type      | Source                                                     |
++============================+===========+============================================================+
+| ``change_request_id``      | Many2one  | Direct link to parent CR                                   |
++----------------------------+-----------+------------------------------------------------------------+
+| ``registrant_id``          | Many2one  | ``change_request_id.registrant_id``                        |
++----------------------------+-----------+------------------------------------------------------------+
+| ``approval_state``         | Selection | ``change_request_id.approval_state``                       |
++----------------------------+-----------+------------------------------------------------------------+
+| ``is_applied``             | Boolean   | ``change_request_id.is_applied``                           |
++----------------------------+-----------+------------------------------------------------------------+
+| ``use_dynamic_approval``   | Boolean   | ``change_request_id.request_type_id.use_dynamic_approval`` |
++----------------------------+-----------+------------------------------------------------------------+
+| ``field_to_modify``        | Selection | Dynamic field selector (populated by                       |
+|                            |           | ``_get_field_to_modify_selection``)                        |
++----------------------------+-----------+------------------------------------------------------------+
 
 CR Type Fields Reference
 ~~~~~~~~~~~~~~~~~~~~~~~~
@@ -862,11 +862,30 @@ Changelog
   ``TypeError``; the blanket fallback swallowed it and the
   **untransformed** value was written to the registrant. A configured
   transform was therefore ignored, reported only as a warning in the
-  log. Failures now log at exception level with the offending
-  expression, since a quiet warning is how this went unnoticed.
-  **Behaviour change:** request types that already have an Expression
-  transform configured will start transforming values on upgrade, having
-  silently passed the raw value through until now.
+  log. **Behaviour change:** request types that already have an
+  Expression transform configured will start transforming values on
+  upgrade, having silently passed the raw value through until now.
+- fix(security): a transform expression can no longer reach the ORM.
+  ``safe_eval`` places no allowlist on attribute access, so a live
+  ``detail``/``registrant`` recordset in the evaluation context exposed
+  ``env``, ``sudo()`` and the database cursor — a change-request
+  manager, who is not a system administrator, could obtain superuser ORM
+  access and raw SQL. The context now carries attribute-readable
+  snapshots of the two records (stored scalar fields only; no methods,
+  no relation traversal, no database handle) instead of the recordsets
+  themselves.
+- fix(security): the transform expression is now restricted to system
+  administrators (``groups="base.group_system"``) rather than only
+  warned against in the help text, and is enforced by the ORM on read
+  and write. The detection path reads the expression as superuser so it
+  keeps working for non-administrator requesters.
+- fix(security): an unevaluable transform expression now fails closed —
+  the change is not applied — instead of falling back to writing the raw
+  value. Because the source value is requester-controlled, the fallback
+  let a requester force the untransformed value onto the registrant by
+  feeding input the transform could not handle. Failures are logged with
+  the expression and error type (never the field value, which is PII);
+  the full traceback is logged only at DEBUG.
 
 19.0.3.1.10
 ~~~~~~~~~~~
diff --git a/spp_change_request_v2/models/change_request_type_mapping.py b/spp_change_request_v2/models/change_request_type_mapping.py
index d4275fe08..c807f4d0f 100644
--- a/spp_change_request_v2/models/change_request_type_mapping.py
+++ b/spp_change_request_v2/models/change_request_type_mapping.py
@@ -32,10 +32,14 @@ class SPPChangeRequestTypeMapping(models.Model):
         default="direct",
     )
     transform_expression = fields.Char(
+        groups="base.group_system",
         help=(
             "Python expression for value transformation. "
-            "Available variables: value, detail, registrant, datetime, date. "
-            "WARNING: Only administrators should configure expressions - "
-            "arbitrary code execution risk."
+            "Available variables: value (the source value), and read-only snapshots "
+            "of detail and registrant exposing their stored scalar fields only - "
+            "no method calls, no relation traversal, no database access - plus "
+            "datetime and date. Restricted to system administrators: it is evaluated "
+            "server-side and an unevaluable expression blocks the change rather than "
+            "writing the raw value."
         ),
     )
diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md
index 2e7839b6f..a0b10e116 100644
--- a/spp_change_request_v2/readme/HISTORY.md
+++ b/spp_change_request_v2/readme/HISTORY.md
@@ -1,6 +1,9 @@
 ### 19.0.3.1.11
 
-- fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. Failures now log at exception level with the offending expression, since a quiet warning is how this went unnoticed. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now.
+- fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now.
+- fix(security): a transform expression can no longer reach the ORM. `safe_eval` places no allowlist on attribute access, so a live `detail`/`registrant` recordset in the evaluation context exposed `env`, `sudo()` and the database cursor — a change-request manager, who is not a system administrator, could obtain superuser ORM access and raw SQL. The context now carries attribute-readable snapshots of the two records (stored scalar fields only; no methods, no relation traversal, no database handle) instead of the recordsets themselves.
+- fix(security): the transform expression is now restricted to system administrators (`groups="base.group_system"`) rather than only warned against in the help text, and is enforced by the ORM on read and write. The detection path reads the expression as superuser so it keeps working for non-administrator requesters.
+- fix(security): an unevaluable transform expression now fails closed — the change is not applied — instead of falling back to writing the raw value. Because the source value is requester-controlled, the fallback let a requester force the untransformed value onto the registrant by feeding input the transform could not handle. Failures are logged with the expression and error type (never the field value, which is PII); the full traceback is logged only at DEBUG.
 
 ### 19.0.3.1.10
 
diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html
index 92241d61c..fb7e67e84 100644
--- a/spp_change_request_v2/static/description/index.html
+++ b/spp_change_request_v2/static/description/index.html
@@ -1160,9 +1160,9 @@ <h1>Methods Reference</h1>
 <tt class="docutils literal">spp.cr.detail.base</tt>):</p>
 <table border="1" class="docutils">
 <colgroup>
-<col width="27%" />
+<col width="28%" />
 <col width="11%" />
-<col width="62%" />
+<col width="61%" />
 </colgroup>
 <thead valign="bottom">
 <tr><th class="head">Field</th>
@@ -1347,11 +1347,30 @@ <h1>19.0.3.1.11</h1>
 <tt class="docutils literal">TypeError</tt>; the blanket fallback swallowed it and the
 <strong>untransformed</strong> value was written to the registrant. A configured
 transform was therefore ignored, reported only as a warning in the
-log. Failures now log at exception level with the offending
-expression, since a quiet warning is how this went unnoticed.
-<strong>Behaviour change:</strong> request types that already have an Expression
-transform configured will start transforming values on upgrade, having
-silently passed the raw value through until now.</li>
+log. <strong>Behaviour change:</strong> request types that already have an
+Expression transform configured will start transforming values on
+upgrade, having silently passed the raw value through until now.</li>
+<li>fix(security): a transform expression can no longer reach the ORM.
+<tt class="docutils literal">safe_eval</tt> places no allowlist on attribute access, so a live
+<tt class="docutils literal">detail</tt>/<tt class="docutils literal">registrant</tt> recordset in the evaluation context exposed
+<tt class="docutils literal">env</tt>, <tt class="docutils literal">sudo()</tt> and the database cursor — a change-request
+manager, who is not a system administrator, could obtain superuser ORM
+access and raw SQL. The context now carries attribute-readable
+snapshots of the two records (stored scalar fields only; no methods,
+no relation traversal, no database handle) instead of the recordsets
+themselves.</li>
+<li>fix(security): the transform expression is now restricted to system
+administrators (<tt class="docutils literal"><span class="pre">groups=&quot;base.group_system&quot;</span></tt>) rather than only
+warned against in the help text, and is enforced by the ORM on read
+and write. The detection path reads the expression as superuser so it
+keeps working for non-administrator requesters.</li>
+<li>fix(security): an unevaluable transform expression now fails closed —
+the change is not applied — instead of falling back to writing the raw
+value. Because the source value is requester-controlled, the fallback
+let a requester force the untransformed value onto the registrant by
+feeding input the transform could not handle. Failures are logged with
+the expression and error type (never the field value, which is PII);
+the full traceback is logged only at DEBUG.</li>
 </ul>
 </div>
 <div class="section" id="section-2">
diff --git a/spp_change_request_v2/strategies/field_mapping.py b/spp_change_request_v2/strategies/field_mapping.py
index be834b53c..86192849a 100644
--- a/spp_change_request_v2/strategies/field_mapping.py
+++ b/spp_change_request_v2/strategies/field_mapping.py
@@ -1,5 +1,6 @@
 import logging
 from datetime import date, datetime
+from types import SimpleNamespace
 
 from odoo import _, models
 from odoo.exceptions import UserError
@@ -53,13 +54,31 @@ def proposed_target_value(self, mapping, detail, registrant):
         value = getattr(detail, mapping.source_field, None)
         if hasattr(value, "id"):
             value = value.id
-        if mapping.transform == "expression" and mapping.transform_expression:
-            value = self._eval_expression(mapping.transform_expression, value, detail, registrant)
+        # The transform is admin-authored configuration. Read it as superuser so
+        # detection -- which runs as the requester, not under sudo -- can see it:
+        # ``transform_expression`` is gated by ``groups="base.group_system"``, so a
+        # plain read by a change-request user would raise AccessError and, worse,
+        # a silent skip would put detection and apply back out of step.
+        config = mapping.sudo()
+        if config.transform == "expression" and config.transform_expression:
+            value = self._eval_expression(config.transform_expression, value, detail, registrant)
         return value
 
     def mapping_changes_value(self, mapping, detail, registrant):
-        """Whether ``mapping`` would write a different value than is stored."""
-        return self.proposed_target_value(mapping, detail, registrant) != self.current_target_value(mapping, registrant)
+        """Whether ``mapping`` would write a different value than is stored.
+
+        A transform that cannot be evaluated fails closed on the apply path
+        (``_eval_expression`` raises ``UserError``). Detection must not crash on
+        that and must not silently drop the mapping: treat an unevaluable
+        transform as a change so the field stays visible to conflict and
+        duplicate detection. ``_run_conflict_checks`` on create is not
+        try-guarded, so a propagating error here would break creation.
+        """
+        try:
+            proposed = self.proposed_target_value(mapping, detail, registrant)
+        except UserError:
+            return True
+        return proposed != self.current_target_value(mapping, registrant)
 
     def apply(self, change_request):
         """Apply field mappings from detail to registrant."""
@@ -131,34 +150,73 @@ def apply(self, change_request):
 
         return True
 
+    def _expression_record_view(self, record):
+        """Attribute-readable snapshot of ``record`` with no ORM handle attached.
+
+        ``safe_eval`` permits arbitrary non-dunder attribute access, so a live
+        recordset in the evaluation context exposes ``record.env`` /
+        ``record.sudo()`` / ``record._cr`` -- the full ORM (as superuser on the
+        apply path, which runs under sudo) and the database cursor. Keeping
+        ``env`` out of the context means nothing while a recordset is in it.
+
+        The snapshot carries stored scalar fields only, so ``registrant.family_name``
+        keeps working while method calls and relation traversal do not, and its
+        ``__dict__`` is blocked by the dunder-name check. Many2one values are
+        reduced to their id, matching how ``proposed_target_value`` normalises.
+        """
+        if not record:
+            return None
+        values = {}
+        for name, field in record._fields.items():
+            if not field.store or field.type in ("one2many", "many2many"):
+                continue
+            value = record[name]
+            values[name] = value.id if field.type == "many2one" else value
+        return SimpleNamespace(**values)
+
     def _eval_expression(self, expr, value, detail, registrant):
-        """Safely evaluate transform expression.
+        """Safely evaluate a field-mapping transform expression.
 
-        Note ``safe_eval`` takes no ``nocopy`` argument in Odoo 19 -- passing it
-        raised ``TypeError`` for every expression, which the fallback below
-        swallowed, so configured transforms were silently ignored and the
-        untransformed value was written instead.
+        Security contract: the context exposes ``value`` and attribute-readable
+        snapshots of ``detail`` and ``registrant`` -- never live recordsets, so
+        no ``env``, ``sudo()`` or cursor is reachable from an expression. It
+        fails closed: an expression that cannot be evaluated raises rather than
+        writing the untransformed, requester-controlled ``value`` through.
         """
         try:
-            # Admin-defined field mapping expressions with restricted context (no env)
             return safe_eval(  # nosemgrep: odoo-unsafe-safe-eval
                 expr,
                 {
                     "value": value,
-                    "detail": detail,
-                    "registrant": registrant,
-                    # env removed for security
+                    "detail": self._expression_record_view(detail),
+                    "registrant": self._expression_record_view(registrant),
                     "datetime": datetime,
                     "date": date,
                 },
                 mode="eval",
             )
-        except Exception:
-            # Falls back to the untransformed value rather than failing the
-            # apply. Logged at exception level with the expression, because a
-            # silent warning is how the ``nocopy`` breakage went unnoticed.
-            _logger.exception("Field mapping transform expression failed, using the raw value: %s", expr)
-            return value
+        except Exception as error:
+            # Fail closed: refuse to write the raw value. ``value`` is
+            # requester-controlled, so falling back would let a requester force
+            # the untransformed value onto the registrant by feeding input the
+            # transform cannot handle. Log the expression and error *type* at
+            # ERROR -- never the wrapped error text, which embeds the field
+            # value (PII) -- and the full traceback only at DEBUG. The UserError
+            # message omits the underlying error for the same reason: it is
+            # persisted to ``apply_error`` on the change request.
+            _logger.error(
+                "Field mapping transform expression failed (%s), refusing to write the raw value: %s",
+                type(error).__name__,
+                expr,
+            )
+            _logger.debug("Transform expression failure detail", exc_info=True)
+            raise UserError(
+                _(
+                    "A configured field-mapping transform expression could not be evaluated, "
+                    "so the change was not applied. Ask an administrator to review the "
+                    "request type's transform expression; the failure detail is in the server log."
+                )
+            ) from None
 
     def _is_value_empty(self, value, record=None, field_name=None):
         """Check if a value should be considered empty and skipped.
diff --git a/spp_change_request_v2/tests/test_field_mapping_transform.py b/spp_change_request_v2/tests/test_field_mapping_transform.py
index 3bf9e46d3..a23b8530c 100644
--- a/spp_change_request_v2/tests/test_field_mapping_transform.py
+++ b/spp_change_request_v2/tests/test_field_mapping_transform.py
@@ -1,14 +1,15 @@
 # Part of OpenSPP. See LICENSE file for full copyright and licensing details.
-"""Field-mapping transform expressions must actually be evaluated.
+"""Field-mapping transform expressions: evaluated, sandboxed, and fail-closed.
 
-``_eval_expression`` passed ``nocopy=True`` to ``safe_eval``, which takes no
-such argument in Odoo 19. Every expression therefore raised ``TypeError``, the
-blanket fallback swallowed it, and the *untransformed* value was written to the
-registrant -- so a configured transform was silently ignored and only a warning
-in the log said so.
+Covers that a configured transform is actually applied (it was once passed a
+``nocopy`` kwarg ``safe_eval`` does not accept, so every expression raised and
+the raw value was written), that no ORM handle -- ``env``, ``sudo()`` or the
+cursor -- is reachable from an expression, that an unevaluable expression fails
+closed instead of writing the requester-controlled raw value, that the failure
+log does not leak the field value, and that the expression is admin-only.
 """
 
-from odoo.exceptions import UserError
+from odoo.exceptions import AccessError, UserError
 from odoo.tests import TransactionCase, tagged
 
 
@@ -76,26 +77,57 @@ def test_transform_result_is_what_gets_compared(self):
         self._apply(cr_type, {"given_name": "jane"})
         self.assertEqual(self.registrant.given_name, "john")
 
-    def test_a_broken_expression_falls_back_to_the_raw_value(self):
+    def test_a_broken_expression_fails_closed(self):
+        """A transform that cannot be evaluated blocks the apply -- it must not
+        fall back to writing the raw value. ``value`` is requester-controlled, so
+        a fallback would let a requester force the untransformed value onto the
+        registrant by feeding input the transform cannot handle."""
         cr_type = self._type_with_transform("tf_broken", "value.no_such_method()")
         with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR") as logs:
-            self._apply(cr_type, {"given_name": "jane"})
+            with self.assertRaises(UserError):
+                self._apply(cr_type, {"given_name": "jane"})
         self.assertEqual(
             self.registrant.given_name,
-            "jane",
-            "a failing expression falls back to the untransformed value",
+            "john",
+            "a failing expression must not write anything to the registrant",
         )
         self.assertTrue(
             any("transform expression failed" in line for line in logs.output),
             "the failure must be logged loudly enough to be diagnosable",
         )
 
-    def test_env_is_not_reachable_from_an_expression(self):
-        """The context deliberately omits env; keep it that way."""
-        cr_type = self._type_with_transform("tf_env", "env['res.users'].search([])")
-        with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR"):
-            self._apply(cr_type, {"given_name": "jane"})
-        self.assertEqual(self.registrant.given_name, "jane")
+    def test_the_failure_log_does_not_leak_the_field_value(self):
+        """The ERROR log carries the error *type* and the expression, never the
+        wrapped error text -- which embeds the (PII) field value."""
+        cr_type = self._type_with_transform("tf_pii", "int(value)")
+        with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR") as logs:
+            with self.assertRaises(UserError):
+                self._apply(cr_type, {"given_name": "Juan Dela Cruz"})
+        error_lines = [line for line in logs.output if line.startswith("ERROR:")]
+        self.assertTrue(error_lines)
+        for line in error_lines:
+            self.assertNotIn("Juan Dela Cruz", line)
+
+    def test_the_orm_is_not_reachable_from_an_expression(self):
+        """``env`` alone is not the boundary: a live recordset in the context
+        carries ``env``, ``sudo()`` and ``_cr`` with it, and ``safe_eval``
+        permits arbitrary non-dunder attribute access. The record snapshots
+        close every one of these; each fails closed rather than escaping."""
+        for index, expression in enumerate(
+            (
+                "env['res.users'].search([])",
+                "registrant.env['res.users'].search([])",
+                "registrant.sudo().family_name",
+                "registrant._cr",
+                "detail.env.cr",
+            )
+        ):
+            with self.subTest(expression=expression):
+                cr_type = self._type_with_transform(f"tf_escape_{index}", expression)
+                with self.assertLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR"):
+                    with self.assertRaises(UserError):
+                        self._apply(cr_type, {"given_name": "jane"})
+                self.assertEqual(self.registrant.given_name, "john")
 
     def test_direct_mappings_are_unaffected(self):
         cr_type = self.env["spp.change.request.type"].create(
@@ -118,3 +150,21 @@ def test_unrelated_apply_still_raises_without_a_detail(self):
         cr.write({"detail_res_id": False})
         with self.assertRaises(UserError):
             self.env["spp.cr.strategy.field_mapping"].apply(cr)
+
+    def test_cr_manager_cannot_write_transform_expression(self):
+        """``transform_expression`` is admin-only (``base.group_system``). A
+        Change Request Manager -- who is not a system administrator -- must not
+        be able to author the server-side expression, so the "administrators
+        only" warning is ORM-enforced rather than merely advisory."""
+        manager = self.env["res.users"].create(
+            {
+                "name": "CR Manager",
+                "login": "cr_manager_tf",
+                "group_ids": [(4, self.env.ref("spp_change_request_v2.group_cr_manager").id)],
+            }
+        )
+        self.assertFalse(manager._has_group("base.group_system"))
+        cr_type = self._type_with_transform("tf_acl", "value.upper()")
+        mapping = cr_type.apply_mapping_ids
+        with self.assertRaises(AccessError):
+            mapping.with_user(manager).write({"transform_expression": "value.lower()"})

From b06c9c89432b85aee99fb40d6d5d6fd8d782b594 Mon Sep 17 00:00:00 2001
From: Ken Lewerentz <ken@openspp.org>
Date: Fri, 28 Aug 2026 09:33:48 +0700
Subject: [PATCH 3/4] fix(change_request): build the expression snapshot from
 requester-readable fields only

The snapshot builder read every stored scalar field as the current user,
and core gates some behind groups (res.partner.signup_type needs
base.group_erp_manager via auth_signup). On the detection path -- which
runs as the requester -- building the registrant snapshot raised
AccessError before the expression ever evaluated, so every
expression-transform mapping was flagged as changed and logged an ERROR
per conflict check, for every non-admin user.

Gated fields are skipped on the apply path too, so detection and apply
build the identical snapshot. Binary fields are excluded to keep image
payloads out of every evaluation, and Reference fields because a stored
Reference value is itself a live recordset -- the handle the snapshot
exists to keep out.

Also annotates the detection-path config read for semgrep.
---
 spp_change_request_v2/README.rst              |  6 +++-
 .../models/change_request_type_mapping.py     |  3 +-
 spp_change_request_v2/readme/HISTORY.md       |  2 +-
 .../static/description/index.html             |  6 +++-
 .../strategies/field_mapping.py               | 14 ++++++--
 .../tests/test_field_mapping_transform.py     | 35 +++++++++++++++++++
 6 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst
index 1c950c415..95f85af7b 100644
--- a/spp_change_request_v2/README.rst
+++ b/spp_change_request_v2/README.rst
@@ -873,7 +873,11 @@ Changelog
   access and raw SQL. The context now carries attribute-readable
   snapshots of the two records (stored scalar fields only; no methods,
   no relation traversal, no database handle) instead of the recordsets
-  themselves.
+  themselves. Group-gated, binary and reference fields are excluded from
+  the snapshot: a gated field cannot be read by the requester on the
+  detection path — and apply must build the identical snapshot or the
+  two disagree again — a binary would haul image payloads into every
+  evaluation, and a stored Reference value is itself a live recordset.
 - fix(security): the transform expression is now restricted to system
   administrators (``groups="base.group_system"``) rather than only
   warned against in the help text, and is enforced by the ORM on read
diff --git a/spp_change_request_v2/models/change_request_type_mapping.py b/spp_change_request_v2/models/change_request_type_mapping.py
index c807f4d0f..53879ffb7 100644
--- a/spp_change_request_v2/models/change_request_type_mapping.py
+++ b/spp_change_request_v2/models/change_request_type_mapping.py
@@ -37,7 +37,8 @@ class SPPChangeRequestTypeMapping(models.Model):
             "Python expression for value transformation. "
             "Available variables: value (the source value), and read-only snapshots "
             "of detail and registrant exposing their stored scalar fields only - "
-            "no method calls, no relation traversal, no database access - plus "
+            "no method calls, no relation traversal, no database access, and no "
+            "group-restricted, binary or reference fields - plus "
             "datetime and date. Restricted to system administrators: it is evaluated "
             "server-side and an unevaluable expression blocks the change rather than "
             "writing the raw value."
diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md
index a0b10e116..54cd843e0 100644
--- a/spp_change_request_v2/readme/HISTORY.md
+++ b/spp_change_request_v2/readme/HISTORY.md
@@ -1,7 +1,7 @@
 ### 19.0.3.1.11
 
 - fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now.
-- fix(security): a transform expression can no longer reach the ORM. `safe_eval` places no allowlist on attribute access, so a live `detail`/`registrant` recordset in the evaluation context exposed `env`, `sudo()` and the database cursor — a change-request manager, who is not a system administrator, could obtain superuser ORM access and raw SQL. The context now carries attribute-readable snapshots of the two records (stored scalar fields only; no methods, no relation traversal, no database handle) instead of the recordsets themselves.
+- fix(security): a transform expression can no longer reach the ORM. `safe_eval` places no allowlist on attribute access, so a live `detail`/`registrant` recordset in the evaluation context exposed `env`, `sudo()` and the database cursor — a change-request manager, who is not a system administrator, could obtain superuser ORM access and raw SQL. The context now carries attribute-readable snapshots of the two records (stored scalar fields only; no methods, no relation traversal, no database handle) instead of the recordsets themselves. Group-gated, binary and reference fields are excluded from the snapshot: a gated field cannot be read by the requester on the detection path — and apply must build the identical snapshot or the two disagree again — a binary would haul image payloads into every evaluation, and a stored Reference value is itself a live recordset.
 - fix(security): the transform expression is now restricted to system administrators (`groups="base.group_system"`) rather than only warned against in the help text, and is enforced by the ORM on read and write. The detection path reads the expression as superuser so it keeps working for non-administrator requesters.
 - fix(security): an unevaluable transform expression now fails closed — the change is not applied — instead of falling back to writing the raw value. Because the source value is requester-controlled, the fallback let a requester force the untransformed value onto the registrant by feeding input the transform could not handle. Failures are logged with the expression and error type (never the field value, which is PII); the full traceback is logged only at DEBUG.
 
diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html
index fb7e67e84..34d67e13b 100644
--- a/spp_change_request_v2/static/description/index.html
+++ b/spp_change_request_v2/static/description/index.html
@@ -1358,7 +1358,11 @@ <h1>19.0.3.1.11</h1>
 access and raw SQL. The context now carries attribute-readable
 snapshots of the two records (stored scalar fields only; no methods,
 no relation traversal, no database handle) instead of the recordsets
-themselves.</li>
+themselves. Group-gated, binary and reference fields are excluded from
+the snapshot: a gated field cannot be read by the requester on the
+detection path — and apply must build the identical snapshot or the
+two disagree again — a binary would haul image payloads into every
+evaluation, and a stored Reference value is itself a live recordset.</li>
 <li>fix(security): the transform expression is now restricted to system
 administrators (<tt class="docutils literal"><span class="pre">groups=&quot;base.group_system&quot;</span></tt>) rather than only
 warned against in the help text, and is enforced by the ORM on read
diff --git a/spp_change_request_v2/strategies/field_mapping.py b/spp_change_request_v2/strategies/field_mapping.py
index 86192849a..1e692f5db 100644
--- a/spp_change_request_v2/strategies/field_mapping.py
+++ b/spp_change_request_v2/strategies/field_mapping.py
@@ -59,7 +59,7 @@ def proposed_target_value(self, mapping, detail, registrant):
         # ``transform_expression`` is gated by ``groups="base.group_system"``, so a
         # plain read by a change-request user would raise AccessError and, worse,
         # a silent skip would put detection and apply back out of step.
-        config = mapping.sudo()
+        config = mapping.sudo()  # nosemgrep: odoo-sudo-without-context
         if config.transform == "expression" and config.transform_expression:
             value = self._eval_expression(config.transform_expression, value, detail, registrant)
         return value
@@ -163,12 +163,22 @@ def _expression_record_view(self, record):
         keeps working while method calls and relation traversal do not, and its
         ``__dict__`` is blocked by the dunder-name check. Many2one values are
         reduced to their id, matching how ``proposed_target_value`` normalises.
+
+        Group-gated fields are excluded on both paths: detection builds the
+        snapshot as the requester, where reading a gated field (e.g.
+        ``res.partner.signup_type``) raises AccessError before the expression
+        runs, and apply -- which runs under sudo, where the read would succeed
+        -- must build the identical snapshot or the two disagree about what a
+        mapping writes. Binary fields are excluded so image payloads are not
+        hauled into every evaluation, and Reference fields because their value
+        is itself a live recordset -- the handle this snapshot exists to keep
+        out.
         """
         if not record:
             return None
         values = {}
         for name, field in record._fields.items():
-            if not field.store or field.type in ("one2many", "many2many"):
+            if not field.store or field.groups or field.type in ("one2many", "many2many", "binary", "reference"):
                 continue
             value = record[name]
             values[name] = value.id if field.type == "many2one" else value
diff --git a/spp_change_request_v2/tests/test_field_mapping_transform.py b/spp_change_request_v2/tests/test_field_mapping_transform.py
index a23b8530c..644bac93c 100644
--- a/spp_change_request_v2/tests/test_field_mapping_transform.py
+++ b/spp_change_request_v2/tests/test_field_mapping_transform.py
@@ -151,6 +151,41 @@ def test_unrelated_apply_still_raises_without_a_detail(self):
         with self.assertRaises(UserError):
             self.env["spp.cr.strategy.field_mapping"].apply(cr)
 
+    def test_detection_snapshot_is_buildable_by_a_plain_cr_user(self):
+        """Detection runs as the requester, and the record snapshot must be
+        buildable without admin groups: core gates stored scalar fields behind
+        ``groups=`` (e.g. ``res.partner.signup_type`` needs
+        ``base.group_erp_manager`` via auth_signup), and reading one as a plain
+        user raises AccessError before the expression ever runs. Detection
+        would then over-flag every expression mapping as changed -- putting
+        detection and apply back out of step -- and log an ERROR on every
+        conflict check. The superuser suite cannot see this, because field
+        group checks are skipped when ``env.su``; hence ``with_user``."""
+        cr_user = self.env["res.users"].create(
+            {
+                "name": "CR User",
+                "login": "cr_user_tf",
+                "group_ids": [
+                    (4, self.env.ref("base.group_user").id),
+                    (4, self.env.ref("spp_change_request_v2.group_cr_user").id),
+                ],
+            }
+        )
+        cr_type = self._type_with_transform("tf_plain_user", "'john'")
+        cr = (
+            self.env["spp.change.request"]
+            .with_user(cr_user)
+            .create({"request_type_id": cr_type.id, "registrant_id": self.registrant.id})
+        )
+        strategy = self.env["spp.cr.strategy.field_mapping"].with_user(cr_user)
+        mapping = cr_type.apply_mapping_ids.with_user(cr_user)
+        with self.assertNoLogs("odoo.addons.spp_change_request_v2.strategies.field_mapping", level="ERROR"):
+            changes = strategy.mapping_changes_value(mapping, cr.get_detail(), cr.registrant_id)
+        self.assertFalse(
+            changes,
+            "a transform landing on the stored value must not be flagged as a change",
+        )
+
     def test_cr_manager_cannot_write_transform_expression(self):
         """``transform_expression`` is admin-only (``base.group_system``). A
         Change Request Manager -- who is not a system administrator -- must not

From 925f77819e5c046e75ff9740462e131209c8e808 Mon Sep 17 00:00:00 2001
From: Ken Lewerentz <ken@openspp.org>
Date: Fri, 28 Aug 2026 10:03:14 +0700
Subject: [PATCH 4/4] chore(change_request): regenerate README with CI's
 generator output

The local oca-gen-addon-readme environment renders RST tables wider than
CI's (a docutils rendering difference the dependency pins do not cover
across Python versions), and the previous regeneration was committed
under --no-verify, so CI's pre-commit rejected the drift. This applies
the exact output CI's hook produces; content is unchanged.
---
 spp_change_request_v2/README.rst              | 32 +++++++++----------
 .../static/description/index.html             |  4 +--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst
index 95f85af7b..cec371c08 100644
--- a/spp_change_request_v2/README.rst
+++ b/spp_change_request_v2/README.rst
@@ -752,22 +752,22 @@ Methods available for override on detail models (all inherited from
 Related fields available on all detail models (from
 ``spp.cr.detail.base``):
 
-+----------------------------+-----------+------------------------------------------------------------+
-| Field                      | Type      | Source                                                     |
-+============================+===========+============================================================+
-| ``change_request_id``      | Many2one  | Direct link to parent CR                                   |
-+----------------------------+-----------+------------------------------------------------------------+
-| ``registrant_id``          | Many2one  | ``change_request_id.registrant_id``                        |
-+----------------------------+-----------+------------------------------------------------------------+
-| ``approval_state``         | Selection | ``change_request_id.approval_state``                       |
-+----------------------------+-----------+------------------------------------------------------------+
-| ``is_applied``             | Boolean   | ``change_request_id.is_applied``                           |
-+----------------------------+-----------+------------------------------------------------------------+
-| ``use_dynamic_approval``   | Boolean   | ``change_request_id.request_type_id.use_dynamic_approval`` |
-+----------------------------+-----------+------------------------------------------------------------+
-| ``field_to_modify``        | Selection | Dynamic field selector (populated by                       |
-|                            |           | ``_get_field_to_modify_selection``)                        |
-+----------------------------+-----------+------------------------------------------------------------+
++--------------------------+-----------+------------------------------------------------------------+
+| Field                    | Type      | Source                                                     |
++==========================+===========+============================================================+
+| ``change_request_id``    | Many2one  | Direct link to parent CR                                   |
++--------------------------+-----------+------------------------------------------------------------+
+| ``registrant_id``        | Many2one  | ``change_request_id.registrant_id``                        |
++--------------------------+-----------+------------------------------------------------------------+
+| ``approval_state``       | Selection | ``change_request_id.approval_state``                       |
++--------------------------+-----------+------------------------------------------------------------+
+| ``is_applied``           | Boolean   | ``change_request_id.is_applied``                           |
++--------------------------+-----------+------------------------------------------------------------+
+| ``use_dynamic_approval`` | Boolean   | ``change_request_id.request_type_id.use_dynamic_approval`` |
++--------------------------+-----------+------------------------------------------------------------+
+| ``field_to_modify``      | Selection | Dynamic field selector (populated by                       |
+|                          |           | ``_get_field_to_modify_selection``)                        |
++--------------------------+-----------+------------------------------------------------------------+
 
 CR Type Fields Reference
 ~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html
index 34d67e13b..705cc88b8 100644
--- a/spp_change_request_v2/static/description/index.html
+++ b/spp_change_request_v2/static/description/index.html
@@ -1160,9 +1160,9 @@ <h1>Methods Reference</h1>
 <tt class="docutils literal">spp.cr.detail.base</tt>):</p>
 <table border="1" class="docutils">
 <colgroup>
-<col width="28%" />
+<col width="27%" />
 <col width="11%" />
-<col width="61%" />
+<col width="62%" />
 </colgroup>
 <thead valign="bottom">
 <tr><th class="head">Field</th>