From 7bd6f9bc95e98a883399d3b74f443282b82fc576 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 24 Jul 2026 11:56:54 +0800 Subject: [PATCH 1/4] security(cr): derive conflict field from field_to_modify, not writable selected_field_name Conflict and duplicate detection treated a non-empty selected_field_name as 'only that field is a proposed change'. selected_field_name is only view-readonly; group_cr_user has write on their own spp.change.request, so a user could create a CR blocked by a field-scoped conflict rule, then RPC-write selected_field_name to a field outside conflict_fields - _filter_by_field_conflicts returned empty, resetting conflict_status to none and allowing submission without the configured override path. The duplicate-detection path (_calculate_similarity) had the same flaw. Derive the effective changed field server-side from the detail's validated field_to_modify Selection via a new _effective_selected_field() helper, gated on use_dynamic_approval (non-dynamic types always use the full conflict_fields). The writable selected_field_name is no longer trusted for these decisions. Bump 19.0.3.0.0 -> 19.0.3.0.3 (above #261's .0.2 and #264's .0.1 on the same module) + HISTORY. No migration: behavior fix on existing fields. Scope: conflict/duplicate detection only. The same writable field is also read by dynamic-approval routing (_resolve_dynamic_approval); filed as a follow-up. --- spp_change_request_v2/__manifest__.py | 2 +- .../models/conflict_mixin.py | 42 +++++++--- spp_change_request_v2/readme/HISTORY.md | 10 +++ .../tests/test_conflict_dynamic_approval.py | 77 +++++++++++++++++++ 4 files changed, 120 insertions(+), 11 deletions(-) diff --git a/spp_change_request_v2/__manifest__.py b/spp_change_request_v2/__manifest__.py index 1e32f323f..516bfd937 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.0.0", + "version": "19.0.3.0.3", "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/models/conflict_mixin.py b/spp_change_request_v2/models/conflict_mixin.py index 4f450a29c..24549de81 100644 --- a/spp_change_request_v2/models/conflict_mixin.py +++ b/spp_change_request_v2/models/conflict_mixin.py @@ -291,12 +291,34 @@ def _get_group_member_ids(self): return list(set(member_ids)) + def _effective_selected_field(self): + """Return the dynamic-approval field actually being changed. + + This is derived from the detail's ``field_to_modify`` — a Selection + validated against the allowed fields — NOT from the CR's + ``selected_field_name`` Char, which is only view-readonly and is + directly writable by any user with write access to their own change + request. Trusting the writable copy let a user re-point it to a field + outside a rule's ``conflict_fields`` to escape field-scoped conflict + and duplicate detection; deriving from the validated source closes that. + + Returns the field name for dynamic-approval CR types, or ``""`` for + non-dynamic types (where field scoping does not apply and the full + ``conflict_fields`` set must always be considered). + """ + self.ensure_one() + if not self.request_type_id.use_dynamic_approval: + return "" + detail = self.get_detail() + return (detail.field_to_modify or "") if detail else "" + def _filter_by_field_conflicts(self, candidates, rule): """Filter candidate CRs by checking if they modify the same fields. - For dynamic-approval CRs (where selected_field_name is set), only the - selected field is treated as a proposed change. Prefilled fields from - the registrant are ignored for conflict purposes. + For dynamic-approval CRs, only the selected field (derived server-side + from the detail's validated ``field_to_modify``) is treated as a + proposed change. Prefilled fields from the registrant are ignored for + conflict purposes. """ self.ensure_one() @@ -309,7 +331,7 @@ def _filter_by_field_conflicts(self, candidates, rule): return self.env["spp.change.request"] # Dynamic approval: only the selected field is a proposed change - my_selected = self.selected_field_name + my_selected = self._effective_selected_field() if my_selected: if my_selected not in conflict_fields: return self.env["spp.change.request"] @@ -325,7 +347,7 @@ def _filter_by_field_conflicts(self, candidates, rule): continue # Determine candidate's effective fields - candidate_selected = candidate.selected_field_name + candidate_selected = candidate._effective_selected_field() if candidate_selected: # Both use dynamic approval: conflict only if same field if my_selected and candidate_selected != my_selected: @@ -439,9 +461,9 @@ def _detect_duplicates(self): def _calculate_similarity(self, other_cr, config): """Calculate similarity percentage between this CR and another. - For dynamic-approval CRs (where selected_field_name is set), only the - selected field is compared. Prefilled fields are ignored to prevent - inflated similarity scores. + For dynamic-approval CRs, only the selected field (derived server-side + from the detail's validated ``field_to_modify``) is compared. Prefilled + fields are ignored to prevent inflated similarity scores. Args: other_cr: Another spp.change.request record @@ -459,8 +481,8 @@ def _calculate_similarity(self, other_cr, config): return 0.0 # Dynamic approval: compare only the selected field - my_selected = self.selected_field_name - other_selected = other_cr.selected_field_name + my_selected = self._effective_selected_field() + other_selected = other_cr._effective_selected_field() if my_selected and other_selected: # Different fields selected = not duplicates if my_selected != other_selected: diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md index 0f9d181e5..a3d0270f4 100644 --- a/spp_change_request_v2/readme/HISTORY.md +++ b/spp_change_request_v2/readme/HISTORY.md @@ -1,3 +1,13 @@ +### 19.0.3.0.3 + +- fix(security): derive the dynamic-approval field used by conflict and + duplicate detection from the detail's validated ``field_to_modify`` selection + instead of the user-writable ``selected_field_name``. ``selected_field_name`` + was only view-readonly, so a change-request user could write it to a field + outside a rule's ``conflict_fields`` to clear a field-scoped conflict and + submit without the configured override path; the detection now ignores the + writable copy and is skipped for non-dynamic-approval request types. + ### 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. diff --git a/spp_change_request_v2/tests/test_conflict_dynamic_approval.py b/spp_change_request_v2/tests/test_conflict_dynamic_approval.py index dfebdd915..bbdeccdd9 100644 --- a/spp_change_request_v2/tests/test_conflict_dynamic_approval.py +++ b/spp_change_request_v2/tests/test_conflict_dynamic_approval.py @@ -481,3 +481,80 @@ def test_static_cr_create_still_runs_conflict_check(self): cr2.conflict_detection_date, "Static CR must run conflict checks at create time (existing behavior).", ) + + # ────────────────────────────────────────────────────────────────────────── + # Security: a user-writable selected_field_name must NOT bypass field-scoped + # conflict/duplicate detection. selected_field_name is only view-readonly; + # CR users have write on their own CRs, so the detection must derive the + # effective changed field from the trusted detail.field_to_modify selection. + # ────────────────────────────────────────────────────────────────────────── + + def test_writing_selected_field_name_cannot_bypass_field_conflict(self): + """Writing selected_field_name to a field outside conflict_fields must + not clear a real field-scoped conflict on a dynamic-approval CR.""" + cr_a = self._create_dynamic_cr() + cr_a.get_detail().write({"field_to_modify": "given_name", "given_name": "NewGivenA"}) + cr_a._run_conflict_checks() + + cr_b = self._create_dynamic_cr() + cr_b.get_detail().write({"field_to_modify": "given_name", "given_name": "NewGivenB"}) + cr_b._run_conflict_checks() + self.assertEqual(cr_b.conflict_status, "warning", "Baseline: same field = conflict.") + + # Attack: re-point selected_field_name to a field NOT in conflict_fields + # (phone). This re-triggers conflict detection via the write override. + cr_b.write({"selected_field_name": "phone"}) + + self.assertEqual( + cr_b.conflict_status, + "warning", + "Writing selected_field_name must not bypass the field-scoped conflict.", + ) + self.assertIn(cr_a, cr_b.conflicting_cr_ids) + + def test_writing_selected_field_name_on_static_cr_cannot_bypass(self): + """On a non-dynamic-approval CR, selected_field_name must be ignored + entirely — writing it cannot narrow the field-scoped conflict.""" + cr_a = self._create_static_cr() + cr_a.get_detail().write({"given_name": "StaticGivenA"}) + cr_a._run_conflict_checks() + + cr_b = self._create_static_cr() + cr_b.get_detail().write({"given_name": "StaticGivenB"}) + cr_b._run_conflict_checks() + self.assertIn(cr_a, cr_b.conflicting_cr_ids, "Baseline: static CRs conflict on given_name.") + + cr_b.write({"selected_field_name": "phone"}) + + self.assertIn( + cr_a, + cr_b.conflicting_cr_ids, + "selected_field_name must not affect conflict detection for non-dynamic CR types.", + ) + + def test_writing_selected_field_name_cannot_bypass_duplicate(self): + """Writing selected_field_name must not drop duplicate similarity to 0.""" + dup_config = self.env["spp.cr.duplicate.config"].create( + {"cr_type_id": self.dynamic_cr_type.id, "similarity_threshold": 50.0} + ) + self.dynamic_cr_type.write({"enable_duplicate_detection": True, "duplicate_detection_config_id": dup_config.id}) + try: + cr_a = self._create_dynamic_cr() + cr_a.get_detail().write({"field_to_modify": "given_name", "given_name": "SameValue"}) + cr_a._run_conflict_checks() + + cr_b = self._create_dynamic_cr() + cr_b.get_detail().write({"field_to_modify": "given_name", "given_name": "SameValue"}) + self.assertEqual(cr_b._calculate_similarity(cr_a, dup_config), 100.0, "Baseline duplicate.") + + # Attack: re-point selected_field_name away from the real field. + cr_b.write({"selected_field_name": "phone"}) + + self.assertEqual( + cr_b._calculate_similarity(cr_a, dup_config), + 100.0, + "Writing selected_field_name must not bypass duplicate detection.", + ) + finally: + self.dynamic_cr_type.write({"enable_duplicate_detection": False, "duplicate_detection_config_id": False}) + dup_config.unlink() From cbe3d904eed8bd2c37641e93f4ba0170963906e3 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 24 Jul 2026 12:13:25 +0800 Subject: [PATCH 2/4] docs(spp_change_request_v2): regenerate README/index for 19.0.3.0.3 changelog Applies CI's pinned oca-gen output verbatim for the new HISTORY fragment. --- spp_change_request_v2/README.rst | 12 ++++++++ .../static/description/index.html | 29 ++++++++++++++----- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst index 914b1a0e5..0d9fd1758 100644 --- a/spp_change_request_v2/README.rst +++ b/spp_change_request_v2/README.rst @@ -853,6 +853,18 @@ Before declaring a new CR type complete: Changelog ========= +19.0.3.0.3 +~~~~~~~~~~ + +- fix(security): derive the dynamic-approval field used by conflict and + duplicate detection from the detail's validated ``field_to_modify`` + selection instead of the user-writable ``selected_field_name``. + ``selected_field_name`` was only view-readonly, so a change-request + user could write it to a field outside a rule's ``conflict_fields`` to + clear a field-scoped conflict and submit without the configured + override path; the detection now ignores the writable copy and is + skipped for non-dynamic-approval request types. + 19.0.3.0.0 ~~~~~~~~~~ diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html index 58f8d5751..bb9b3ac13 100644 --- a/spp_change_request_v2/static/description/index.html +++ b/spp_change_request_v2/static/description/index.html @@ -1339,6 +1339,19 @@

Changelog

+

19.0.3.0.3

+
    +
  • fix(security): derive the dynamic-approval field used by conflict and +duplicate detection from the detail’s validated field_to_modify +selection instead of the user-writable selected_field_name. +selected_field_name was only view-readonly, so a change-request +user could write it to a field outside a rule’s conflict_fields to +clear a field-scoped conflict and submit without the configured +override path; the detection now ignores the writable copy and is +skipped for non-dynamic-approval request types.
    • +
+
+

19.0.3.0.0

  • feat(change_request): redesign the group/membership CR flows (#242) — @@ -1360,7 +1373,7 @@

    19.0.3.0.0

    must adapt (see #1133).
-
+

19.0.2.0.8

  • fix(views): disable inline creation of CR document types on the Change @@ -1371,7 +1384,7 @@

    19.0.2.0.8

    Documents” modal (missing Name field) that blocked saving (#1125)
-
+

19.0.2.0.7

  • fix(security): align CR Requestor / CR Local Validator / CR HQ @@ -1383,7 +1396,7 @@

    19.0.2.0.7

    dependencies.
-
+

19.0.2.0.6

  • fix(views): route post-submit CRs (pending / approved / applied / @@ -1398,7 +1411,7 @@

    19.0.2.0.6

    list so row-click goes through the stage router.
-
+

19.0.2.0.5

  • fix(security): add a global ir.rule on spp.change.request that @@ -1411,27 +1424,27 @@

    19.0.2.0.5

    roles).
-
+

19.0.2.0.3

  • fix: add HTML escaping to all computed Html fields with sanitize=False to prevent stored XSS (#50)
-
+

19.0.2.0.2

  • fix: fix batch approval wizard line deletion (#130)
-
+

19.0.2.0.1

  • fix: skip field types before getattr and isolate detail prefetch (#129)
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • From 116b92eda7ec504f2a9e80dd0822e285114bbe6c Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 24 Jul 2026 13:24:52 +0800 Subject: [PATCH 3/4] security(cr): scope conflict/duplicate detection to actual diff, not a writable label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staff review found the first fix incomplete: it derived the effective field from detail.field_to_modify, but that field is ALSO user-writable and does not constrain apply — field_mapping.apply writes every mapped field that differs from the registrant, not just field_to_modify. So the bypass survived with one extra field: write({'field_to_modify': 'phone', 'given_name': 'X'}) cleared the given_name conflict while still applying the given_name change. Derive the proposed changes server-side from the detail-vs-registrant diff (the exact set apply writes) via _proposed_changed_fields(), and scope both _filter_by_field_conflicts and _calculate_similarity to it. Neither selected_field_name nor field_to_modify is trusted for the decision. Add field mappings to the dynamic test type (a field_mapping type needs them) and two regression tests for the mislabel bypass (own-side and candidate-side). 339 tests pass. --- .../models/conflict_mixin.py | 130 ++++++++++-------- spp_change_request_v2/readme/HISTORY.md | 17 ++- .../tests/test_conflict_dynamic_approval.py | 53 +++++++ 3 files changed, 139 insertions(+), 61 deletions(-) diff --git a/spp_change_request_v2/models/conflict_mixin.py b/spp_change_request_v2/models/conflict_mixin.py index 24549de81..cfb754b0c 100644 --- a/spp_change_request_v2/models/conflict_mixin.py +++ b/spp_change_request_v2/models/conflict_mixin.py @@ -291,34 +291,61 @@ def _get_group_member_ids(self): return list(set(member_ids)) - def _effective_selected_field(self): - """Return the dynamic-approval field actually being changed. - - This is derived from the detail's ``field_to_modify`` — a Selection - validated against the allowed fields — NOT from the CR's - ``selected_field_name`` Char, which is only view-readonly and is - directly writable by any user with write access to their own change - request. Trusting the writable copy let a user re-point it to a field - outside a rule's ``conflict_fields`` to escape field-scoped conflict - and duplicate detection; deriving from the validated source closes that. - - Returns the field name for dynamic-approval CR types, or ``""`` for - non-dynamic types (where field scoping does not apply and the full - ``conflict_fields`` set must always be considered). + def _proposed_changed_fields(self): + """Return the detail (source) fields a dynamic-approval CR actually + proposes to change. + + These are the mapped fields whose detail value differs from the + registrant's current value — exactly the set the ``field_mapping`` apply + strategy will write. This is derived server-side from the actual data, + NOT from a declared label: both ``selected_field_name`` (only + view-readonly) and the detail's ``field_to_modify`` (freely writable and + validated only to be a real field name, not tied to what apply changes) + are attacker-controlled. A user could otherwise clear a field-scoped + conflict/duplicate by labelling a different, unchanged field while still + changing a scoped field. Scoping to the real diff makes those labels + irrelevant to the security decision. + + Returns a set of detail field names for dynamic-approval CR types, or + ``None`` for non-dynamic types (where field scoping does not apply and + the full configured field set must always be considered — their details + are not registrant-prefilled snapshots). """ self.ensure_one() if not self.request_type_id.use_dynamic_approval: - return "" + return None detail = self.get_detail() - return (detail.field_to_modify or "") if detail else "" + registrant = self.registrant_id + if not detail or not registrant: + return set() + changed = set() + for mapping in self.request_type_id.apply_mapping_ids: + source_field = mapping.source_field + target_field = mapping.target_field + if source_field not in detail._fields or target_field not in registrant._fields: + continue + detail_value = self._normalize_field_value(getattr(detail, source_field, None)) + registrant_value = self._normalize_field_value(getattr(registrant, target_field, None)) + if detail_value != registrant_value: + changed.add(source_field) + return changed + + def _effective_conflict_fields(self, conflict_fields): + """Return the subset of ``conflict_fields`` this CR actually proposes to + change (the security-relevant scope). Full set for non-dynamic types.""" + self.ensure_one() + conflict_fields = set(conflict_fields) + proposed = self._proposed_changed_fields() + if proposed is None: + return conflict_fields + return proposed & conflict_fields def _filter_by_field_conflicts(self, candidates, rule): """Filter candidate CRs by checking if they modify the same fields. - For dynamic-approval CRs, only the selected field (derived server-side - from the detail's validated ``field_to_modify``) is treated as a - proposed change. Prefilled fields from the registrant are ignored for - conflict purposes. + For dynamic-approval CRs, the proposed changes are the conflict fields + whose value actually differs from the registrant (derived server-side), + not a user-writable label. Prefilled/unchanged fields are ignored. """ self.ensure_one() @@ -330,14 +357,10 @@ def _filter_by_field_conflicts(self, candidates, rule): if not my_detail: return self.env["spp.change.request"] - # Dynamic approval: only the selected field is a proposed change - my_selected = self._effective_selected_field() - if my_selected: - if my_selected not in conflict_fields: - return self.env["spp.change.request"] - my_effective_fields = [my_selected] - else: - my_effective_fields = conflict_fields + my_effective_fields = self._effective_conflict_fields(conflict_fields) + if not my_effective_fields: + # This CR changes none of the rule's fields — nothing to conflict on. + return self.env["spp.change.request"] matching = self.env["spp.change.request"] @@ -346,18 +369,10 @@ def _filter_by_field_conflicts(self, candidates, rule): if not candidate_detail: continue - # Determine candidate's effective fields - candidate_selected = candidate._effective_selected_field() - if candidate_selected: - # Both use dynamic approval: conflict only if same field - if my_selected and candidate_selected != my_selected: - continue - candidate_effective = [candidate_selected] - else: - candidate_effective = conflict_fields + candidate_effective = candidate._effective_conflict_fields(conflict_fields) - # Check overlapping effective fields - fields_to_check = set(my_effective_fields) & set(candidate_effective) + # Overlap = fields BOTH CRs actually propose to change. + fields_to_check = my_effective_fields & candidate_effective for field_name in fields_to_check: if field_name not in my_detail._fields: @@ -480,22 +495,29 @@ def _calculate_similarity(self, other_cr, config): if not my_detail or not other_detail: return 0.0 - # Dynamic approval: compare only the selected field - my_selected = self._effective_selected_field() - other_selected = other_cr._effective_selected_field() - if my_selected and other_selected: - # Different fields selected = not duplicates - if my_selected != other_selected: + # Dynamic approval: compare only the fields actually changed (derived + # server-side from the detail-vs-registrant diff, not a writable label). + my_changed = self._proposed_changed_fields() + other_changed = other_cr._proposed_changed_fields() + if my_changed is not None and other_changed is not None: + # Different set of changed fields (or neither changed anything) = + # not duplicates. + if my_changed != other_changed or not my_changed: return 0.0 - # Same field: compare that field's value only - if my_selected in my_detail._fields and my_selected in other_detail._fields: - my_value = self._normalize_field_value(getattr(my_detail, my_selected, None)) - other_value = self._normalize_field_value(getattr(other_detail, my_selected, None)) - if my_value == other_value: - return 100.0 - elif self._are_similar(my_value, other_value): - return 80.0 - return 0.0 + all_match = True + any_similar = False + for field_name in my_changed: + if field_name not in my_detail._fields or field_name not in other_detail._fields: + continue + my_value = self._normalize_field_value(getattr(my_detail, field_name, None)) + other_value = self._normalize_field_value(getattr(other_detail, field_name, None)) + if my_value != other_value: + all_match = False + if self._are_similar(my_value, other_value): + any_similar = True + if all_match: + return 100.0 + return 80.0 if any_similar else 0.0 # Static CRs (or mixed): original logic check_fields = config.get_check_fields_list() diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md index a3d0270f4..6e953628f 100644 --- a/spp_change_request_v2/readme/HISTORY.md +++ b/spp_change_request_v2/readme/HISTORY.md @@ -1,12 +1,15 @@ ### 19.0.3.0.3 -- fix(security): derive the dynamic-approval field used by conflict and - duplicate detection from the detail's validated ``field_to_modify`` selection - instead of the user-writable ``selected_field_name``. ``selected_field_name`` - was only view-readonly, so a change-request user could write it to a field - outside a rule's ``conflict_fields`` to clear a field-scoped conflict and - submit without the configured override path; the detection now ignores the - writable copy and is skipped for non-dynamic-approval request types. +- fix(security): scope dynamic-approval conflict and duplicate detection to the + fields that actually differ from the registrant (the changes the apply + strategy will write), instead of a user-writable label. Both + ``selected_field_name`` (view-readonly only) and the detail's + ``field_to_modify`` are writable by a change-request user and do not constrain + what apply changes, so a user could point them at a field outside a rule's + ``conflict_fields`` to clear a field-scoped conflict and submit without the + configured override path. Detection now derives the changed fields + server-side from the detail-vs-registrant diff and ignores those labels; it + is skipped for non-dynamic-approval request types. ### 19.0.3.0.0 diff --git a/spp_change_request_v2/tests/test_conflict_dynamic_approval.py b/spp_change_request_v2/tests/test_conflict_dynamic_approval.py index bbdeccdd9..aa3ef09fd 100644 --- a/spp_change_request_v2/tests/test_conflict_dynamic_approval.py +++ b/spp_change_request_v2/tests/test_conflict_dynamic_approval.py @@ -70,6 +70,19 @@ def _test_field_to_modify_selection(self): "enable_conflict_detection": True, } ) + # Field-mapping definitions: which detail fields map to which registrant + # fields. Conflict/duplicate detection derives the "actually changed" + # fields from these (detail value vs registrant), so a field_mapping + # type needs them — same-named here (given_name -> given_name, etc.). + cls.dynamic_cr_type.write( + { + "apply_mapping_ids": [ + Command.create({"source_field": "given_name", "target_field": "given_name"}), + Command.create({"source_field": "family_name", "target_field": "family_name"}), + Command.create({"source_field": "phone", "target_field": "phone"}), + ], + } + ) # Field-scope conflict rule: checks given_name, family_name cls.field_rule = cls.env["spp.cr.conflict.rule"].create( @@ -532,6 +545,46 @@ def test_writing_selected_field_name_on_static_cr_cannot_bypass(self): "selected_field_name must not affect conflict detection for non-dynamic CR types.", ) + def test_mislabeled_field_to_modify_cannot_bypass_conflict(self): + """Labelling field_to_modify as an unchanged field must not bypass the + conflict on the field actually changed. field_to_modify is user-writable + and does not constrain apply (field_mapping applies every changed field), + so detection must scope to what actually differs from the registrant.""" + cr_a = self._create_dynamic_cr() + cr_a.get_detail().write({"field_to_modify": "given_name", "given_name": "NewGivenA"}) + cr_a._run_conflict_checks() + + # cr_b really changes given_name (a conflict field) but labels the CR as + # modifying phone (unchanged — still the registrant's prefilled value). + cr_b = self._create_dynamic_cr() + cr_b.get_detail().write({"field_to_modify": "phone", "given_name": "NewGivenB"}) + cr_b._run_conflict_checks() + + self.assertEqual( + cr_b.conflict_status, + "warning", + "A real change to a conflict field must be detected regardless of field_to_modify.", + ) + self.assertIn(cr_a, cr_b.conflicting_cr_ids) + + def test_mislabeled_candidate_still_detected_as_conflict(self): + """A prior CR that mislabels field_to_modify while actually changing a + conflict field must still be found as a conflicting candidate.""" + # cr_a really changes given_name but labels field_to_modify = phone. + cr_a = self._create_dynamic_cr() + cr_a.get_detail().write({"field_to_modify": "phone", "given_name": "MislabeledGivenA"}) + cr_a._run_conflict_checks() + + cr_b = self._create_dynamic_cr() + cr_b.get_detail().write({"field_to_modify": "given_name", "given_name": "NewGivenB"}) + cr_b._run_conflict_checks() + + self.assertIn( + cr_a, + cr_b.conflicting_cr_ids, + "A candidate that actually changed the conflict field must be detected even if mislabeled.", + ) + def test_writing_selected_field_name_cannot_bypass_duplicate(self): """Writing selected_field_name must not drop duplicate similarity to 0.""" dup_config = self.env["spp.cr.duplicate.config"].create( From d38ae9991e8e9d1cb527e3305e1bb6ea63f1122e Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 24 Jul 2026 13:32:16 +0800 Subject: [PATCH 4/4] docs(spp_change_request_v2): regenerate README/index for updated 19.0.3.0.3 changelog Applies CI's pinned oca-gen output verbatim for the corrected HISTORY fragment. --- spp_change_request_v2/README.rst | 19 +++++++++++-------- .../static/description/index.html | 19 +++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst index 0d9fd1758..52b1d2669 100644 --- a/spp_change_request_v2/README.rst +++ b/spp_change_request_v2/README.rst @@ -856,14 +856,17 @@ Changelog 19.0.3.0.3 ~~~~~~~~~~ -- fix(security): derive the dynamic-approval field used by conflict and - duplicate detection from the detail's validated ``field_to_modify`` - selection instead of the user-writable ``selected_field_name``. - ``selected_field_name`` was only view-readonly, so a change-request - user could write it to a field outside a rule's ``conflict_fields`` to - clear a field-scoped conflict and submit without the configured - override path; the detection now ignores the writable copy and is - skipped for non-dynamic-approval request types. +- fix(security): scope dynamic-approval conflict and duplicate detection + to the fields that actually differ from the registrant (the changes + the apply strategy will write), instead of a user-writable label. Both + ``selected_field_name`` (view-readonly only) and the detail's + ``field_to_modify`` are writable by a change-request user and do not + constrain what apply changes, so a user could point them at a field + outside a rule's ``conflict_fields`` to clear a field-scoped conflict + and submit without the configured override path. Detection now derives + the changed fields server-side from the detail-vs-registrant diff and + ignores those labels; it is skipped for non-dynamic-approval request + types. 19.0.3.0.0 ~~~~~~~~~~ diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html index bb9b3ac13..0d39262b6 100644 --- a/spp_change_request_v2/static/description/index.html +++ b/spp_change_request_v2/static/description/index.html @@ -1341,14 +1341,17 @@

    Changelog

    19.0.3.0.3

      -
    • fix(security): derive the dynamic-approval field used by conflict and -duplicate detection from the detail’s validated field_to_modify -selection instead of the user-writable selected_field_name. -selected_field_name was only view-readonly, so a change-request -user could write it to a field outside a rule’s conflict_fields to -clear a field-scoped conflict and submit without the configured -override path; the detection now ignores the writable copy and is -skipped for non-dynamic-approval request types.
      • +
    • fix(security): scope dynamic-approval conflict and duplicate detection +to the fields that actually differ from the registrant (the changes +the apply strategy will write), instead of a user-writable label. Both +selected_field_name (view-readonly only) and the detail’s +field_to_modify are writable by a change-request user and do not +constrain what apply changes, so a user could point them at a field +outside a rule’s conflict_fields to clear a field-scoped conflict +and submit without the configured override path. Detection now derives +the changed fields server-side from the detail-vs-registrant diff and +ignores those labels; it is skipped for non-dynamic-approval request +types.