Skip to content

fix(change_request): evaluate field-mapping transform expressions again #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonzalesedwin1123 merged 5 commits into from
Aug 28, 2026
Merged

fix(change_request): evaluate field-mapping transform expressions again #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
78333f1
fix(change_request): evaluate field-mapping transform expressions again
kneckinator Aug 26, 2026
8259953
Merge branch '19.0' into fix/field-mapping-transform-safe-eval
kneckinator Aug 26, 2026
38e05b5
fix(security): sandbox and fail-close field-mapping transform express…
kneckinator Aug 27, 2026
b06c9c8
fix(change_request): build the expression snapshot from requester-rea…
kneckinator Aug 28, 2026
925f778
chore(change_request): regenerate README with CI's generator output
kneckinator Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions spp_change_request_v2/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,44 @@ 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. **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. 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.

19.0.3.1.10
~~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion spp_change_request_v2/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "OpenSPP Change Request V2",
"version": "19.0.3.1.10",
"version": "19.0.3.1.11",
"sequence": 50,
"category": "OpenSPP",
"summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",
Expand Down
11 changes: 8 additions & 3 deletions spp_change_request_v2/models/change_request_type_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ 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, 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."
),
)
7 changes: 7 additions & 0 deletions spp_change_request_v2/readme/HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
### 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. 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.

### 19.0.3.1.10

- fix(security): conflict and duplicate detection now decide whether a mapped field changed using the same comparison the apply strategy uses. Detection compared through a helper that lowercases and strips strings while apply compares raw, so a case- or whitespace-only edit was invisible to detection yet still written to the registrant — enough to sidestep a field-scoped conflict rule with a cosmetic edit. Detection also ignored transform expressions, which apply evaluates before comparing. Similarity scoring is unchanged and stays case-insensitive, since that is the point of a fuzzy match.
Expand Down
77 changes: 58 additions & 19 deletions spp_change_request_v2/static/description/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,45 @@ <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. <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. 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
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">
<h1>19.0.3.1.10</h1>
<ul class="simple">
<li>fix(security): conflict and duplicate detection now decide whether a
Expand Down Expand Up @@ -1378,7 +1417,7 @@ <h1>19.0.3.1.10</h1>
configured mapping.</li>
</ul>
</div>
<div class="section" id="section-2">
<div class="section" id="section-3">
<h1>19.0.3.1.9</h1>
<ul class="simple">
<li>fix(security): duplicate detection now scores the fields both change
Expand All @@ -1395,7 +1434,7 @@ <h1>19.0.3.1.9</h1>
requester-writable <tt class="docutils literal">selected_field_name</tt> / <tt class="docutils literal">field_to_modify</tt>.</li>
</ul>
</div>
<div class="section" id="section-3">
<div class="section" id="section-4">
<h1>19.0.3.1.8</h1>
<ul class="simple">
<li>fix(security): scope the Create-Group member wizards to the parent
Expand All @@ -1413,7 +1452,7 @@ <h1>19.0.3.1.8</h1>
access-control entry grants.</li>
</ul>
</div>
<div class="section" id="section-4">
<div class="section" id="section-5">
<h1>19.0.3.1.7</h1>
<ul class="simple">
<li>fix(security): require change-request manager rights to apply a change
Expand All @@ -1428,7 +1467,7 @@ <h1>19.0.3.1.7</h1>
endpoint.</strong></li>
</ul>
</div>
<div class="section" id="section-5">
<div class="section" id="section-6">
<h1>19.0.3.1.6</h1>
<ul class="simple">
<li>fix(security): derive conflict and duplicate detection from the change
Expand All @@ -1442,7 +1481,7 @@ <h1>19.0.3.1.6</h1>
an empty one, so detection cannot silently disable itself.</li>
</ul>
</div>
<div class="section" id="section-6">
<div class="section" id="section-7">
<h1>19.0.3.1.5</h1>
<ul class="simple">
<li>fix(security): scope the CR Requestor, Local Validator and HQ
Expand All @@ -1454,7 +1493,7 @@ <h1>19.0.3.1.5</h1>
are <tt class="docutils literal">noupdate</tt>.</li>
</ul>
</div>
<div class="section" id="section-7">
<div class="section" id="section-8">
<h1>19.0.3.1.4</h1>
<ul class="simple">
<li>fix(security): add ownership and area record rules to every concrete
Expand All @@ -1471,7 +1510,7 @@ <h1>19.0.3.1.4</h1>
unrestricted delete their access-control entries grant.</li>
</ul>
</div>
<div class="section" id="section-8">
<div class="section" id="section-9">
<h1>19.0.3.1.3</h1>
<ul class="simple">
<li>fix(security): route and apply the same single field for
Expand All @@ -1484,7 +1523,7 @@ <h1>19.0.3.1.3</h1>
the routing selector.</li>
</ul>
</div>
<div class="section" id="section-9">
<div class="section" id="section-10">
<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
Expand All @@ -1493,7 +1532,7 @@ <h1>19.0.3.1.2</h1>
(#1136)</li>
</ul>
</div>
<div class="section" id="section-10">
<div class="section" id="section-11">
<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
Expand All @@ -1507,7 +1546,7 @@ <h1>19.0.3.1.1</h1>
applied) so the constraint applies cleanly on upgrade.</li>
</ul>
</div>
<div class="section" id="section-11">
<div class="section" id="section-12">
<h1>19.0.3.1.0</h1>
<ul class="simple">
<li>revert(change_request): restore the create-a-new-individual <strong>Add
Expand All @@ -1525,7 +1564,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-12">
<div class="section" id="section-13">
<h1>19.0.3.0.0</h1>
<ul class="simple">
<li>feat(change_request): redesign the group/membership CR flows (#242) —
Expand All @@ -1547,7 +1586,7 @@ <h1>19.0.3.0.0</h1>
must adapt (see #1133).</li>
</ul>
</div>
<div class="section" id="section-13">
<div class="section" id="section-14">
<h1>19.0.2.0.8</h1>
<ul class="simple">
<li>fix(views): disable inline creation of CR document types on the Change
Expand All @@ -1558,7 +1597,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-14">
<div class="section" id="section-15">
<h1>19.0.2.0.7</h1>
<ul class="simple">
<li>fix(security): align CR Requestor / CR Local Validator / CR HQ
Expand All @@ -1570,7 +1609,7 @@ <h1>19.0.2.0.7</h1>
dependencies.</li>
</ul>
</div>
<div class="section" id="section-15">
<div class="section" id="section-16">
<h1>19.0.2.0.6</h1>
<ul class="simple">
<li>fix(views): route post-submit CRs (pending / approved / applied /
Expand All @@ -1585,7 +1624,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-16">
<div class="section" id="section-17">
<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
Expand All @@ -1598,27 +1637,27 @@ <h1>19.0.2.0.5</h1>
roles).</li>
</ul>
</div>
<div class="section" id="section-17">
<div class="section" id="section-18">
<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-18">
<div class="section" id="section-19">
<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-19">
<div class="section" id="section-20">
<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-20">
<div class="section" id="section-21">
<h1>19.0.2.0.0</h1>
<ul class="simple">
<li>Initial migration to OpenSPP2</li>
Expand Down
Loading
Loading