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

Move security audit checks to a daily schedule [SAME VERSION] #264

Merged
merged 7 commits into from Mar 11, 2021

Conversation

romoh
Copy link
Contributor

@romoh romoh commented Mar 3, 2021

What this PR does / why we need it:
As part of check-rust workflow, we also run cargo audit to detect any security vulnerabilities in Akri's dependencies. This was added to catch newly added dependencies imposing security risks to Akri. However, we ended up with those checks blocking PRs & code merges because of existing dependencies being out of date or a new vulnerability is discovered in a version of a crate that Akri is already consuming, so this check ended up being very noisy and hence this PR.

This change:

  • Removes security audit checks from the 'check-rust'
  • Creates a new workflow that runs daily that takes care of running the security audit and sends akri team an email in the event a security vulnerability was detected.

A v2 of this change would be to add another step in check-rust workflow to audit newly added dependencies. Another potential improvement is to attempt re-running the audit after a cargo update and in case that succeeds the workflow can kick off the autoupdate-dependencies workflow instead of sending an email to minimize manual intervention.

Note: The audit github action will create an issue when a vulnerability is detected. I'm not very happy with that as that is not a recommended/secure approach. Usually in the event a security vulnerability is detected in some codebase, it is best to fix asap rather than documenting it through issues where others become aware of that and can leverage to compromise the code. I opened an issue on audit-check and will investigate other github action options.

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)

Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

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

Will this automatically post to the same GitHub Issue each time or is that a vNext?

@romoh
Copy link
Contributor Author

romoh commented Mar 9, 2021

Will this automatically post to the same GitHub Issue each time or is that a vNext?

Based on the current github action implementation, for each vulnerability it will search the existing issues and check if an issue is already created, if not then it will create an issue for that vulnerability.

While we can use other actions (with no issue creation), t the one above is the one recommended by rs tools and cargo audit so I'm leaning towards keeping it at the moment and maybe later we can contributed back to make issue creation optional.

@romoh romoh merged commit 062d5c4 into project-akri:main Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants