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

Add Trivy Security Scans #17

Merged
merged 10 commits into from
Aug 9, 2021
Merged

Add Trivy Security Scans #17

merged 10 commits into from
Aug 9, 2021

Conversation

celanthe
Copy link
Contributor

@celanthe celanthe commented Jul 21, 2021

Description: This pull request introduces two Trivy security scans running in repo mode on both PUSH and PULL requests.

The first scan checks for unknown, low, and medium security vulnerabilities, returns an exit-code: 0, and formats the scan results into a template.

The second scan runs in repo mode and checks for high and critical vulnerabilities, fails with exit-code: 1 if vulnerabilities are found, and formats the scan results into a template.

Description: This pull request introduces two Trivy security scans running in repo mode on both <code> PUSH </code> and <code> PULL </code> requests.

The first scan checks for unknown, low, and medium security vulnerabilities, returns an exit-code: 0, and formats the scan results into  a template. 

The second scan runs in repo mode and checks for high and critical vulnerabilities, and fails with exit-code: 1 if vulnerabilities are found.
@celanthe celanthe added the enhancement New feature or request label Jul 21, 2021
@celanthe celanthe requested a review from Langleu July 21, 2021 14:39
@celanthe celanthe self-assigned this Jul 21, 2021
celanthe added 3 commits July 21, 2021 10:25
Updated YAML formatting and added additional context to outputs
Attempting to fix YAML formatting
Corrected formatting of YAML and ran through https://jsonformatter.org/yaml-validator
Copy link
Contributor

@Langleu Langleu left a comment

Choose a reason for hiding this comment

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

Hey @celanthe, sadly the proposed change is not possible.

An additional action can not be defined within another action.
So deleting line 68 till 78 would work in theory.
Sadly the action is a composite action, this means that currently it does not support the uses keyword.

There is a PR in the making - actions/runner#1144 , which will support this at some point in the future, but it's not clear yet when it would be released.

So for now we would have to come up with a different strategy.
There is either the option to include it via scripts, e.g. download Trivy via script and execute it ... essentially doing what the already defined trivy action does.
Alternatively, ask contributors to include an example trivy workflow and possibly depend on it for the release action.

@celanthe
Copy link
Contributor Author

Hi @Langleu! Interestingly enough, it seems that the PR you linked had some movement as of earlier today! :) It looks like adding the Trivy scans on lines 68-78 might be possible to do now, if I'm reading this GitHub Actions documentation render correctly?

I welcome your thoughts!

@Langleu
Copy link
Contributor

Langleu commented Jul 28, 2021

@celanthe , great news! I didn't expect them to merge that quickly as it was stalling for a while.
Sure, go ahead and give it a try!

@celanthe celanthe requested a review from Langleu July 28, 2021 14:48
celanthe added 2 commits July 28, 2021 09:52
Added a missing - to line 1 column 1
Updated YAML formatting to correct test failure
@celanthe
Copy link
Contributor Author

celanthe commented Jul 28, 2021

Awesome @Langleu! That's great news! I ran the YAML through one last check and it's all valid! :)

Unfortunately, I'm running into an issue with the pre-commit check. All my tests come back 'passed' except for 'prettier'. Which is still failing, causing the pre-commit check to fail. It looks like the workflows and actions will work, however!

Validate GitHub Workflows................................................Passed
Validate GitHub Actions..................................................Passed
Validate Example Workflows...............................................Passed
Shellcheck Bash Linter...................................................Passed

The message in the logs says, "pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: pre-commit run --all-files.
To run pre-commit as part of git workflow, use pre-commit install.
All changes made by hooks:
diff --git a/action.yml b/action.yml
index c3e13d6..4255359 100644"

I've dug through google and the logs and this seems to be intentional in that prettier hooks will fail if a hook modifies a commit. It seems like I have a few options here:

  1. Accept the automatic modifications and git add
  2. Adjust pre-commit hook settings and re-commit

I'm not quite sure how to reproduce locally and tell prettier that everything is okay. I welcome your thoughts here!

@Langleu
Copy link
Contributor

Langleu commented Jul 29, 2021

Hey @celanthe, you can fix those errors by running pre-commit locally and push the changes.
When cloning the repo and developing locally, a pre-commit hook will be triggered with every commit you try to add, this will lint and format your code changes so you won't be able to push anything out of the line.
In this case you'll have to run the suggested pre-commit run --all-files as your changes are already commited.
For more information on pre-commit and how to install it, have a look at their website https://pre-commit.com/ .
Generally, the PR was just an ADR (Architecture Decision Record), so till the actual implementation is done it can still take quite some time.
As mentioned last time, line 68-78 would have to be deleted anyway as you can't define a GitHub Action within another.
So for now the only possible way to integrate it into this composite action would be to manually retrieve the trivy binary and then execute the steps like that via bash.

@celanthe
Copy link
Contributor Author

celanthe commented Jul 29, 2021

Hi @Langleu! Awesome, I fixed this up! :)

I added a bash script that should download and install Trivy, and deleted lines 68-78! Fingers crossed that should get us a bit closer to this working! And all the tests are passing now. I got pre-check working and all sorted! :D

I was wondering if it's possible to make this optional? I suppose the best way to do that would be to release a new version as we'd discussed with this new code included, and people can upgrade if they so choose? Or would another path forward be potentially breaking out lines 68-98 into their own action? Or perhaps running a [dispatch action](https://github.com/marketplace/actions/dispatch-action] where this would be step one, and if an exit-code: 0 is returned it would send a message to kick off the community-action-maven-release workflow?

Just thinking out loud! :)

@Langleu
Copy link
Contributor

Langleu commented Jul 30, 2021

Hey @celanthe,
one could introduce an additional input called e.g. trivy or activate-trivy enable-vuln-scan, something that fits. Set it to default "false" or "0". In the bash script, one could then only run the trivy stuff if the input is set to either true or 1.

Or would another path forward be potentially breaking out lines 68-98 into their own action?

Yes, this would definitely be an option. The only downside is that it will require more "effort" from the user base to include all of it or exclude it if a different solution is introduced in the future and is, therefore, more error-prone.
Your idea would then be to trigger the release action via Trivy, which should certainly be possible as well.
As I said it will require a lot of changes from the implementation side and also from the user base for a potentially temporary implementation.

One way to test if your action is working would be to use our demo repository - https://github.com/camunda/infra-github-demo .
E.g. create an extra branch, and change in there the following line to https://github.com/camunda/infra-github-demo/blob/a96978ad29f91e4a8dbdda195eaf80ff3d0482d8/.github/workflows/deploy.yml#L32 e.g. camunda-community-hub/community-action-maven-release@celanthe-patch-2 and then create a release based on the branch, which should, in turn, run the branch you referenced of the release action. In case it is successful, don't forget to delete/ drop without releasing the repository in Sonatype as we don't want to publish it to maven central.

Let me know what you think and whether we should have a synchronous call on the topic to iterate over it / pair programming?

@celanthe
Copy link
Contributor Author

HI @Langleu! I like the idea of introducing an additional input called enable-trivy-vuln-scan and setting it to default "false" or "0", and as you'd mentioned n the bash script, one could then only run the Trivy piece if the input is set to either true or 1.

I would love to pair program on this part, for sure! I unfortunately am getting a 404 error when I try to access the infra-github-demo you linked. Is that a GitHub permissions issue? Would I need to open a ticket with IT to get access to that repo for testing purposes?

Thank you so much! I look forward to pairing, and will try to find a time on our calendars over the next few weeks to make it work!

@celanthe
Copy link
Contributor Author

celanthe commented Aug 2, 2021

Hi @Langleu! I got access to the infra-demo repository and set up a new branch/test workflow and changed L32 as you'd suggested. I can get the workflow from this PR to build successfully 🎊 , but it's failing on the deploy step. Both your run https://github.com/camunda/infra-github-demo/actions/runs/1077574813 and mine at https://github.com/camunda/infra-github-demo/actions/runs/1091353319 have the same four errors, too, which is interesting. I've done some digging and it seems to maybe be something related to templating in GitHub Actions...

https://stackoverflow.com/questions/66581573/unexpected-type-encountered-while-reading-resources-the-type-mappingtoken

Not sure if that's what's causing our issue here, but it might be a good place to start? :) I look forward to connecting!

@Langleu Langleu force-pushed the celanthe-patch-2 branch 2 times, most recently from fc0a8e3 to 5254577 Compare August 6, 2021 08:27
@celanthe celanthe merged commit 2ae2b43 into main Aug 9, 2021
@celanthe celanthe deleted the celanthe-patch-2 branch August 9, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants