Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/django_nh3/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
class DjangoNh3AppConfig(AppConfig):
name = "django_nh3"
verbose_name = "django-nh3"

def ready(self):
from . import checks # noqa: F401
33 changes: 33 additions & 0 deletions src/django_nh3/checks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from django.conf import settings
from django.core.checks import Tags, Warning, register


@register(Tags.security)
def check_nh3_settings(app_configs, **kwargs):
"""
Inspects the NH3_ALLOWED_ATTRIBUTES setting to ensure that the 'style'
attribute is not allowed, as nh3 does not sanitize CSS content.
"""
errors = []
allowed_attributes = getattr(settings, "NH3_ALLOWED_ATTRIBUTES", {})

found_style = False
if isinstance(allowed_attributes, dict):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like this block has test coverage - at least according to the coverage report.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I have updated the tests to explicitly cover scenarios where NH3_ALLOWED_ATTRIBUTES is None or an empty dictionary. This ensures the isinstance check is fully exercised and should satisfy the coverage report.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent - thank you!

for _tag, attrs in allowed_attributes.items():
if "style" in attrs:
found_style = True
break

if found_style:
errors.append(
Warning(
"The 'style' attribute is allowed in NH3_ALLOWED_ATTRIBUTES.",
hint=(
"Allowing 'style' poses a security risk (XSS) because nh3 "
"does not sanitize CSS content. Ensure you strictly trust "
"the input if this attribute is required."
),
id="django_nh3.W001",
)
)
return errors
16 changes: 16 additions & 0 deletions tests/test_checks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from django.test import override_settings

from django_nh3.checks import check_nh3_settings


@override_settings(NH3_ALLOWED_ATTRIBUTES={"div": {"style"}})
def test_check_warns_about_style():
errors = check_nh3_settings(None)
assert len(errors) == 1
assert errors[0].id == "django_nh3.W001"


@override_settings(NH3_ALLOWED_ATTRIBUTES={"div": {"class", "href"}})
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is not valid for div elements. Consider using a valid attribute for div (e.g., id, data-*) or testing with an a tag if href is needed. While this doesn't affect the test's purpose, using semantically correct HTML attributes in tests improves clarity.

Suggested change
@override_settings(NH3_ALLOWED_ATTRIBUTES={"div": {"class", "href"}})
@override_settings(NH3_ALLOWED_ATTRIBUTES={"div": {"class", "id"}})

Copilot uses AI. Check for mistakes.
def test_check_is_silent_when_safe():
errors = check_nh3_settings(None)
assert len(errors) == 0
Loading