Skip to content

feat(ci): add trivy job for security #3261

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 4, 2021
Merged

feat(ci): add trivy job for security #3261

merged 1 commit into from
May 4, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Apr 29, 2021 •
edited
Loading

This PR adds a new job to our CI called trivy which uses trivy-action to scan our code and upload the Trivy scan results to the GitHub Security tab.

Fixes #3177

@jsjoeio jsjoeio requested a review from a team as a code owner April 29, 2021 18:20
@jsjoeio jsjoeio self-assigned this Apr 29, 2021
@jsjoeio jsjoeio added the security Security related label Apr 29, 2021
@jsjoeio jsjoeio added this to the v3.9.4 milestone Apr 29, 2021
jawnsy
jawnsy approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

Nice work Joe! ❤️

jsjoeio
jsjoeio commented Apr 29, 2021
View reviewed changes
@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch 2 times, most recently from ed2f27a to 281f0b5 Compare April 29, 2021 19:17
@jsjoeio jsjoeio requested a review from jawnsy April 29, 2021 19:17
@codecov
Copy link

codecov bot commented Apr 29, 2021 •
edited
Loading

Codecov Report

Merging #3261 (f1491ec) into main (f8d8ad3) will decrease coverage by 4.12%.
The diff coverage is n/a.

❗ Current head f1491ec differs from pull request most recent head 6d5c603. Consider uploading reports for the commit 6d5c603 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3261      +/-   ##
==========================================
- Coverage   51.02%   46.90%   -4.13%     
==========================================
  Files          23       23              
  Lines        1266     1196      -70     
  Branches      286      237      -49     
==========================================
- Hits          646      561      -85     
+ Misses        498      451      -47     
- Partials      122      184      +62
Impacted Files Coverage Δ
src/common/util.ts 72.34% <0.00%> (-27.66%) ⬇️
src/node/cli.ts 60.95% <0.00%> (-16.74%) ⬇️
src/node/proxy.ts 66.66% <0.00%> (-11.12%) ⬇️
src/common/http.ts 92.30% <0.00%> (-7.70%) ⬇️
src/browser/register.ts 92.30% <0.00%> (-7.70%) ⬇️
src/node/constants.ts 92.85% <0.00%> (-7.15%) ⬇️
src/node/util.ts 44.56% <0.00%> (-2.38%) ⬇️
src/node/heart.ts 70.37% <0.00%> (-2.05%) ⬇️
src/node/plugin.ts 67.59% <0.00%> (-0.59%) ⬇️
src/node/socket.ts 89.65% <0.00%> (-0.35%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8d8ad3...6d5c603. Read the comment docs.

jawnsy
jawnsy approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

LGTM! Just some minor thoughts for your consideration

@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch from 281f0b5 to 44a9534 Compare April 29, 2021 21:16
@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch 2 times, most recently from 9d2290c to 8402786 Compare April 29, 2021 21:23
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 29, 2021

2021-04-29T21:24:34.963Z FATAL scan error: image scan failed: failed analysis: walk dir: failed to analyze file: analyze file (.): unable to open a file (.): unable to read file: read .: is a directory

Hmm time to figure out why this failed 🤔 link to line in logs

@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch from ba9e4d3 to 7cc821e Compare April 29, 2021 21:48
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 29, 2021

Step 7/14 : COPY release-packages/code-server*.deb /tmp/

Turns out the build-docker-image script relies on the release-packages being present 😅 so I refactored it back into ci.yaml since they get built in that workflow.

oxy
oxy reviewed Apr 30, 2021
View reviewed changes
@jsjoeio jsjoeio marked this pull request as draft April 30, 2021 19:09
jawnsy
jawnsy reviewed May 3, 2021
View reviewed changes
@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch from 600a6a3 to e46ad39 Compare May 3, 2021 23:02
@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 3, 2021

Okay I finally got it all working thanks to help from @code-asher 🎉

Though it says no vulnerabilities were found 🤔

https://github.com/cdr/code-server/security/code-scanning?query=ref%3Arefs%2Fpull%2F3261%2Fmerge+tool%3ATrivy

@jsjoeio jsjoeio requested a review from jawnsy May 3, 2021 23:43
@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 3, 2021

image

Thought maybe the findings were not new? https://github.com/cdr/code-server/pull/3261/checks?check_run_id=2496406312

jawnsy
jawnsy approved these changes May 3, 2021
View reviewed changes
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

Nice stuff! It looks so easy when it's done :)

@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 3, 2021

Nice stuff! It looks so easy when it's done :)

😂 I know right? With Asher's help, it was a lot easier.

oxy
oxy reviewed May 4, 2021
View reviewed changes
@jsjoeio jsjoeio marked this pull request as ready for review May 4, 2021 18:27
@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch from f1491ec to e229c41 Compare May 4, 2021 18:27
@jsjoeio
feat(ci): add trivy scans to workflow
6d5c603 
This adds both a trivy scan for the repo and a trivy scan for our Docker image.
@jsjoeio jsjoeio force-pushed the jsjoeio/add-trivy branch from e229c41 to 6d5c603 Compare May 4, 2021 18:32
@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 4, 2021

Note: for the trivy-action version I went with the commit SHA.

Using the commit SHA of a released action version is the safest for stability and security.

GitHub Docs

@jsjoeio jsjoeio merged commit 1070db7 into main May 4, 2021
@jsjoeio jsjoeio deleted the jsjoeio/add-trivy branch May 4, 2021 18:59
@jsjoeio jsjoeio mentioned this pull request May 14, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jawnsy jawnsy jawnsy approved these changes

@oxy oxy oxy left review comments

Assignees

@jsjoeio jsjoeio

Labels
security Security related
Projects
None yet
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

Scan built docker images using trivy or grype
3 participants
@jsjoeio @jawnsy @oxy