Skip to content
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.

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

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

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

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 🚧 In progress in Clean Up via automation Apr 29, 2021
@jsjoeio jsjoeio added this to the v3.9.4 milestone Apr 29, 2021
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! ❤️

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@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

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.

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

.github/workflows/trivy.yml Outdated Show resolved Hide resolved
.github/workflows/trivy.yml Outdated Show resolved Hide resolved
@jsjoeio jsjoeio added the merge when passing Merge the PR automatically once all status checks have passed label Apr 29, 2021
@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
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.

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@jsjoeio jsjoeio removed the merge when passing Merge the PR automatically once all status checks have passed label Apr 30, 2021
@jsjoeio jsjoeio marked this pull request as draft April 30, 2021 19:09
ci/release-image/build.sh Outdated Show resolved Hide resolved
@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

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 :)

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@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.

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@jsjoeio jsjoeio marked this pull request as ready for review May 4, 2021 18:27
This adds both a trivy scan for the repo and a trivy scan for our Docker image.
@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
Clean Up automation moved this from 🚧 In progress to ✅ Done May 4, 2021
@jsjoeio jsjoeio deleted the jsjoeio/add-trivy branch May 4, 2021 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Security related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scan built docker images using trivy or grype
3 participants