Skip to content

feat(finding): multiple CWEs per finding#15143

Open
valentijnscholten wants to merge 6 commits into
devfrom
feat/cwe-vuln-id-consolidation
Open

feat(finding): multiple CWEs per finding#15143
valentijnscholten wants to merge 6 commits into
devfrom
feat/cwe-vuln-id-consolidation

Conversation

@valentijnscholten

@valentijnscholten valentijnscholten commented Jul 2, 2026

Copy link
Copy Markdown
Member

🔗 PR stack — CWE / vulnerability-ID consolidation (OSS)

Merge bottom-up:

  • #15145 — autodetected vulnerability-ID type + uniqueness constraint
    • #15143 — multiple CWEs per finding
      • #15154 — docs: Pro set-based dedup hash-code fields
      • #15155 — pluggable false-positive-history candidate filter

👉 This PR: #15143


Stacked on top of: #15145

Description

A finding could previously store only one CWE — the integer Finding.cwe field. This PR lets a finding carry multiple CWEs, using the same approach we already use for vulnerability ids (CVE, GHSA, …):

  • The primary CWE stays on Finding.cwe (unchanged). Legacy behaviour, hash codes and existing deduplication continue to use it exactly as before.
  • Additional CWEs are stored in a new Finding_CWE relationship (unique per (finding, cwe)), exactly like Vulnerability_Id rows hang off a finding for vulnerability ids.
  • The finding exposes the merged list via finding.cwes (primary first, then additional, deduplicated) — mirroring the existing finding.vulnerability_ids property.

CWEs can now be added in three places, matching how vulnerability ids already flow through the system:

  • UI — the finding add/edit forms and the finding detail page accept and display multiple CWEs.
  • API — findings expose a cwes list field that reads and writes the additional CWEs, mirroring the existing vulnerability_ids field.
  • Parsers — parsers can populate finding.unsaved_cwes; on import the primary CWE plus any additional CWEs the parser supplies are persisted. Tier-2 parsers that surface more than one CWE per finding now emit all of them.

CWEs are stored canonically as CWE-<n> strings. A CWE is a weakness class, not a vulnerability instance identifier, so it is modelled separately from vulnerability ids and does not participate in hash_code or the cve field. Because of that separation, existing hash codes and deduplication are unaffected by this change.

Stacked on top of #15145. This PR is stacked on the vulnerability-id-type change (#15145). Its base is dev, so until #15145 merges the diff/commits below also include #15145's changes — review and merge #15145 first, then this PR. The CWE-specific work is the top commit; its migrations 0279_finding_cwe / 0280_backfill_finding_cwe chain after #15145's 02760278. After #15145 merges to dev, this PR's diff will reduce to the CWE-only changes.

Migrations (this PR adds the last two; the first three come from #15145)

  • 0279_finding_cwe — creates the Finding_CWE table (unique per (finding, cwe)).
  • 0280_backfill_finding_cwe — seeds Finding_CWE rows from the legacy Finding.cwe values.

To backfill CWEs for existing findings after upgrading, run the idempotent command:

manage.py migrate_cwe

Test results

  • New unittests/test_finding_cwe.py covers the CWE string helpers, the Finding_CWE model, the finding.cwes property, copy-on-clone, and the cwes API field.
  • Parser tests updated/added across ~30 parsers (with new multi-CWE scan fixtures) to assert that multiple CWEs are parsed and persisted per finding.
  • Performance baselines (test_importers_performance.py, test_tag_inheritance_perf.py) updated for the added Finding_CWE store/reconcile queries.

Documentation

  • docs/content/releases/os_upgrading/3.2.md describes both stacked changes, the migrations, and the migrate_cwe backfill command.

Checklist

  • Rebased against the latest dev.
  • Submitted against dev.
  • Model changes include the necessary migrations in dojo/db_migrations.
  • Added tests to the unit tests.

…aint

Adds Vulnerability_Id.vulnerability_id_type, autodetected from the id's
leading prefix (CVE-2024-1234 -> CVE, GHSA-... -> GHSA), stored and indexed
so identifiers can be filtered/grouped by type. Populated on import (bulk)
and on save(); existing rows backfilled by migration.

Also de-duplicates (finding, vulnerability_id) rows and adds a unique
constraint on the pair. CWE is a weakness class and is intentionally NOT
part of this change; vulnerability_id_type does not participate in
hash_code, so existing hash codes and deduplication are unaffected.

Migrations: 0276 (type column + lookup index), 0277 (dedupe + backfill,
data), 0278 (unique constraint).
Stacked on top of the vulnerability_id-type change (feat/vulnerability-id-type).

Adds a Finding_CWE relationship so a finding can carry multiple CWEs, mirroring
vulnerability ids: the primary CWE stays on Finding.cwe; additional CWEs live in
the relationship and are exposed via finding.cwes. Wired through the UI, the API
(cwes field), and parsers (finding.unsaved_cwes). CWE is a weakness class, kept
out of hash_code and the cve field, so existing hash codes/dedup are unaffected.

Migrations 0279_finding_cwe (create table) and 0280_backfill_finding_cwe (seed
from legacy Finding.cwe), chained after the vulnerability_id migrations (0278).
Backfill existing findings with: manage.py migrate_cwe
@valentijnscholten valentijnscholten force-pushed the feat/cwe-vuln-id-consolidation branch from e0216e4 to e984c77 Compare July 2, 2026 20:37
@valentijnscholten valentijnscholten marked this pull request as ready for review July 2, 2026 21:06

@mtesauro mtesauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

The generic JSON and CSV importers emitted only a single `cwe` per finding,
so a finding could not be imported with a CWE *set*. Mirror the existing
`vulnerability_ids` handling to accept multiple CWEs:

- JSON: accept an optional `cwes` list (popped like `vulnerability_ids`,
  since it is not a Finding field). The first entry becomes the primary
  `finding.cwe` when `cwe` is not already given; the full set is kept on
  `finding.unsaved_cwes`. Mixed int / "CWE-<n>" forms are supported.
- CSV: accept a delimited `CweIds` column alongside the existing single
  `CweId`, with the same primary-plus-set behavior; merge CWE sets across
  internally-deduplicated rows like vulnerability ids.

The import pipeline persists the set via `Finding_CWE`; normalization and
deduplication happen in `finding_cwe_labels()`. The legacy single `cwe`
path is unchanged.

Adds a fabricated multi-CWE fixture and parser tests (JSON + CSV).
Add 'cwes' to HASHCODE_ALLOWED_FIELDS and hash it via a new get_cwes(), which
mirrors get_vulnerability_ids. Because the primary cwe is a scalar that is always
set (so Finding.cwes is non-empty even before the extra Finding_CWE rows are
written during import), get_cwes prefers unsaved_cwes whenever present, so the
pre-save and post-save import hashes agree.

Tests: get_cwes prefers unsaved_cwes, is stable across save, empty with no cwe,
and cwes participates in compute_hash_code.
@github-actions github-actions Bot added the settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR label Jul 5, 2026
Mirror the existing cve/vulnerability_ids deprecation note: the single primary
cwe field is merged into the cwes set for backward compatibility and can be
removed once no longer needed.
… cached property

get_cwes() used the cwes @cached_property for the saved path; if finding.cwes was
accessed before the Finding_CWE rows were written (serializer/signal/log), it cached
an empty/partial set and the hash then used that stale value -> nondeterministic
hash_code (flaky dedup under parallel test runs). Mirror get_vulnerability_ids: read
the finding_cwe_set reverse relation directly (uncached, prefetch-honoring), falling
back to the unsaved values before save. save_cwes stores the primary cwe as a row too,
so finding_cwe_set carries the full set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs New Migration Adding a new migration file. Take care when merging. parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants