diff --git a/netbox/extras/constants.py b/netbox/extras/constants.py index d88fdb3c3..bae808f24 100644 --- a/netbox/extras/constants.py +++ b/netbox/extras/constants.py @@ -1,3 +1,5 @@ +from jinja2 import ChainableUndefined, DebugUndefined, StrictUndefined, Undefined + from core.events import * from extras.choices import LogLevelChoices @@ -32,11 +34,47 @@ WEBHOOK_EVENT_TYPES = { JOB_ERRORED: 'job_ended', } -# Jinja environment parameters which support path imports -JINJA_ENV_PARAMS_WITH_PATH_IMPORT = ( - 'undefined', - 'finalize', -) +# Allowed Jinja2 environment parameters and their permitted values. +# Only keys listed here may appear in a template's environment_params. +# 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). +JINJA_ENV_PARAMS_ALLOWED = { + # Boolean / scalar params (accept any JSON-serializable value) + 'auto_reload': None, + 'autoescape': None, + 'cache_size': None, + 'enable_async': None, + 'keep_trailing_newline': None, + 'lstrip_blocks': None, + 'optimized': None, + 'trim_blocks': None, + # String params (template syntax delimiters) + 'block_start_string': None, + 'block_end_string': None, + 'comment_start_string': None, + 'comment_end_string': None, + 'line_comment_prefix': None, + 'line_statement_prefix': None, + 'newline_sequence': None, + 'variable_start_string': None, + 'variable_end_string': None, + # Mapped params (value must be a key in the dict; resolved to the dict value) + 'undefined': { + 'jinja2.ChainableUndefined': ChainableUndefined, + 'jinja2.DebugUndefined': DebugUndefined, + 'jinja2.StrictUndefined': StrictUndefined, + 'jinja2.Undefined': Undefined, + }, + # 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 + # 'loader' — accepts arbitrary object +} # Dashboard DEFAULT_DASHBOARD = [ diff --git a/netbox/extras/models/mixins.py b/netbox/extras/models/mixins.py index 79df67a11..7eee9ac3c 100644 --- a/netbox/extras/models/mixins.py +++ b/netbox/extras/models/mixins.py @@ -4,6 +4,7 @@ import os import sys from collections import defaultdict +from django.core.exceptions import ValidationError from django.core.files.storage import storages from django.db import models from django.http import HttpResponse @@ -11,7 +12,7 @@ from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ from core.models import ObjectType -from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_WITH_PATH_IMPORT +from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_ALLOWED from extras.utils import filename_from_model, filename_from_object from utilities.jinja2 import render_jinja2 @@ -134,15 +135,84 @@ class RenderTemplateMixin(models.Model): return _context + def clean(self): + super().clean() + + params = self.environment_params or {} + for key, value in params.items(): + # finalize is deprecated: block new use but preserve existing stored values + if key == 'finalize': + raise ValidationError({ + 'environment_params': _( + 'The "{key}" parameter is deprecated and may not be set on new or modified templates.' + ).format(key=key) + }) + if key not in JINJA_ENV_PARAMS_ALLOWED: + raise ValidationError({ + 'environment_params': _( + '"{key}" is not a permitted Jinja2 environment parameter.' + ).format(key=key) + }) + allowed = JINJA_ENV_PARAMS_ALLOWED[key] + if type(allowed) is dict: + if value not in allowed: + raise ValidationError({ + 'environment_params': _( + 'Invalid value "{value}" for parameter "{key}". ' + 'Allowed values are: {allowed}' + ).format( + value=value, + key=key, + allowed=', '.join(sorted(allowed.keys())) + ) + }) + + @staticmethod + 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. + """ + return { + key: value for key, value in params.items() + if key in JINJA_ENV_PARAMS_ALLOWED or key == 'finalize' + } + + @staticmethod + def _resolve_mapped_params(params): + """ + Resolve allowlisted params that have a value mapping (e.g. undefined class names) + to their Python objects via direct dict lookup. Returns a new dict with resolved values; + unresolved params are passed through unchanged. + """ + resolved = {} + for name, value in params.items(): + allowed = JINJA_ENV_PARAMS_ALLOWED.get(name) + if type(allowed) is dict and value in allowed: + resolved[name] = allowed[value] + else: + resolved[name] = value + return resolved + + @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(). + """ + if 'finalize' in params and type(params['finalize']) is str: + return {**params, 'finalize': import_string(params['finalize'])} + return params + def get_environment_params(self): """ - Pre-processing of any defined Jinja environment parameters (e.g. to support path resolution). + Pre-processing of any defined Jinja environment parameters. """ - # Shallow-copy so resolved imports don't replace the string values on the model field itself. + # Shallow-copy so resolved values don't replace the strings on the model field. params = dict(self.environment_params or {}) - for name, value in params.items(): - if name in JINJA_ENV_PARAMS_WITH_PATH_IMPORT and type(value) is str: - params[name] = import_string(value) + params = self._filter_environment_params(params) + params = self._resolve_mapped_params(params) + params = self._resolve_finalize(params) return params def render(self, context=None, queryset=None): diff --git a/netbox/extras/tests/test_models.py b/netbox/extras/tests/test_models.py index d3a2ad6fd..794f216fb 100644 --- a/netbox/extras/tests/test_models.py +++ b/netbox/extras/tests/test_models.py @@ -10,7 +10,7 @@ from django.core.files.storage import Storage from django.core.files.uploadedfile import SimpleUploadedFile from django.forms import ValidationError from django.test import TestCase, tag -from jinja2 import StrictUndefined, TemplateError, TemplateSyntaxError, UndefinedError +from jinja2 import DebugUndefined, StrictUndefined, TemplateError, TemplateSyntaxError, UndefinedError from PIL import Image from core.events import OBJECT_CREATED @@ -27,6 +27,7 @@ from extras.models import ( Tag, TaggedItem, ) +from extras.models.mixins import RenderTemplateMixin from tenancy.models import Tenant, TenantGroup from utilities.exceptions import AbortRequest from utilities.jinja2 import render_jinja2 @@ -1032,7 +1033,11 @@ class RenderTemplateMixinRenderTest(TestCase): with self.assertRaises(UndefinedError): strict.render({}) - def test_environment_params_finalize_path_import(self): + def test_environment_params_finalize_legacy_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(). + """ t = ConfigTemplate( name='finalize', template_code='{{ v }}', @@ -1180,3 +1185,199 @@ class EventRuleTestCase(TestCase): with self.assertRaises(ValidationError) as cm: rule.clean() self.assertIn('action_data', cm.exception.message_dict) + + +class JinjaEnvironmentParamsCleanTest(TestCase): + """Tests for RenderTemplateMixin.clean() validation of environment_params.""" + + def _make_template(self, environment_params): + return ConfigTemplate( + name='test', + template_code='{{ "test" }}', + environment_params=environment_params, + ) + + def test_allowed_scalar_params_pass(self): + template = self._make_template({'trim_blocks': True, 'lstrip_blocks': True}) + template.clean() + + def test_autoescape_boolean_passes(self): + template = self._make_template({'autoescape': True}) + template.clean() + + def test_valid_undefined_passes(self): + for value in ( + 'jinja2.Undefined', + 'jinja2.ChainableUndefined', + 'jinja2.DebugUndefined', + 'jinja2.StrictUndefined', + ): + template = self._make_template({'undefined': value}) + template.clean() + + def test_invalid_undefined_rejected(self): + template = self._make_template({'undefined': 'subprocess.getoutput'}) + with self.assertRaises(ValidationError) as cm: + template.clean() + self.assertIn('environment_params', cm.exception.message_dict) + + def test_unknown_key_rejected(self): + template = self._make_template({'extensions': ['os']}) + with self.assertRaises(ValidationError) as cm: + template.clean() + self.assertIn('environment_params', cm.exception.message_dict) + + def test_finalize_blocked_from_new_use(self): + template = self._make_template({'finalize': 'subprocess.getoutput'}) + with self.assertRaises(ValidationError) as cm: + template.clean() + self.assertIn('environment_params', cm.exception.message_dict) + + def test_empty_params_pass(self): + template = self._make_template({}) + template.clean() + + def test_none_params_pass(self): + template = self._make_template(None) + template.clean() + + def test_exporttemplate_clean_rejects_unknown_key(self): + """MRO smoke test: ExportTemplate.clean() reaches RenderTemplateMixin.clean().""" + obj = ExportTemplate( + name='test', + template_code='{{ "test" }}', + environment_params={'loader': 'some.loader'}, + ) + with self.assertRaises(ValidationError) as cm: + obj.clean() + self.assertIn('environment_params', cm.exception.message_dict) + + def test_configtemplate_clean_rejects_finalize(self): + """MRO smoke test: ConfigTemplate.clean() reaches RenderTemplateMixin.clean().""" + obj = ConfigTemplate( + name='test', + template_code='{{ "test" }}', + environment_params={'finalize': 'subprocess.getoutput'}, + ) + with self.assertRaises(ValidationError) as cm: + obj.clean() + self.assertIn('environment_params', cm.exception.message_dict) + + +class JinjaEnvironmentParamsFilterTest(TestCase): + """Tests for RenderTemplateMixin._filter_environment_params().""" + + def test_allowed_keys_pass_through(self): + params = {'trim_blocks': True, 'autoescape': False} + result = RenderTemplateMixin._filter_environment_params(params) + self.assertEqual(result, params) + + def test_unknown_keys_stripped(self): + params = {'extensions': ['os'], 'loader': 'x', 'trim_blocks': True} + result = RenderTemplateMixin._filter_environment_params(params) + self.assertEqual(result, {'trim_blocks': True}) + + def test_finalize_preserved_as_legacy(self): + params = {'finalize': 'some.module.func', 'trim_blocks': True} + result = RenderTemplateMixin._filter_environment_params(params) + self.assertEqual(result, params) + + def test_empty_params(self): + self.assertEqual(RenderTemplateMixin._filter_environment_params({}), {}) + + +class JinjaEnvironmentParamsResolveTest(TestCase): + """Tests for RenderTemplateMixin._resolve_mapped_params().""" + + def test_undefined_resolved_to_class(self): + params = {'undefined': 'jinja2.StrictUndefined'} + result = RenderTemplateMixin._resolve_mapped_params(params) + self.assertIs(result['undefined'], StrictUndefined) + + def test_unrecognized_undefined_value_passed_through(self): + params = {'undefined': 'not.a.real.class'} + result = RenderTemplateMixin._resolve_mapped_params(params) + self.assertEqual(result['undefined'], 'not.a.real.class') + + def test_scalar_params_passed_through(self): + params = {'trim_blocks': True, 'autoescape': False} + result = RenderTemplateMixin._resolve_mapped_params(params) + self.assertEqual(result, params) + + def test_empty_params(self): + self.assertEqual(RenderTemplateMixin._resolve_mapped_params({}), {}) + + +class JinjaEnvironmentParamsFinalizeTest(TestCase): + """Tests for RenderTemplateMixin._resolve_finalize() legacy carve-out.""" + + def test_finalize_string_resolved_via_import_string(self): + params = {'finalize': 'extras.tests.test_models.finalize_none_to_dash'} + result = RenderTemplateMixin._resolve_finalize(params) + self.assertIs(result['finalize'], finalize_none_to_dash) + + def test_finalize_non_string_passed_through(self): + params = {'finalize': 42} + result = RenderTemplateMixin._resolve_finalize(params) + self.assertEqual(result['finalize'], 42) + + def test_no_finalize_key_unchanged(self): + params = {'trim_blocks': True} + result = RenderTemplateMixin._resolve_finalize(params) + self.assertEqual(result, {'trim_blocks': True}) + + def test_invalid_import_path_raises_import_error(self): + params = {'finalize': 'nonexistent.module.func'} + with self.assertRaises(ImportError): + RenderTemplateMixin._resolve_finalize(params) + + def test_empty_params(self): + self.assertEqual(RenderTemplateMixin._resolve_finalize({}), {}) + + +class JinjaEnvironmentParamsIntegrationTest(TestCase): + """Integration tests for get_environment_params() end-to-end.""" + + def _make_template(self, environment_params): + return ConfigTemplate( + name='test', + template_code='{{ "test" }}', + environment_params=environment_params, + ) + + def test_full_pipeline_with_undefined(self): + template = self._make_template({'undefined': 'jinja2.StrictUndefined', 'trim_blocks': True}) + params = template.get_environment_params() + self.assertIs(params['undefined'], StrictUndefined) + self.assertIs(params['trim_blocks'], True) + + def test_full_pipeline_strips_unknown_and_resolves(self): + template = self._make_template({ + 'extensions': ['os'], + 'undefined': 'jinja2.DebugUndefined', + 'trim_blocks': True, + }) + params = template.get_environment_params() + self.assertNotIn('extensions', params) + self.assertIs(params['undefined'], DebugUndefined) + self.assertIs(params['trim_blocks'], True) + + def test_full_pipeline_finalize_resolves(self): + template = self._make_template({ + 'finalize': 'extras.tests.test_models.finalize_none_to_dash', + }) + params = template.get_environment_params() + self.assertIs(params['finalize'], finalize_none_to_dash) + + def test_does_not_mutate_stored_value(self): + template = self._make_template({'undefined': 'jinja2.StrictUndefined'}) + template.get_environment_params() + self.assertEqual(template.environment_params['undefined'], 'jinja2.StrictUndefined') + + def test_none_environment_params(self): + template = self._make_template(None) + self.assertEqual(template.get_environment_params(), {}) + + def test_empty_environment_params(self): + template = self._make_template({}) + self.assertEqual(template.get_environment_params(), {})