Galileo-Galilei / kedro-mlflow

A kedro-plugin for integration of mlflow capabilities inside kedro projects (especially machine learning model versioning and packaging)
https://kedro-mlflow.readthedocs.io/
Apache License 2.0
195 stars 29 forks source link

Snyk Advisor mentioning "Security Review Needed" #178

Closed grechut closed 3 years ago

grechut commented 3 years ago

Description

Snyk (OS package health monitor) is pointing out that kedro-mlflow might be facing some vulnerabilities. https://snyk.io/advisor/python/kedro-mlflow

It's pointing out 2 indirect vulnerabilities: "A security vulnerability was detected in an indirect dependency that is added to your project when the latest version of kedro-mlflow is installed."

However, the main dependencies are https://snyk.io/advisor/python/kedro and https://snyk.io/advisor/python/mlflow , which are labeled as okay, so I'm not sure what exactly it is pointing to.

Context

Assessing whether package is okay to use using Snyk

Steps to Reproduce

  1. Open https://snyk.io/advisor/python/kedro-mlflow

Expected Result

Getting to Snyk status: "No known security issues", which is an indicator of healthy package

Galileo-Galilei commented 3 years ago

Hello @grechut,

thank you very much for raising this. I'll get a look at Snyk report and come back to you. This is indeed very weird, since kedro and mlflow are the only two dependencies of the package. It may be worth adding a workflow for automatic security check on PR, though.

Galileo-Galilei commented 3 years ago

Oops, sorry , my tests with the CI polluted the issue. Actually, it seems to be a false positive:

(Sorry, I don't know how I can publish publicly this report through Snyk, and replace the current public one)

Required packages missing: mlflow

Please run `pip install -r requirements.txt`. If the issue persists try again with --skip-unresolved.

despite the package being obviously installed. I tried adding the --command=python3 as recommended in this github issue and installing packages in a conda environment, without success.

Are you familiar with snyk and could you help to solve this?

Galileo-Galilei commented 3 years ago

I think I figured out what's going on: the UI on snyk advisor claims that no dependencies are found. I guess it cannot parse the requirements.txt, and hence raise an indirect error to warn about this. It is hard to be 100% sure without the log, because when I parse it on my snyk account, everything works fine and no security issue is raised (see above report).

Since the snyk "advisor" seems to be in beta, I guess I should reach the developers to report a bug.

Galileo-Galilei commented 3 years ago

As of today, Snyk no longer display any code vulnerability even if I have not changed anything. I can't really say what is going on, but if it remains so during one week, I'll lose this issue. Are you ok with this?

grechut commented 3 years ago

Thanks for investigating so deeply, that is very much appreciated ;)

As you say, now it looks okay for 0.7.0 image

So I think we can close the issue and I wish you getting "popularity" section to the green colour :)

I only recently started using kedro-mlflow, if I get more familiar and have any good ideas, will be writing here.

Thanks a lot once again :)

Galileo-Galilei commented 3 years ago

A security warning is a serious issue and it is totally normal to check out what is going on, thank you to raise the issue at first !

Regarding the "popularity" section, since kedro is still in an "orange" zone I am litterally thousands stars away from this so It should not happen any time soon :) (and for your record the kedro core team is actively working to integrate tracking functionalities inside kedro, and they reached me privately to discuss about kedro-mlflow's so it is more likely that the plugin will be deprecated in a not-so-far future rather that become very popular. I don't know what is their timeline though, and I'll bet on several month at least, maybe one year. The other scenario is that the plugin will continue to exist to provide some specific functionalities (e.g. serving), but some other functionalities will be removed once they are deeply integrated in core code).

Do not hesitate to report bugs / suggest new features or even open PR, any contributions will be greatly appreciated.