perf(finding): drop four dojo_finding indexes that no query uses - #15659
Open
devGregA wants to merge 1 commit into
Open
perf(finding): drop four dojo_finding indexes that no query uses#15659devGregA wants to merge 1 commit into
devGregA wants to merge 1 commit into
Conversation
dojo_finding is the busiest table in the schema and carries close to fifty indexes, every one
of which is maintained on each row written. Import is where that is felt: a scan creating
thousands of findings pays for all of them, per row.
These four measured as completely unused (pg_stat_user_indexes.idx_scan = 0) on four
long-running production instances, three with roughly seventeen months of accumulated
statistics, ranging from a few thousand to ~184k findings:
(epss_percentile) the largest of the four
(line)
(known_exploited)
idx_finding_sev_open_unver partial: (severity, -numerical_severity)
WHERE active AND NOT verified
The EPSS pair is the informative one. (epss_score) IS used, while (epss_percentile) is unused
everywhere: findings are sorted by score, and the percentile is stored and displayed but is
never an access path.
WHAT WAS DELIBERATELY LEFT ALONE. Several other indexes were unused on SOME instances and busy
on others -- (test_id, false_p), (test_id, unique_id_from_tool, duplicate),
(test_id, out_of_scope), (unique_id_from_tool), and the three review-workflow FK indexes. They
track optional features, so a single-instance reading would have "proved" them dead and broken
the installs that use them. Only indexes unused across every instance sampled are dropped here.
Built with DROP INDEX CONCURRENTLY in a non-atomic migration, following 0280, so it takes no
exclusive lock on a large dojo_finding. IF EXISTS makes it idempotent; the reverse rebuilds
concurrently, so a downgrade does not lock the table either.
Verified:
- makemigrations dojo --check --dry-run reports "No changes detected", so the state operations
match the model and a later makemigrations stays a no-op.
- Applied against a migrated database: dojo_finding goes 49 indexes -> 45.
- Reversed: back to 49.
- ruff (repo config) clean.
|
This pull request contains critical security findings because two sensitive files were modified by an author not on the allowed list. Specifically, changes to
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in
|
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0295_drop_unused_finding_indexes.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by '' (commit aba7dc6) who is not in the allowed authors list. |
🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/models.py (drs_96652083)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/models.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by '' (commit aba7dc6) 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.
Member
There was a problem hiding this comment.
Reviewed with a focus on migration mechanics and whether any real query path still wants these indexes.
Per-index check against actual query paths
epss_percentile— one precision note on "never an access path": the column header sort is client-side DataTables only (nodojo_sortlink,findings_list_snippet.html:330) and the API ordering allowlist excludes it, but the UI filter panel's Ordering dropdown does offer "EPSS Percentile" (dojo/finding/ui/filters.py:227,:860), producing a genuine server-sideORDER BY epss_percentile. That said, those queries are always permission/product-scoped, so the planner would essentially never satisfy the sort via a single-column index scan — consistent withidx_scan = 0even on instances where users do use that ordering. Drop still looks safe; the claim just deserves the softer phrasing.
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
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.
What
dojo_findingis the busiest table in the schema and carries close to fifty indexes, every one maintained on each row written. Import is where that lands: a scan creating thousands of findings pays for all of them, per row.This drops four that no query uses.
dojo_findin_epss_pe_567499_idx(epss_percentile)— the largest of the fourdojo_findin_line_fea329_idx(line)dojo_findin_known_e_8c584e_idx(known_exploited)idx_finding_sev_open_unver(severity, -numerical_severity) WHERE active AND NOT verifiedEvidence
pg_stat_user_indexes.idx_scan = 0on four long-running production instances, three of them carrying roughly seventeen months of accumulated statistics, ranging from a few thousand to ~184k findings.The EPSS pair is the informative one:
(epss_score)is used,(epss_percentile)is not — anywhere. Findings get sorted by score; the percentile is stored and displayed but is never an access path.What was deliberately left alone
Several other indexes were unused on some instances and busy on others —
(test_id, false_p),(test_id, unique_id_from_tool, duplicate),(test_id, out_of_scope),(unique_id_from_tool), and the three review-workflow FK indexes.Those track optional features. A single-instance reading would have "proved" them dead and broken every install that uses them, so only indexes unused across every instance sampled are dropped here.
The limit of that evidence, stated plainly: four instances is a sample, and they share a deployment style. If any of these four serves a use case that sampling wouldn't see, I'd rather hear it than merge it — happy to drop any of the four from this PR.
Migration safety
DROP INDEX CONCURRENTLYin a non-atomic migration, following the pattern in0280_vulnerability_id_upper_index, so it takes no exclusive lock on a largedojo_finding.IF EXISTSmakes it idempotent, and the reverse rebuilds concurrently, so a downgrade doesn't lock the table either.Verification
makemigrations dojo --check --dry-run→ "No changes detected" — state operations match the model, so a latermakemigrationsstays a no-opruff(repo config) clean