Skip to content

fix(sbom): detect sha256 hashes in expression-form licenses in needs_fix#1911

Open
mesutoezdil wants to merge 1 commit into
NVIDIA:mainfrom
mesutoezdil:fix/sbom-needs-fix-expression
Open

fix(sbom): detect sha256 hashes in expression-form licenses in needs_fix#1911
mesutoezdil wants to merge 1 commit into
NVIDIA:mainfrom
mesutoezdil:fix/sbom-needs-fix-expression

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

CycloneDX allows licenses in 2 forms:

  • {"license": {"id": "MIT"}} // handled correctly
  • {"expression": "MIT OR Apache-2.0"} // not checked in needs_fix

needs_fix only looked at the license key, so components with a sha256 hash in expression form were silently skipped and never resolved.

This is the parallel fix to #1898, which fixed the same gap in extract_licenses. Adds tests covering both forms.

CycloneDX allows licenses as either {"license": {"id": "..."}} or
{"expression": "..."}. needs_fix only checked the license form, so
expression entries with sha256 hashes were silently skipped.

Add expression-form check to needs_fix, mirroring the fix in
extract_licenses (NVIDIA#1898). Add tests covering both forms.
@mesutoezdil mesutoezdil requested review from a team, derekwaynecarr and mrunalp as code owners June 15, 2026 19:41
@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@johntmyers johntmyers self-assigned this Jun 24, 2026

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix(sbom): detect sha256 hashes in expression-form licenses in needs_fix

Findings

  • [P1] deploy/sbom/test_resolve_licenses.py:11 will fail Python lint. The file mutates sys.path and then imports resolve_licenses on line 12; Ruff selects E in pyproject.toml:80, and python:lint runs against deploy/sbom/*.py via tasks/python.toml:200, so this trips E402 before the tests can help protect the fix. Use an import pattern that does not require a post-statement module import, or add an explicit local exemption if that is the accepted pattern.

  • [P2] deploy/sbom/test_resolve_licenses.py:15 is not covered by the default test runner. mise run test:python runs uv run pytest python/, and pytest is configured with testpaths = ["python"] plus python_files = ["*_test.py"] in pyproject.toml:107-109. This new file is under deploy/sbom/ and named test_*.py, so the regression tests are effectively dead unless invoked manually.

  • [P2] tasks/sbom.toml:80 still has the old unresolved-license detector. sbom:check only checks license.id and license.name, so an SBOM containing {"expression": "sha256:..."} can still report “All licenses resolved.” after this PR. That leaves the same false-negative in the user-facing SBOM check path; it should either reuse needs_fix or mirror the new expression handling.

Notes

The needs_fix production change itself matches the PR description.

Testing

Not run. This was a code-only review per the review workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants