Skip to content

Fix DjangoGetter invoking callables with alters_data during serialization#1696

Open
chidoziemanagwu wants to merge 1 commit intovitalik:masterfrom
chidoziemanagwu:fix/1686-prevent-alters-data-callable-invocation
Open

Fix DjangoGetter invoking callables with alters_data during serialization#1696
chidoziemanagwu wants to merge 1 commit intovitalik:masterfrom
chidoziemanagwu:fix/1686-prevent-alters-data-callable-invocation

Conversation

@chidoziemanagwu
Copy link

@chidoziemanagwu chidoziemanagwu commented Mar 14, 2026

Closes #1686 @stephane @robhudson @quique @vitalik

Context

When integrating Pydantic schemas with Django models, security and data sovereignty are paramount. Issue #1686 highlighted a critical vulnerability where DjangoGetter._convert_result unconditionally executed any callable attribute that matched a schema field name. This meant that if a schema included a field named delete or save, the corresponding Django model method would be invoked during serialization, potentially leading to unintended and silent data destruction.

The Approach

Rather than implementing a hardcoded blocklist of "dangerous" method names—which would be brittle and incomplete—this PR leverages Django's built-in alters_data safety mechanism.

By adding a single check (and not getattr(result, "alters_data", False)), we align DjangoGetter with the exact same safety paradigm used by Django's template engine. This surgically prevents destructive operations (like delete(), save(), and full_clean()) from executing during read-only serialization, while fully preserving backward compatibility for legitimate read-only method aliases (e.g., Field(alias="get_boss_title")).

Impact

This patch secures the serialization pipeline against accidental data loss, reinforcing the framework's architecture and reliability for enterprise use cases where data integrity is non-negotiable.


Visual Evidence & Proof

Code Changes:

--- a/ninja/schema.py
+++ b/ninja/schema.py
@@ -96,7 +96,7 @@ class DjangoGetter:
         elif isinstance(result, getattr(QuerySet, "__origin__", QuerySet)):
             return list(result)
 
-        if callable(result):
+        if callable(result) and not getattr(result, "alters_data", False):
             return result()
# Added 2 new tests to tests/test_schema.py:
def test_callable_with_alters_data_not_called():
    """Callables with alters_data=True must not be invoked (issue #1686)."""
    class FakeModel:
        name = "Widget"
        def delete(self):
            raise AssertionError("delete() should not be called during serialization")
        delete.alters_data = True
    # ... assert SafeSchema.from_orm(FakeModel()).dict() == {"name": "Widget"}

def test_safe_callable_still_called():
    """Callables without alters_data are still called as before."""
    # ... proves existing functionality is unbroken

Before/After Behavior:

  • Before: ItemSchema.from_orm(item) where schema contains delete: str executes Item.objects.all().delete().
  • After: Safely ignores the alters_data callable, preserving database integrity.

Test Coverage (100% Passing):

django-ninja> pytest tests/test_schema.py -v
============================= test session starts =============================
platform win32 -- Python 3.13.0, pytest-9.0.2, pluggy-1.6.0
django: version: 6.0.3
plugins: anyio-4.12.1, asyncio-1.3.0, django-4.12.0

tests\test_schema.py::test_callable_with_alters_data_not_called PASSED   [ 91%]
tests\test_schema.py::test_safe_callable_still_called PASSED             [100%]

======================== 11 passed, 1 skipped in 0.62s ========================

Full Suite Verification:

django-ninja> pytest tests/ -v --tb=short
...
================ 768 passed, 1 skipped, 55 warnings in 10.45s =================

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] DjangoGetter._convert_result invokes callable model attributes during serialization (can trigger delete())

1 participant