Skip to content

Conversation

@8R0WNI3
Copy link
Member

@8R0WNI3 8R0WNI3 commented Nov 28, 2025

What this PR does / why we need it:
Instead of hardcoding a list of finding types for which to re-use discovery dates, re-use them by default if the data key (i.e. their identity) matches. This behaviour may be adjusted per finding type (e.g. for vulnerabilities, only re-use if the package and CVE match).
Finding types can still be configured via the findings-cfg to not honour existing discovery dates at all, e.g.:

- type: finding/<type>
  reuse_discovery_date:
    enabled: true | false
    max_reuse_time: 30d

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

The discovery dates of findings are now re-used by default based on the finding's identity. This behaviour can be disabled via the findings-cfg `[].reuse_discovery_date.enabled` flag. For `sast` findings it is typically desirable to disable it because a missing SAST scan is always bound to a specific source snapshot and has no earlier discovery date in that sense.

@8R0WNI3 8R0WNI3 requested a review from a team as a code owner November 28, 2025 09:42
@zkdev
Copy link
Member

zkdev commented Nov 28, 2025

do we consider this as a breaking change from a semantical PoV?

@8R0WNI3 8R0WNI3 force-pushed the 8R0WNI3-discovery-dates branch from 97103dd to bd09956 Compare November 28, 2025 09:58
@8R0WNI3
Copy link
Member Author

8R0WNI3 commented Nov 28, 2025

do we consider this as a breaking change from a semantical PoV?

Looking at the findings types which will now also re-use the discovery dates:

  • falco -> not being processed (yet)
  • kyverno -> not being processed (yet)
  • ghas -> we don't have any of those findings (yet)
  • inventory -> should have been the desired behaviour anyways -> rather considerable as a bugfix
  • malware -> always had "blocking" semantics anyways -> an earlier discovery date won't change this -> rather considerable as a bugfix
  • sast -> always had "blocking" semantics anyways -> an earlier discovery date won't change this BUT a sast finding is only ever applicable to exactly one OCM artefact version, so it might always be considered as a new finding

I think for sast findings we should disable the re-use of discovery dates, so I agree we should rather consider this as breaking. Wdyt?

@8R0WNI3 8R0WNI3 force-pushed the 8R0WNI3-discovery-dates branch from bd09956 to 6e5a163 Compare November 28, 2025 10:14
@8R0WNI3 8R0WNI3 requested a review from zkdev November 28, 2025 10:14
Instead of hardcoding a list of finding types for which to re-use
discovery dates, re-use them by default if the data key (i.e. their
identity) matches. This behaviour may be adjusted per finding type
(e.g. for vulnerabilities, only re-use if the package and CVE match).
Finding types can still be configured via the findings-cfg to not honour
existing discovery dates at all, e.g.:

```
- type: finding/<type>
  reuse_discovery_date:
    enabled: true | false
    max_reuse_time: 30d
```

Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Since the data key is used per default now, these code paths (which also
(indirectly) implemented a check against the data key) became obsolete
now.

Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
@8R0WNI3 8R0WNI3 force-pushed the 8R0WNI3-discovery-dates branch from 6e5a163 to a6ab805 Compare November 28, 2025 11:09
@8R0WNI3 8R0WNI3 requested review from TuanAnh17N and ccwienk December 2, 2025 11:37
@ccwienk
Copy link
Contributor

ccwienk commented Dec 3, 2025

do we consider this as a breaking change from a semantical PoV?

I think for sast findings we should disable the re-use of discovery dates, so I agree we should rather consider this as breaking. Wdyt?

do we know whether there are other usages of sast-extension than our own (gardener)? if not, we can still mark this as breaking, but we will not need to take any precautions w.r.t. rollout.

@8R0WNI3
Copy link
Member Author

8R0WNI3 commented Dec 3, 2025

do we know whether there are other usages of sast-extension than our own (gardener)?

No, there are no other (active) usages yet.

if not, we can still mark this as breaking, but we will not need to take any precautions w.r.t. rollout.

Agree, I have adjusted the release-note accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants