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

Duplicate build-system external references #1356

Closed
1 of 2 tasks
jeremylong opened this issue Jan 28, 2025 · 10 comments · Fixed by #1355
Closed
1 of 2 tasks

Duplicate build-system external references #1356

jeremylong opened this issue Jan 28, 2025 · 10 comments · Fixed by #1355
Assignees
Labels
bug Something isn't working

Comments

@jeremylong
Copy link
Contributor

Describe the bug

PR #1349 creates multiple duplicate build-system external references for some projects.

To Reproduce

I do not have a shareable project that replicates the issue. However, with the current implementation, I've seen the plugin produce records like:

"externalReferences": [
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.internal.vcs/org/repo",
          "type": "vcs",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentVCS\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        }
      ]

Expected behavior

There should not be duplicate build-system external references.

Screenshots or output-paste

See above

Environment

  • @cyclonedx/webpack-plugin version: 4.0.0
  • webpack version: 5.95.0
  • Node version: 22.12.0
  • OS: osX

Additional context

Proposed fix #1355 - uses the same mechanism to prevent multiple entries as the external references for VCS so I assumed there would be no issue.

Contribution

  • I am willing to provide a fix
  • I will wait until somebody else fixes it
@jeremylong jeremylong added the bug Something isn't working label Jan 28, 2025
@jeremylong jeremylong changed the title [BUG] [BUG] Duplicate build-system external references Jan 28, 2025
@jkowalleck
Copy link
Member

jkowalleck commented Jan 28, 2025

PR #1349 creates multiple duplicate build-system external references for some projects.

for the follwing reasons:

  • one is taken from the package maniffest
  • the other is taken from config setting

original request was aware of this - as of #1344 (comment)
Therefore, I'd argue that this is not a bug, but a feature. Actually, a feature that the user has control over.

Therefoa, the original "expected behaviour" is debatable.

There should not be duplicate build-system external references.

Therefore, i tend to closing this ticket as "not planned."

@jkowalleck
Copy link
Member

In case the original "expected behaviour" was valid,
there are multiple solutions:

  • override the existing BuildSyste URL in the SBOM
  • add the BuildSystem URL from the options only, of none existed

both solutions are a breaking change -- not a blocker, just a remark/justification

My recommendation for a solution: override.
Reasoning: the user setting should override the defaults - and the defaults are coming from package manifests

@jkowalleck jkowalleck added question Further information is requested breaking-change and removed bug Something isn't working labels Jan 28, 2025
@jkowalleck jkowalleck changed the title [BUG] Duplicate build-system external references Duplicate build-system external references Jan 28, 2025
@jeremylong
Copy link
Contributor Author

The build system is not part of the package manifest.

@jeremylong
Copy link
Contributor Author

If you want another discussion around the handling of the VCS we can open a new issue.

@jkowalleck
Copy link
Member

jkowalleck commented Jan 28, 2025

The build system is not part of the package manifest.

you are right. i was wrong.

Now that i understood, can you craft a reproducible setup, so that we might have a regression test for this.
after this is sorted out, i need to re-think about #1356 (comment)

@jeremylong
Copy link
Contributor Author

Regarding the concern with the VCS configuration overriding the default from the package.json - I'm 100% on board with this.

I'll try and put together a regression test.

@jkowalleck jkowalleck added bug Something isn't working and removed question Further information is requested labels Jan 29, 2025
@jkowalleck
Copy link
Member

thank you so much for all your effort. And thank you even more for your persistence 👍

jkowalleck pushed a commit that referenced this issue Jan 29, 2025
PR #1349, for some projects, ends up creating multiple duplicate
`build-system` external references. The fix is to ensure we have not
already added an external reference of type: `build-system`.

With the current implementation, I've seen the plugin produce records
like:

```json
"externalReferences": [
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.internal.vcs/org/repo",
          "type": "vcs",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentVCS\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        },
        {
          "url": "https://some.build.system.internal/job/88",
          "type": "build-system",
          "comment": "as declared via cyclonedx-webpack-plugin config \"rootComponentBuildSystem\""
        }
      ]
```


fixes #1356

Signed-off-by: Jeremy Long <[email protected]>
@jkowalleck
Copy link
Member

@jeremylong
Copy link
Contributor Author

As a maintainer myself - I hate it when people continue to post on closed issues. I just wanted to comment about the rootComponentVCS and let you decide if you want a new issue.

  1. You had stated the feature was not needed because the VCS should be in the package.json. While I agree it should, it is not always there and I have reasons to need the VCS added to the BOM. Thus we are where we are with the feature.
  2. When implementing the feature if I had rootComponentVCS override the VCS retrieved from the package.json the report ended up looking very weird because the external references would have a VCS defined by the rootComponentVCS but the package URL would have the VCS from the package.json. Rather than open a discussion about this I went with if it is defined in the package.json we should not override the value. In hindsight, I should have opened up the topic for discussion.

@jkowalleck
Copy link
Member

re: #1356 (comment)

if you wanted to propose a change of the process, feel free to open a new ticket describing the use-cases and reasoning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants