fix(importers): stop the write-back re-creating a deleted import target

[Maffooch]

Description

An object deleted while its scan was being processed was re-created by the importer's closing write-back.

Model.save() on an instance whose primary key is already set issues an UPDATE, and Django falls back to an INSERT when that UPDATE matches no rows. Both importers load their test and engagement at the start of a run that can take minutes, then save them again at the end (default_importer.py / default_reimporter.py, "Save the test and engagement for changes to take affect"). A delete landing in between turned that write-back into an INSERT of the deleted row from the stale in-memory copy. Two outcomes, both wrong:

• The parent went with it. An engagement delete cascades to its tests, so the INSERT carried a dangling engagement_id. Django declares its foreign keys DEFERRABLE INITIALLY DEFERRED, so the violation was only raised at COMMIT — past any handler that still knew what the import was doing. The API caller got an opaque 500 naming a PostgreSQL constraint (dojo_test_engagement_id_..._fk_dojo_engagement_id) instead of the reason the import failed.
• The parent survived. The INSERT succeeded and silently resurrected a row the user had deleted, without the findings and history that were cascaded away with it.

Observed on a production instance on the synchronous reimport-scan path, repeatedly, on an instance whose CI creates and tears down short-lived per-pull-request engagements. The tell was the failure order: the constraint violation named the test row as already having an id, which is only possible if the UPDATE-to-INSERT fallback had run.

Fix

Both write-back sites and update_test_progress() now save through a new BaseImporter.save_without_resurrecting(), which confirms the row is still there and otherwise raises a ValidationError naming what was deleted. The import still fails — its target is gone, and it cannot complete — but it fails with the error that says so, and nothing is re-inserted.

The row is checked before the write rather than with save(force_update=True). A forced update raises from inside the atomic block Model.save_base opens without a savepoint, which marks the whole surrounding transaction for rollback; the caller's failure handling would then hit TransactionManagementError instead of being able to record why the import failed. Confirmed experimentally while developing this — the follow-up query needed to classify the error was itself refused. Costing one indexed primary-key lookup keeps the transaction usable.

This is deliberately scoped to the resurrection. It does not change whether such an import succeeds, and it does not address why a target may be deleted mid-run — that is the caller's workflow.

Performance impact

One primary-key SELECT 1 ... LIMIT 1 per guarded save: +3 queries per import/reimport, independent of finding count. Expected counts in test_importers_performance.py are updated accordingly — all 34 deltas are exactly +3, which is itself the evidence that the cost is O(1) and not O(findings). Async task counts are unchanged.

Impact on the Pro plugin

Checked as part of the acceptance criteria; no Pro change is required.

• Pro has no test.save() / engagement.save() of its own on the importer write-back path, so there is no second copy of this defect to fix there.
• ProImporter / ProReImporter derive from DefaultImporter / DefaultReImporter and do not override update_test_progress, so they inherit the guard. ConnectorsImporter and its subclasses inherit it too.
• Pro's importer wraps this process_scan in a try/except that records a failed status and re-raises as type(exception)(exception). A Django ValidationError re-wraps cleanly, so the failure path is unaffected — and the message the caller now receives names the deleted object instead of an internal table.
• Follow-up worth considering on the Pro side (not needed for this fix): Pro surfaces this as a 500. A ValidationError from a vanished target is arguably a 4xx, which would be a change to Pro's import-error humanizer rather than to open source.

Test results

New unittests/test_importers_deleted_target.py, 6 tests. Every one was confirmed failing before the fix:

FAILED (failures=4, errors=1)   # the 6th is the control case, green throughout
AssertionError: ValidationError not raised
AttributeError: 'DefaultReImporter' object has no attribute 'save_without_resurrecting'

The pre-fix reimport did not merely fail — it completed successfully and left the deleted row re-inserted, which a direct probe confirmed:

deleted test row exists again after reimport: True

Coverage: the reimport write-back (test deleted, and engagement deleted so the test is cascaded away), the import write-back, update_test_progress, the engagement half of the guard, and a control case asserting a run whose target is still present is unchanged and still persists percent_complete.

After the fix, run against PostgreSQL (not SQLite — the deferred-constraint behaviour above is backend-specific):

• unittests/test_importers_deleted_target.py — 6 passed
• test_importers_performance — 11 passed (was 11 passed on the unmodified base, then 34 count assertions failing, now green on the updated counts)
• test_importers_importer, test_importers_closeold, test_importers_deduplication, test_import_reimport, test_update_import_history, test_importers_performance, test_apiv2_scan_import_options, test_import_execution_mode, test_reimport_prefetch, test_endpoint_meta_import, test_generic_meta_import — 325 passed, 24 skipped, 0 failures
• ruff check --config ruff.toml (0.15.20, the pinned version) clean on dojo/importers/ and both touched test files

Note on the environment: this sandbox has no Docker, so the suites were run through manage.py test against a locally provisioned PostgreSQL 16 cluster on Python 3.13 rather than run-unittest.sh. For the same reason scripts/update_performance_test_counts.py (which shells out to run-unittest.sh) could not be used; the counts were taken from the actual values the assertions reported and each one re-verified. CI is the authority.

Documentation

No documentation change — this is a bug fix to internal importer behaviour, with no new or changed inputs, outputs, or settings.

Checklist

• Submitted against bugfix (bug fix, not a feature).
• Ruff compliant.
• Python 3.13 compliant.
• No model changes, so no migrations.
• Tests added to the unit tests.

---

Generated by Claude Code

[dryrunsecurity]

This pull request contains critical security findings because the author 'claude' modified sensitive files in the 'dojo/importers/' directory without being on the allowed authors list.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/base_importer.py (drs_9a0244e3)

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 'claude' (commit 0625b7e) who is not in the allowed authors list.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/default_importer.py (drs_c9e277b5)

Vulnerability	Configured Sensitive Codepath Modified by Non-Allowed Author
Description	File 'dojo/importers/default_importer.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'claude' (commit 0625b7e) who is not in the allowed authors list.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/default_reimporter.py (drs_c34043b5)

Vulnerability	Configured Sensitive Codepath Modified by Non-Allowed Author
Description	File 'dojo/importers/default_reimporter.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'claude' (commit 0625b7e) 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.