Skip to content

feat: improved sbom filename extension handling - #4919

Closed
Saksham-Sirohi wants to merge 3 commits into
ossf:mainfrom
Saksham-Sirohi:improved-sbom-filename-extension-handling
Closed

Saksham-Sirohi wants to merge 3 commits into
ossf:mainfrom
Saksham-Sirohi:improved-sbom-filename-extension-handling

Conversation

@Saksham-Sirohi

Copy link
Copy Markdown
Contributor

Summary

This PR fixes SBOM parsing test failures caused by invalid CycloneDX file extensions. It enforces strict extension validation for CycloneDX files and improves error handling while ensuring backward compatibility.


Changes Introduced

  • Extension Validation:
    • CycloneDX SBOMs now require .json or .xml extensions. Invalid extensions trigger explicit error logging.
  • Test Fixes:
    • Updated test cases to use valid SBOM files from the test/sbom directory.
    • Added checks for error logging and empty results when invalid extensions are detected.
  • Code Refactoring:
    • Simplified extension validation logic in parse.py.

Checklist

  • ✅ Code adheres to project standards.
  • ✅ Tests pass (including updated SBOM test cases).

Steps to Test

pytest test/test_sbom.py -v

Verified

  • Tests for invalid CycloneDX extensions log errors and return empty results.
  • Valid CycloneDX files (JSON/XML) parse successfully.

Related Issues
Fixes #4836

Comment thread cve_bin_tool/sbom_manager/sbom_detection.py Fixed
Comment thread cve_bin_tool/sbom_manager/sbom_detection.py Fixed
Comment thread cve_bin_tool/sbom_manager/sbom_detection.py Fixed
@Saksham-Sirohi
Saksham-Sirohi force-pushed the improved-sbom-filename-extension-handling branch from 4389a69 to 780892a Compare March 13, 2025 09:43
@Saksham-Sirohi

Copy link
Copy Markdown
Contributor Author

hi @terriko required changes for URL sanitation has been made please take a look

@terriko

terriko commented Apr 14, 2025

Copy link
Copy Markdown
Collaborator

Ugh, looking at this again, I realize that we're going to have issues because splitting on . will cause problems when we have things like \. in the filename. Which I realize is unlikely but sloppy filename parsing in a security tool obviously is a bad look. I don't have the time necessary to help you get this into PR shape so I'm going to close it, but if you want to look at again please consider using the built in stuff from pathlib to make this work.

@terriko terriko closed this Apr 14, 2025
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.

bug: improved sbom filename extension handling

3 participants