Skip to content

Generic Findings Import: fix CSV KEV/fix booleans, CVSSV3_score and empty cells; document every field - #16043

Merged
Maffooch merged 6 commits into
bugfixfrom
fix/generic-csv-bool-cvss3-score
Sep 23, 2026
Merged

Maffooch merged 6 commits into
bugfixfrom
fix/generic-csv-bool-cvss3-score

Conversation

@Maffooch

Copy link
Copy Markdown
Contributor

Description

Fixes three bugs in the Generic Findings Import CSV parser (dojo/tools/generic/csv_parser.py) and brings its documentation up to date with how the parser actually behaves.

Parser fixes:

  • known_exploited, ransomware_used and fix_available were converted with bool(str), so any non-empty value, including FALSE, became true. They now use the same _convert_bool rule as the other CSV booleans (true when the value starts with t or T). An empty cell leaves the model default (false, false, and unset).
  • CVSSV3_score was documented but never read. It is now parsed like CVSSV4_score. Finding.save() still recalculates the score from a valid CVSSV3 vector.
  • An empty cell in CweId, epss_score, epss_percentile, CVSSV4_score, MitigatedDate or kev_date aborted the whole import (int(''), float(''), dateutil ParserError). Empty and whitespace-only cells are now skipped, which matches the JSON format, where an omitted field stays unset. A non-numeric value such as N/A still fails the import.

Documentation (docs/content/supported_tools/parsers/):

  • file/generic.md is now a field reference. Every CSV column and JSON field has its data type, example values and a note. The audit behind it also documents behavior the old page left out or got wrong:
    • CSV: required columns, case-sensitive headers, a byte order mark breaking the first column, row merging, and the CweIds, CVE and Vulnerability Id columns.
    • JSON: the cwes key, the endpoint and file formats, rejection of unknown keys, severity normalization, YYYY-MM-DD-only date fields, and repeated unique_id_from_tool handling.
    • A new Test Type metadata section explains that static_tool, dynamic_tool and soc are written to the shared Test Type.
    • The old example JSON had a trailing comma and did not parse. It has been replaced.
  • generic_findings_import.md is rewritten as the entry page. It explains choosing CSV or JSON, importing and reimporting, what makes an import fail, Test Type naming (the report-level name key has no effect), and tuning deduplication by Test Type name. It links out to the relevant sections.

Test results

  • New TestGenericCSVParserCellValues in unittests/tools/test_generic_parser.py, plus a new fixture unittests/scans/generic/generic_csv_empty_cells_and_booleans.csv. It covers TRUE, FALSE, empty and whitespace-only rows, CVSSV3_score, and empty numeric and date cells. Before the fix these tests failed with known_exploited parsed as True, 9.8 != None, and ParserError: String does not contain a date. They pass now.
  • unittests.tools.test_generic_parser, unittests.test_parsers, unittests.test_importers_importer and unittests.test_generic_meta_import: 130 tests, all passing.
  • ruff check --config ruff.toml (0.16.5) passes on the changed Python files.
  • Built the docs site with Hugo and checked both pages in a browser. All internal links and section anchors on the overview page resolve.

Documentation

Included in this PR.

Checklist

  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is Ruff compliant.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

🤖 Generated with Claude Code

Maffooch and others added 6 commits September 22, 2026 17:44
…ric Findings Import

Audit the Generic Findings Import parser guide against dojo/tools/generic
and the importer, and rewrite it as per-field reference tables with a data
type, example values and behavior notes for every CSV column and JSON key.

Gaps closed:
- CSV: required columns, case-sensitive headers, BOM breakage, row merging
  on severity+title+description, CweIds / CVE / Vulnerability Id columns,
  empty-cell failures for numeric/date columns, the Active empty-cell
  behavior, and that known_exploited / ransomware_used / fix_available
  read any non-empty value (including FALSE) as true.
- CSV: remove CVSSV3_score, which the parser never reads.
- JSON: cwes key, endpoints/files/tags schemas, report-level keys
  (version, description, static_tool, dynamic_tool, Pro-only soc; name is
  unused), unknown-key rejection, severity normalization, numeric
  coercion, YYYY-MM-DD-only date fields, thread_id is an integer,
  numerical_severity is ignored, repeated unique_id_from_tool handling.
- Fix the example JSON, which had a trailing comma and did not parse.
- Test Type naming: CSV always uses Generic Findings Import; reimport
  type-mismatch rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, skip empty cells

The Generic Findings Import CSV parser:
- converted known_exploited, ransomware_used and fix_available with
  bool(str), so any non-empty value, including "FALSE", became True. They
  now use the same rule as the other CSV booleans (_convert_bool), and an
  empty cell leaves the model default in place.
- never read the CVSSV3_score column. It is now parsed like CVSSV4_score;
  Finding.save() still recalculates it from a valid CVSSV3 vector.
- aborted the whole import on an empty CweId, epss_score, epss_percentile,
  CVSSV4_score, MitigatedDate or kev_date cell (int('') / float('') /
  dateutil ParserError). Empty cells are now skipped, matching JSON, where
  an omitted field stays unset.

Docs: update the CSV column table and boolean rules to match, and add a
Test Type metadata section explaining that static_tool, dynamic_tool and
soc are written to the shared Test Type (last import wins, omitted keeps
the current value, a report without "type" changes the built-in Generic
Findings Import Test Type, quoted "false" rejects the import).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d numeric cells

Gives the Generic Findings Import CSV fix a scan fixture (true, false,
empty and whitespace-only rows), exercised by a unit test here and usable
by downstream parser parity harnesses that walk unittests/scans.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…able

The CSV column, JSON report-level and JSON finding-field tables now use
three columns (name, type and examples, notes), with the type above its
examples. At narrower content widths the four-column layout squeezed the
notes into a thin strip. Also shortens two unbreakable examples that
pushed the finding-field table past the content width.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fills the 15 JSON finding-field rows that had no note: what the field
holds, plus the non-obvious behavior where it exists (an import-request
service overrides the report's and scopes close-old-findings; tags must
be a list; component and SAST source fields feed Locations; thread_id
defaults to 0; blank component values are stored as empty).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Makes the overview page accurate and a hub into the rest of the docs:
- CSV vs JSON, with links to each format section and example in the
  Parser Guide.
- How to import (Import Scan form, /api/v2/import-scan/) and reimport,
  including the Test type mismatch rule.
- What makes an import fail for CSV and JSON, and that an unrecognized
  CSV severity becomes Info.
- Test Type naming with examples that no longer imply the report-level
  name field does anything (it is ignored), plus the shared Test Type
  metadata caveat.
- Deduplication: the default hash fields and how to tune a generic tool
  by its Test Type name in open source (settings) and Pro (Deduplication
  Tuning).
- Links the canonical Universal Parser page and the sample reports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@skywalke34 skywalke34 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.

Thanks for this. It overlaps almost completely with a docs-only accuracy pass I'd been preparing for these two pages after a user reported errors importing pen-test results they had converted to JSON with AI. Rather than open a competing PR, I've written the remaining findings as a prompt you can hand straight to your agent on this branch. Each item has already been checked against bugfix and a running 3.3.200 instance, but the prompt asks for independent confirmation before anything changes.

You are working on the branch for DefectDojo PR #16043 ("Generic Findings Import: fix CSV KEV/fix
booleans, CVSSV3_score and empty cells; document every field"). An independent audit of the PR's
docs against the code produced the items below. For EACH item:

  1. Verify the claim yourself against the code on this branch. Cite file:line. Where static reading
     is not conclusive, prove it by running the parser or the model-field conversion (parser level
     only; do not write to the database).
  2. Report CONFIRMED or REFUTED with the evidence. Do not apply an item you could not confirm.
  3. Apply each CONFIRMED item to docs/content/supported_tools/parsers/file/generic.md and/or
     docs/content/supported_tools/parsers/generic_findings_import.md, matching the PR's existing
     style (tables, "Type and examples" / "Notes" columns, root-relative links).

Items:

A. CORRECTION, file/generic.md > "File rules", merge bullet. It says merged rows have "their
   vulnerability IDs and CWEs are combined". Claim: only `CweIds` values are combined. The merge
   block in dojo/tools/generic/csv_parser.py extends unsaved_vulnerability_ids, unsaved_cwes and the
   locations/endpoints, but a different `CweId` on a later row is discarded and the first row's
   `CweId` stays primary. Proposed wording: "their vulnerability IDs and `CweIds` values are combined
   (the first row's `CweId` is kept)".

B. CORRECTION, file/generic.md > "Attached files" example. The `data` value is the truncated
   placeholder "iVBOR...<...>TkSuQmCC", which fails base64.b64decode with "Incorrect padding" if
   copied. Replace it with a real 1x1 PNG so the example imports as-is:
   iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGNgAAIAAAUAAXpeqz8AAAAASUVORK5CYII=
   Verify by decoding it and by running the JSON through GenericParser().get_tests().

C. GAP, file/generic.md > "Test Type metadata", "Send JSON booleans" bullet (and the matching
   JSON bullet in generic_findings_import.md > "What stops an import"). Claim: when a quoted or
   non-boolean `static_tool`/`dynamic_tool` fails the import, the Test has already been created.
   dojo/importers/base_importer.py creates the Test (create_test, around the
   resolve_dynamic_test_type_name call) before update_test_type_from_internal_test raises on
   test_type.save(), with no surrounding transaction, so an empty Test is left behind (reported
   stuck at 50%), plus a new Test Type if `type` was new. The Django error text, e.g.
   '"manual penetration test" value must be either True or False.', does not name the field.
   Proposed addition: "The error does not name the field, and the Test has already been created,
   so delete the empty Test before re-importing the corrected file."

D. GAP, file/generic.md > "Finding fields", `title` row. Claim: titles are title-cased on save
   (Finding.persisted_title() calls titlecase(), which also collapses whitespace), so
   "Stored XSS in profile display name" is stored as "Stored XSS in Profile Display Name". Verify
   with Finding.persisted_title(...). Proposed note: "Stored in title case, so the imported title
   may differ in capitalization from the source."

E. GAP, generic_findings_import.md. Add a short "Recording manual penetration test results"
   section. Manual pen-test findings (often transcribed from PDF reports with AI help) are the most
   common non-tool use, and those users are the ones who trip over label-style `dynamic_tool`
   values, extra keys and quoted booleans. Confirm that this example parses and resolves to the
   Test Type "Manual Penetration Test Scan (Generic Findings Import)":

   {
     "type": "Manual Penetration Test",
     "description": "Findings transcribed from the Q3 2026 external pen test report.",
     "findings": [
       {
         "title": "Stored XSS in profile display name",
         "severity": "High",
         "description": "The display name field is rendered without output encoding.",
         "date": "2026-09-15",
         "cwe": 79,
         "mitigation": "HTML-encode user-supplied values on output.",
         "active": true,
         "verified": true,
         "unique_id_from_tool": "PT-2026-Q3-001",
         "endpoints": ["https://portal.example.com/profile"],
         "tags": ["manual-pentest", "pentest-2026-q3"]
       }
     ]
   }

   Points to make with it: `type` gives a dedicated, filterable Test Type; only `title`, `severity`
   and `description` are required; tags can track the engagement or quarter;
   `unique_id_from_tool` must be unique within the file; and the title will be shown title-cased.

F. MINOR, Test Type naming table, suffix row (both pages). Claim: the verbatim rule requires a
   leading space, because base_importer.resolve_dynamic_test_type_name checks
   raw_type.endswith(f" ({self.scan_type})"). "Tool1(Generic Findings Import)" becomes
   "Tool1(Generic Findings Import) Scan (Generic Findings Import)". Proposed: add "(with a space
   before the parenthesis)".

G. MINOR, reimport note (both pages). Claim: the report-level `description` and `version` are
   re-applied to the Test on every reimport, not only the Test Type flags, because
   update_test_from_internal_test runs on both the import and reimport paths. Proposed: add a
   half-sentence next to the `Test type mismatch` note.

After applying the confirmed items:
  - Validate every JSON code block on both pages with json.loads, and run each complete example
    through GenericParser().get_tests("Generic Findings Import", fh).
  - Build the docs as .github/workflows/validate_docs_build.yml does and run its lychee
    internal-link check. Confirm there are no new errors on these two pages.
  - Confirm unittests/test_parsers.py still passes for the generic docs.
  - Reply with a table: item, CONFIRMED/REFUTED, evidence (file:line or probe output), and change
    made.

Out of scope for this PR (open issues instead if you agree): validate `static_tool`/`dynamic_tool`
in the parser before the Test is created and name the key in the error; turn a missing required
CSV column into a validation error instead of a KeyError (a 500 through the API); and make an
unknown endpoint-object key a clean error instead of a TypeError.

Everything else in the PR matched the code when I checked it: severity handling on import and reimport, the date formats (including YYYYMMDD for the strict fields), cwe string handling, unique_id_from_tool de-duplication, the upload extension list, endpoint keys in both Locations modes, a single-string tags being ignored, the 400 mapping for ValidationError, the BOM behavior, and the new CSV empty-cell and boolean handling.

@Maffooch
Maffooch added this pull request to the merge queue Sep 23, 2026
Merged via the queue into bugfix with commit 83580ec Sep 23, 2026
49 checks passed
@Maffooch
Maffooch deleted the fix/generic-csv-bool-cvss3-score branch September 23, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants