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

feat: add root component and unscanned dependency #13

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

xieshenzh
Copy link
Contributor

@xieshenzh xieshenzh commented Feb 20, 2024

To support reporting package vulnerabilities in base image:

  1. add root component's purl and recommended component's purl to analysis report api.
  2. add unscanned packages' purls to provider report.

Jira: https://issues.redhat.com/browse/APPENG-2250 and https://issues.redhat.com/browse/APPENG-2251

Copy link
Collaborator

@ruromero ruromero left a comment

Choose a reason for hiding this comment

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

@xieshenzh let me know what you think about my comments.

type: string
RootComponent:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add this as a new provider redhat otherwise this will break the general data hierarchy. These recommendations come from some provider and maybe it eventually might include not only recommendations but also vulnerabilities and remediations for such packages.

Copy link
Contributor Author

@xieshenzh xieshenzh Feb 21, 2024

Choose a reason for hiding this comment

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

I'll remove the recommendation part and use a new provider instead.

But I need to store the purl of the original sbom somewhere. The purl will be used to match the image names in the docker file and also to show the image name in the html report.
@ruromero where do you think is a good place to store the purl? Also in the redhat provider as a dependency report, and store it as the only direct dependency?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The DependencyReport has a ref which should be the original purl and the recommendation which is the purl of what you are recommending. Is that what you are asking?

Copy link
Contributor Author

@xieshenzh xieshenzh Feb 22, 2024

Choose a reason for hiding this comment

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

Yes, that's what I am asking.

In case there are multiple DependencyReport from provider redhat, how could we determine which DependencyReport is for the root component of the sbom in the request ? The one without any issues? Or always insert it at the first postion in the DependencyReport list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently there are no vulnerabilities or anything reported from the redhat provider. Actually, thinking more about it and comparing it to the other package recommendations (e.g. Quarkus). For any provider/source we always add the redhat recommendations so you should just add it.

Imagine we start from the alpine image. This turns into pkg:oci/library/[email protected] (just an example). Then this will be sent to the providers and might not be supported by one or many but it will include the recommendation for the pkg:oci/redhat.io/ubi8/[email protected]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand. But the main purpose of adding the purl is to create an id for the AnalysisReport, with which we can match the AnalysisReport with an sbom in the array of the request body, and also distinguish different AnalysisReport in the html report (e.g. use the id as tag title in the html report).

If we store the purl as a dependency in provider/source and don't create an id for the AnalysisReport, how should we match the sbom and generate tag titles for the html report? Should we use the position in the array to match the AnalysisReport to an sbom from the input array? Should we create a new variable to the html report for tag titles instead of getting the titles from the AnalysisReport?

Also, if we store the purl as a dependency in provider/source, then each provider would have at least one recommendation for the image. In the meantime, we just want to emphasize the single recommendation for UBI.
In this case, should we just get the first recommendation from any provider and assume it will always be the UBI that we want to recommend?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can, as you say rely on the array order or turn the list into a dictionary

Copy link
Contributor Author

@xieshenzh xieshenzh Feb 23, 2024

Choose a reason for hiding this comment

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

Another issue is that the root component is not really a dependency, it is the application itself.
If we add the root component to the DependencyReport list in the Source, should all other dependencies become transitive?
And how should we handle the root component in the SourceSummary and in the html report? Ignore it?
In addition, for snyk, oci packages are not supported. So the image will be treated as an unscanned package, and there will not be an item in the DependencyReport list for the image.

Should we create a new property for the root component in the Source, instead of adding it to the DependencyReport list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

An SBOM is a bill of materials, it doesn't necessarily need to be a tree. In our case the SBOM will contain the image + rpms + python packages + etc. there is no dependency tree. They will all be components at the same level, then of course we will generate a tree for the python transitive deps etc.
Does that make sense? Maybe I'm missing something.

api/v4/openapi.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@ruromero ruromero left a comment

Choose a reason for hiding this comment

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

Can you also increase the version to 4.1.0 ?

@@ -164,6 +227,10 @@ components:
type: object
additionalProperties:
$ref: '#/components/schemas/Source'
unscanned:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should belong to the Source. Sources are the ones that provide the vulnerabilities. Besides it seems reasonable to also add unscanned to the SourceSummary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then, in case a provider has multiple Source, the unscanned list will be duplicated to each Source of the provider, which is fine, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

correct, we recommend the same package in each case

Copy link
Collaborator

@ruromero ruromero left a comment

Choose a reason for hiding this comment

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

LGTM

@ruromero ruromero merged commit e737e5d into RHEcosystemAppEng:main Feb 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants