[19.0][FIX] endpoint: validate registry sync

endpoint/__manifest__.py

@@ -4,7 +4,7 @@
 {
     "name": "Endpoint",
     "summary": """Provide custom endpoint machinery.""",
-    "version": "19.0.1.2.0",
+    "version": "19.0.1.2.1",
     "license": "LGPL-3",
     "development_status": "Beta",
     "author": "Camptocamp,Odoo Community Association (OCA)",

endpoint/models/endpoint_mixin.py

@@ -3,6 +3,7 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
 import json
+import symtable
 import textwrap
 
 import jsonschema
@@ -43,6 +44,10 @@
     ["new", "compare_digest"],
 )
 
+# ``safe_eval`` injects these built-ins at runtime. Keep validation aligned with
+# Odoo instead of maintaining a second, potentially outdated list here.
+_SAFE_EVAL_BUILTINS = frozenset(safe_eval._BUILTINS)
+
 
 @disable_rpc()  # Block ALL RPC calls
 class EndpointMixin(models.AbstractModel):
@@ -98,6 +103,62 @@ def _validate_exec__code(self):
                 )
             )
 
+    def _registry_sync_errors(self):
+        errors = super()._registry_sync_errors()
+        snippet = self.code_snippet or ""
+        syntax_error = safe_eval.test_python_expr(snippet, mode="exec")
+        if syntax_error:
+            errors.append(
+                self.env._("Invalid code snippet: %(error)s", error=syntax_error)
+            )
+            return errors
+
+        unavailable_names = self._code_snippet_unavailable_names(snippet)
+        if unavailable_names:
+            errors.append(
+                self.env._(
+                    "The code snippet uses unavailable variable(s): %(names)s. "
+                    "Available system variables are: %(available_names)s.",
+                    names=", ".join(unavailable_names),
+                    available_names=", ".join(
+                        sorted(self._code_snippet_system_variable_names())
+                    ),
+                )
+            )
+        return errors
+
+    def _code_snippet_system_variable_names(self):
+        # Derive names from the actual evaluation context so this validation
+        # stays accurate when another system variable is added.
+        return set(self._get_code_snippet_eval_context(request=None))
+
+    def _code_snippet_unavailable_names(self, snippet):
+        """Find global names that safe_eval will not provide at runtime."""
+        symbol_table = symtable.symtable(snippet, "<endpoint>", "exec")
+        referenced_globals = set()
+
+        # ``symtable`` distinguishes global lookups from local names in nested
+        # functions and comprehensions, avoiding warnings for valid variables.
+        def collect_globals(table):
+            for symbol in table.get_symbols():
+                if symbol.is_referenced() and symbol.is_global():
+                    referenced_globals.add(symbol.get_name())
+            for child in table.get_children():
+                collect_globals(child)
+
+        collect_globals(symbol_table)
+        assigned_names = {
+            symbol.get_name()
+            for symbol in symbol_table.get_symbols()
+            if symbol.is_assigned() or symbol.is_imported()
+        }
+        available_names = (
+            self._code_snippet_system_variable_names()
+            | _SAFE_EVAL_BUILTINS
+            | assigned_names
+        )
+        return sorted(referenced_globals - available_names)
+
     def _get_request_content_schema_applicable_for_types(self):
         """Content types for which ``request_content_schema`` applies."""
         return ["application/json", "application/xml"]

endpoint/tests/test_endpoint.py

@@ -246,6 +246,83 @@ def test_registry_sync(self):
                 partial_func.func.__name__, "_handle_registry_sync_post_commit"
             )
 
+    def test_invalid_route_can_be_saved_but_not_synchronized(self):
+        endpoint = self.env["endpoint.endpoint"].create(
+            {
+                "name": "Invalid route",
+                "route": "/invalid/<date:value>",
+                "exec_mode": "code",
+                "code_snippet": 'result = {"payload": "ok"}',
+                "request_method": "GET",
+                "auth_type": "user_endpoint",
+            }
+        )
+        registry = endpoint._endpoint_registry
+        key = endpoint._endpoint_registry_unique_key()
+        self.assertEqual(registry._get_rule(key), None)
+
+        with (
+            self.assertRaisesRegex(exceptions.UserError, "converter 'date'"),
+            self.env.cr.savepoint(),
+        ):
+            endpoint.registry_sync = True
+        self.assertEqual(registry._get_rule(key), None)
+
+        endpoint.route = "/valid/<string:value>"
+        endpoint._handle_registry_sync()
+        self.assertEqual(registry._get_rule(key).route, "/valid/<string:value>")
+
+    def test_inactive_invalid_route_can_be_removed_from_registry(self):
+        endpoint = self.env["endpoint.endpoint"].create(
+            {
+                "name": "Route to remove",
+                "route": "/route/to/remove",
+                "exec_mode": "code",
+                "code_snippet": 'result = {"payload": "ok"}',
+                "request_method": "GET",
+                "auth_type": "user_endpoint",
+            }
+        )
+        registry = endpoint._endpoint_registry
+        key = endpoint._endpoint_registry_unique_key()
+        endpoint._handle_registry_sync()
+        self.assertIsNotNone(registry._get_rule(key))
+
+        endpoint.write({"route": "/invalid/<date:value>", "active": False})
+        endpoint._handle_registry_sync()
+        self.assertEqual(registry._get_rule(key), None)
+
+    def test_invalid_code_can_be_saved_but_not_synchronized(self):
+        endpoint = self.env["endpoint.endpoint"].create(
+            {
+                "name": "Invalid code",
+                "route": "/invalid/code",
+                "exec_mode": "code",
+                "code_snippet": "result = date(request.params.get('since'))",
+                "request_method": "GET",
+                "auth_type": "user_endpoint",
+            }
+        )
+        with self.assertRaisesRegex(exceptions.UserError, "unavailable.*date"):
+            endpoint._handle_registry_sync()
+
+        endpoint.code_snippet = "result = {"
+        with self.assertRaisesRegex(exceptions.UserError, "Invalid code snippet"):
+            endpoint._handle_registry_sync()
+
+    def test_locally_defined_code_variables_are_available(self):
+        self.endpoint.code_snippet = textwrap.dedent(
+            """
+            value = request.params.get("value")
+            result = {"payload": value}
+            """
+        )
+        self.assertEqual(self.endpoint._registry_sync_errors(), [])
+
+    def test_safe_eval_builtins_are_available(self):
+        self.endpoint.code_snippet = "result = {'count': len(request.params)}"
+        self.assertEqual(self.endpoint._registry_sync_errors(), [])
+
     def test_duplicate(self):
         endpoint = self.endpoint.copy()
         self.assertTrue(endpoint.route.endswith("/COPY_FIXME"))

endpoint_route_handler/__manifest__.py

@@ -4,7 +4,7 @@
 {
     "name": "Endpoint route handler",
     "summary": """Provide mixin and tool to generate custom endpoints on the fly.""",
-    "version": "19.0.1.2.0",
+    "version": "19.0.1.2.1",
     "license": "LGPL-3",
     "development_status": "Beta",
     "author": "Camptocamp,Odoo Community Association (OCA)",

endpoint_route_handler/models/endpoint_route_handler.py

@@ -4,6 +4,8 @@
 
 import logging
 
+import werkzeug
+
 from odoo import api, exceptions, fields, models
 
 ENDPOINT_ROUTE_CONSUMER_MODELS = {
@@ -187,6 +189,28 @@ def _check_route(self):
                     )
                 )
 
+    def _registry_sync_errors(self):
+        errors = super()._registry_sync_errors()
+        try:
+            # Binding the rule performs the same converter lookup as Odoo's
+            # routing map without modifying the live endpoint registry.
+            routing_map = werkzeug.routing.Map(
+                strict_slashes=False,
+                converters=self.env["ir.http"]._get_converters(),
+            )
+            rule = werkzeug.routing.Rule(self.route)
+            rule.merge_slashes = False
+            routing_map.add(rule)
+        except (LookupError, TypeError, ValueError) as error:
+            errors.append(
+                self.env._(
+                    "Invalid route %(route)s: %(error)s",
+                    route=self.route,
+                    error=error,
+                )
+            )
+        return errors
+
     @api.constrains("request_method", "request_content_type")
     def _check_request_method(self):
         for rec in self:
@@ -207,6 +231,8 @@ def _endpoint_registry_unique_key(self):
     # TODO: consider if useful or not for single records
     def _register_single_controller(self, options=None, key=None, init=False):
         """Shortcut to register one single controller."""
+        # Programmatic callers can bypass registry_sync and must be protected too.
+        self._validate_registry_sync(active_only=False)
         rule = self._make_controller_rule(options=options, key=key)
         self._endpoint_registry.update_rules([rule], init=init)
         self.env.registry.clear_cache("routing")

endpoint_route_handler/models/endpoint_route_sync_mixin.py

@@ -6,6 +6,7 @@
 from functools import partial
 
 from odoo import api, fields, models
+from odoo.exceptions import UserError
 
 from ..registry import EndpointRegistry
 
@@ -43,6 +44,9 @@ def write(self, vals):
         if any([x in vals for x in self._routing_impacting_fields() + ("active",)]):
             # Mark as out of sync
             vals["registry_sync"] = False
+        if vals.get("registry_sync"):
+            # Validate only when publishing so invalid drafts remain editable.
+            self._validate_registry_sync()
         res = super().write(vals)
         if vals.get("registry_sync"):
             # NOTE: this is not done on create to allow bulk reload of the envs
@@ -51,6 +55,38 @@ def write(self, vals):
             self._add_after_commit_hook(self.ids)
         return res
 
+    def _registry_sync_errors(self):
+        """Return errors preventing this record from being registered."""
+        self.ensure_one()
+        return []
+
+    def _validate_registry_sync(self, active_only=True):
+        """Protect the global routing map from invalid endpoint records.
+
+        Inactive records can be skipped because synchronization only removes
+        their existing rules from the registry.
+        """
+        invalid_records = []
+        records = self.filtered("active") if active_only else self
+        for record in records:
+            errors = record._registry_sync_errors()
+            if errors:
+                invalid_records.append(
+                    self.env._(
+                        "%(name)s:\n- %(errors)s",
+                        name=record.display_name,
+                        errors="\n- ".join(errors),
+                    )
+                )
+        if invalid_records:
+            raise UserError(
+                self.env._(
+                    "The registry cannot be synchronized because some active "
+                    "records are invalid:\n\n%(errors)s",
+                    errors="\n\n".join(invalid_records),
+                )
+            )
+
     @api.model
     def _add_after_commit_hook(self, record_ids):
         self.env.cr.postcommit.add(
@@ -91,6 +127,8 @@ def unlink(self):
     def _register_controllers(self, init=False, options=None, clear_cache=True):
         if not self:
             return
+        # Startup and programmatic callers do not necessarily pass through write().
+        self._validate_registry_sync(active_only=False)
         rules = self._prepare_endpoint_rules(options=options)
         self._endpoint_registry.update_rules(rules, init=init)
         if clear_cache:

endpoint_route_handler/tests/test_endpoint.py

@@ -3,7 +3,7 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 from contextlib import contextmanager
 
-from odoo import api, modules
+from odoo import api, exceptions, modules
 from odoo.tests import common
 from odoo.tools import mute_logger
 
@@ -50,6 +50,26 @@ def test_as_tool_base_data(self):
         new_route.route += "/new"
         self.assertNotEqual(new_route.endpoint_hash, first_hash)
 
+    def test_invalid_route_cannot_be_registered(self):
+        for route, error in (
+            ("/my/test/<date:value>", "converter 'date' does not exist"),
+            ("/my/test/<value", "malformed url rule"),
+        ):
+            with self.subTest(route=route):
+                new_route = make_new_route(self.env, route=route)
+                with self.assertRaisesRegex(exceptions.UserError, error):
+                    new_route._register_controllers()
+
+    def test_inactive_invalid_route_can_be_synchronized(self):
+        new_route = make_new_route(
+            self.env,
+            route="/my/test/<date:value>",
+            active=False,
+        )
+        new_route._validate_registry_sync()
+        with self.assertRaisesRegex(exceptions.UserError, "converter 'date'"):
+            new_route._register_controllers()
+
     def test_auth_type_routing_info(self):
         for auth_type in ("public", "user_endpoint", "bearer"):
             new_route = make_new_route(self.env, auth_type=auth_type)