security(cr): route and apply the same single field for dynamic approvals

spp_change_request_v2/README.rst

@@ -853,6 +853,17 @@ Before declaring a new CR type complete:
 Changelog
 =========
 
+19.0.3.0.1
+~~~~~~~~~~
+
+- fix(security): for dynamic-approval CR types, the ``field_mapping``
+  apply strategy now writes only the routed/approved field, and the
+  proposed change is frozen once submitted (selected field, mapped field
+  values, and the detail record pointer). Previously a user could route
+  a low-risk field to a weak approval and smuggle changes to other
+  mapped fields — or swap the field/value/detail after routing — so
+  unapproved changes reached the registrant.
+
 19.0.3.0.0
 ~~~~~~~~~~
 

spp_change_request_v2/__manifest__.py

@@ -1,6 +1,6 @@
 {
     "name": "OpenSPP Change Request V2",
-    "version": "19.0.3.0.0",
+    "version": "19.0.3.0.1",
     "sequence": 50,
     "category": "OpenSPP",
     "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",

spp_change_request_v2/models/change_request.py

@@ -663,6 +663,52 @@ def create(self, vals_list):
                 record._run_conflict_checks()
         return records
 
+    # Fields that bind a submitted CR to exactly what was routed and approved:
+    # the dynamic-approval selection (synced from the detail's field_to_modify in
+    # draft) and the detail record pointer that get_detail() resolves for both
+    # routing and apply. Once the CR leaves draft/revision these are frozen — else
+    # a user could route on a low-risk field / benign detail and then swap the
+    # selection or repoint detail_res_id to a substituted detail before apply.
+    # Editing requires reset to draft, which re-routes. (These fields are never
+    # written by the apply strategies, so the guard needs no apply-path exemption.)
+    _FROZEN_ON_SUBMIT_FIELDS = (
+        "selected_field_name",
+        "selected_field_old_value",
+        "selected_field_new_value",
+        "detail_res_id",
+        "detail_res_model",
+    )
+
+    @staticmethod
+    def _normalize_frozen_value(value):
+        """Normalize a value for change detection: recordset -> id, None -> False.
+
+        Odoo stores unset fields as ``False``, but a write payload (JSON-RPC /
+        integrations) may pass ``None`` for the same field, or a Many2one as a
+        recordset. Normalizing both sides prevents an idempotent re-save from
+        being mistaken for a real change and wrongly locked out.
+        """
+        if hasattr(value, "id"):
+            value = value.id
+        return value if value is not None else False
+
+    def write(self, vals):
+        guarded = [f for f in self._FROZEN_ON_SUBMIT_FIELDS if f in vals]
+        if guarded:
+            norm = self._normalize_frozen_value
+            for rec in self:
+                if rec.approval_state in ("draft", "revision") or not rec.approval_state:
+                    continue
+                if any(norm(vals[f]) != norm(rec[f]) for f in guarded):
+                    raise UserError(
+                        _(
+                            "A submitted change request is locked to the change it was "
+                            "routed and approved for; its selected field and detail record "
+                            "cannot be changed. Reset the request to draft to re-route."
+                        )
+                    )
+        return super().write(vals)
+
     def unlink(self):
         """Delete associated detail records and archive DMS directory."""
         directories_to_archive = self.env["spp.dms.directory"]

spp_change_request_v2/models/change_request_detail_base.py

@@ -72,7 +72,64 @@ def _get_field_to_modify_selection(self):
         """
         return []
 
+    def _protected_content_fields(self, change_request):
+        """Fields whose value defines the proposed change / approval routing.
+
+        These must not change once the CR has left draft/revision, otherwise a
+        user could re-route the approval (change the selected field) or alter the
+        value that was routed and approved (see dynamic-approval routing). For
+        the field_mapping strategy that is the routing selector plus every mapped
+        source field; apply-output fields (e.g. created_*_id) are NOT included so
+        the apply strategies can still record their results post-approval.
+        """
+        protected = {"field_to_modify"}
+        cr_type = change_request.request_type_id
+        if cr_type.apply_strategy == "field_mapping":
+            protected |= {m.source_field for m in cr_type.apply_mapping_ids if m.source_field}
+        return protected
+
+    @staticmethod
+    def _normalize_frozen_value(value):
+        """Normalize a value for change detection: recordset -> id, None -> False.
+
+        Odoo stores unset fields as ``False``, but a write payload may pass
+        ``None`` for the same field or a Many2one as a recordset; normalizing
+        both sides prevents an idempotent re-save from being mistaken for a real
+        change and wrongly locked out.
+        """
+        if hasattr(value, "id"):
+            value = value.id
+        return value if value is not None else False
+
+    def _assert_content_editable(self, vals):
+        """Reject edits to proposed-change fields once the CR is submitted.
+
+        Mirrors the view-level readonly (approval_state not in draft/revision) at
+        the server so it cannot be bypassed via RPC. Editing requires resetting
+        the CR to draft, which re-routes the approval.
+        """
+        for rec in self:
+            change_request = rec.change_request_id
+            state = change_request.approval_state
+            if not change_request or state in ("draft", "revision") or not state:
+                continue
+            for field_name in rec._protected_content_fields(change_request):
+                if field_name not in vals or field_name not in rec._fields:
+                    continue
+                # Normalize both sides (recordset -> id, None -> False) so an
+                # idempotent re-save, a Many2one written as a recordset, or a
+                # JSON-RPC None is not mistaken for a real change and locked out.
+                if self._normalize_frozen_value(vals[field_name]) != self._normalize_frozen_value(rec[field_name]):
+                    raise UserError(
+                        _(
+                            "This change request has already been submitted for approval, "
+                            "so its proposed changes are locked. Reset it to draft to edit "
+                            "(this re-routes the approval)."
+                        )
+                    )
+
     def write(self, vals):
+        self._assert_content_editable(vals)
         result = super().write(vals)
         if "field_to_modify" in vals:
             for rec in self:

spp_change_request_v2/readme/HISTORY.md

@@ -1,3 +1,7 @@
+### 19.0.3.0.1
+
+- fix(security): for dynamic-approval CR types, the `field_mapping` apply strategy now writes only the routed/approved field, and the proposed change is frozen once submitted (selected field, mapped field values, and the detail record pointer). Previously a user could route a low-risk field to a weak approval and smuggle changes to other mapped fields — or swap the field/value/detail after routing — so unapproved changes reached the registrant.
+
 ### 19.0.3.0.0
 
 - feat(change_request): redesign the group/membership CR flows (#242) — Create Group (#876), Add Member now searches an existing member (#871), Remove Member first-page/review cleanup (#872), Change Head of Household via a per-member role table (#873), and Split Household as a relational member move with single-head validation (#877). Review pages render the real data as tables / detail sections.

spp_change_request_v2/static/description/index.html

@@ -1339,6 +1339,18 @@ <h2>Changelog</h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.3.0.1</h1>
+<ul class="simple">
+<li>fix(security): for dynamic-approval CR types, the <tt class="docutils literal">field_mapping</tt>
+apply strategy now writes only the routed/approved field, and the
+proposed change is frozen once submitted (selected field, mapped field
+values, and the detail record pointer). Previously a user could route
+a low-risk field to a weak approval and smuggle changes to other
+mapped fields — or swap the field/value/detail after routing — so
+unapproved changes reached the registrant.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.3.0.0</h1>
 <ul class="simple">
 <li>feat(change_request): redesign the group/membership CR flows (#242) —
@@ -1360,7 +1372,7 @@ <h1>19.0.3.0.0</h1>
 must adapt (see #1133).</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.2.0.8</h1>
 <ul class="simple">
 <li>fix(views): disable inline creation of CR document types on the Change
@@ -1371,7 +1383,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-3">
+<div class="section" id="section-4">
 <h1>19.0.2.0.7</h1>
 <ul class="simple">
 <li>fix(security): align CR Requestor / CR Local Validator / CR HQ
@@ -1383,7 +1395,7 @@ <h1>19.0.2.0.7</h1>
 dependencies.</li>
 </ul>
 </div>
-<div class="section" id="section-4">
+<div class="section" id="section-5">
 <h1>19.0.2.0.6</h1>
 <ul class="simple">
 <li>fix(views): route post-submit CRs (pending / approved / applied /
@@ -1398,7 +1410,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-5">
+<div class="section" id="section-6">
 <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
@@ -1411,27 +1423,27 @@ <h1>19.0.2.0.5</h1>
 roles).</li>
 </ul>
 </div>
-<div class="section" id="section-6">
+<div class="section" id="section-7">
 <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-7">
+<div class="section" id="section-8">
 <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-8">
+<div class="section" id="section-9">
 <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-9">
+<div class="section" id="section-10">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_change_request_v2/strategies/field_mapping.py

@@ -15,17 +15,35 @@ class SPPCRStrategyFieldMapping(models.AbstractModel):
     _inherit = "spp.cr.strategy.base"
     _description = "CR Apply Strategy: Field Mapping"
 
+    def _effective_mappings(self, change_request):
+        """Return the mappings that may be applied for this change request.
+
+        For dynamic-approval CR types the approval workflow is routed and
+        approved based on a single selected field, so ONLY that field's mapping
+        may be written to the registrant — regardless of any other mapped detail
+        fields that were also changed. This keeps the applied change in lockstep
+        with what was actually approved. Fail closed: if no field is selected, or
+        the selection maps to no configured field, nothing is applied.
+        """
+        cr_type = change_request.request_type_id
+        mappings = cr_type.apply_mapping_ids
+        if not cr_type.use_dynamic_approval:
+            return mappings
+        selected = change_request.selected_field_name
+        if not selected:
+            return mappings.browse()
+        return mappings.filtered(lambda m: m.source_field == selected)
+
     def apply(self, change_request):
         """Apply field mappings from detail to registrant."""
         registrant = change_request.registrant_id
         detail = change_request.get_detail()
-        cr_type = change_request.request_type_id
 
         if not detail:
             raise UserError(_("No detail record found."))
 
         values = {}
-        for mapping in cr_type.apply_mapping_ids:
+        for mapping in self._effective_mappings(change_request):
             source_value = getattr(detail, mapping.source_field, None)
             current_value = getattr(registrant, mapping.target_field, None)
 
@@ -147,13 +165,14 @@ def preview(self, change_request):
         """Preview what changes will be applied."""
         registrant = change_request.registrant_id
         detail = change_request.get_detail()
-        cr_type = change_request.request_type_id
 
         if not detail:
             return {}
 
         changes = {}
-        for mapping in cr_type.apply_mapping_ids:
+        # Mirror apply(): a dynamic-approval CR previews only the selected field,
+        # so the approver sees exactly what will be written.
+        for mapping in self._effective_mappings(change_request):
             source_raw = getattr(detail, mapping.source_field, None)
             current_raw = getattr(registrant, mapping.target_field, None)