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

Follow-up: Address remaining "High" vulnerabilities in KServe repo from SNYK scans #91

Closed
3 of 4 tasks
heyselbi opened this issue Sep 27, 2023 · 10 comments · Fixed by #93 or #94
Closed
3 of 4 tasks

Follow-up: Address remaining "High" vulnerabilities in KServe repo from SNYK scans #91

heyselbi opened this issue Sep 27, 2023 · 10 comments · Fixed by #93 or #94
Assignees
Labels
kind/security Indicates that this is a security issue that should be addressed odh-1.11 rhods-1.34

Comments

@heyselbi
Copy link

heyselbi commented Sep 27, 2023

SNYK scan of KServe repo

  • Deactivate files in /python directory that are not used in the build
  • Address high vulnerabilities in /python directory that are used in the build, if any (perhaps one in /python/kserve/?)
  • Address high vulnerabilities in Code analysis
  • Address high vulnerabilities in go.mod
@heyselbi heyselbi changed the title Follow-up: Address "High" vulnerabilities in KServe repo from SNYK scans Follow-up: Address remaining "High" vulnerabilities in KServe repo from SNYK scans Sep 27, 2023
@heyselbi heyselbi added the kind/security Indicates that this is a security issue that should be addressed label Sep 27, 2023
@heyselbi
Copy link
Author

SNYK also creates automatic PRs which might be of use:
https://github.com/red-hat-data-services/kserve/pulls

@israel-hdez
Copy link

Fix in: #93

  • The only high entry in under python/kserve should be solved.
    • Snyk still reports a vulnerability because its database hasn't been updated yet.
  • There is only one true positive in the code analysis:
    • The Server-Side Request Forgery is fixed.
    • All Improper Certificate Validation entries refer to docs/samples which we don't deliver in the final container images.
    • The Hardcoded Secret refer either to docs/samples which we don't deliver, or are false positives because are alerts over an attribute_map which is doing a mapping between JSON and YAML attributes (and is not hard-coding any secret).
  • All entries in go.mod seem to be false positives.
    • Snyk CLI reports: ✔ Tested 810 dependencies for known issues, no vulnerable paths found.
    • Perhaps, related to this.

@israel-hdez
Copy link

I cannot deactivate the other python entries that we are not using.

Deactivating does not remove from the list... Maybe we can keep as is, since those still will introduce noise?

@israel-hdez
Copy link

Upstream PR: kserve#3157

@heyselbi
Copy link
Author

Downstream commit that is part of rhods 1.34: red-hat-data-services@820006e

Closing the issue. The kserve upstream issue, whenever it is merged, will be part of new kserve release.

@bdattoma
Copy link

bdattoma commented Oct 11, 2023

I cannot deactivate the other python entries that we are not using.

If these files are not used, could they be deleted? @israel-hdez

What about docs/apis/Dockerfile? it's in docs, but it's still a Dockerfile. Is it used or intended to be used by someone else?

@israel-hdez
Copy link

https://kserve.github.io/website/0.11/reference/api/

I cannot deactivate the other python entries that we are not using.

If these files are not used, could they be deleted? @israel-hdez

My rationale for not deleting, is that upstream community DO use that code and they maintain it.

We could delete these files, but by doing so we introduce a point of conflict; i.e. git will constantly complain with "we deleted, theirs modified" and will ask for manual conflict resolution. So, code syncs with upstream repos will become less automatable.

What about docs/apis/Dockerfile? it's in docs, but it's still a Dockerfile. Is it used or intended to be used by someone else?

I'm not fully familiar with all the code base. My guess is that this Dockerfile is used to generate this reference documentation: https://github.com/kserve/website/blob/main/docs/reference/api.md, which is the source for this web-doc: https://kserve.github.io/website/0.11/reference/api/. If my guess is right, the vulnerabilities are not a concern, because the deliverable artifact is the md file (which is just plain text) and the container image is just developer tooling.

@bdattoma
Copy link

https://kserve.github.io/website/0.11/reference/api/

I cannot deactivate the other python entries that we are not using.

If these files are not used, could they be deleted? @israel-hdez

My rationale for not deleting, is that upstream community DO use that code and they maintain it.

We could delete these files, but by doing so we introduce a point of conflict; i.e. git will constantly complain with "we deleted, theirs modified" and will ask for manual conflict resolution. So, code syncs with upstream repos will become less automatable.

What about docs/apis/Dockerfile? it's in docs, but it's still a Dockerfile. Is it used or intended to be used by someone else?

I'm not fully familiar with all the code base. My guess is that this Dockerfile is used to generate this reference documentation: https://github.com/kserve/website/blob/main/docs/reference/api.md, which is the source for this web-doc: https://kserve.github.io/website/0.11/reference/api/. If my guess is right, the vulnerabilities are not a concern, because the deliverable artifact is the md file (which is just plain text) and the container image is just developer tooling.

understood. So, our downstream image doesn't contain these files right?

@israel-hdez
Copy link

understood. So, our downstream image doesn't contain these files right?

That's right.

I just want to stress that files under python/kserve are in downstream, and I fixed issues under this path. And, well, anything under docs/ is not present in our images.

@bdattoma
Copy link

got it, thank you for the clarifications @israel-hdez

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/security Indicates that this is a security issue that should be addressed odh-1.11 rhods-1.34
Projects
Status: Done
Status: No status
Status: Done
3 participants