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

Does repository_license_choices has an effect on CycloneDXReporter sbom output? #7182

Closed
mawl opened this issue Jun 22, 2023 · 7 comments
Closed
Labels
bug Issues that are considered to be bugs reporter About the reporter tool

Comments

@mawl
Copy link

mawl commented Jun 22, 2023

Hey,

I wonder if repository_license_choices has an effect on sbom output generated by CycloneDXReporter.

licenseChoice = LicenseChoice().apply { licenses = licenseObjects }

Here's an example:

analyzer-result.yml

   - id: "NPM::pizzip:3.1.4"
      purl: "pkg:npm/pizzip@3.1.4"
      authors:
      - "Edgar Hipp"
      declared_licenses:
      - "(MIT OR GPL-3.0)"
      declared_licenses_processed:
        spdx_expression: "GPL-3.0-only OR MIT"
        mapped:
          (MIT OR GPL-3.0): "MIT OR GPL-3.0-only"
      description: "Create, read and edit .zip files synchronously with Javascript"
      homepage_url: "https://github.com/open-xml-templating/pizzip#readme"
...

evaluation-result.yml

 config:
...
    license_choices:
      repository_license_choices:
      - given: "GPL-3.0-only OR MIT"
        choice: "MIT"
      - given: "MIT OR GPL-3.0-only"
        choice: "MIT"
...
   - id: "NPM::pizzip:3.1.4"
      purl: "pkg:npm/pizzip@3.1.4"
      authors:
      - "Edgar Hipp"
      declared_licenses:
      - "(MIT OR GPL-3.0)"
      declared_licenses_processed:
        spdx_expression: "GPL-3.0-only OR MIT"
        mapped:
          (MIT OR GPL-3.0): "MIT OR GPL-3.0-only"
      description: "Create, read and edit .zip files synchronously with Javascript"
      homepage_url: "https://github.com/open-xml-templating/pizzip#readme"

ORT Scan Report > Tree

Effective SPDX is MIT, license choice works:
image

bom.cyclonedx.json

Still both licenses in sbom - license choice has no effect.

    {
      "group": "",
      "name": "pizzip",
      "version": "3.1.4",
      "description": "Create, read and edit .zip files synchronously with Javascript",
      "scope": "required",
      "hashes": [
        {
          "alg": "SHA-1",
          "content": "0c2578506ce5b487fa00bc2dd62eebeb291ee677"
        }
      ],
      "licenses": [
        {
          "license": {
            "id": "GPL-3.0-only",
            "text": {
              "encoding": "base64",
              "contentType": "plain/text",
              "content": "..."
            }
          }
        },
        {
          "license": {
            "id": "MIT",
            "text": {
              "encoding": "base64",
              "contentType": "plain/text",
              "content": "..."
            }
          }
        }
      ],

Or did I miss something?

@sschuberth
Copy link
Member

sschuberth commented Jun 22, 2023

As a side note, I thought this would be too complicated:

  repository_license_choices:
  - given: "GPL-3.0-only OR MIT"
    choice: "MIT"
  - given: "MIT OR GPL-3.0-only"
    choice: "MIT"

But I indeed just verified that the order of OR operands matters in given directives! Is that intended, @MarcelBochtler? It seems highly counter-intuitive to me.

@sschuberth sschuberth added bug Issues that are considered to be bugs reporter About the reporter tool labels Jun 22, 2023
@fviernau
Copy link
Member

fviernau commented Jun 22, 2023

But I indeed just verified that the order of OR operands matters in given directives! Is that intended, @MarcelBochtler? It seems high counter-intuitive to me.

IIUC, the ordering issue is a bug which was already on the radar, see #6721.

@sschuberth
Copy link
Member

sschuberth commented Jun 22, 2023

IIUC, the ordering issue is a bug which was already on the radar, see #6721.

I don't think that's related. Creating the DNF does not alter / create a deterministic order of licenses.

But I remember that we once discussed whether SpdxExpression.normalize() should also sort licenses by name, which IMO would solve this (if DNF would also normalize), but IIRC we ditched the sorting due to performance concerns or so...

@sschuberth
Copy link
Member

sschuberth commented Jun 22, 2023

But I remember that we once discussed whether SpdxExpression.normalize() should also sort licenses by name, which IMO would solve this (if DNF would also normalize), but IIRC we ditched the sorting due to performance concerns or so...

It was almost like at: #4077

@mawl
Copy link
Author

mawl commented Jun 23, 2023

As a side note, I thought this would be too complicated:

  repository_license_choices:
  - given: "GPL-3.0-only OR MIT"
    choice: "MIT"
  - given: "MIT OR GPL-3.0-only"
    choice: "MIT"

But I indeed just verified that the order of OR operands matters in given directives! Is that intended, @MarcelBochtler? It seems highly counter-intuitive to me.

Yes, I have declared both combinations just to exclude possible "given" order issues.

But my first question is, if repository_license_choices would work for CycloneDXReporter or not. So, Just to clarify: is a license choice only important during report phase or should it change the licenses of a dependency even in the evaluation-result.yml?

@sschuberth
Copy link
Member

sschuberth commented Jun 23, 2023

Any (general) reporter should take licenses choices into account (unless it's a very special reporter that somehow needs to provide access to "raw" data), and licenses choices are also applied before the evaluation phase (otherwise you'd never be able to fix rule violations by making a license choice).

So, your bug report sounds valid, but I haven't verified it yet.

sschuberth added a commit that referenced this issue Jun 26, 2023
Fixes #7182.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
sschuberth added a commit that referenced this issue Jun 26, 2023
Fixes #7182.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
@mawl
Copy link
Author

mawl commented Jul 12, 2023

Tested and it works. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that are considered to be bugs reporter About the reporter tool
Projects
None yet
Development

No branches or pull requests

3 participants