-
Notifications
You must be signed in to change notification settings - Fork 1.8k
docs: Expand CycloneDX parser documentation with detailed field mappings #13750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
docs: Expand CycloneDX parser documentation with detailed field mappings #13750
Conversation
Added detailed documentation for CycloneDX parser including: - Field mapping tables for both JSON and XML formats - Special processing notes for dates, severity, status conversion - CVSS processing details and component lookup mechanics - Deduplication and vulnerability ID collection explanations - Legacy format support and namespace handling (XML) Authored by T. Walker - DefectDojo
|
Thanks for the PR @skywalke34, looks like it needs a small bit of tuning to pass the unit tests that ensures consistency across parser docs: |
| | serialNumber | - | N/A | BOM serial number, not used in findings | | ||
| | version | - | N/A | BOM version number, not used in findings | | ||
| | metadata.timestamp | date | 17-20 | Parsed and set as finding date if present | | ||
| | components | - | 21, 148-156 | Flattened into dictionary for lookup by bom-ref | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the conclusion of earlier talks about this. But my view is that adding (and maintaining) line numbers here is too fine grained. It adds work to anyone making changes to the parser and it adds work to us as reviewers to double check any changes. I think the line numbers are only useful for people who can read source code and once they can, they'll be able to find the code sections easily even if we don't specify the line numbers in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the above, would create an ongoing docs maintenance requirement and would have limited relevance to a viewer in this specific context ^ FYI @skywalke34
|
|
||
| **Implementation:** | ||
| ```python | ||
| # JSON: json_parser.py lines 17-20 | ||
| report_date = None | ||
| if "metadata" in data and "timestamp" in data["metadata"]: | ||
| report_date = dateutil.parser.parse(data["metadata"]["timestamp"]) | ||
|
|
||
| # XML: xml_parser.py lines 31-34 | ||
| report_date = tree.find("b:metadata/b:timestamp", ns) | ||
| if report_date is not None: | ||
| report_date = dateutil.parser.parse(report_date.text) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preference would be to leave this out.
|
|
||
| **JSON Format:** The parser sets `vuln_id_from_tool` (line 78) which is used by DefectDojo's deduplication logic. | ||
|
|
||
| **XML Format:** The parser sets `vuln_id_from_tool` (lines 137, 224) which may be used by DefectDojo's deduplication algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readers may wonder what the "... may be used ..." means?
| ### Total Fields in JSON | ||
|
|
||
| - Total data fields: 45 | ||
| - Total data fields parsed: 20 | ||
| - Total data fields NOT parsed: 25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it looks nice, but does it provide real value? My first instinct is we should leave it out.
| CycloneDX is a lightweight software bill of materials (SBOM) standard designed for use in application security contexts and supply chain component analysis. | ||
|
|
||
| From: https://www.cyclonedx.org/ | ||
| # CycloneDX Parser Documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line, or a title of any kind, is not required as we have the parser title in the front matter already (see line 2)
fyi @skywalke34 I am going to take this out
| | vulnerabilities[].ratings[].source.name | - | N/A | Rating source name not mapped | | ||
| | vulnerabilities[].ratings[].source.url | - | N/A | Rating source URL not mapped | | ||
| | vulnerabilities[].ratings[].score | cvssv3_score | 91-99 | Extracted from CVSSv3 vector calculation | | ||
| | vulnerabilities[].ratings[].severity | severity | 36-38, 95-97 | Fixed via fix_severity helper, overridden by CVSS calculation if available | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not consistent with what the parser says - in the case of CycloneDX we don't always override with CVSS. Instead CVSS is used as a fallback if Severity is not present in the file.
|
here would be my feedback if you want to apply more of these template changes:
There's at least one error in this PR where the language misunderstands our source code, see my comment for an example. I don't think we can merge this without checking each mapping detail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Authored by T. Walker - DefectDojo