StatCan / aaw-kubeflow-containers

Containers built to be used with Kubeflow for Data Science
Other
24 stars 21 forks source link

Fix failing builds (code-server and sql-language-server) #293

Closed Jose-Matsuda closed 3 years ago

Jose-Matsuda commented 3 years ago

EDIT 07/10/2021

Re-purposing to be more general as more failures in our build appeared. Initial issue was that code-server had vulnerabilities Second issue was sql-language-server got vulnerabilities Third issue was the azure/container-scan started to fail due to trivy updating and azure/container-scan not being updated at this time.


See error report in my branch

image

https://github.com/advisories/GHSA-9j49-mfvp-vmhm

Since we use our own release of code-server we would need to patch ours to fix it.

The upstream fix is here

Might be good to just merge whatever they have into our fork (but keeping the download button removal of course) and then create a new release.

Could be a size medium depending on how this is tackled (ie if we bring their stuff into our fork im not sure how much work that is) or a small if we just update the packages

Jose-Matsuda commented 3 years ago

I don't think I have push permissions on this fork. I've made a branch where I rebased on top of the most recent commit in code-server's main branch but cannot push to my branch

Jose-Matsuda commented 3 years ago

ahhhhh they made an update to code-server which moved around where the download button was kept will need to look into it

Jose-Matsuda commented 3 years ago

Well looks like the folks over at code-server had to fork vscode and make their own changes as it wasn't co-operating with code-server https://github.com/cdr/code-server/pull/4135

and the removal of a lot of files https://github.com/cdr/code-server/pull/4169

So if we want to go that way where we just remove the download button, we may need to fork vscode and then in this fork of code-server change it to use our forked version of vscode that has the button removed

wg102 commented 3 years ago

One of the two vulnerabilities in code-server is fixed. The other one is set-value. This is a bit trickier because the issue is not direct, it seems to come from dependencies. It is also troublesome because the first vulnerability was visible when doing a yarn audit locally, as the second one is not. And therefore we cannot see if it is fixed until we do a PR with auto-deploy. Currently because of another issue, that is not possible.

These are the two packages that introduce the CVE:

wg102 commented 3 years ago

I tried something using selective-version-resolutions. I am not sure if it will work or not. I generated the deb file and made a new release using the steps here: https://github.com/StatCan/code-server/blob/main/docs/CONTRIBUTING.md#build

yarn build
yarn build:vscode
yarn release

and

yarn release:standalone
yarn test:standalone-release
yarn package
wg102 commented 3 years ago

If this fix (https://github.com/StatCan/code-server/releases/tag/v3.10.2-nodownload-v3) does not work, there is a possibility we could look into https://yarnpkg.com/package/patch-package#readme to edit or rewrite the troublesome packages that introduce the dependency.

Jose-Matsuda commented 3 years ago

I have comments in the PR https://github.com/StatCan/aaw-kubeflow-containers/pull/296#issuecomment-934769074 regarding another vulnerability that popped up from our sql-language-server install.

Note that it uses node-ssh-forward which itself hasn't been touched in about 1.5 years but I have opened up an issue on their repository here in case they are able to look at it. If that is updated then we can open up an issue to the sql-language-server folks to get them to update which node-ssh-forward they are looking at.

Jose-Matsuda commented 3 years ago

For code-server itself it's also blocked upstream by two packages but they do have open PRs to fix the vulnerability https://github.com/jonschlinkert/union-value/issues/11 https://github.com/jonschlinkert/cache-base/issues/22

And we do have another issue with our container scans failing because of https://github.com/Azure/container-scan/issues/104 but again there is an open PR to fix the trivy scans