[IMP] core: enforce custom fields to starts with x_

The `api.constrains` `_check_name` on `ir.model.fields`
ensures users do not create custom fields not starting with `x_`.
It offers a user-friendly message when users try to do that.

However, this `api.constrains` can be bypassed,
for instance when updating the field name through raw SQL.

This revision aims to no longer load custom fields
if they do not starts with `x_`.

In addition, it replaces the Python constraint
with an SQL constraint in order to have a stronger constraint.
Following bugs or unexpected flows, people
were able to add custom fields not starting with `x_`
with the `api.constrains` alone.
Adding the SQL constraint will tighten the constraint.

However, even without that SQL constraint,
for instance if users drop the SQL constraint manually,
custom field not starting by `x_` must not be loaded.
A user would be able to drop that constraint
through a SQL shell or a `cr.execute` in a server action.

This is to prevent users to override class attributes.

closes odoo/odoo#127702

Related: odoo/upgrade#4908

odoo/addons/base/models/ir_model.py

@@ -596,11 +596,9 @@ class IrModelFields(models.Model):
         for field in self:
             safe_eval(field.domain or '[]')
 
-    @api.constrains('name', 'state')
+    @api.constrains('name')
     def _check_name(self):
         for field in self:
-            if field.state == 'manual' and not field.name.startswith('x_'):
-                raise ValidationError(_("Custom fields must have a name that starts with 'x_'!"))
             try:
                 models.check_pg_name(field.name)
             except ValidationError:
@@ -610,6 +608,11 @@ class IrModelFields(models.Model):
     _sql_constraints = [
         ('name_unique', 'UNIQUE(model, name)', "Field names must be unique per model."),
         ('size_gt_zero', 'CHECK (size>=0)', 'Size of the field cannot be negative.'),
+        (
+            "name_manual_field",
+            "CHECK (state != 'manual' OR name LIKE 'x_%')",
+            "Custom fields must have a name that starts with 'x_'!"
+        ),
     ]
 
     def _related_field(self):

odoo/addons/base/tests/test_ir_actions.py

@@ -379,13 +379,13 @@ class TestCustomFields(common.TransactionCase):
 
     def test_create_custom(self):
         """ custom field names must be start with 'x_' """
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'):
             self.create_field('foo')
 
     def test_rename_custom(self):
         """ custom field names must be start with 'x_' """
         field = self.create_field('x_foo')
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'):
             field.name = 'foo'
 
     def test_create_valid(self):
@@ -399,6 +399,26 @@ class TestCustomFields(common.TransactionCase):
         with self.assertRaises(ValidationError):
             field.name = 'x_foo bar'
 
+    def test_add_field_valid(self):
+        """ custom field names must start with 'x_', even when bypassing the constraints
+
+        If a user bypasses all constraints to add a custom field not starting by `x_`,
+        it must not be loaded in the registry.
+
+        This is to forbid users to override class attributes.
+        """
+        field = self.create_field('x_foo')
+        # Drop the SQL constraint, to bypass it,
+        # as a user could do through a SQL shell or a `cr.execute` in a server action
+        self.env.cr.execute("ALTER TABLE ir_model_fields DROP CONSTRAINT ir_model_fields_name_manual_field")
+        self.env.cr.execute("UPDATE ir_model_fields SET name = 'foo' WHERE id = %s", [field.id])
+        with self.assertLogs('odoo.addons.base.models.ir_model') as log_catcher:
+            # Trick to reload the registry. The above rename done through SQL didn't reload the registry. This will.
+            field.write({'field_description': 'foo'})
+            self.assertIn(
+                f'The field `{field.name}` is not defined in the `{field.model}` Python class', log_catcher.output[0]
+            )
+
     def test_create_unique(self):
         """ one cannot create two fields with the same name on a given model """
         self.create_field('x_foo')

odoo/models.py

@@ -613,12 +613,28 @@ class BaseModel(metaclass=MetaModel):
     def _add_field(self, name, field):
         """ Add the given ``field`` under the given ``name`` in the class """
         cls = type(self)
-        # add field as an attribute and in cls._fields (for reflection)
+
+        # Assert the name is an existing field in the model, or any model in the _inherits
+        # or a custom field (starting by `x_`)
+        is_class_field = any(
+            isinstance(getattr(model, name, None), fields.Field)
+            for model in [cls] + [self.env.registry[inherit] for inherit in cls._inherits]
+        )
+        if not (is_class_field or name.startswith('x_')):
+            raise ValidationError(
+                f"The field `{name}` is not defined in the `{cls._name}` Python class and does not start with 'x_'"
+            )
+
+        # Assert the attribute to assign is a Field
+        if not isinstance(field, fields.Field):
+            raise ValidationError("You can only add `fields.Field` objects to a model fields")
+
         if not isinstance(getattr(cls, name, field), Field):
             _logger.warning("In model %r, field %r overriding existing value", cls._name, name)
         setattr(cls, name, field)
         field._toplevel = True
         field.__set_name__(cls, name)
+        # add field as an attribute and in cls._fields (for reflection)
         cls._fields[name] = field
 
     @api.model