From 10981c85560e4a55a4253f02841c21dbfdc0ad27 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 29 Apr 2026 20:54:45 -0600 Subject: [PATCH 1/4] security: remove pickle from forms and Celery serializer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pickle deserialization on attacker-controllable bytes is arbitrary code execution. Two sites used pickle: - The survey choice-question form pickled/unpickled a list of strings through MultiExampleField.compress / MultiWidgetBasic.decompress and pickle.loads in survey/views.py. Switched to json — the data is just a list of up to 6 strings. - Celery defaulted to the pickle serializer with CELERY_ACCEPT_CONTENT including pickle/yaml/msgpack so dispatch sites could pass Django model instances and a Dojo_User on the wire. Made every task argument JSON-serializable: async_delete_task takes (model_label, pk) and refetches; SLA recompute takes (sla_config_id, product_ids); per-channel notification sends run inline inside the surrounding async_create_notification task instead of dispatching an inner Celery task with model instances; user context is injected as async_user_id and resolved in the worker. Flipped DD_CELERY_TASK_SERIALIZER default to json, tightened CELERY_ACCEPT_CONTENT to ["json"], and added CELERY_RESULT_SERIALIZER = "json". Added unittests/test_no_pickle.py as a regression net (asserts no pickle imports in dojo/ and that the Celery serializer settings stay JSON-only) and unittests/test_survey_forms.py for the widget round-trip. Operator note: workers running this version reject in-flight pickled messages with ContentDisallowed. Drain the broker (\`celery -A dojo purge -f\`) or scale workers to zero before deploy. Co-Authored-By: Claude Opus 4.7 (1M context) --- dojo/celery.py | 26 ++++++++++++------------ dojo/celery_dispatch.py | 7 ++++--- dojo/decorators.py | 2 +- dojo/forms.py | 6 +++--- dojo/models.py | 13 ++++++++++-- dojo/notifications/helper.py | 34 +++++++++----------------------- dojo/settings/settings.dist.py | 6 +++--- dojo/sla_config/helpers.py | 7 ++++++- dojo/survey/views.py | 4 ++-- dojo/tasks.py | 4 +++- dojo/utils.py | 15 ++++++++++++-- unittests/test_no_pickle.py | 35 +++++++++++++++++++++++++++++++++ unittests/test_survey_forms.py | 36 ++++++++++++++++++++++++++++++++++ 13 files changed, 140 insertions(+), 55 deletions(-) create mode 100644 unittests/test_no_pickle.py create mode 100644 unittests/test_survey_forms.py diff --git a/dojo/celery.py b/dojo/celery.py index 336fd420aca..83d2e5d1e08 100644 --- a/dojo/celery.py +++ b/dojo/celery.py @@ -28,23 +28,26 @@ def __call__(self, *args, **kwargs): """ Restore user context in the celery worker via crum.impersonate. - The apply_async method injects ``async_user`` into kwargs when a task - is dispatched. Here we pop it and set it as the current user in - thread-local storage so that all downstream code — including nested - dojo_dispatch_task calls — sees the correct user via - get_current_user(). + The apply_async method injects ``async_user_id`` into kwargs when a task + is dispatched. Here we pop it, resolve to a user instance, and set it + as the current user in thread-local storage so that all downstream + code — including nested dojo_dispatch_task calls — sees the correct + user via get_current_user(). - When a task is called directly (not via apply_async), async_user is + When a task is called directly (not via apply_async), async_user_id is not in kwargs. In that case we leave the existing crum context intact so that callers who already set a user (e.g. via crum.impersonate in tests or request middleware) are not disrupted. """ - if "async_user" not in kwargs: + if "async_user_id" not in kwargs: return super().__call__(*args, **kwargs) import crum # noqa: PLC0415 - user = kwargs.pop("async_user") + from dojo.models import Dojo_User # noqa: PLC0415 circular import + + user_id = kwargs.pop("async_user_id") + user = Dojo_User.objects.filter(pk=user_id).first() if user_id else None with crum.impersonate(user): return super().__call__(*args, **kwargs) @@ -59,8 +62,9 @@ def apply_async(self, args=None, kwargs=None, **options): # Inject user context for Dojo tasks only. Celery built-in tasks (e.g. # celery.backend_cleanup) do not accept custom kwargs. task_name = self.name or "" - if not task_name.startswith("celery.") and "async_user" not in kwargs: - kwargs["async_user"] = get_current_user() + if not task_name.startswith("celery.") and "async_user_id" not in kwargs: + user = get_current_user() + kwargs["async_user_id"] = user.id if user else None # Control flag used for sync/async decision; never pass into the task itself kwargs.pop("sync", None) @@ -135,8 +139,6 @@ def __call__(self, *args, **kwargs): app = Celery("dojo", task_cls=PgHistoryTask) -# Using a string here means the worker will not have to -# pickle the object when using Windows. app.config_from_object("django.conf:settings", namespace="CELERY") app.autodiscover_tasks(lambda: settings.INSTALLED_APPS) diff --git a/dojo/celery_dispatch.py b/dojo/celery_dispatch.py index c835717efe2..fde94336ec4 100644 --- a/dojo/celery_dispatch.py +++ b/dojo/celery_dispatch.py @@ -18,10 +18,11 @@ def apply_async(self, args: Any | None = None, kwargs: Any | None = None, **opti def _inject_async_user(kwargs: Mapping[str, Any] | None) -> dict[str, Any]: result: dict[str, Any] = dict(kwargs or {}) - if "async_user" not in result: + if "async_user_id" not in result: from dojo.utils import get_current_user # noqa: PLC0415 circular import - result["async_user"] = get_current_user() + user = get_current_user() + result["async_user_id"] = user.id if user else None return result @@ -58,7 +59,7 @@ def dojo_dispatch_task(task_or_sig: _SupportsSi | _SupportsApplyAsync | Signatur """ Dispatch a task/signature using DefectDojo semantics. - - Inject `async_user` if missing. + - Inject `async_user_id` if missing. - Capture and inject pghistory context if available. - Respect `sync=True` (foreground execution) and user `block_execution`. - Support `countdown=` for async dispatch. diff --git a/dojo/decorators.py b/dojo/decorators.py index f3045402079..bc3b898a3e1 100644 --- a/dojo/decorators.py +++ b/dojo/decorators.py @@ -88,7 +88,7 @@ def __wrapper__(*args, **kwargs): from dojo.utils import get_current_user # noqa: PLC0415 circular import user = get_current_user() - kwargs["async_user"] = user + kwargs["async_user_id"] = user.id if user else None # Capture pghistory context to pass to Celery worker # The PgHistoryTask base class will apply this context in the worker diff --git a/dojo/forms.py b/dojo/forms.py index b9f924754ed..21267dc0ada 100644 --- a/dojo/forms.py +++ b/dojo/forms.py @@ -1,5 +1,5 @@ +import json import logging -import pickle import re import warnings from datetime import date, datetime @@ -3522,7 +3522,7 @@ def __init__(self, attrs=None): def decompress(self, value): if value: - return pickle.loads(value) + return json.loads(value) return [None, None, None, None, None, None] def format_output(self, rendered_widgets): @@ -3542,7 +3542,7 @@ def __init__(self, *args, **kwargs): super().__init__(list_fields, *args, **kwargs) def compress(self, values): - return pickle.dumps(values) + return json.dumps(values) class CreateChoiceQuestionForm(forms.Form): diff --git a/dojo/models.py b/dojo/models.py index d4dbae62afe..5d7a4bbfa2d 100644 --- a/dojo/models.py +++ b/dojo/models.py @@ -1094,7 +1094,12 @@ def save(self, *args, **kwargs): from dojo.sla_config.helpers import async_update_sla_expiration_dates_sla_config_sync # noqa: I001, PLC0415 circular import from dojo.celery_dispatch import dojo_dispatch_task # noqa: PLC0415 circular import - dojo_dispatch_task(async_update_sla_expiration_dates_sla_config_sync, self, products, severities=severities) + dojo_dispatch_task( + async_update_sla_expiration_dates_sla_config_sync, + self.id, + list(products.values_list("id", flat=True)), + severities=severities, + ) def clean(self): sla_days = [self.critical, self.high, self.medium, self.low] @@ -1256,7 +1261,11 @@ def save(self, *args, **kwargs): from dojo.sla_config.helpers import async_update_sla_expiration_dates_sla_config_sync # noqa: I001, PLC0415 circular import from dojo.celery_dispatch import dojo_dispatch_task # noqa: PLC0415 circular import - dojo_dispatch_task(async_update_sla_expiration_dates_sla_config_sync, sla_config, Product.objects.filter(id=self.id)) + dojo_dispatch_task( + async_update_sla_expiration_dates_sla_config_sync, + sla_config.id, + [self.id], + ) def get_absolute_url(self): return reverse("view_product", args=[str(self.id)]) diff --git a/dojo/notifications/helper.py b/dojo/notifications/helper.py index c7161b81227..60911b22019 100644 --- a/dojo/notifications/helper.py +++ b/dojo/notifications/helper.py @@ -18,7 +18,6 @@ from dojo import __version__ as dd_version from dojo.authorization.roles_permissions import Permissions from dojo.celery import app -from dojo.celery_dispatch import dojo_dispatch_task from dojo.decorators import we_want_async from dojo.labels import get_labels from dojo.models import ( @@ -833,19 +832,19 @@ def _process_notifications( # Some errors should not be pushed to all channels, only to alerts. # For example reasons why JIRA Issues: https://github.com/DefectDojo/django-DefectDojo/issues/11575 + # Per-channel sends run synchronously inside the surrounding async_create_notification + # task body. Dispatching inner Celery tasks would require JSON-serializable kwargs, but + # callers pass model instances (finding/test/engagement/product/...) and refetching every + # one of them per channel would multiply DB queries; running synchronously avoids both. if not alert_only: + user_id = getattr(notifications.user, "id", None) if self.system_settings.enable_slack_notifications and "slack" in getattr( notifications, event, notifications.other, ): logger.debug("Sending Slack Notification") - dojo_dispatch_task( - send_slack_notification, - event, - user_id=getattr(notifications.user, "id", None), - **kwargs, - ) + send_slack_notification.run(event, user_id=user_id, **kwargs) if self.system_settings.enable_msteams_notifications and "msteams" in getattr( notifications, @@ -853,12 +852,7 @@ def _process_notifications( notifications.other, ): logger.debug("Sending MSTeams Notification") - dojo_dispatch_task( - send_msteams_notification, - event, - user_id=getattr(notifications.user, "id", None), - **kwargs, - ) + send_msteams_notification.run(event, user_id=user_id, **kwargs) if self.system_settings.enable_mail_notifications and "mail" in getattr( notifications, @@ -866,12 +860,7 @@ def _process_notifications( notifications.other, ): logger.debug("Sending Mail Notification") - dojo_dispatch_task( - send_mail_notification, - event, - user_id=getattr(notifications.user, "id", None), - **kwargs, - ) + send_mail_notification.run(event, user_id=user_id, **kwargs) if self.system_settings.enable_webhooks_notifications and "webhooks" in getattr( notifications, @@ -879,12 +868,7 @@ def _process_notifications( notifications.other, ): logger.debug("Sending Webhooks Notification") - dojo_dispatch_task( - send_webhooks_notification, - event, - user_id=getattr(notifications.user, "id", None), - **kwargs, - ) + send_webhooks_notification.run(event, user_id=user_id, **kwargs) @app.task diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index c37b787925e..f5fe6acb82e 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -88,7 +88,7 @@ DD_CELERY_RESULT_BACKEND=(str, "django-db"), DD_CELERY_RESULT_EXPIRES=(int, 86400), DD_CELERY_BEAT_SCHEDULE_FILENAME=(str, root("dojo.celery.beat.db")), - DD_CELERY_TASK_SERIALIZER=(str, "pickle"), + DD_CELERY_TASK_SERIALIZER=(str, "json"), DD_CELERY_LOG_LEVEL=(str, "INFO"), # Hard ceiling on task runtime. When reached, the worker process is sent SIGKILL — no cleanup # code runs. Always set higher than DD_CELERY_TASK_SOFT_TIME_LIMIT. (0 = disabled, no limit) @@ -1267,8 +1267,9 @@ def saml2_attrib_map_format(din): CELERY_TIMEZONE = TIME_ZONE CELERY_RESULT_EXPIRES = env("DD_CELERY_RESULT_EXPIRES") CELERY_BEAT_SCHEDULE_FILENAME = env("DD_CELERY_BEAT_SCHEDULE_FILENAME") -CELERY_ACCEPT_CONTENT = ["pickle", "json", "msgpack", "yaml"] +CELERY_ACCEPT_CONTENT = ["json"] CELERY_TASK_SERIALIZER = env("DD_CELERY_TASK_SERIALIZER") +CELERY_RESULT_SERIALIZER = "json" CELERY_LOG_LEVEL = env("DD_CELERY_LOG_LEVEL") if env("DD_CELERY_TASK_TIME_LIMIT") > 0: @@ -1292,7 +1293,6 @@ def saml2_attrib_map_format(din): "add-alerts": { "task": "dojo.tasks.add_alerts", "schedule": timedelta(hours=1), - "args": [timedelta(hours=1)], "options": { "expires": int(60 * 60 * 1 * 1.2), # If a task is not executed within 72 minutes, it should be dropped from the queue. Two more tasks should be scheduled in the meantime. }, diff --git a/dojo/sla_config/helpers.py b/dojo/sla_config/helpers.py index 045456f38d7..6cabe47c29d 100644 --- a/dojo/sla_config/helpers.py +++ b/dojo/sla_config/helpers.py @@ -8,7 +8,12 @@ @app.task -def async_update_sla_expiration_dates_sla_config_sync(sla_config: SLA_Configuration, products: list[Product], *args, severities: list[str] | None = None, **kwargs): +def async_update_sla_expiration_dates_sla_config_sync(sla_config_id: int, product_ids: list[int], *args, severities: list[str] | None = None, **kwargs): + sla_config = SLA_Configuration.objects.filter(pk=sla_config_id).first() + if sla_config is None: + logger.info("SLA_Configuration with id %s no longer exists, skipping update", sla_config_id) + return + products = Product.objects.filter(pk__in=product_ids) if method := get_custom_method("FINDING_SLA_EXPIRATION_CALCULATION_METHOD"): method(sla_config, products, severities=severities) else: diff --git a/dojo/survey/views.py b/dojo/survey/views.py index 60c8cd58a15..d17e25e6d2c 100644 --- a/dojo/survey/views.py +++ b/dojo/survey/views.py @@ -1,4 +1,4 @@ -import pickle +import json from datetime import date, timedelta from django.contrib import messages @@ -490,7 +490,7 @@ def create_question(request): order=form.cleaned_data["order"], text=form.cleaned_data["text"], multichoice=choiceQuestionFrom.cleaned_data["multichoice"]) - choices_to_process = pickle.loads(choiceQuestionFrom.cleaned_data["answer_choices"]) + choices_to_process = json.loads(choiceQuestionFrom.cleaned_data["answer_choices"]) for c in choices_to_process: if c is not None and len(c) > 0: diff --git a/dojo/tasks.py b/dojo/tasks.py index dbc2135e560..9427de8bb92 100644 --- a/dojo/tasks.py +++ b/dojo/tasks.py @@ -32,7 +32,9 @@ def log_generic_alert(source, title, description): @app.task(bind=True) -def add_alerts(self, runinterval, *args, **kwargs): +def add_alerts(self, *args, **kwargs): + # Run interval matches the beat schedule for this task (see CELERY_BEAT_SCHEDULE). + runinterval = timedelta(hours=1) now = timezone.now() upcoming_engagements = Engagement.objects.filter(target_start__gt=now + timedelta(days=3), target_start__lt=now + timedelta(days=3) + runinterval).order_by("target_start") diff --git a/dojo/utils.py b/dojo/utils.py index a056bc4fe53..9d0e75e8bf4 100644 --- a/dojo/utils.py +++ b/dojo/utils.py @@ -1986,10 +1986,13 @@ def _get_object_name(obj): @app.task -def async_delete_task(obj, **kwargs): +def async_delete_task(model_label, pk, **kwargs): """ Delete an object and all its related objects using the SQL cascade walker. + Takes ``(model_label, pk)`` (e.g. ``("dojo.product", 42)``) so the task + arguments are JSON-serializable. The instance is refetched in the worker. + Handles Python-level concerns (duplicates, integrators, M2M, file cleanup, product grading) explicitly, then uses cascade_delete_related_objects() for efficient bottom-up SQL deletion of all FK-related tables. The top-level @@ -1998,12 +2001,20 @@ def async_delete_task(obj, **kwargs): Accepts **kwargs for _pgh_context injected by dojo_dispatch_task. Uses PgHistoryTask base class (default) to preserve pghistory context for audit trail. """ + from django.apps import apps # noqa: PLC0415 + from dojo.finding.helper import ( # noqa: PLC0415 circular import bulk_delete_findings, prepare_duplicates_for_delete, ) from dojo.utils_cascade_delete import cascade_delete_related_objects # noqa: PLC0415 circular import + Model = apps.get_model(model_label) + obj = Model.objects.filter(pk=pk).first() + if obj is None: + logger.info("ASYNC_DELETE: %s pk=%s already gone, nothing to do", model_label, pk) + return + logger.debug("ASYNC_DELETE: Deleting %s: %s", _get_object_name(obj), obj) if not isinstance(obj, ASYNC_DELETE_SUPPORTED_TYPES): logger.debug("ASYNC_DELETE: %s async delete not supported. Deleting normally: %s", _get_object_name(obj), obj) @@ -2105,7 +2116,7 @@ def delete(self, obj, **kwargs): """ from dojo.celery_dispatch import dojo_dispatch_task # noqa: PLC0415 circular import - dojo_dispatch_task(async_delete_task, obj, **kwargs) + dojo_dispatch_task(async_delete_task, obj._meta.label_lower, obj.pk, **kwargs) @staticmethod def get_object_name(obj): diff --git a/unittests/test_no_pickle.py b/unittests/test_no_pickle.py new file mode 100644 index 00000000000..f9da16df86b --- /dev/null +++ b/unittests/test_no_pickle.py @@ -0,0 +1,35 @@ +""" +Guard tests preventing the reintroduction of pickle into the dojo app. + +Pickle deserialization of attacker-controllable bytes is arbitrary code +execution. We removed all uses (form widgets, Celery serializer) and these +tests fail loudly if a future change adds them back. +""" + +import re +from pathlib import Path + +from django.conf import settings + +import dojo +from unittests.dojo_test_case import DojoTestCase + + +class TestNoPickle(DojoTestCase): + def test_no_pickle_imports_in_dojo(self): + dojo_root = Path(dojo.__file__).resolve().parent + offenders = [] + import_re = re.compile(r"^\s*(?:import\s+pickle|from\s+pickle\s+import)\b", re.MULTILINE) + for path in dojo_root.rglob("*.py"): + text = path.read_text(encoding="utf-8") + if import_re.search(text): + offenders.append(str(path.relative_to(dojo_root.parent))) + self.assertFalse( + offenders, + f"pickle is forbidden in dojo source. Offenders: {offenders}", + ) + + def test_celery_serializer_is_json_only(self): + self.assertEqual(settings.CELERY_TASK_SERIALIZER, "json") + self.assertEqual(settings.CELERY_ACCEPT_CONTENT, ["json"]) + self.assertEqual(settings.CELERY_RESULT_SERIALIZER, "json") diff --git a/unittests/test_survey_forms.py b/unittests/test_survey_forms.py new file mode 100644 index 00000000000..0e4acf8c324 --- /dev/null +++ b/unittests/test_survey_forms.py @@ -0,0 +1,36 @@ +import json + +from dojo.forms import MultiExampleField, MultiWidgetBasic +from unittests.dojo_test_case import DojoTestCase + + +class TestSurveyChoiceWidget(DojoTestCase): + def test_compress_returns_json_string(self): + field = MultiExampleField(required=False) + values = ["a", "b", "c", None, None, None] + compressed = field.compress(values) + + self.assertIsInstance(compressed, str) + self.assertEqual(json.loads(compressed), values) + + def test_decompress_round_trips(self): + widget = MultiWidgetBasic() + values = ["red", "green", "blue", "yellow", None, None] + compressed = json.dumps(values) + + self.assertEqual(widget.decompress(compressed), values) + + def test_decompress_empty_returns_blank_list(self): + widget = MultiWidgetBasic() + self.assertEqual(widget.decompress(None), [None, None, None, None, None, None]) + self.assertEqual(widget.decompress(""), [None, None, None, None, None, None]) + + def test_no_pickle_in_form_module(self): + """Guard test: pickle must not be reintroduced into dojo/forms.py.""" + from pathlib import Path + + forms_path = Path(__file__).resolve().parent.parent / "dojo" / "forms.py" + contents = forms_path.read_text() + self.assertNotIn("import pickle", contents) + self.assertNotIn("pickle.loads", contents) + self.assertNotIn("pickle.dumps", contents) From 424f4903433310543c651fe0a9cb95c46d04db96 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:09:01 -0600 Subject: [PATCH 2/4] test: update importer perf-test query counts after pickle removal The async_user_id user resolution and refetch in async_delete_task / SLA recompute add 1-6 queries per scenario; CI auto-generated the new expected counts. Co-Authored-By: Claude Opus 4.7 (1M context) --- unittests/test_importers_performance.py | 36 ++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/unittests/test_importers_performance.py b/unittests/test_importers_performance.py index 02a8a234d81..cbd2a561746 100644 --- a/unittests/test_importers_performance.py +++ b/unittests/test_importers_performance.py @@ -338,11 +338,11 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=152, + expected_num_queries1=155, expected_num_async_tasks1=2, - expected_num_queries2=122, + expected_num_queries2=123, expected_num_async_tasks2=1, - expected_num_queries3=36, + expected_num_queries3=37, expected_num_async_tasks3=1, expected_num_queries4=100, expected_num_async_tasks4=0, @@ -363,13 +363,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=159, + expected_num_queries1=165, expected_num_async_tasks1=4, - expected_num_queries2=129, + expected_num_queries2=133, expected_num_async_tasks2=3, - expected_num_queries3=40, + expected_num_queries3=44, expected_num_async_tasks3=3, - expected_num_queries4=107, + expected_num_queries4=109, expected_num_async_tasks4=2, ) @@ -509,9 +509,9 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=87, + expected_num_queries1=90, expected_num_async_tasks1=2, - expected_num_queries2=80, + expected_num_queries2=83, expected_num_async_tasks2=2, ) @@ -599,11 +599,11 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=161, + expected_num_queries1=164, expected_num_async_tasks1=2, - expected_num_queries2=133, + expected_num_queries2=134, expected_num_async_tasks2=1, - expected_num_queries3=46, + expected_num_queries3=47, expected_num_async_tasks3=1, expected_num_queries4=101, expected_num_async_tasks4=0, @@ -624,13 +624,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=171, + expected_num_queries1=177, expected_num_async_tasks1=4, - expected_num_queries2=143, + expected_num_queries2=147, expected_num_async_tasks2=3, - expected_num_queries3=50, + expected_num_queries3=54, expected_num_async_tasks3=3, - expected_num_queries4=111, + expected_num_queries4=113, expected_num_async_tasks4=2, ) @@ -744,8 +744,8 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=96, + expected_num_queries1=99, expected_num_async_tasks1=2, - expected_num_queries2=191, + expected_num_queries2=194, expected_num_async_tasks2=2, ) From f726b1d2306ced02c53f51e7539f5c7560b146b6 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:16:21 -0600 Subject: [PATCH 3/4] fix(sla): reset in-memory async_updating after dispatch Before the pickle removal, the SLA recompute task received the SLA_Configuration / Product instances by reference (Celery's sync .apply() does not serialize). The inner update function set async_updating=False on those shared instances, so the dispatcher's local self.async_updating ended up False as well. After switching the dispatch to pass IDs and refetch in the task, the inner function only resets async_updating on its refetched copies. The dispatcher's in-memory self.async_updating stayed True, so a subsequent save() on the same instance triggered the lock-revert path at SLA_Configuration.save() line 1058 and overwrote the caller's field changes (e.g. enforce_critical) with the DB values. Manifested as test_sla_expiration_date_after_sla_not_enforced failing: sla_config.enforce_critical=False was reverted to True on save. Reset async_updating on the in-memory caller instances after dispatch returns to keep them consistent with the post-task DB state. Co-Authored-By: Claude Opus 4.7 (1M context) --- dojo/models.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dojo/models.py b/dojo/models.py index 2da81fc4eed..153177bc20e 100644 --- a/dojo/models.py +++ b/dojo/models.py @@ -1099,6 +1099,10 @@ def save(self, *args, **kwargs): list(products.values_list("id", flat=True)), severities=severities, ) + # The async task refetches and resets async_updating on its own copy. + # Mirror that on this in-memory instance so a subsequent save() on the + # same instance does not trigger the lock-revert path at line 1058. + self.async_updating = False def clean(self): sla_days = [self.critical, self.high, self.medium, self.low] @@ -1265,6 +1269,12 @@ def save(self, *args, **kwargs): sla_config.id, [self.id], ) + # The async task refetches and resets async_updating on its own copies. + # Mirror that on this in-memory product and the in-memory sla_config so a + # subsequent save() on either does not trigger their lock-revert paths. + self.async_updating = False + if sla_config: + sla_config.async_updating = False def get_absolute_url(self): return reverse("view_product", args=[str(self.id)]) From 7eac1d63297fdf0474398ac32b6fe778224a1c53 Mon Sep 17 00:00:00 2001 From: Ross Esposito Date: Mon, 4 May 2026 08:52:53 -0500 Subject: [PATCH 4/4] Adding try/except to channels --- dojo/notifications/helper.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/dojo/notifications/helper.py b/dojo/notifications/helper.py index a2aa6a632dc..1de664c4a6d 100644 --- a/dojo/notifications/helper.py +++ b/dojo/notifications/helper.py @@ -845,7 +845,10 @@ def _process_notifications( notifications.other, ): logger.debug("Sending Slack Notification") - send_slack_notification.run(event, user_id=user_id, **kwargs) + try: + send_slack_notification.run(event, user_id=user_id, **kwargs) + except Exception: + logger.exception("Failed to send Slack notification for event %s", event) if self.system_settings.enable_msteams_notifications and "msteams" in getattr( notifications, @@ -853,7 +856,10 @@ def _process_notifications( notifications.other, ): logger.debug("Sending MSTeams Notification") - send_msteams_notification.run(event, user_id=user_id, **kwargs) + try: + send_msteams_notification.run(event, user_id=user_id, **kwargs) + except Exception: + logger.exception("Failed to send MSTeams notification for event %s", event) if self.system_settings.enable_mail_notifications and "mail" in getattr( notifications, @@ -861,7 +867,10 @@ def _process_notifications( notifications.other, ): logger.debug("Sending Mail Notification") - send_mail_notification.run(event, user_id=user_id, **kwargs) + try: + send_mail_notification.run(event, user_id=user_id, **kwargs) + except Exception: + logger.exception("Failed to send Mail notification for event %s", event) if self.system_settings.enable_webhooks_notifications and "webhooks" in getattr( notifications, @@ -869,7 +878,10 @@ def _process_notifications( notifications.other, ): logger.debug("Sending Webhooks Notification") - send_webhooks_notification.run(event, user_id=user_id, **kwargs) + try: + send_webhooks_notification.run(event, user_id=user_id, **kwargs) + except Exception: + logger.exception("Failed to send Webhooks notification for event %s", event) def process_tag_notifications(request, note, parent_url, parent_title):