From 8e68d91124e7803d6ed666b7d3179d5a87373257 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Mon, 14 Sep 2026 11:52:53 -0500 Subject: [PATCH 1/2] Fixes #23167: Sanitize JSON schema property descriptions used as form help text `JSONSchemaProperty.to_form_field()` assigned a schema property's description directly to the form field's `help_text`, which `form_helpers/render_field.html` renders through the safe filter. Any element a module type profile schema author put in a property description reached the DOM intact, including ones outside `HTML_ALLOWED_TAGS`. Pass the description through `render_markdown()`, which applies the allowlist via `clean_html()` before `mark_safe()`. This matches how custom field descriptions are handled in `CustomField.to_form_field()`, so both kinds of user-defined attribute render their help text the same way. Descriptions stored since v4.3.0 are now interpreted as Markdown, so one beginning with "#" renders as a heading and one beginning with "1." renders as a list item. Custom field descriptions took the same change in #12685. Sanitizing inside `to_form_field()` rather than at the call site in `dcim/forms/model_forms.py` means plugins calling this utility are covered too. --- netbox/dcim/tests/test_forms.py | 41 +++++++ netbox/utilities/jsonschema.py | 3 +- netbox/utilities/tests/test_jsonschema.py | 137 ++++++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index cd1d29e7d..88b31aeb5 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -1,6 +1,7 @@ from unittest.mock import patch from django import forms +from django.template.loader import render_to_string from django.test import TestCase from dcim.choices import ( @@ -229,6 +230,46 @@ class ModuleTypeFormTestCase(TestCase): self.assertEqual(module_type.attribute_data, {'media': ['copper', 'qsfp28']}) +class ModuleTypeProfileDescriptionRenderingTestCase(TestCase): + """ + A profile schema property's description is rendered as the attribute field's help text via the + `safe` filter, so markup outside HTML_ALLOWED_TAGS must not reach the DOM as a live element. + Verified end to end because the sanitization and the `safe` filter that makes it necessary sit + in different layers. + """ + + @classmethod + def setUpTestData(cls): + cls.manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + cls.profile = ModuleTypeProfile.objects.create( + name='Disk', + schema={ + 'properties': { + 'capacity': { + 'type': 'integer', + 'title': 'Capacity (GB)', + 'description': 'Gross disk size ', + }, + }, + }, + ) + + def test_help_text_is_rendered_without_disallowed_markup(self): + form = ModuleTypeForm(data={ + 'manufacturer': self.manufacturer.pk, + 'model': 'Module Type 1', + 'profile': self.profile.pk, + 'attr_capacity': 500, + }) + rendered = render_to_string('form_helpers/render_field.html', {'field': form['attr_capacity']}) + + self.assertInHTML( + '' + '

Gross disk size

', + rendered, + ) + + class ModuleBayTemplateImportFormTestCase(TestCase): def test_module_bay_types_prefers_manufacturer_specific_match_over_global(self): diff --git a/netbox/utilities/jsonschema.py b/netbox/utilities/jsonschema.py index a25e24f21..418eb300e 100644 --- a/netbox/utilities/jsonschema.py +++ b/netbox/utilities/jsonschema.py @@ -11,6 +11,7 @@ from jsonschema.exceptions import SchemaError from jsonschema.validators import validator_for from utilities.string import title +from utilities.templatetags.builtins.filters import render_markdown from utilities.validators import MultipleOfValidator __all__ = ( @@ -88,7 +89,7 @@ class JSONSchemaProperty: """ field_kwargs = { 'label': self.title or title(name), - 'help_text': self.description, + 'help_text': render_markdown(self.description), 'required': required, 'initial': self.default, } diff --git a/netbox/utilities/tests/test_jsonschema.py b/netbox/utilities/tests/test_jsonschema.py index 2526bc84d..b6d421e47 100644 --- a/netbox/utilities/tests/test_jsonschema.py +++ b/netbox/utilities/tests/test_jsonschema.py @@ -44,3 +44,140 @@ class JSONSchemaPropertyTestCase(TestCase): self.assertIsInstance(field, SimpleArrayField) self.assertIsInstance(field.base_field, forms.CharField) self.assertEqual(field.clean('ge-0/0/0,ge-0/0/1'), ['ge-0/0/0', 'ge-0/0/1']) + + +class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): + """ + A property's description becomes the form field's help_text, which is rendered through the + `safe` filter in form_helpers/render_field.html. It is passed through render_markdown(), which + applies the HTML_ALLOWED_TAGS allowlist, matching the custom field path in + extras.models.customfields.CustomField.to_form_field(). + + Each test asserts the complete help text, so a payload that survived anywhere in it would fail + the comparison. Asserting only on the absence of a substring would not, because escaping and + stripping both leave a payload's text behind as character data. + """ + + def test_disallowed_element_is_stripped(self): + prop = JSONSchemaProperty( + type='integer', + title='Capacity (GB)', + description='Gross disk size ', + ) + + field = prop.to_form_field('capacity') + + self.assertInHTML( + '

Gross disk size

', + field.help_text, + ) + + def test_script_element_is_stripped(self): + prop = JSONSchemaProperty( + type='string', + description='Vendor code ', + ) + + field = prop.to_form_field('vendor_code') + + self.assertInHTML( + '

Vendor code

', + field.help_text, + ) + + def test_event_handler_attribute_is_stripped(self): + """An allowed tag carrying a disallowed attribute keeps the tag but loses the attribute.""" + prop = JSONSchemaProperty( + type='string', + description='Vendor code', + ) + + field = prop.to_form_field('vendor_code') + + self.assertInHTML( + '

Vendor code

', + field.help_text, + ) + + def test_javascript_uri_is_stripped(self): + prop = JSONSchemaProperty( + type='string', + description='Vendor code', + ) + + field = prop.to_form_field('vendor_code') + + self.assertInHTML( + '
' + '

Vendor code

', + field.help_text, + ) + + def test_disallowed_element_is_stripped_from_mixed_markup(self): + """A disallowed element is dropped while its allowed siblings are kept.""" + prop = JSONSchemaProperty( + type='string', + description='Vendor code ', + ) + + field = prop.to_form_field('vendor_code') + + self.assertInHTML( + '

Vendor code

', + field.help_text, + ) + + def test_allowed_markup_is_preserved(self): + """ + render_markdown() applies the HTML_ALLOWED_TAGS allowlist, so markup inside it survives. + This is the behavior that keeps schema descriptions consistent with custom field + descriptions. + """ + prop = JSONSchemaProperty( + type='integer', + description='Gross disk size in GB', + ) + + field = prop.to_form_field('capacity') + + self.assertInHTML( + '

Gross disk size in GB

', + field.help_text, + ) + + def test_markdown_is_rendered(self): + """Descriptions are interpreted as Markdown, matching the custom field path.""" + prop = JSONSchemaProperty( + type='integer', + description='Gross disk size in **GB**', + ) + + field = prop.to_form_field('capacity') + + self.assertInHTML( + '
' + '

Gross disk size in GB

', + field.help_text, + ) + + def test_description_text_is_retained(self): + """Sanitization must not discard the author's actual help text.""" + prop = JSONSchemaProperty( + type='string', + description='Gross disk size in gigabytes', + ) + + field = prop.to_form_field('capacity') + + self.assertInHTML( + '

Gross disk size in gigabytes

', + field.help_text, + ) + + def test_absent_description_yields_no_help_text(self): + """A property without a description must not gain help text from the sanitizer.""" + prop = JSONSchemaProperty(type='string', title='Vendor Code') + + field = prop.to_form_field('vendor_code') + + self.assertFalse(field.help_text) From f96e6f86b9cfdd6f2fbe339a5dfe8b9283bae3ca Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Mon, 14 Sep 2026 12:24:02 -0500 Subject: [PATCH 2/2] Assert the full help text rather than a fragment of it The sanitization tests used `assertInHTML`, which checks containment. Each needle happened to span the whole output, so they passed, but a payload surviving outside the `rendered-markdown` div would not have failed them. `assertHTMLEqual` is what the docstring already claimed these tests did. --- netbox/utilities/tests/test_jsonschema.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/netbox/utilities/tests/test_jsonschema.py b/netbox/utilities/tests/test_jsonschema.py index b6d421e47..50210fc48 100644 --- a/netbox/utilities/tests/test_jsonschema.py +++ b/netbox/utilities/tests/test_jsonschema.py @@ -53,9 +53,9 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): applies the HTML_ALLOWED_TAGS allowlist, matching the custom field path in extras.models.customfields.CustomField.to_form_field(). - Each test asserts the complete help text, so a payload that survived anywhere in it would fail - the comparison. Asserting only on the absence of a substring would not, because escaping and - stripping both leave a payload's text behind as character data. + Each test compares the entire help text, so a payload surviving anywhere in it fails the + assertion. Asserting only on the absence of a substring would not, because stripping an + element leaves its text behind as character data. """ def test_disallowed_element_is_stripped(self): @@ -67,7 +67,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('capacity') - self.assertInHTML( + self.assertHTMLEqual( '

Gross disk size

', field.help_text, ) @@ -80,7 +80,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('vendor_code') - self.assertInHTML( + self.assertHTMLEqual( '

Vendor code

', field.help_text, ) @@ -94,7 +94,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('vendor_code') - self.assertInHTML( + self.assertHTMLEqual( '

Vendor code

', field.help_text, ) @@ -107,7 +107,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('vendor_code') - self.assertInHTML( + self.assertHTMLEqual( '', field.help_text, @@ -122,7 +122,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('vendor_code') - self.assertInHTML( + self.assertHTMLEqual( '

Vendor code

', field.help_text, ) @@ -140,7 +140,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('capacity') - self.assertInHTML( + self.assertHTMLEqual( '

Gross disk size in GB

', field.help_text, ) @@ -154,7 +154,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('capacity') - self.assertInHTML( + self.assertHTMLEqual( '
' '

Gross disk size in GB

', field.help_text, @@ -169,7 +169,7 @@ class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): field = prop.to_form_field('capacity') - self.assertInHTML( + self.assertHTMLEqual( '

Gross disk size in gigabytes

', field.help_text, )