Explorar o código

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

* 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.

* Fixes #23375: Raise on template code mismatch and document finalize deprecation

When a template has a saved finalize value but its template code no longer matches the saved record, render now raises a TemplateError instead of rendering without finalize. Update the export and config template docs, which still described finalize as settable.

* Fixes #23375: Raise when the stored template was edited or deleted after load
Jason Novinger hai 1 día
pai
achega
33730d030f

+ 5 - 1
docs/models/extras/configtemplate.md

@@ -24,7 +24,7 @@ Jinja2 template code, if being defined locally rather than replicated from a dat
 
 A dictionary of any additional parameters to pass when instantiating the [Jinja2 environment](https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment). Jinja2 supports various optional parameters which can be used to modify its default behavior.
 
-The `undefined` and `finalize` Jinja environment parameters, which must reference a Python class or function, can define a dotted path to the desired resource. For example:
+The `undefined` Jinja environment parameter, which must reference a Python class, can define a dotted path to the desired resource. For example:
 
 ```json
 {
@@ -32,6 +32,10 @@ The `undefined` and `finalize` Jinja environment parameters, which must referenc
 }
 ```
 
+!!! note "Changed in NetBox v4.6.1"
+
+    The `finalize` parameter is deprecated and cannot be set on new or modified templates. Templates that already have it saved continue to honor it.
+
 ### MIME Type
 
 The MIME type to indicate in the response when rendering the configuration template (optional). Defaults to `text/plain`.

+ 5 - 1
docs/models/extras/exporttemplate.md

@@ -24,7 +24,7 @@ Jinja2 template code for rendering the exported data.
 
 A dictionary of any additional parameters to pass when instantiating the [Jinja2 environment](https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment). Jinja2 supports various optional parameters which can be used to modify its default behavior.
 
-The `undefined` and `finalize` Jinja environment parameters, which must reference a Python class or function, can define a dotted path to the desired resource. For example:
+The `undefined` Jinja environment parameter, which must reference a Python class, can define a dotted path to the desired resource. For example:
 
 ```json
 {
@@ -32,6 +32,10 @@ The `undefined` and `finalize` Jinja environment parameters, which must referenc
 }
 ```
 
+!!! note "Changed in NetBox v4.6.1"
+
+    The `finalize` parameter is deprecated and cannot be set on new or modified templates. Templates that already have it saved continue to honor it.
+
 ### MIME Type
 
 The MIME type to indicate in the response when rendering the export template (optional). Defaults to `text/plain`.

+ 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
 }
 

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

@@ -12,6 +12,7 @@ from django.http import HttpResponse
 from django.utils.http import content_disposition_header
 from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
+from jinja2 import TemplateError
 
 from core.models import ObjectType
 from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_ALLOWED, SCRIPT_MODULE_NAME_PREFIX
@@ -191,7 +192,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 +218,42 @@ 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 the template has no stored value. Raises TemplateError if
+        this saved template's row is gone or its stored code differs from this instance's. 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()
+            # Fail instead of rendering code the stored row no longer backs, which gives different output (#23375).
+            if stored is None or stored[1] != self.template_code:
+                raise TemplateError(
+                    _('The template differs from the saved template. Reload the template and try again.')
+                )
+            stored_finalize = (stored[0] or {}).get('finalize')
+        if stored_finalize is None:
+            logger.warning(
+                'Ignoring the deprecated "finalize" environment parameter on %s (pk=%s): '
+                'it is not stored on the 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 +262,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
 

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

@@ -1429,12 +1429,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'},
@@ -1594,6 +1594,89 @@ 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_render_fails_when_template_code_differs_from_stored(self):
+        t = self._create_stored()
+        t.template_code = '{{ "changed" }}'
+        with self.assertRaises(TemplateError):
+            t.render()
+
+    @tag('regression')
+    def test_render_fails_when_stored_template_is_edited_after_load(self):
+        t = self._create_stored()
+        # An edit that saves new code must also remove 'finalize', since clean() rejects it
+        ExportTemplate.objects.filter(pk=t.pk).update(template_code='{{ "edited" }}', environment_params={})
+        with self.assertRaises(TemplateError):
+            t.render()
+
+    @tag('regression')
+    def test_render_fails_when_stored_template_is_deleted_after_load(self):
+        t = self._create_stored()
+        ExportTemplate.objects.filter(pk=t.pk).delete()
+        with self.assertRaises(TemplateError):
+            t.render()
+
+    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)."""
 
@@ -1888,9 +1971,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)