Skip to content
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

SBOM workflow using "npm sbom" #4521

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Conversation

martinkuba
Copy link
Contributor

@martinkuba martinkuba commented Mar 2, 2024

Which problem is this PR solving?

This replaces #4479

This workflow is using the npm sbom command to generate multiple SBOM files: one for each package and one for the whole repository (opentelemetry-js.spdx). All files are combined in a single zip file. The workflow is triggered when a release is published.

NOTE: The npm sbom command makes it possible to exclude dev dependencies (--omit dev configuration). However, there seems to be a bug where sometimes dependencies are not captured (see npm/cli#7204 for more details). Once the npm issue is resolved, the --omit dev configuration should be added to the commands in this workflow).

An example output from this workflow is available here.

@martinkuba martinkuba requested a review from a team March 2, 2024 22:01
Copy link

codecov bot commented Mar 2, 2024

Codecov Report

Merging #4521 (ea28153) into main (3a426e8) will increase coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4521      +/-   ##
==========================================
+ Coverage   92.83%   92.84%   +0.01%     
==========================================
  Files         328      328              
  Lines        9486     9486              
  Branches     2035     2035              
==========================================
+ Hits         8806     8807       +1     
+ Misses        680      679       -1     

see 1 file with indirect coverage changes

@trentm
Copy link
Contributor

trentm commented Mar 6, 2024

Looking, for example at part of opentelemetry-js_api.spdx:

{
  "spdxVersion": "SPDX-2.3",
  "dataLicense": "CC0-1.0",
  "SPDXID": "SPDXRef-DOCUMENT",
  "name": "opentelemetry@0.1.0",
  "documentNamespace": "http://spdx.org/spdxdocs/opentelemetry-0.1.0-b79ed234-0b6a-4cbd-8ccb-b280340bb394",
  "creationInfo": {
    "created": "2024-03-02T21:48:21.872Z",
    "creators": [
      "Tool: npm/cli-10.5.0"
    ]
  },
  "documentDescribes": [
    "SPDXRef-Package-opentelemetry-0.1.0"
  ],
  "packages": [
    {
      "name": "opentelemetry",
      "SPDXID": "SPDXRef-Package-opentelemetry-0.1.0",
      "versionInfo": "0.1.0",
      "packageFileName": "",
      "description": "OpenTelemetry is a distributed tracing and stats collection framework.",
      "primaryPackagePurpose": "LIBRARY",
      "downloadLocation": "NOASSERTION",
      "filesAnalyzed": false,
      "homepage": "https://github.com/open-telemetry/opentelemetry-js#readme",
      "licenseDeclared": "Apache-2.0",
      "externalRefs": [
        {
          "referenceCategory": "PACKAGE-MANAGER",
          "referenceType": "purl",
          "referenceLocator": "pkg:npm/opentelemetry@0.1.0"
        }
      ]
    },
    {
      "name": "@opentelemetry/api",
      "SPDXID": "SPDXRef-Package-opentelemetry.api-1.8.0",
      "versionInfo": "1.8.0",
      "packageFileName": "node_modules/@opentelemetry/api",
      "description": "Public API for OpenTelemetry",
      "downloadLocation": "NOASSERTION",
      "filesAnalyzed": false,
      "homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/api",
      "licenseDeclared": "Apache-2.0",
      "externalRefs": [
        {
          "referenceCategory": "PACKAGE-MANAGER",
          "referenceType": "purl",
          "referenceLocator": "pkg:npm/%40opentelemetry/api@1.8.0"
        }
      ]
    },

Is it misleading that it references an opentelemetry@0.1.0 package? There actually is (perhaps by accident?) such a package published to npm: https://www.npmjs.com/package/opentelemetry?activeTab=versions
(That package is not malicious, BTW. It just contains a small package.json and useless one-line "index.js".)

Would setting "private": true in the top-level package.json change something here?

Perhaps all of this is fine. I don't have any experience with real use cases for SBOM files.

@trentm
Copy link
Contributor

trentm commented Mar 6, 2024

@martinkuba I guess you don't need final review until this is adjusted somehow to add this asset to the relevant GitHub Release?

I searched a bit for a way to do that and while there is a github.com provided "actions/upload-release-asset", it is unmaintained: https://github.com/actions/upload-release-asset
Basically it just calls the "upload-a-release-asset" GH API:
https://github.com/actions/upload-release-asset/blob/main/src/upload-release-asset.js#L22-L30

@martinkuba
Copy link
Contributor Author

@martinkuba I guess you don't need final review until this is adjusted somehow to add this asset to the relevant GitHub Release?

@trentm @pichlermarc Trent is correct that we would need an extra step in the workflow to add the artifact to a release. This brings up a question whether it should be attached to the API and experimental releases as well. And if yes, should we have different workflows for the core SDK, the API, and experimental packages? I think the answer is probably yes, and I can split this into three different workflows (or conditional steps). What do you think?

@martinkuba
Copy link
Contributor Author

Is it misleading that it references an opentelemetry@0.1.0 package? There actually is (perhaps by accident?) such a package published to npm: npmjs.com/package/opentelemetry?activeTab=versions (That package is not malicious, BTW. It just contains a small package.json and useless one-line "index.js".)

I tried with "private": true and did not make a difference. The version 0.1.0 is in the top-level package.json, so it is technically correct. Per the documentation of the field, the important part is that the value is unique for each file, which it is since it gets a unique UUID appended. In any case, the npm sbom does not have a way to customize it.

@martinkuba
Copy link
Contributor Author

I have made the following updates

  • added .json file extension to the generated SBOM files
  • removed the SBOM file for the whole repository
  • SBOM files are generated conditionally for core packages, experimental packages, or API based on the release tag
  • the SBOM zip file is added to the release automatically as an artifact

Copy link
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 🙂

@pichlermarc pichlermarc merged commit aabd1a9 into open-telemetry:main Mar 20, 2024
20 checks passed
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
* add sbom workflow

* generate sbom for each package

* generate sbom API

* add prefix to all files

* conditionally add artifacts to releases

---------

Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
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.

3 participants