Skip to content

fix(sbom): handle SPDX expression licenses in extract_licenses#1898

Merged
maxamillion merged 1 commit into
NVIDIA:mainfrom
mesutoezdil:fix/sbom-csv-expression-license
Jun 22, 2026
Merged

fix(sbom): handle SPDX expression licenses in extract_licenses#1898
maxamillion merged 1 commit into
NVIDIA:mainfrom
mesutoezdil:fix/sbom-csv-expression-license

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

CycloneDX allows licenses in 2 forms:

{"license": {"id": "MIT"}}; handled correctly

{"expression": "MIT OR Apache-2.0"}; silently dropped

extract_licenses only checked for the license key, so any component
using the expression form got an empty license field in the CSV output.

Add a check for expression before falling back to the license block.

CycloneDX allows licenses as either {"license": {"id": "..."}} or
{"expression": "MIT OR Apache-2.0"}. The expression form was silently
dropped, producing an empty license field in the CSV output.
@mesutoezdil mesutoezdil requested review from a team, derekwaynecarr and mrunalp as code owners June 13, 2026 10:42
@copy-pr-bot

copy-pr-bot Bot commented Jun 13, 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 added the gator:watch-pipeline Gator is monitoring PR CI/CD status label Jun 13, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This is a small, concentrated SBOM tooling bug fix. The PR addresses a documented CycloneDX license form (expression) that the existing CSV export dropped, touches one deploy script, and no duplicate issue or PR was found for the same fix.
Head SHA: e18f4270b57875c60e817219db31c120e0c95bef

Review findings:

  • No blocking findings remain.
  • Non-blocking test note: there is no direct regression coverage for deploy/sbom/sbom_to_csv.py; a focused test for both {"expression": "MIT OR Apache-2.0"} and {"license": {"id": "MIT"}} would reduce reintroduction risk.

Docs: Fern docs update is not needed because this only changes generated SBOM CSV extraction behavior and does not alter a documented user-facing workflow, CLI contract, provider flow, policy syntax, or published API.

E2E: No test:e2e, test:e2e-gpu, or test:e2e-kubernetes label is needed for this SBOM CSV utility change.

Next state: gator:watch-pipeline

@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test e18f427

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Maintainer Approval Needed

Gator validation and PR monitoring are complete.

Validation: This is a small, concentrated SBOM CSV extraction fix for CycloneDX SPDX expression licenses in deploy/sbom/sbom_to_csv.py.
Review: No blocking findings remain. I noted only non-blocking regression-test coverage risk for extract_licenses.
Docs: Fern docs update is not needed because this changes generated SBOM CSV extraction behavior only and does not alter a documented user-facing workflow, CLI contract, provider flow, policy syntax, or published API.
Checks: Required gates are green for head e18f4270b57875c60e817219db31c120e0c95bef: OpenShell / Branch Checks, OpenShell / Helm Lint, OpenShell / E2E, and OpenShell / GPU E2E.
E2E: No test:e2e, test:e2e-gpu, or test:e2e-kubernetes label was needed; the required E2E gate statuses passed as not applied.

Human maintainer approval or merge decision is now required.

@johntmyers johntmyers added gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Jun 13, 2026
@elezar

elezar commented Jun 15, 2026

Copy link
Copy Markdown
Member

Question: do we know how the CSV licenses column is consumed downstream? Preserving licenses[].expression verbatim seems right for review output, but an SPDX expression like MIT OR Apache-2.0 can represent multiple license choices. If any consumer splits this field expecting only individual SPDX IDs joined by |, we may need to document that the column can contain SPDX expressions or normalize it differently.

@mesutoezdil

Copy link
Copy Markdown
Contributor Author

Question: do we know how the CSV licenses column is consumed downstream? Preserving licenses[].expression verbatim seems right for review output, but an SPDX expression like MIT OR Apache-2.0 can represent multiple license choices. If any consumer splits this field expecting only individual SPDX IDs joined by |, we may need to document that the column can contain SPDX expressions or normalize it differently.

i v checked the repo, and no downstream consumer of the csv was found. resolve_licenses.py only processes json files and never touches the csvoutput. csv appears to be generated for manual review only.
if there is a known pipeline consumer expecting pipe-separated spdxids, i can normalize expression values before writing to the column. or verbatim seems fine as-is.. any case, happy to add a column comment/header note either way.

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Re-check After Author Update

I re-evaluated latest head e18f4270b57875c60e817219db31c120e0c95bef after mesutoezdil's 2026-06-15 12:37 UTC response to the maintainer question about downstream consumers of the CSV licenses column.

Disposition: resolved for gator gating. I checked the repository references and found deploy/sbom/sbom_to_csv.py is the only in-repo CSV writer/reader for this SBOM CSV path, while deploy/sbom/resolve_licenses.py operates on CycloneDX JSON and does not consume the CSV output. Preserving the CycloneDX SPDX expression verbatim is still the least surprising representation for manual review output unless a maintainer knows of an external consumer that requires normalization.

Remaining items:

  • No blocking gator items remain.
  • A human maintainer still needs to make the approval or merge decision.

Next state: gator:approval-needed

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Maintainer Review Nudge

This PR has been in gator:approval-needed for more than 48 business hours with no maintainer approval or merge decision. The latest gator approval-needed disposition was posted on 2026-06-15 15:25 UTC, and the required status contexts are still green for head e18f4270b57875c60e817219db31c120e0c95bef.

@NVIDIA/openshell-maintainers @NVIDIA/openshell-codeowners @mrunalp @maxamillion @derekwaynecarr, can someone review and either approve, request changes, or close this out?

@mesutoezdil

Copy link
Copy Markdown
Contributor Author

any news?

@johntmyers

johntmyers commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Re-check After Author Update

I re-evaluated latest head e18f4270b57875c60e817219db31c120e0c95bef after mesutoezdil's 2026-06-22 07:14 UTC comment asking for an update.

Disposition: no new gator blocker was found. The PR remains ready for human maintainer approval or merge decision; the required status contexts are still green for the current head.

Remaining items:

  • No blocking gator review items remain.
  • A human maintainer still needs to approve, request changes, merge, or close the PR.

Next state: gator:approval-needed

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Maintainer Review Nudge

This PR has been in gator:approval-needed for more than 48 business hours since the previous maintainer nudge on 2026-06-18 14:37 UTC, with no maintainer approval or merge decision. The author asked for an update on 2026-06-22 07:14 UTC, and gator re-checked head e18f4270b57875c60e817219db31c120e0c95bef; no new blocker was found and the required status contexts remain green.

@NVIDIA/openshell-maintainers @NVIDIA/openshell-codeowners @mrunalp @maxamillion @derekwaynecarr, can someone review and either approve, request changes, merge, or close this out?

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Approval Received

Maintainer approval was recorded for head e18f4270b57875c60e817219db31c120e0c95bef: #1898 (review)

Delta since the latest gator nudge: approval is now present, the PR is mergeable, and the required status contexts remain green. Existing same-SHA gator review disposition: #1898 (comment)

Remaining item: a maintainer merge or close decision.

Next state: gator:approval-needed

@johntmyers johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Jun 22, 2026
@maxamillion maxamillion merged commit 85c52bb into NVIDIA:main Jun 22, 2026
34 of 36 checks passed
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: The PR was in gator:merge-ready for head e18f4270b57875c60e817219db31c120e0c95bef; maintainer approval was present, required checks were green, and no blocking gator review items remained before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

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.

4 participants