Просмотр исходного кода

Fixes #23375: Honor finalize only from the stored template record

The deprecated finalize environment parameter is now resolved only with the value saved on the template's database record. Unsaved templates and in-memory changes no longer supply a finalize value. When no stored value applies, the parameter is dropped and a warning is logged.
Jason Novinger 2 дней назад
Родитель
Сommit
286135509b
3 измененных файлов с 108 добавлено и 11 удалено
  1. 3 4
      netbox/extras/constants.py
  2. 31 3
      netbox/extras/models/mixins.py
  3. 74 4
      netbox/extras/tests/test_models.py

+ 3 - 4
netbox/extras/constants.py

@@ -53,9 +53,8 @@ WEBHOOK_EVENT_TYPES = {
 #   None  = any JSON-serializable value accepted (scalars, booleans, etc.)
 #   dict  = only dict keys accepted; dict values are the resolved Python objects
 #
-# Note: 'finalize' is intentionally absent. It is deprecated and handled as a
-# legacy carve-out in RenderTemplateMixin (blocked from new use, but existing
-# stored values continue to resolve via import_string at render time).
+# Note: 'finalize' is intentionally absent. RenderTemplateMixin resolves only the
+# stored value of this deprecated parameter (#23375).
 JINJA_ENV_PARAMS_ALLOWED = {
     # Boolean / scalar params (accept any JSON-serializable value)
     'auto_reload': None,
@@ -86,7 +85,7 @@ JINJA_ENV_PARAMS_ALLOWED = {
     # Excluded (dangerous — accept callables or trigger imports):
     #   'bytecode_cache' — accepts arbitrary object
     #   'extensions'     — Jinja2 internally calls import_string() on string entries
-    #   'finalize'       — deprecated; legacy carve-out in RenderTemplateMixin
+    #   'finalize'       — deprecated and stored-only, see RenderTemplateMixin
     #   'loader'         — accepts arbitrary object
 }
 

+ 31 - 3
netbox/extras/models/mixins.py

@@ -191,7 +191,7 @@ class RenderTemplateMixin(models.Model):
     def _filter_environment_params(params):
         """
         Return a copy of params with only permitted keys. Keys not in the allowlist are
-        stripped, except 'finalize' which is a deprecated legacy carve-out.
+        stripped, except 'finalize', which is deprecated and stored-only.
         """
         return {
             key: value for key, value in params.items()
@@ -217,13 +217,40 @@ class RenderTemplateMixin(models.Model):
     @staticmethod
     def _resolve_finalize(params):
         """
-        Legacy carve-out: resolve the deprecated 'finalize' parameter via import_string().
-        Existing templates with finalize continue to work; new use is blocked by clean().
+        Resolve the deprecated 'finalize' parameter via import_string(). Call
+        _restrict_finalize_to_stored() first. clean() blocks new use.
         """
         if 'finalize' in params and type(params['finalize']) is str:
             return {**params, 'finalize': import_string(params['finalize'])}
         return params
 
+    def _restrict_finalize_to_stored(self, params):
+        """
+        Return a copy of params with 'finalize' replaced by the value stored on this template's
+        database record, or removed if no stored value applies. Runs one query when params
+        contain 'finalize', and none otherwise.
+        """
+        if 'finalize' not in params:
+            return params
+        stored_finalize = None
+        if self.pk is not None:
+            stored = type(self)._base_manager.using(self._state.db).filter(pk=self.pk).values_list(
+                'environment_params', 'template_code'
+            ).first()
+            if stored is not None:
+                stored_params, stored_code = stored
+                # A stored value applies only to the template code it was stored with (#23375).
+                if stored_code == self.template_code:
+                    stored_finalize = (stored_params or {}).get('finalize')
+        if stored_finalize is None:
+            logger.warning(
+                'Ignoring the deprecated "finalize" environment parameter on %s (pk=%s): '
+                'it does not match the stored template.',
+                type(self).__name__, self.pk,
+            )
+            return {key: value for key, value in params.items() if key != 'finalize'}
+        return {**params, 'finalize': stored_finalize}
+
     def get_environment_params(self):
         """
         Pre-processing of any defined Jinja environment parameters.
@@ -232,6 +259,7 @@ class RenderTemplateMixin(models.Model):
         params = dict(self.environment_params or {})
         params = self._filter_environment_params(params)
         params = self._resolve_mapped_params(params)
+        params = self._restrict_finalize_to_stored(params)
         params = self._resolve_finalize(params)
         return params
 

+ 74 - 4
netbox/extras/tests/test_models.py

@@ -1417,12 +1417,12 @@ class RenderTemplateMixinRenderTestCase(TestCase):
         with self.assertRaises(UndefinedError):
             strict.render({})
 
-    def test_environment_params_finalize_legacy_resolution(self):
+    def test_environment_params_stored_finalize_resolution(self):
         """
-        Existing finalize values continue to resolve via import_string() as a
-        legacy carve-out (CVE-2026-29514). New use is blocked by clean().
+        A stored 'finalize' value still resolves via import_string(). clean() blocks new use (CVE-2026-29514).
         """
-        t = ConfigTemplate(
+        # create() does not call clean(). This matches a value stored before clean() blocked it.
+        t = ConfigTemplate.objects.create(
             name='finalize',
             template_code='{{ v }}',
             environment_params={'finalize': 'extras.tests.test_models.finalize_none_to_dash'},
@@ -1582,6 +1582,74 @@ class ExportTemplateRenderTestCase(TestCase):
         self.assertEqual(response.content.decode(), 'Site A\nSite B\nSite C\n')
 
 
+class ExportTemplateStoredOnlyFinalizeTestCase(TestCase):
+    """
+    Tests that render() honors 'finalize' only with the value stored on the template's database record.
+    """
+    TEMPLATE_CODE = '{{ "netbox" }}'
+
+    def _create_stored(self):
+        """
+        Create a template with a stored 'finalize' value.
+        """
+        # create() does not call clean(). This matches a value stored before clean() blocked it.
+        return ExportTemplate.objects.create(
+            name='stored',
+            template_code=self.TEMPLATE_CODE,
+            environment_params={'finalize': 'builtins.len'},
+        )
+
+    @tag('regression')
+    def test_unsaved_template_ignores_finalize(self):
+        t = ExportTemplate(
+            name='unsaved',
+            template_code=self.TEMPLATE_CODE,
+            environment_params={'finalize': 'builtins.len'},
+        )
+        with self.assertLogs('extras.models.mixins', 'WARNING'):
+            self.assertEqual(t.render(), 'netbox')
+
+    def test_stored_finalize_is_applied(self):
+        self._create_stored()
+        t = ExportTemplate.objects.get(name='stored')
+        self.assertEqual(t.render(), '6')
+
+    @tag('regression')
+    def test_finalize_added_in_memory_is_ignored(self):
+        t = ExportTemplate.objects.create(name='saved', template_code=self.TEMPLATE_CODE)
+        t.environment_params = {'finalize': 'builtins.len'}
+        with self.assertLogs('extras.models.mixins', 'WARNING'):
+            self.assertEqual(t.render(), 'netbox')
+
+    @tag('regression')
+    def test_finalize_changed_in_memory_uses_stored_value(self):
+        t = self._create_stored()
+        t.environment_params = {'finalize': 'builtins.repr'}
+        with self.assertNoLogs('extras.models.mixins', 'WARNING'):
+            self.assertEqual(t.render(), '6')
+
+    @tag('regression')
+    def test_finalize_ignored_when_template_code_changed_in_memory(self):
+        t = self._create_stored()
+        t.template_code = '{{ "changed" }}'
+        with self.assertLogs('extras.models.mixins', 'WARNING'):
+            self.assertEqual(t.render(), 'changed')
+
+    def test_no_query_without_finalize(self):
+        t = ExportTemplate.objects.create(
+            name='plain',
+            template_code=self.TEMPLATE_CODE,
+            environment_params={'trim_blocks': True},
+        )
+        with self.assertNumQueries(0):
+            t.get_environment_params()
+
+    def test_one_query_with_stored_finalize(self):
+        t = self._create_stored()
+        with self.assertNumQueries(1):
+            t.get_environment_params()
+
+
 class WebhookPayloadUrlValidationTestCase(TestCase):
     """Tests for Webhook.clean()'s validation of payload_url (#22828)."""
 
@@ -1876,9 +1944,11 @@ class JinjaEnvironmentParamsIntegrationTestCase(TestCase):
         self.assertIs(params['trim_blocks'], True)
 
     def test_full_pipeline_finalize_resolves(self):
+        # Only a stored 'finalize' value resolves (#23375).
         template = self._make_template({
             'finalize': 'extras.tests.test_models.finalize_none_to_dash',
         })
+        template.save()
         params = template.get_environment_params()
         self.assertIs(params['finalize'], finalize_none_to_dash)