Skip to content

Reimport with group_by no longer 500s on duplicate same-name finding groups - #16065

Merged
Maffooch merged 2 commits into
bugfixfrom
fix/finding-group-multiple-objects
Sep 23, 2026
Merged

Maffooch merged 2 commits into
bugfixfrom
fix/finding-group-multiple-objects

Conversation

@Maffooch

@Maffooch Maffooch commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

[sc-15673]

Description

POST /api/v2/reimport-scan/ with group_by set could fail with a 500:

Finding_Group.MultipleObjectsReturned: get() returned more than one Finding_Group -- it returned 2!

add_findings_to_auto_group in dojo/finding/helper.py looked the group up with
Finding_Group.objects.get_or_create(test=test, creator=creator, name=name[:255]). Keying on the
creator caused two problems:

  • A reimport by a different user than the one who created the group did not find that group, so it
    created a second group with the same name in the same test and split the findings between them.
  • When a test already held two same-name groups for the importing user (two imports racing on the
    same test can leave this, since get_or_create has no constraint behind it), get_or_create
    raised MultipleObjectsReturned and the whole reimport returned a 500. This is the path the API
    takes by default, because create_finding_groups_for_all_findings defaults to true.

The lone-finding branch (create_finding_groups_for_all_findings=false with one finding) had a
related problem. It called Finding_Group.objects.get(test=test, name=name) inside a bare except,
so duplicates raised MultipleObjectsReturned, the except swallowed it, and the finding was left
out of any group.

The fix treats an auto group as one per (test, name). A new helper,
get_or_create_auto_finding_group, filters by test and truncated name, orders by id, and reuses the
oldest group whoever created it. It only creates a group, owned by the importing user, when none
exists. The lone-finding branch uses the same oldest-group lookup in place of the get() and bare
except, and now also truncates the name to 255 characters to match the stored value.

This PR adds no uniqueness constraint or migration. Existing databases can already hold duplicate
same-name groups, and a constraint would fail on them. The lookup tolerates those rows and stops
the reimport path from creating new duplicates when a different user reimports. Two imports that
race on the same test can still both insert a group. After this change the next reimport resolves
to the oldest one instead of failing.

Test results

New unittests/test_reimport_finding_group_duplicates.py reimports a one-finding Trivy report
through the API with group_by=finding_title, close_old_findings=true and
do_not_reactivate=true:

  • Two same-name groups owned by the importing user, with create_finding_groups_for_all_findings
    true and false: the reimport returns 201 and the finding lands only in the oldest group. Before
    the fix, the true case returned 500 with MultipleObjectsReturned raised from
    add_findings_to_auto_group, and the false case left the finding ungrouped.
  • One group created by a different user, true and false: the finding joins that group and no second
    group is created. Before the fix, the true case created a second group owned by the importer.
  • Control with no existing group: exactly one group is created, owned by the importer.

Before the fix, 3 of the 5 tests failed. After the fix, all 5 pass. These suites also pass with the
change: test_import_reimport, test_reimport_batch_flush, test_reimport_final_drain,
test_reimport_prefetch, test_reimporter_persist_seam, test_apiv2_scan_import_options (148
tests, 1 skipped), plus test_importers_importer, test_jira_import_and_pushing_api,
test_jira_finding_group_perf, test_finding_group_cleanup, test_reimport_group_jira_sync,
test_finding_group_filter_context, test_update_import_history and test_importers_performance
(155 tests, 7 skipped). ruff check with the pinned version passes on both changed files.

After rebasing on the latest bugfix, the new module plus test_import_reimport, test_finding_group_cleanup, test_reimport_group_jira_sync and test_apiv2_scan_import_options ran 146 tests, OK.

Documentation

No documentation change. The documented behavior of group_by does not change.

Checklist

  • Make sure to rebase your PR against the very latest dev. (Bug fix: rebased on the latest bugfix.)
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is Ruff compliant (see ruff.toml).
  • Your code is python 3.13 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

🤖 Generated with Claude Code

Auto grouping looked up the group with get_or_create keyed on
(test, creator, name). A reimport by a different user than the group's
creator therefore made a second same-name group in the same test, and
once a test held two same-name groups for the importing user (for
example after two imports raced), get_or_create raised
Finding_Group.MultipleObjectsReturned and reimport-scan returned a 500.
The lone-finding path hit the same duplicates through a get() inside a
bare except, which swallowed the error and left the finding ungrouped.

Look the group up by (test, name) only and reuse the oldest one by id,
creating a group (owned by the importing user) only when none exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch Maffooch added this to the 3.3.300 milestone Sep 23, 2026
@Maffooch
Maffooch enabled auto-merge September 23, 2026 14:47
@Maffooch
Maffooch added this pull request to the merge queue Sep 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 23, 2026
@Maffooch
Maffooch enabled auto-merge September 23, 2026 23:20
@Maffooch
Maffooch added this pull request to the merge queue Sep 23, 2026
Merged via the queue into bugfix with commit 96c62fc Sep 23, 2026
49 checks passed
@Maffooch
Maffooch deleted the fix/finding-group-multiple-objects branch September 23, 2026 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants