Skip to content

feat(finding): multiple CWEs per finding#15143

Merged
Maffooch merged 10 commits into
devfrom
feat/cwe-vuln-id-consolidation
Jul 14, 2026
Merged

feat(finding): multiple CWEs per finding#15143
Maffooch merged 10 commits into
devfrom
feat/cwe-vuln-id-consolidation

Conversation

@valentijnscholten

@valentijnscholten valentijnscholten commented Jul 2, 2026 •
edited
Loading

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 0276–0278. 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.
image image image

@github-actions github-actions Bot added New Migration Adding a new migration file. Take care when merging. docs unittests ui parser labels Jul 2, 2026
@valentijnscholten valentijnscholten mentioned this pull request Jul 2, 2026
Merged
4 tasks
@valentijnscholten valentijnscholten added this to the 3.2.0 milestone Jul 2, 2026
@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 approved these changes Jul 2, 2026
View reviewed changes

@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

This was referenced Jul 4, 2026
Merged
Closed
@github-actions github-actions Bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR conflicts-detected labels Jul 5, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Maffooch
Maffooch requested a review from blakeaowens as a code owner July 14, 2026 00:24
@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from 354058e to 518df6c Compare July 14, 2026 01:07
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from da8c84a to 2f3a611 Compare July 14, 2026 02:13
valentijnscholten and others added 8 commits July 13, 2026 21:09
@valentijnscholten @Maffooch
feat(finding): multiple CWEs per finding
0b9d750 
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 @Maffooch
feat(generic): support multiple CWEs per finding in generic importers
f54df99 
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).
@valentijnscholten @Maffooch
feat(finding): make the CWE set a hashable dedup field (cwes)
8702d69 
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.
@valentijnscholten @Maffooch
docs(finding): note the primary cwe field can be removed in the future
b2e7f28 
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.
@valentijnscholten @Maffooch
fix(finding): compute cwe hash from finding_cwe_set directly, not the…
a42f9c7 
… 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.
@valentijnscholten @Maffooch
refactor(finding): move CWE helpers to dojo/finding/cwe.py
9ec9a1c 
Move cwe_number, cwe_label, parse_cwes, finding_cwe_labels out of
finding/vulnerability_id.py into a dedicated finding/cwe.py (CWEs are a weakness
class, distinct from vulnerability identifiers). resolve_vulnerability_id_type
stays in vulnerability_id.py. All importers updated.
@Maffooch @claude
fix(cwe): guard save_cwes on partial update; bound CWE number to colu…
86ca913 
…mn width

Two data-safety fixes for the multi-CWE relation:

- FindingSerializer.update() called save_cwes() unconditionally, so any
  PATCH/PUT that omitted `cwes` deleted a finding's extra Finding_CWE rows
  (leaving only the primary). Guard the call on `parsed_cwes is not None`,
  mirroring the vulnerability-id path.

- cwe_number() had no upper bound, but Finding_CWE.cwe stores "CWE-<n>" in a
  varchar(11). A number > 9,999,999 passed validation then raised DataError
  at insert (API/form 500, and would abort the 0280 backfill's bulk_create,
  since ignore_conflicts does not swallow DataError). Reject it as invalid
  input (None) so it becomes a clean 400/skip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch @claude
chore(migrations): renumber CWE migrations 0283/0284 after restack
a3af3c2 
Restacked onto the renumbered vulnerability-id chain; the CWE migrations move
0279/0280 -> 0283/0284 and re-parent onto 0282_unique_finding_vulnerability_id
so the branch has a single linear migration leaf again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch @claude
docs(upgrade): fix migration numbers and the redundant migrate_cwe st…
0b291ea 
…ep (F10)

- Update the migration list to the renumbered chain (0280-0284).
- Remove the contradiction: 0284_backfill_finding_cwe already backfills every
  existing finding automatically, so migrate_cwe is NOT a required upgrade
  step (it would repeat that work). Reframe migrate_cwe as an optional,
  idempotent re-scan for later use (e.g. after a parser CWE change).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from 2f3a611 to 0b291ea Compare July 14, 2026 03:15
@Maffooch @claude
test(perf): re-baseline importer perf counts on merged dev (CWE)
a269d08 
The pghistory_no_async_with_product_grading counts were placeholders from the
pre-merge branch base. Update import1/reimport1 to the actual counts on the
merged dev (from CI): the multi-CWE work adds ~2 queries per import phase and
the async-task counts follow dev's current values.

  Small:          import1 182q/5t, reimport1 141q/4t
  SmallLocations: import1 194q/5t, reimport1 155q/4t

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch
Maffooch merged commit 3edfceb into dev Jul 14, 2026
150 checks passed
@Maffooch
Maffooch deleted the feat/cwe-vuln-id-consolidation branch July 14, 2026 03:54
@Maffooch Maffooch mentioned this pull request Jul 14, 2026
Merged
@valentijnscholten valentijnscholten mentioned this pull request Jul 21, 2026
Merged
Maffooch pushed a commit that referenced this pull request Jul 23, 2026
@valentijnscholten
fix(api): explicit scalar cwe stays primary when cwes list is also su…
d604c30 
…pplied (#15305)

PR #15143 added multiple-CWE support (cwes object list, writable on
create and update). When a request supplied BOTH the scalar cwe and a
cwes list, cwes[0] unconditionally overwrote the explicit scalar - the
more specific input lost.

Precedence now mirrors the vulnerability_ids->cve pattern: an explicit
scalar cwe in the request stays the primary and every cwes entry is
treated as an extra row (save_cwes dedupes the overlap); without a
scalar, cwes[0] becomes the primary and is mirrored into the scalar.
Detection uses initial_data presence - the established pattern in this
serializer (FindingTemplateSerializer).

Behavior change only for the unreleased cwes field when both inputs
appear in one request; no released consumer can depend on the old
behavior. Wire shape unchanged.

+8 API tests: precedence on create/update, cwes-only primary promotion,
replace, omission-untouched, explicit-empty semantics, read-back shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mtesauro mtesauro mtesauro approved these changes
@Maffooch Maffooch Maffooch approved these changes
@blakeaowens blakeaowens Awaiting requested review from blakeaowens blakeaowens is a code owner

Assignees

No one assigned

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

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@valentijnscholten @mtesauro @Maffooch