Add finding lifecycle provenance events (3/3) - #15152
Closed
devGregA wants to merge 11 commits into
Closed
Conversation
github-actions
Bot
added
New Migration
devGregA
changed the title
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Maffooch
force-pushed
the
devgrega/finding-lifecycle-events
branch
from
July 8, 2026 04:34
38ad3cb to
e8f7a83
Compare
Contributor
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Contributor
|
Fixed, tested, and approving (milestone 3.1.100) — must merge after #15150. Issues found in review:
Fixes:
Verified (in the combined #15150 + #15152 state, which is what production will have):
The lifecycle provenance feature still meets its goal; the re-dedup double-count is closed and the migration ordering is documented correctly. Looks good now (merge after #15150). |
valentijnscholten
left a comment
valentijnscholten
left a comment
Member
There was a problem hiding this comment.
During the selection of the django-auditlog replacement we found it a benefit of pghistory because it allows for adding extra context to data edits and/or group edit together and show them as a single event. I can understand the benefit of having a "life cycle event" only table. It would thought be the third table capturing finding events next to Test_Import_Finding_Action and pghistory. I thin it's worth a small exploration if we could achieve the same nice "life cycle event" view on the existing pghistory pages and by using pghistory contexts to enrich the events.
devGregA
force-pushed
the
devgrega/finding-lifecycle-events
branch
from
July 27, 2026 22:56
986ce44 to
ac6117a
Compare
|
This pull request contains critical security findings because multiple database migration files in the sensitive
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in
|
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0281_finding_processing_status.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by '' (commit df8aa76) who is not in the allowed authors list. |
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0282_remove_finding_insert_insert_and_more.py (drs_7a3064e8)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0282_remove_finding_insert_insert_and_more.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by '' (commit df8aa76) who is not in the allowed authors list. |
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0282_finding_lifecycle_event.py (drs_bb28b93a)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0282_finding_lifecycle_event.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by '' (commit 235c154) who is not in the allowed authors list. |
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0283_finding_lifecycle_event.py (drs_1e59404f)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0283_finding_lifecycle_event.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by '' (commit 6b5f49c) who is not in the allowed authors list. |
We've notified @mtesauro.
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.
Findings now carry an explicit post-import processing state: importers create findings as "pending" and post_process_findings_batch stamps "processed" on completion or "failed" on error (the exception still propagates). Findings created outside the import pipeline (manual entry, API) default to "processed" and never enter "pending". This makes silently-dying post-processing (worker OOM, task crash) visible on the findings themselves, and defines the post-import pipeline (dedupe, false-positive history, issue updater, grading, JIRA push) as a single terminating chain — groundwork for pending/failed counts on import pages and stuck-finding health checks. Performance: - column default "processed": metadata-only ALTER on PostgreSQL 11+, no backfill; all pre-existing findings read correctly - stamping is one bulk UPDATE per existing 1000-finding batch; no signals, no per-row saves - partial index covers only the small, hot PENDING working set - pghistory finding-history triggers regenerated for the new columns API: fields exposed read-only on the Finding API, plus a ?processing_status= filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
post_process_findings_batch now ends with one bulk UPDATE stamping the batch's processing lifecycle, so every import/reimport step that runs a post-processing batch costs exactly one additional query. Adjust the perf baselines accordingly (+1 per step; steps with no batch, like empty-report reimports, are unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review: add_jira_issue/update_jira_issue swallow errors internally and report them as a (success, message) tuple, so a failed push never raised and the batch stamped processed even when the ticket was never created — in every execution mode. The push tasks are now the writer of record for the JIRA outcome (record_finding_push_outcome in dojo/jira/helper.py): - a (False, message) result stamps the finding failed with the helper's message in the new processing_error field; group pushes fan the outcome out to member findings, and the group task's separate-ticket updates are stamped individually by their own results; - a later successful push heals failed back to processed and clears the error; successful pushes never touch pending findings — the batch owns the happy-path stamp. Ordering: the batch's final bulk UPDATE excludes failed rows, so an inline (eager/force_sync) push failure stamped inside the try block survives the finally, while in async execution the push task simply runs after the batch and is the last writer. The batch's own exception path now records str(e) in processing_error before re-raising. A finding stamped failed by a push deliberately stays failed through later non-pushing batches until a push succeeds — the failure is unresolved until then. Also corrects the (bool, str) return annotations on the five push/issue helpers (they were declared tuple[str, bool], which had already misled the finding-group perf test's mock — updated to return a real tuple now that the tasks unpack it). Migration 0279 adds processing_error (blank text, editable=False → read-only in the API) and regenerates the pghistory triggers. Tests: 6 new in unittests/test_processing_status.py — failure stamping with reason, heal on success, pending untouched by successful pushes, group fan-out, eager-ordering (real dispatch path under CELERY_TASK_ALWAYS_EAGER), and batch exception reason capture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
….300 sync master-into-bugfix (DefectDojo#15362) brought 0280_vulnerability_id_upper_index onto bugfix; renumber and re-parent so the graph keeps a single leaf. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
devGregA
force-pushed
the
devgrega/finding-lifecycle-events
branch
2 times, most recently
from
July 27, 2026 23:33
c24c1dc to
0c418ca
Compare
New append-only model Finding_Lifecycle_Event records the SEMANTIC
transitions in a finding's life - the "why" behind a large class of
support questions:
- created: by which import/reimport (only findings actually created)
- closed: by close_old_findings / re-upload, with the reason
- reopened: reactivated by a re-upload
- marked_duplicate: of which original (covers batch dedupe and
transitive re-points)
- pushed_jira: as which issue key
This complements, and deliberately does not duplicate, existing
history: pghistory captures field-level diffs and
Test_Import_Finding_Action records per-import actions; neither can
express why a transition happened.
Exposed read-only at /api/v2/findings/{id}/lifecycle_events/.
Performance design: transition-only writes (a matched-unchanged
reimport writes zero rows - tested), bulk_create at the existing
1000-finding import batch boundaries, no signals, detail values
truncated. The FK carries no DB constraint with on_delete=DO_NOTHING
so bulk finding deletion never touches this table; orphans and old
events are swept by a nightly retention purge task
(DD_FINDING_LIFECYCLE_EVENTS_RETENTION_DAYS, default 540). Kill
switch: DD_FINDING_LIFECYCLE_EVENTS_ENABLED. Two indexes: the
(finding, created) timeline read and (created) for the purge.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The provenance ledger writes exactly one query per transition, and the baselines now document that cost precisely: - +1 on steps that create findings (one bulk_create of CREATED events per import batch) - +0 on unchanged-match reimports (transition-only discipline, now enforced by the perf baselines as well as the unit test) - +1 per finding closed by close_old_findings (CLOSED event inside mitigate_finding, alongside the ~10 queries a close already costs) - +1 per duplicate marked by dedupe (MARKED_DUPLICATE event in set_duplicate) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
set_duplicate recorded a MARKED_DUPLICATE lifecycle event on every call, so a product-wide re-deduplication appended a redundant event for findings that were already duplicates of the same original. Guard the event on a genuine transition (skip when new_finding is already a duplicate of existing_finding), preserving the transition-only provenance guarantee. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…UPLICATE event Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…82; lint - rebase onto the updated 1/3 branch so 0280/0281 exist in-tree (CI previously failed with NodeNotFoundError on the old 0278 parent) - renumber 0279_finding_lifecycle_event -> 0282, re-parented on 0281 - combine query-count baselines with the 1/3 branch's stamps - move set_duplicate import to top level (ruff import-outside-top-level) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bugfix took 0280 in the 3.1.300 master sync; the 1/3 branch is now 0281/0282, so this becomes 0283 parented on 0282. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 3.2.0-sync Small classes predate lifecycle provenance; each synchronous import/reimport step gains one bulk event insert. Verified against both branches' perf runs: 1/3 alone matches the original baselines, this branch runs one query higher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
devGregA
force-pushed
the
devgrega/finding-lifecycle-events
branch
from
July 27, 2026 23:45
0c418ca to
9e47fb8
Compare
blakeaowens
approved these changes
Jul 28, 2026
Maffooch
approved these changes
Jul 28, 2026
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR and #15150 both update the pinned query-count baselines in
unittests/test_importers_performance.pyandunittests/test_tag_inheritance_perf.py, each measured againstdevwithout the other. #15150 must merge first: this PR's migration0279_finding_lifecycle_eventdepends on0278_finding_processing_status(added in #15150), so applying #15152 on its own fails withNodeNotFoundError. They are otherwise functionally independent. Separately, whichever merges second needs a rebase with re-measured baselines, because the deltas add up. Example:EXPECTED_ZAP_IMPORT_V2is 287 on dev, 288 in each PR alone, and 289 once both are in (+1 for this PR's created-events batch, +1 for #15150's batch stamp). I'll push the combined-baseline rebase on whichever PR lands second.Description
Adds a finding lifecycle provenance ledger: an append-only record of the semantic transitions in a finding's life, answering the questions behind a large class of support tickets — "why did this finding close?", "why is this a duplicate?", "when was this ticketed?"
New model
Finding_Lifecycle_Event(finding,actor_type,action,detailJSON,created), written at five capture points:createdclosedmitigate_finding(close_old_findings / re-upload)reopenedmarked_duplicateset_duplicate(covers batch dedupe; transitive re-points record their own event)pushed_jiraadd_jira_issuesuccessThis complements — deliberately does not duplicate — existing history: pghistory triggers capture field-level diffs, and
Test_Import_Finding_Actionrecords per-import actions. Neither can express why: which key matched, which re-upload closed it, what it's a duplicate of. That's what this table records.API:
GET /api/v2/findings/{id}/lifecycle_events/— the finding's provenance timeline, newest first (read-only).Performance / operational design (this table must never become a problem):
bulk_createevents at the existing 1,000-finding batch boundaries; no signals, no per-row saves; detail values truncated to 256 chars.db_constraint=False+on_delete=DO_NOTHING, so bulk finding deletion never touches this table (no ORM cascade collection, no DB cascade). Orphans are swept by retention.(finding, created)for the timeline read;(created)for the purge.DD_FINDING_LIFECYCLE_EVENTS_RETENTION_DAYS(default 540), batched deletes.DD_FINDING_LIFECYCLE_EVENTS_ENABLED(default true) turns all writes into no-ops.test_importers_performance.pyandtest_tag_inheritance_perf.pyare updated accordingly and now double as a regression tripwire for this table's write discipline.Test results
New module
unittests/test_finding_lifecycle_events.py(5 tests), including a full reimport cycle over the semgrep close-old fixtures (created → matched-with-zero-events → closed with reason on unique-id change → reactivated), dedupe originals, the API endpoint, retention purge, and the kill switch.Regression:
test_importers_closeold,test_importers_deduplication,test_deduplication_logic(135 tests) andtest_rest_framework.FindingsTest(27 tests) all pass against PostgreSQL via the unit-test compose image.makemigrations --checkclean; ruff (0.15.20, repo config) clean.Not covered by an automated test: the
pushed_jiraevent (requires a mocked JIRA stack; the capture is three lines on the existing success path). Happy to extend a JIRA test if maintainers prefer.Documentation
Additive feature; the API action is schema-annotated. Happy to add a docs page (finding lifecycle events + settings reference) in this PR or as a follow-up, whichever maintainers prefer.
🤖 Generated with Claude Code