Exclude volatile ReplicaSet name from Trivy Operator service field - #16028
tallesgois-shipay wants to merge 7 commits into
Conversation
The Trivy Operator handlers (vulnerability, checks, secrets) built the `service` field as `namespace/kind/name`. For Pods owned by a ReplicaSet, `name` is the ReplicaSet's generated name, which changes on every deploy. Since close_old_findings matches on an exact service string, this made service change on every redeploy and permanently prevented old findings from ever being closed, causing unbounded finding accumulation across redeploys. Fixes DefectDojo#16026
|
This pull request contains multiple low-severity findings where the author 'tallesgois-shipay' modified files in configured sensitive codepaths without being on the allowed authors list. These issues are non-blocking and do not prevent the merge.
Configured Sensitive Codepath Modified by Non-Allowed Author in
|
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/announcement/os_message.py' matches configured sensitive codepath pattern 'dojo/announcement/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/authorization/query_registrations.py (drs_30debfc2)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/authorization/query_registrations.py' matches configured sensitive codepath pattern 'dojo/authorization/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0268_release_authorization_to_pro.py (drs_23d40114)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0268_release_authorization_to_pro.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/deduplication.py (drs_94a914ce)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/deduplication.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/helper.py (drs_dec384dd)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/helper.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/auto_create_context.py (drs_fabe1ba9)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/auto_create_context.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/base_importer.py (drs_7e25de47)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/base_importer.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'tallesgois-shipay' (commit 67ed0e0) who is not in the allowed authors list. |
Comment to provide feedback on these findings.
Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]
Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing
All finding details can be found in the DryRun Security Dashboard.
devGregA
left a comment
There was a problem hiding this comment.
Thanks for the detailed write-up, and sorry about the table growth you ran into. Two things came up in review.
First, close_old_findings filters on the service passed with the import request, not on the value the parser sets (dojo/importers/default_importer.py lines 413-417). When no service is passed, only findings with an empty service are candidates, so Trivy Operator findings are never closed whatever the parser builds. Passing a stable per-workload service on import is what makes closing work today.
Second, the parser's service does feed the hash code, so making it stable is still worth doing, but dropping the name entirely means two workloads of the same kind with the same container name in a namespace end up sharing one service. Could you make these changes:
- Retarget the PR to
bugfix. - Keep
resource_name, and only strip the trailing pod-template-hash segment when the kind isReplicaSet, in all three handlers (vulnerability_handler.pyline 29,checks_handler.pyline 22,secrets_handler.pyline 29). - Use the same normalized name in the
**resource.name:**line of the description (vulnerability_handler.pyline 82 and the matching lines in the other two handlers).descriptionis a hash code field for Trivy Operator Scan, so otherwise the hash still changes on every rollout. - Extend the test to cover a ReplicaSet (suffix stripped), a Deployment or StatefulSet (name unchanged), and two different ReplicaSets in the same namespace (different services).
Fixes #16026
Problem
The Trivy Operator handlers (
vulnerability_handler.py,checks_handler.py,secrets_handler.py) buildFinding.serviceasnamespace/kind/name:For Pods owned by a ReplicaSet,
resource_name(from thetrivy-operator.resource.namelabel) is the ReplicaSet's generated name, which changes on every deploy (new hash suffix each rollout).close_old_findingsscopes its query by an exactservicematch:Since
servicechanges on every redeploy for these three scan types,close_old_findingscan never match findings from a previous deploy generation — old findings are never closed, only ever added to, regardless ofclose_old_findings=true. In our environment this let onedojo_findingtable grow from ~218k to 5.7M+ rows over ~3 months, degrading Postgres enough to cause dashboard timeouts and an availability incident (details in #16026).This is the same pattern discussed in #11500, where a maintainer confirmed the fix works "as long as you don't include a version number and the value stays the same across reimports" — but there the volatile value came from caller-supplied API input. Here it's built inside DefectDojo's own bundled parsers, so callers (e.g.
trivy-dojo-report-operator) have no way to work around it.Fix
Drop
resource_namefrom theservicestring in all three handlers, keepingnamespace/kind[/container]. This keepsservicestable across redeploys of the same workload while still distinguishing different containers/workloads within a namespace.checks_handler.pyandsecrets_handler.pyhad byte-identical logic tovulnerability_handler.pyand are fixed the same way for consistency, since they share the exact same volatility problem.Tests
Added
test_vulnerabilityreport_service_excludes_volatile_replicaset_nametounittests/tools/test_trivy_operator_parser.py, asserting the fixture's ReplicaSet-generatedresource.name("nginx-deployment-965685897") does not end up infinding.service. No existing test asserted onserviceformat, so nothing else needed updating.Deployment method