-
Notifications
You must be signed in to change notification settings - Fork 39
twoliter: add check-advisories task to lint BRSAs #467
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -416,6 +416,7 @@ done | |
| [tasks.check] | ||
| dependencies = [ | ||
| "check-cargo-version", | ||
| "check-advisories", | ||
| "unit-tests", | ||
| "check-fmt", | ||
| "check-lints", | ||
|
|
@@ -540,6 +541,61 @@ fi | |
| ''' | ||
| ] | ||
|
|
||
| # Task to lint Bottlerocket Security Advisories by checking for valid CVE or GHSA IDs | ||
| # and verifying that each versioned directory under "advisories" has a corresponding | ||
| # tag on the Twoliter project this task runs against. | ||
| [tasks.check-advisories] | ||
| script_runner = "bash" | ||
| script = [ | ||
| ''' | ||
| if find advisories -name '*.toml' -type f >/dev/null 2>&1 ; then | ||
|
|
||
| # Ensure each versioned advisories directory has a corresponding release tag. | ||
| for version in $(find advisories/* -type d -not -path advisories/staging); do | ||
| set +e; grep -q v$(basename ${version})$ <(PAGER= git tag); rc="$?"; set -e; | ||
| if [ "${rc}" -ne 0 ]; then | ||
| echo "error: no corresponding tag found for ${version} advisories directory" >&2 | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
|
|
||
| # Not all BRSAs might have a CVE; there can be cases where an advisory | ||
| # is for a GHSA, for example, but no corresponding CVE, and vice versa. | ||
| # Check separately for GHSA ID regex when a 'ghsa' is included and for | ||
| # CVE ID regex when a 'cve' is included. | ||
|
|
||
| # 1. If ghsa line exists and GHSA ID does not match regex, error | ||
|
|
||
| # Regex to strictly match a GHSA identifier in an advisory | ||
| # https://github.com/github/advisory-database?tab=readme-ov-file#ghsa-ids | ||
| ghsa_regex="^ghsa\s+=\s+\"GHSA(-[23456789cfghjmpqrvwx]{4}){3}\"" | ||
|
|
||
| # Find all non-matching GHSA lines and report | ||
| ghsa_found="$(grep -L --include '*.toml' -R -P ${ghsa_regex} advisories | xargs awk '/ghsa/')" | ||
| if [ ! -z "${ghsa_found}" ] ; then | ||
| echo "error: advisory GHSA ID did not match expression '${ghsa_regex}' and may contain non-ASCII characters" >&2 | ||
| echo "${ghsa_found}" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # 2. If cve line exists and CVE ID does not match regex, error | ||
|
|
||
| # Regex to strictly match CVE identifier in an advisory | ||
| # https://cve.mitre.org/cve/identifiers/tech-guidance.html#input_format | ||
| cve_regex="^cve\s+=\s+\"CVE-\d{4}-(0\d{3}|[1-9]\d{3,})\"" | ||
|
|
||
| # Find all non-matching CVE lines and report | ||
| cve_found="$(grep -L --include '*.toml' -R -P ${cve_regex} advisories | xargs awk '/cve/')" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand right, these checks will fail if an advisory document contains the string I think what we actually want to do is semantically parse the advisory TOML document and check that the CVE and GHSA lines are valid, right? I think parsing with regexes like this leads to a lot of potential for surprising and valid inputs to get caught in the crossfire. Can we at least constrain the mismatch checks to lines that start with In general I want to see code like this in Twoliter hoisted into Rust so that we can get unit test coverage and a higher ability to take advantage of software libraries like a toml parser. We don't really have a "recommended" way to do that. In a pinch we could probably have Twoliter provide a helper binary (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You're right, the
I totally agree; that along with including advisory types, tools, and models in twoliter are a more robust long term effort |
||
| if [ ! -z "${cve_found}" ] ; then | ||
| echo "error: advisory CVE ID did not match expression '${cve_regex}' and may contain non-ASCII characters" >&2 | ||
| echo "${cve_found}" >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
| ''' | ||
| ] | ||
|
|
||
| [tasks.check-golangci-lint] | ||
| script = [ | ||
| ''' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use full filepaths in case the current working directory is not set as expected. There are a few cases in here that refer to the
advisoriesdirectory that require this.