Fix #10268: Make Finding Group filter context-aware (Test/Engagement/Product) by AbhimanyuGit2507 · Pull Request #14271 · DefectDojo/django-DefectDojo

AbhimanyuGit2507 · 2026-02-09T12:10:33Z

Description

Fixes #10268 — the Finding Group filter displayed groups from unrelated Tests or Engagements when viewing findings within a specific context.

This PR makes the Finding Group filter context-aware so that the dropdown only shows relevant groups:

Test view: only groups belonging to that test
Engagement view: groups from tests in that engagement
Product view: groups from tests in that product
Global findings view: all authorized finding groups (unchanged behavior)

Implementation details

Added a helper function to build a Finding Group queryset based on context (test → engagement → product → global).
Passed engagement and test context from finding views into the filter.
Updated both FindingFilter and FindingFilterWithoutObjectLookups to ensure consistent behavior.
Authorization is applied after context filtering, following the same pattern used in finding groups: filter by product if applicable #12711.
Added minor query optimization to load only required fields for the dropdown.
No model changes or migrations were introduced.

Test results

Added unit tests in dojo/unittests/test_finding_group_filter_context.py covering:

Filtering in Test context
Filtering in Engagement context
Filtering in Product context
Global context behavior
Hierarchy precedence (test > engagement > product)
Behavior of both filter classes

All tests pass locally.

Documentation

No documentation updates required. This change corrects filter behavior without altering user workflows or APIs.

Checklist

Make sure to rebase your PR against the very latest dev.
Bugfix submitted against the correct branch.
Give a meaningful name to your PR.
Your code is flake8 compliant.
Your code is python 3.13 compliant.
Add applicable tests to the unit tests.
Add the proper label to categorize your PR (bugfix).

dryrunsecurity · 2026-02-10T05:05:09Z

🔴 Risk threshold exceeded.

This pull request includes edits to sensitive codepaths in multiple files (dojo/filters.py, dojo/finding/views.py, dojo/test/views.py), which the scanner flagged as potentially sensitive and configurable via .dryrunsecurity.yaml; review and verification of authorship and changes are recommended.

🔴 Configured Codepaths Edit in dojo/filters.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in `.dryrunsecurity.yaml`.

🔴 Configured Codepaths Edit in dojo/finding/views.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in `.dryrunsecurity.yaml`.

🔴 Configured Codepaths Edit in dojo/test/views.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in `.dryrunsecurity.yaml`.

🔴 Configured Codepaths Edit in dojo/filters.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in `.dryrunsecurity.yaml`.

We've notified @mtesauro.

All finding details can be found in the DryRun Security Dashboard.

valentijnscholten · 2026-02-10T18:40:34Z

I asked Sonnet 4.5, can you check if there's some valid point in there?

1. Missing defensive checks in `set_related_object_fields()`

In FindingFilterWithoutObjectLookups.__init__() (lines 57-80), field deletions use defensive checks:

if "test__engagement__product__name" in self.form.fields:
    del self.form.fields["test__engagement__product__name"]

However, in FindingFilter.set_related_object_fields() (lines 112-120), fields are deleted without checks:

del self.form.fields["test__engagement__product"]
del self.form.fields["test__engagement__product__prod_type"]
del self.form.fields["test__engagement"]
del self.form.fields["test"]

Recommendation: Add defensive if field in self.form.fields checks for consistency and to prevent potential KeyError exceptions.

2. Potential N+1 query in engagement context

Line 122 performs a database query each time the filter is instantiated:

engagement = Engagement.objects.get(id=self.eid)

Recommendation: Consider caching this or refactoring to avoid the extra query if multiple filters are created.

valentijnscholten

see above

AbhimanyuGit2507 · 2026-02-10T19:18:22Z

I asked Sonnet 4.5, can you check if there's some valid point in there?

1. Missing defensive checks in set_related_object_fields()

In FindingFilterWithoutObjectLookups.__init__() (lines 57-80), field deletions use defensive checks:
if "test__engagement__product__name" in self.form.fields:
    del self.form.fields["test__engagement__product__name"]
However, in FindingFilter.set_related_object_fields() (lines 112-120), fields are deleted without checks:
del self.form.fields["test__engagement__product"]
del self.form.fields["test__engagement__product__prod_type"]
del self.form.fields["test__engagement"]
del self.form.fields["test"]
Recommendation: Add defensive if field in self.form.fields checks for consistency and to prevent potential KeyError exceptions.

2. Potential N+1 query in engagement context

Line 122 performs a database query each time the filter is instantiated:
engagement = Engagement.objects.get(id=self.eid)
Recommendation: Consider caching this or refactoring to avoid the extra query if multiple filters are created.

yes both points are valid, I will update the field deletions to protect against missing fields. I’ll also switch the engagement lookup to a safer pattern to avoid unnecessary queries.

…lter - Implemented hierarchical context filtering (test > engagement > product > global) - Created get_finding_group_queryset_for_context() helper function to eliminate code duplication - Modified FindingFilter and FindingFilterWithoutObjectLookups to accept eid/tid parameters - Updated filter to show only Finding Groups from current test/engagement/product context - Added query optimization with .only("id", "name") for Finding Groups - Fixed user parameter passing to get_authorized_finding_groups_for_queryset() - Updated finding/views.py and test/views.py to pass context parameters to filters - Created comprehensive unit tests (8 test methods) covering all context levels This ensures users only see relevant Finding Groups in the filter dropdown based on their current page context, preventing confusion from seeing unrelated groups.

Use DojoTestCase instead of plain TestCase to align with DefectDojo testing conventions and ensure proper test setup/teardown.

AbhimanyuGit2507 · 2026-02-11T03:52:14Z

Both changes are done as requested, let me know if any other changes are required.

valentijnscholten

did some manual testing also and looks good to me

AbhimanyuGit2507 · 2026-02-12T21:00:46Z

did some manual testing also and looks good to me

thanks for the review, it seems 2 more approvals are required from other maintainers.

mtesauro · 2026-02-13T01:58:48Z

did some manual testing also and looks good to me

thanks for the review, it seems 2 more approvals are required from other maintainers.

We're all in different timezones so approvals can be scatterd

mtesauro

Approved

AbhimanyuGit2507 requested review from Maffooch and mtesauro as code owners February 9, 2026 12:10

github-actions bot added the unittests label Feb 9, 2026

AbhimanyuGit2507 force-pushed the master branch 2 times, most recently from 0198418 to 6fb77ab Compare February 9, 2026 18:52

Maffooch closed this Feb 10, 2026

Maffooch reopened this Feb 10, 2026

valentijnscholten reviewed Feb 10, 2026

View reviewed changes

AbhimanyuGit2507 added 2 commits February 11, 2026 01:04

Switch to DojoTestCase for better test compatibility

da8f604

Use DojoTestCase instead of plain TestCase to align with DefectDojo testing conventions and ensure proper test setup/teardown.

AbhimanyuGit2507 force-pushed the master branch from 6fb77ab to da8f604 Compare February 10, 2026 19:35

valentijnscholten approved these changes Feb 11, 2026

View reviewed changes

valentijnscholten added this to the 2.56.0 milestone Feb 11, 2026

Maffooch approved these changes Feb 13, 2026

View reviewed changes

Maffooch requested review from Jino-T and blakeaowens February 13, 2026 00:36

mtesauro approved these changes Feb 13, 2026

View reviewed changes

Jino-T approved these changes Feb 13, 2026

View reviewed changes

Merge branch 'dev' into master

e65e14b

Maffooch merged commit 656bbe6 into DefectDojo:dev Feb 14, 2026
147 of 148 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #10268: Make Finding Group filter context-aware (Test/Engagement/Product)#14271

Fix #10268: Make Finding Group filter context-aware (Test/Engagement/Product)#14271
Maffooch merged 3 commits intoDefectDojo:devfrom
AbhimanyuGit2507:master

AbhimanyuGit2507 commented Feb 9, 2026

Uh oh!

dryrunsecurity bot commented Feb 10, 2026 •

edited

Loading

Uh oh!

valentijnscholten commented Feb 10, 2026

Uh oh!

valentijnscholten left a comment

Uh oh!

AbhimanyuGit2507 commented Feb 10, 2026

1. Missing defensive checks in `set_related_object_fields()`

2. Potential N+1 query in engagement context

Uh oh!

AbhimanyuGit2507 commented Feb 11, 2026

Uh oh!

valentijnscholten left a comment

Uh oh!

AbhimanyuGit2507 commented Feb 12, 2026

Uh oh!

mtesauro commented Feb 13, 2026

Uh oh!

mtesauro left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

AbhimanyuGit2507 commented Feb 9, 2026

Uh oh!

dryrunsecurity bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔴 Risk threshold exceeded.

Uh oh!

valentijnscholten commented Feb 10, 2026

1. Missing defensive checks in set_related_object_fields()

2. Potential N+1 query in engagement context

Uh oh!

valentijnscholten left a comment

Choose a reason for hiding this comment

Uh oh!

AbhimanyuGit2507 commented Feb 10, 2026

1. Missing defensive checks in set_related_object_fields()

2. Potential N+1 query in engagement context

Uh oh!

AbhimanyuGit2507 commented Feb 11, 2026

Uh oh!

valentijnscholten left a comment

Choose a reason for hiding this comment

Uh oh!

AbhimanyuGit2507 commented Feb 12, 2026

Uh oh!

mtesauro commented Feb 13, 2026

Uh oh!

mtesauro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dryrunsecurity bot commented Feb 10, 2026 •

edited

Loading

1. Missing defensive checks in `set_related_object_fields()`

1. Missing defensive checks in `set_related_object_fields()`