search

ThalesGroup / helm-spray

Helm plugin for installing or upgrading sub-charts from an umbrella-chart using dependency orders
https://thalesgroup.github.io/helm-spray/
Apache License 2.0
74 stars 18 forks source link

Possible injection ? exec.Command() #63

Open AYDEV-FR opened 3 years ago

AYDEV-FR commented 3 years ago

Hi, I have already posted an issue on your project, and while re-reading the code, I wonder about possible injections. By using exec.Command(), especially the line exec.Command("sh -c", params). https://github.com/ThalesGroup/helm-spray/blob/945927b0641376bf3003092b5fca246c241e27dd/pkg/helm/helm.go#L184

Also in the UpgradeWithValues function, even if the use of the bianire helm in the first argument of exec.Command() prevents some injection.https://github.com/ThalesGroup/helm-spray/blob/945927b0641376bf3003092b5fca246c241e27dd/pkg/helm/helm.go#L136

Why not use the Helm Go SDK for most actions on Helm. https://pkg.go.dev/helm.sh/helm/v3@v3.3.3/pkg/action#Upgrade for example.

cvila84 commented 3 years ago

Hello,

I agree there is room for improvement here. I would be happy to review any PR for this :)

AYDEV-FR commented 3 years ago

Hi,

I will fork and try to change only the functions: https://github.com/ThalesGroup/helm-spray/blob/945927b0641376bf3003092b5fca246c241e27dd/pkg/helm/helm.go#L69 https://github.com/ThalesGroup/helm-spray/blob/945927b0641376bf3003092b5fca246c241e27dd/pkg/helm/helm.go#L98 https://github.com/ThalesGroup/helm-spray/blob/945927b0641376bf3003092b5fca246c241e27dd/pkg/helm/helm.go#L158 from /pkg/helm/helm.go

For the List() function we can probably use : https://pkg.go.dev/helm.sh/helm/v3@v3.3.3/pkg/action#List For the UpgradeWithValues()we can probably use : https://pkg.go.dev/helm.sh/helm/v3@v3.3.3/pkg/action#Upgrade For the Fetch()function we can probably use : https://pkg.go.dev/helm.sh/helm/v3@v3.3.3/pkg/action#ChartPull

I think a work is to be considered also on the /pkg/kubectl/kubectl.go file, which also uses a lot of exec.Command(), while there is the Kubernetes Client SDK here : https://github.com/kubernetes/client-go

I will surely propose a PR if I have the time.