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

What is the point of merge-vex? #156

Open
mmarseu opened this issue Apr 10, 2024 · 4 comments
Open

What is the point of merge-vex? #156

mmarseu opened this issue Apr 10, 2024 · 4 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@mmarseu
Copy link
Collaborator

mmarseu commented Apr 10, 2024

The documentation isn't clear on this, so I'd like to ask what the merge-vex command is for.

The documentation simply states:

This command requires two input files, a SBOM and a VEX file that shell be merged. The VEX file needs to be compatible with the SBOM.

But what is a "VEX file"? That isn't an established term. Even if you google it, you'll find a VEX document only described in an abstract manner, as a set of requirements but not as a complete data format. At the very least, there are currently two implementations which fulfil these requirements and therefore could constitute a VEX file: CycloneDX and CSAF.

Now, if we wanted to merge a CSAF VEX document into the SBOM that would probably justify its own command. At the moment, the command only works for CycloneDX, though, so why do we need a separate command if we already have merge for merging two CycloneDX files?

The only thing the current implementation apparently does differently from the regular merge is check whether all vulnerabilities reference components contained in the SBOM (see also #155). If they don't the command fails silently and simply outputs the original SBOM.

Could you explain the need for a separate command for this in the documentation?

@mmarseu mmarseu added the documentation Improvements or additions to documentation label Apr 10, 2024
mmarseu added a commit that referenced this issue Apr 11, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Apr 11, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
@italvi
Copy link
Collaborator

italvi commented Apr 16, 2024

@mmarseu Thanks for starting this discussion as this is something we also discussed internally since #35 and #36. And we decided to deprecate merge-vex in favor of a vex command. The vex command should have several sub-commands.

The ones we currently think of:

  • merge. This would merge two VEX-files, i.e. not requiring an SBOM anymore. Part of this merge would be to check, whether the same CVE from one VEX-file was maybe updated and therefore the current analyis.state should be overwritten (one of the use-cases I can think of there).
  • list. This would list me all CVEs in the VEX. Here I could even think of further flags, e.g. "--affected" to only see CVEs where (after analysis) I am really affected.
  • prune/trim. This would remove not required entries, e.g. if you want to remove the false-positives CVEs from the VEX-file.

I think this command would bring a real benefit and justify a separate command for VEX-files.

Meaning for your integration test: Ignore it.

@CBeck-96 maybe for the time being, till the rework happens, we should add a deprecation notice to the command. This way we also see whether somebody reacts to it, as up to now only one user is known to us.

@CBeck-96
Copy link
Collaborator

Besides the ones mentioned above, we also have plans for a plausibility check regarding VEX.

@italvi i also agree with the deprecation notice, since the implementation might not happen for some time

@mmarseu
Copy link
Collaborator Author

mmarseu commented Apr 22, 2024

@mmarseu Thanks for starting this discussion as this is something we also discussed internally since #35 and #36. And we decided to deprecate merge-vex in favor of a vex command. The vex command should have several sub-commands.

The ones we currently think of:

  • merge. This would merge two VEX-files, i.e. not requiring an SBOM anymore. Part of this merge would be to check, whether the same CVE from one VEX-file was maybe updated and therefore the current analyis.state should be overwritten (one of the use-cases I can think of there).

That raises the same question again: Vex is CycloneDX (at least in the sense that we use it. Other implementations exist but we don't support them). Why do we need two separate merge commands for CycloneDX files, depending on the contents?

  • list. This would list me all CVEs in the VEX. Here I could even think of further flags, e.g. "--affected" to only see CVEs where (after analysis) I am really affected.
  • prune/trim. This would remove not required entries, e.g. if you want to remove the false-positives CVEs from the VEX-file.

Okay, I can see possible use-cases for these.

Meaning for your integration test: Ignore it.

Cool 😆

@italvi
Copy link
Collaborator

italvi commented Apr 23, 2024

That raises the same question again: Vex is CycloneDX (at least in the sense that we use it. Other implementations exist but we don't support them). Why do we need two separate merge commands for CycloneDX files, depending on the contents?

Ok, agree. It would only make sense, if we start to support other formats like OpenVEX and CSAF VEX profile. And before doing so, we should then have something like convert.

Okay, I can see possible use-cases for these.

Glad, that this seems to be of use for you, too 😉. One more use-case could be extract for only getting the VEX, if somebody provides an embedded VEX.

mmarseu added a commit that referenced this issue May 23, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue May 28, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Jun 20, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Jul 2, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Jul 2, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
@italvi italvi added this to the 1.0.0 milestone Jul 3, 2024
mmarseu added a commit that referenced this issue Jul 4, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Jul 10, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
mmarseu added a commit that referenced this issue Jul 10, 2024
Add integration tests for all commands except merge-vex.
merge-vex will be added once issue #156 has been answered.

Some tests fail due to open issues:

- #154 makes TestBuildPublic fail
- #150 makes TestValidate fail
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants