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

CycloneDX Ruby Support #410

Merged
merged 12 commits into from
Aug 4, 2021
Merged

CycloneDX Ruby Support #410

merged 12 commits into from
Aug 4, 2021

Conversation

jeffrey778zhan
Copy link
Contributor

@jeffrey778zhan jeffrey778zhan commented Jul 28, 2021

Add Ruby Dependency Component builder with unit tests. Generate CycloneDX components follow spec version 1.3, specifically making use of the new properties type.

@jeffrey778zhan jeffrey778zhan changed the title CycloneDC Ruby CycloneDX Ruby Support Jul 28, 2021
@coderpatros
Copy link

Hi @jeffrey778zhan, noticed CycloneDX support was being added. Happy to provide some feedback if you like.

@jeffrey778zhan
Copy link
Contributor Author

Hi @jeffrey778zhan, noticed CycloneDX support was being added. Happy to provide some feedback if you like.

Hey @coderpatros, absolutely! Any feedback is welcome.

@jeffrey778zhan jeffrey778zhan marked this pull request as ready for review July 29, 2021 16:29
lib/cyclonedx/base.rb Outdated Show resolved Hide resolved
@nishils
Copy link
Contributor

nishils commented Jul 29, 2021

@jeffrey778zhan thanks for starting this up!

Could you take a first stab at writing the docs for this scanner (and maybe some edits to the readme) before we merge and while we review the PR?

@jeffrey778zhan
Copy link
Contributor Author

@jeffrey778zhan thanks for starting this up!

Could you take a first stab at writing the docs for this scanner (and maybe some edits to the readme) before we merge and while we review the PR?

Sure! I can add in the relevant information into salus_reports.md

@ghbren
Copy link
Contributor

ghbren commented Jul 31, 2021

Does CycloneDX provide a function for you to validate the result against a schema?

Copy link
Contributor

@ghbren ghbren left a comment

Choose a reason for hiding this comment

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

Have you tested something like docker build ..., then docker run ... to generate the cycloneDX output file, to ensure no strange behavior?

@coderpatros
Copy link

Does CycloneDX provide a function for you to validate the result against a schema?

You can use the CycloneDX CLI tool or web tool for validation.

Alternatively you can grab a copy of the schemas from the specification repo if you want to perform your own validation.

@ghbren
Copy link
Contributor

ghbren commented Aug 1, 2021

Alternatively you can grab a copy of the schemas from the specification repo if you want to perform your own validation.

Jeffrey, we included a copy of the sarif schema in salus to validate against our sarif results. Can you look into whether it's possible to do something similar for CycloneDX?

@jeffrey778zhan
Copy link
Contributor Author

Does CycloneDX provide a function for you to validate the result against a schema?

Yes, #415

Does CycloneDX provide a function for you to validate the result against a schema?

Yep, PR to integrate that here #415

@jeffrey778zhan
Copy link
Contributor Author

Have you tested something like docker build ..., then docker run ... to generate the cycloneDX output file, to ensure no strange behavior?

Yep, if I'm understanding your question correctly currently this PR is scoped for only the changes needed to build CycloneDX reports from Ruby dependencies so there's still config changes needed to be made (i.e to add option to read the cyclonedx format option in the config) before we can generate the cycloneDX output file from docker. And at that point we'll be ready to test something like docker build ..., then docker run ...

@jeffrey778zhan
Copy link
Contributor Author

jeffrey778zhan commented Aug 2, 2021

Hi @coderpatros, for the purl field is the version required to be absolute? For example, would something likepkg:maven/com.acme/tomcat-catalina@~> 9.0 be a valid purl? Thanks!

@coderpatros
Copy link

Hi @coderpatros, for the purl field is the version required to be absolute? For example, would something likepkg:maven/com.acme/tomcat-catalina@~> 9.0 be a valid purl? Thanks!

I don't think it would be valid. Besides the version not being encoded, I think it should be a specific version only or omitted. @pombredanne @stevespringett is that statement correct for purls?

@pombredanne
Copy link

@jeffrey778zhan you wrote:

Hi @coderpatros, for the purl field is the version required to be absolute? For example, would something like pkg:maven/com.acme/tomcat-catalina@~> 9.0 be a valid purl? Thanks!

I don't think it would be valid. Besides the version not being encoded, I think it should be a specific version only or omitted. @pombredanne @stevespringett is that statement correct for purls?

Short answer: a version need to be a version, e.g. a resolved concrete version and not a range, constraints, wildcard or specifier of sorts. ~> 9.0 is a version constraint/range/specifier and this is not meant to be used as a version in a purl.

So:

  • pkg:maven/com.acme/tomcat-catalina@~> 9.0 is not valid
  • pkg:maven/com.acme/[email protected] would be valid
  • pkg:maven/com.acme/tomcat-catalina@%7E%3E%209.0 would be technically valid encoding-wise but ugly and impossible to resolve since ~> 9.0 is not a version, so this is also invalid.

Eventually we want to a spec how to encode version ranges likely as a purl qualifier and there is work underway.

The general directions I favor is explained at aboutcode-org/vulnerablecode#119 (comment) and package-url/purl-spec#84 (comment)

@jeffrey778zhan
Copy link
Contributor Author

Thanks @coderpatros and @pombredanne for the confirmation. Will only include resolved concrete versions in the purl for now.

@jeffrey778zhan jeffrey778zhan merged commit cafa754 into master Aug 4, 2021
@jeffrey778zhan jeffrey778zhan deleted the jeffreyz/CycloneDX-Ruby branch August 4, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants