Validate codecov uploader before executing.#125
Open
nuclearsandwich wants to merge 1 commit intomoveit:masterfrom
Open
Validate codecov uploader before executing.#125nuclearsandwich wants to merge 1 commit intomoveit:masterfrom
nuclearsandwich wants to merge 1 commit intomoveit:masterfrom
Conversation
nuclearsandwich
commented
May 3, 2021
| # Download and validate codecov bash uploader script | ||
| travis_run --title "Download codecov uploader" "curl -s 'https://codecov.io/bash' > codecov" | ||
| local codecov_version="$(grep -o 'VERSION=\"[0-9\.]*\"' codecov | cut -d'"' -f2);" | ||
| travis_run --title "Validate codecov uploader" shasum -a 512 -c <(curl -s "https://raw.githubusercontent.com/codecov/codecov-bash/${codecov_version}/SHA512SUM" | grep -w codecov) |
Author
There was a problem hiding this comment.
The shasum utility is provided by perl on most systems which is a dependency of git on most systems. If there is a way to explicitly note that dependency in this script I think doing so is worth it.
After the recent Codecov security incident[1] I've been reviewing codecov usage across ROS repositories. This script is fetching the codecov bash uploader and env scripts without performing the recommended validation step. The validation step does not appear to have been widely explained or publicised and even the official codecov GitHub action was not validating the script until the recent security incident. I have made an attempt to validate the bash uploader here. The environment script is also used but early enough in the process that it wasn't convenient to validate with my lack of familiarity in the travis scripting style. If there's interest I can probably refactor this to fetch and validate both scripts during the setup phase instead of trying to do the bash uploader inline. However I wanted to start with a minimal change. [1]: https://about.codecov.io/security-update/
809cc62 to
55be2f4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After the recent Codecov security incident1 I've been reviewing
codecov usage across ROS repositories.
This script is fetching the codecov bash uploader and env scripts
without performing the recommended validation step.
The validation step does not appear to have been widely explained or
publicised and even the official codecov GitHub action was not
validating the script until the recent security incident.
I have made an attempt to validate the bash uploader here.
The environment script is also used but early enough in the process that
it wasn't convenient to validate with my lack of familiarity in the
travis scripting style.
If there's interest I can probably refactor this to fetch and validate
both scripts during the setup phase instead of trying to do the bash
uploader inline. However I wanted to start with a minimal change.
Author's note: I initially pushed this branch directly to the moveit_ci parent repository in my capacity as an org owner. I have deleted that branch and re-opened the PR from my own fork. Maintainer edits are enabled.