Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: update code-server for vuln #296

Merged
merged 15 commits into from
Oct 13, 2021
Merged

chore: update code-server for vuln #296

merged 15 commits into from
Oct 13, 2021

Conversation

Jose-Matsuda
Copy link
Contributor

@Jose-Matsuda Jose-Matsuda commented Oct 4, 2021

Description

What your PR adds/fixes/removes
Closes #293

Tests / Quality Checks

Automated Testing/build and deployment

  • [?] Does the image pass CI successfully (build, pass vulnerability scan, and pass automated test suite)?
  • [?] If new features are added (new image, new binary, etc), have new automated tests been added to cover these?
  • If new features are added that require in-cluster testing (e.g. a new feature that needs to interact with kubernetes), have you added the auto-deploy tag to the PR before pushing in order to build and push the image to ACR so you can test it in cluster as a custom image?

JupyterLab extensions

  • [?] Are all extensions "enabled" (jupyter labextension list from inside the notebook)?

VS Code tests

  • [?] Does VS Code open?
  • [?] Can you install extensions?

Code review

  • Have you added the auto-deploy tag to your PR before your most recent push to this repo? This causes CI to build the image and push to our ACR, letting reviewers access the built image without having to create it themselves
  • Have you chosen a reviewer, attached them as a reviewer to this PR, and messaged them with the SHA-pinned image name for the final image to test (e.g. k8scc01covidacr.azurecr.io/machine-learning-notebook-cpu:746d058e2f37e004da5ca483d121bfb9e0545f2b)?

@Jose-Matsuda Jose-Matsuda added the auto-deploy Trigger manual CI steps for this PR label Oct 4, 2021
rohank07 and others added 2 commits October 4, 2021 14:30
update the datascience-notebook to see if
that will resolve the security issue
@Jose-Matsuda
Copy link
Contributor Author

Jose-Matsuda commented Oct 5, 2021

New errors of
image
appeared recently, I'm trying to update the FROM image like
image
but as you can see in the error there are some shenanigans going on.
^ cant update because ml-metadata is not supported in python 3.9 yet and that new image uses python 3.9

google/ml-metadata#56
and
google/ml-metadata#37

Seem to indicate it's resolved but I guess I will need to do more digging, in the meantime I will try to build the image without it and scan it

@Jose-Matsuda
Copy link
Contributor Author

Jose-Matsuda commented Oct 5, 2021

Did some scanning locally using the following command trivy image --severity CRITICAL jupyter/datascience-notebook:tag and here are some results.

This first screenshot is what we currently use
image

The second is the result after updating to a recent push from 8 days ago

image

Running it on our actual built image jupyterlab-cpu (this was with a recent image as well) we get
image
(as well as the vulnerability from code-server)

Using our current file we get the two critical vulnerabilities
image

Going through our current image
image

@Jose-Matsuda
Copy link
Contributor Author

Jose-Matsuda commented Oct 5, 2021

Yea we might want to evaluate the importance of the sql-language-server that we install as this is where the vulnerabilities come from.

ssh2 is fixed in the patch for 1.4.0 but upon digging to the most recent commit for sql-language-server 3 days ago it relies on "node-ssh-forward": "^0.6.3", and going looking at node-ssh-forward and then going into the repo -which has not been updated in over a year- uses "ssh2": "^0.8.9"

Upon removing the sql-language-server and building it and then running the trivy scan both vulnerabilities are now gone
image

@Jose-Matsuda
Copy link
Contributor Author

Jose-Matsuda commented Oct 8, 2021

I think we should ignore the one issue for pyyaml due to
image

GHSA-8q59-q68h-6hv4 and
GHSA-6757-jp84-gxfx <--- this issue fix is 'incomplete' so maybe we will add this to allowed vulnerabilities

So we should pin pyyaml to 5.4.1 at the least and not 5.3.1

@Jose-Matsuda
Copy link
Contributor Author

Jose-Matsuda commented Oct 13, 2021

This last error of Cryptography is funky as when I do a pip list inside vscode I get `
image

vs in terminal
image

Using pip show
image
image

So the vulnerable one is in the conda environment

Copy link

@skyeturriff skyeturriff left a comment

Choose a reason for hiding this comment

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

Looks ok to me

@Jose-Matsuda Jose-Matsuda merged commit d1a6b8c into master Oct 13, 2021
@Jose-Matsuda Jose-Matsuda deleted the update-cdr branch December 14, 2021 16:37
JaeggerJose pushed a commit to wycc/cgu-kubeflow-containers that referenced this pull request Dec 30, 2024
* chore: update code-server for vuln
removed sql-lang-server temporarily as it has a vulnerability
and one of its dependencies does not appear to be maintained
Pinned various packages to avoid vulnerabilities
 
Co-authored-by: rohank07 <rohank_17@hotmail.ca>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
auto-deploy Trigger manual CI steps for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix failing builds (code-server and sql-language-server)
3 participants