Skip to content

feat: improved sbom filename extension handling #4919

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

Conversation

Saksham-Sirohi
Copy link
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

@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
Contributor Author

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

@terriko
Copy link
Contributor

terriko commented Apr 14, 2025

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
2 participants