Add system check warning for unsafe 'style' attribute

[shayanTaki]

This PR adds a Django System Check (W001) to warn developers if style is included in NH3_ALLOWED_ATTRIBUTES.

Rationale

By default, nh3 does not parse or sanitize the content of the style attribute. Enabling it allows potential XSS vectors via CSS (e.g., background-image: url('javascript:...')).

While some use cases might require inline styles, users should be explicitly warned about the security implications if they override the defaults.

Changes

• Added check_nh3_settings in src/django_nh3/checks.py.
• Registered the check in src/django_nh3/apps.py.
• Added unit tests covering both safe and unsafe configurations in tests/test_checks.py.

[marksweb]

@shayanTaki excellent - thanks!

Could you just take a look at the mypy issues please? Looks like it needs some type hints on the check.

[Copilot]

Pull request overview

This PR adds a Django system check (W001) to warn developers when the style attribute is included in NH3_ALLOWED_ATTRIBUTES, as nh3 does not sanitize CSS content, which poses an XSS security risk.

Key changes:

• New system check function that inspects NH3_ALLOWED_ATTRIBUTES for unsafe style attribute usage
• Registration of the check in the app's ready() method
• Comprehensive test coverage for both warning and safe configuration scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/django_nh3/checks.py	Implements the security check function that detects style attribute in allowed attributes configuration
src/django_nh3/apps.py	Imports the checks module in the app's ready() method to register the system check
tests/test_checks.py	Adds test cases validating the check triggers warnings for unsafe configs and remains silent for safe ones

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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"}})

[shayanTaki]

Thanks for the review!

I have applied the fixes:

Added full type hints to checks.py to satisfy mypy.
Updated the test case to use semantically valid HTML attributes (id instead of href for div), as suggested by Copilot.
Ready for another look.

[Pantzan]

Currently Django-Nh3 does not allow filter_style_properties to restrict specific style rules to except from sanitation thus accepts all rules and potentially malicious tags can be inserted. Instead of discouraging users to use the style argument, which is essential in applications using rich document forms, It is better to implement the allow filter_style_properties and then show the warning if this style is set and allow filter_style_properties is not or set empty.
On the other hand, warning messages on local development can be ignored. Warnings are also not shown if the app is run using a web server, for example Gunicorn. I see more beneficial a logger.warning so it is visible in all runtime occasions.

I am working in a PR for the allow filter_style_properties option

[shayanTaki]

Thank you for the comprehensive explanation, @Pantzan. I agree with your overall assessment, and introducing an allowlist-based mechanism for handling style appears to be a more appropriate and sustainable approach than discouraging its usage—particularly for applications where rich-text content is a core requirement.

That said, a secure implementation should not be limited to filtering CSS properties alone. Proper validation and sanitization at the value level is equally important, as constructs such as url(), expression, or similar patterns may otherwise introduce new attack vectors if not explicitly handled.

It is also important that the configuration and exposure of this functionality remain aligned with the existing project structure and public API, avoiding breaking changes and preserving predictable default behavior.

Regarding warnings, replacing environment-limited warnings with a logging-based approach (e.g., logger.warning) would provide better visibility into potentially unsafe configurations across all runtime environments, including production deployments.

I am looking forward to reviewing your Pull Request, and I would be glad to assist with review or implementation if that would be helpful.

[marksweb]

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

[shayanTaki]

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.

[marksweb]

Excellent - thank you!