Skip to content

Commit a286dcb

Browse files
Maffoochclaude
andauthored
fix(auditlog): make pghistory context JSON-safe before Celery dispatch (#15204)
get_serializable_pghistory_context returned ctx.metadata.copy() verbatim, so a non-JSON-serializable value in the active context (e.g. a user set to a User object instead of its pk) made kombu raise EncodeError and abort task dispatch. Coerce metadata to JSON-safe values (model instances -> pk, recursing into dict/list/tuple) so a stray value can never crash dispatch. Fixes DJANGO-42W8 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent af3716f commit a286dcb

2 files changed

Lines changed: 89 additions & 2 deletions

File tree

dojo/auditlog/helpers.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,18 +166,43 @@ def process_events_for_display(events):
166166
# ---------------------------------------------------------------------------
167167

168168

169+
def _json_safe_metadata_value(value):
170+
"""
171+
Coerce a pghistory context metadata value into something the JSON task
172+
serializer can encode.
173+
174+
The context is injected into Celery task kwargs as ``_pgh_context`` and
175+
serialized by kombu's JSON encoder at dispatch time. A stray model instance
176+
in the metadata (e.g. a ``user`` set to a ``User`` object instead of its pk)
177+
raises ``EncodeError: Object of type User is not JSON serializable`` and
178+
aborts the whole task dispatch. Reduce model instances to their pk and
179+
recurse into dict/list/tuple so nested values are covered too.
180+
"""
181+
from django.db.models import Model # noqa: PLC0415 -- avoid import cycle at app load
182+
183+
if isinstance(value, Model):
184+
return value.pk
185+
if isinstance(value, dict):
186+
return {k: _json_safe_metadata_value(v) for k, v in value.items()}
187+
if isinstance(value, (list, tuple)):
188+
return [_json_safe_metadata_value(v) for v in value]
189+
return value
190+
191+
169192
def get_serializable_pghistory_context():
170193
"""
171194
Capture the current pghistory context for passing to Celery tasks.
172195
173196
Returns a JSON-serializable dict with context id and metadata,
174-
or None if no context is active.
197+
or None if no context is active. Metadata values are coerced to
198+
JSON-safe types (model instances -> pk) so a non-serializable value in
199+
the active context can never crash Celery task dispatch.
175200
"""
176201
if hasattr(pghistory_runtime._tracker, "value"):
177202
ctx = pghistory_runtime._tracker.value
178203
return {
179204
"id": str(ctx.id),
180-
"metadata": ctx.metadata.copy(),
205+
"metadata": {k: _json_safe_metadata_value(v) for k, v in ctx.metadata.items()},
181206
}
182207
return None
183208

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
"""
2+
Regression: dispatching a Celery task while the active pghistory context holds a
3+
non-JSON-serializable metadata value (e.g. a ``user`` set to a User *object*
4+
instead of its pk) crashed dispatch with
5+
``EncodeError: Object of type User is not JSON serializable``.
6+
7+
``get_serializable_pghistory_context`` must coerce metadata to JSON-safe values
8+
so the injected ``_pgh_context`` can always be serialized by kombu.
9+
"""
10+
11+
import pghistory
12+
from kombu.utils.json import dumps as kombu_json_dumps
13+
14+
from dojo.auditlog.helpers import get_serializable_pghistory_context
15+
from dojo.models import Dojo_User
16+
17+
from .dojo_test_case import DojoTestCase
18+
19+
20+
class TestPghistoryContextSerialization(DojoTestCase):
21+
22+
def setUp(self):
23+
super().setUp()
24+
self.user = Dojo_User.objects.create(username="pgh-ctx-serialize-user")
25+
26+
def test_model_instance_in_context_metadata_is_coerced_to_pk(self):
27+
# A model instance in the context metadata must be reduced to its pk.
28+
with pghistory.context(user=self.user, url="/api/v2/products/1/"):
29+
ctx = get_serializable_pghistory_context()
30+
31+
self.assertEqual(
32+
ctx["metadata"]["user"], self.user.pk,
33+
msg=f"expected user coerced to pk={self.user.pk}, got {ctx['metadata']['user']!r}",
34+
)
35+
36+
def test_context_with_model_instance_is_kombu_serializable(self):
37+
# This is the exact operation Celery dispatch performs on the injected
38+
# `_pgh_context`; it raised EncodeError before the fix.
39+
with pghistory.context(user=self.user, url="/api/v2/products/1/"):
40+
ctx = get_serializable_pghistory_context()
41+
42+
payload = {"kwargs": {"_pgh_context": ctx}}
43+
# Must not raise.
44+
kombu_json_dumps(payload)
45+
46+
def test_nested_model_instance_is_coerced(self):
47+
with pghistory.context(actors=[self.user], details={"owner": self.user}):
48+
ctx = get_serializable_pghistory_context()
49+
50+
self.assertEqual(ctx["metadata"]["actors"], [self.user.pk])
51+
self.assertEqual(ctx["metadata"]["details"], {"owner": self.user.pk})
52+
kombu_json_dumps({"kwargs": {"_pgh_context": ctx}})
53+
54+
def test_plain_metadata_is_unchanged(self):
55+
# Control: already-serializable metadata (pk int, strings) passes through.
56+
with pghistory.context(user=self.user.pk, url="/x", remote_addr="1.2.3.4"):
57+
ctx = get_serializable_pghistory_context()
58+
59+
self.assertEqual(ctx["metadata"]["user"], self.user.pk)
60+
self.assertEqual(ctx["metadata"]["url"], "/x")
61+
self.assertEqual(ctx["metadata"]["remote_addr"], "1.2.3.4")
62+
kombu_json_dumps({"kwargs": {"_pgh_context": ctx}})

0 commit comments

Comments
 (0)