twoliter: add check-advisories task to lint BRSAs #467

ginglis13 · 2025-02-24T22:57:29Z

Issue number:

Closes #466

Description of changes:

Lint BRSAs for CVE / GHSA IDs in a new task, check-advisories. Also ensure that each directory under "advisories" in a project has an associated tag on the project.

Add this to the list in the meta task "check"

Testing done:

[x] Run against kernel-kit with bad CVE IDs

╭─fedora ~/kernel-kit  ‹v1.1.3*›
╰─➤  ./tools/twoliter/twoliter make check-advisories --cargo-home /home/fedora/kernel-kit/.cargo --arch $(uname -m)
[2025-02-28T20:20:24Z INFO  twoliter::project::lock] Resolving SDK project reference to check against lock file
[2025-02-28T20:20:25Z INFO  twoliter::project::lock::image] Resolving dependency image dependency '[email protected]/bottlerocket/bottlerocket-sdk:v0.50.1'.
[cargo-make] INFO - cargo make 0.37.24
[cargo-make] INFO -
[cargo-make] INFO - Build File: /home/fedora/kernel-kit/build/tools/Makefile.toml
[cargo-make] INFO - Task: check-advisories
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: check-advisories
error: advisory CVE ID did not match expression '^cve\s+=\s+"CVE-\d{4}-(0\d{3}|[1-9]\d{3,})"' and may contain non-ASCII characters
cve = "CVE‑2024‑53869"
cve = "CVE‑2024‑0150"
cve = "CVE‑2024‑0131"
cve = "CVE‑2024‑0149"
cve = "CVE‑2024‑0147"
Error while executing command, exit code: 1
Error: Command was unsuccessful, exit code 105

Fixed in bottlerocket-os/bottlerocket-kernel-kit#55

[x] Run against kernel-kit with bad GHSA ID (mocked advisory)

╰─➤  ./tools/twoliter/twoliter make check-advisories --cargo-home /home/fedora/kernel-kit/.cargo --arch $(uname -m)
[2025-02-28T20:22:47Z INFO  twoliter::project::lock] Resolving SDK project reference to check against lock file
[2025-02-28T20:22:47Z INFO  twoliter::project::lock::image] Resolving dependency image dependency '[email protected]/bottlerocket/bottlerocket-sdk:v0.50.1'.
[cargo-make] INFO - cargo make 0.37.24
[cargo-make] INFO -
[cargo-make] INFO - Build File: /home/fedora/kernel-kit/build/tools/Makefile.toml
[cargo-make] INFO - Task: check-advisories
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: check-advisories
error: advisory GHSA ID did not match expression '^ghsa\s+=\s+"GHSA(-[23456789cfghjmpqrvwx]{4}){3}"' and may contain non-ASCII characters
ghsa = "GHSA-0000-5224-cccc"
Error while executing command, exit code: 1
Error: Command was unsuccessful, exit code 105

[x] Run in a Twoliter project without an advisories subdirectory

After ASCII fixes, run against kernel-kit:

╭─fedora ~/kernel-kit  ‹develop*›
╰─➤  ./tools/twoliter/twoliter make check-advisories --cargo-home /home/fedora/kernel-kit/.cargo --arch $(uname -m)
[2025-02-25T22:57:43Z INFO  twoliter::project::lock] Resolving SDK project reference to check against lock file
[2025-02-25T22:57:43Z INFO  twoliter::project::lock::image] Resolving dependency image dependency '[email protected]/bottlerocket/bottlerocket-sdk:v0.50.1'.
[cargo-make] INFO - cargo make 0.37.24
[cargo-make] INFO -
[cargo-make] INFO - Build File: /home/fedora/kernel-kit/build/tools/Makefile.toml
[cargo-make] INFO - Task: check-advisories
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: check-advisories
[cargo-make] INFO - Build Done in 0.22 seconds.

Test case of unexpected directory under advisories/:

╭─fedora ~/kernel-kit  ‹develop*›
╰─➤  ls advisories
0  1.0.2  1.0.4  1.0.6  1.0.7  1.1.0  staging
╭─fedora ~/kernel-kit  ‹develop*›
╰─➤  ./tools/twoliter/twoliter make check-advisories --cargo-home /home/fedora/kernel-kit/.cargo --arch $(uname -m)
[2025-02-25T22:55:54Z INFO  twoliter::project::lock] Resolving SDK project reference to check against lock file
[2025-02-25T22:55:55Z INFO  twoliter::project::lock::image] Resolving dependency image dependency '[email protected]/bottlerocket/bottlerocket-sdk:v0.50.1'.
[cargo-make] INFO - cargo make 0.37.24
[cargo-make] INFO -
[cargo-make] INFO - Build File: /home/fedora/kernel-kit/build/tools/Makefile.toml
[cargo-make] INFO - Task: check-advisories
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: check-advisories
error: no corresponding tag found for advisories/0 directory
Error while executing command, exit code: 1
Error: Command was unsuccessful, exit code 105

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

ginglis13 · 2025-02-25T00:43:30Z

error: cannot install package `cargo-deny 0.18.0`, it requires rustc 1.85.0 or newer, while the currently active rustc version is 1.81.0-nightly
`cargo-deny 0.17.0` supports rustc 1.81.0

I'll update to a newer nightly rust compiler in an additional commit to this PR

ginglis13 · 2025-02-25T01:15:31Z

^ added a commit for updating nightly rust toolchain to 2025-02-24. Still working through some issues with cargo-deny. Currently it is flagging all workspace dependencies as unused.

e: working through

Error: -25 01:21:22 [ERROR] failed to interpret `cargo metadata`'s json: expected `,` or `}` at line 1 column 251366: expected `,` or `}` at line 1 column 251366

now. This is strange because as far as I can tell, cargo metadata is returning valid JSON - cargo metadata | jq works fine.

ginglis13 · 2025-02-25T22:39:52Z

^ force push adds a check to ensure that each versioned directory under advisories/ has a corresponding git tag

ginglis13 · 2025-02-25T22:43:14Z

twoliter/embedded/Makefile.toml

+    grep v$(basename ${version})$ <(PAGER= git tag)
+    if [ "$?" -ne 0  ]; then


This implies a dependency on the project this check runs against having all tags fetched. Curious for feedback on either:

fetching tags in this check

or, removing this as a default check and running this check as a new GitHub CI step on projects. Those projects will set fetch-tags: true in respective actions/checkout steps

ginglis13 · 2025-02-25T22:59:44Z

^ force push handles return code from grep more gracefully with error message. Testing added to PR description

ginglis13 · 2025-02-26T20:24:20Z

^ force push pins cargo-deny to v0.17.0 and reverts toolchain upgrade

ginglis13 · 2025-02-28T20:37:12Z

^ force push based on some offline feedback from @cbgbt . We should only be linting on structured data, which in this case is a CVE or GHSA ID. Adjusted the lint to validate those IDs rather than lint entire advisory files for non-ASCII chars

Also updated to latest rust nightly toolchain now that cargo-deny 0.18.1 is released with a fix for EmbarkStudios/krates#100

Lint BRSAs for non-ASCII characters that may be included in advisory information in a new task, check-advisories. Also ensure that each directory under "advisories" in a project has an associated tag on the project Add this to the list in the meta task "check" Signed-off-by: Gavin Inglis <[email protected]>

Update rust toolchain to the latest nightly release. Address lints and license clarifications as a result. Signed-off-by: Gavin Inglis <[email protected]>

Signed-off-by: Gavin Inglis <[email protected]>

ginglis13 · 2025-03-01T00:20:50Z

^ force push links against musl in CI for make build. integ test failures like

 0.166 /host/build/tools/unplug: /lib64/libc.so.6: version `GLIBC_2.39' not found (required by /host/build/tools/unplug)

Indicated that builds were failing due to an older glibc coming from the bottlerocket-sdk https://github.com/bottlerocket-os/bottlerocket-sdk/blob/7b29868e87346b1538403509a79c22a67a463bce/Dockerfile#L186

cbgbt · 2025-03-03T19:24:57Z

twoliter/embedded/Makefile.toml

+script_runner = "bash"
+script = [
+'''
+if find advisories -name '*.toml' -type f >/dev/null 2>&1 ; then


Suggested change

if find advisories -name '*.toml' -type f >/dev/null 2>&1 ; then

if find ${BUILDSYS_ROOT_DIR}/advisories -name '*.toml' -type f >/dev/null 2>&1 ; then

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 advisories directory that require this.

cbgbt · 2025-03-03T19:49:52Z

twoliter/embedded/Makefile.toml

+  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/')"


If I understand right, these checks will fail if an advisory document contains the string cve or ghsa but does not contain a valid CVE or GHSA line.

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 cve/ghsa rather than the entirety of the document? That way we don't misfire on freeform text e.g. in an advisory description that happens to have the character string cve in it.

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 (oneliter?) under tools/ to run some of these tasks, but in the longer term we probably want to express the task graph currently in Makefile.toml inside Twoliter and drop cargo-make as a dependency.

Can we at least constrain the mismatch checks to lines that start with cve/ghsa rather than the entirety of the document? That way we don't misfire on freeform text e.g. in an advisory description that happens to have the character string cve in it.

You're right, the awk half of this pipeline would break down on any inclusion of "cve"/"ghsa", including advisory descriptions. I'll modify those respective checks to match on line start.

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.

I totally agree; that along with including advisory types, tools, and models in twoliter are a more robust long term effort

ginglis13 · 2025-03-20T21:27:28Z

Chatted offline with @cbgbt . I'm going to take this approach - add a new lints crate under twoliter/tools that provides multiple binaries. I'll write a binary for the lint I've prepared in this PR, embed that binary in twoliter, and invoke it under the check-advisories task

ginglis13 requested review from bcressey and cbgbt February 24, 2025 22:57

ginglis13 mentioned this pull request Feb 25, 2025

fix incorrect hyphens used in BRSA for nvidia-container-toolkit CVE-2025-23359 bottlerocket-os/bottlerocket-core-kit#381

Merged

ginglis13 force-pushed the check-advisories branch from f93c95f to 64b8cad Compare February 25, 2025 22:39

ginglis13 commented Feb 25, 2025

View reviewed changes

ginglis13 force-pushed the check-advisories branch from 64b8cad to 9c32cf6 Compare February 25, 2025 22:59

ginglis13 mentioned this pull request Feb 26, 2025

krates 0.18.0 fails to parse cargo metadata of project that uses unstable artifacts dependency declarations EmbarkStudios/krates#100

Closed

ginglis13 force-pushed the check-advisories branch from 9c32cf6 to 1018ea5 Compare February 26, 2025 20:13

ginglis13 force-pushed the check-advisories branch 2 times, most recently from e0c189c to cf07a98 Compare February 28, 2025 20:33

ginglis13 force-pushed the check-advisories branch 6 times, most recently from f0abf30 to e631461 Compare March 1, 2025 00:02

ginglis13 added 3 commits March 1, 2025 00:16

update rust toolchain to latest nightly

967d91c

Update rust toolchain to the latest nightly release. Address lints and license clarifications as a result. Signed-off-by: Gavin Inglis <[email protected]>

ci: link against musl

276d91a

Signed-off-by: Gavin Inglis <[email protected]>

ginglis13 force-pushed the check-advisories branch from e631461 to 276d91a Compare March 1, 2025 00:16

jmt-lab approved these changes Mar 1, 2025

View reviewed changes

cbgbt reviewed Mar 3, 2025

View reviewed changes

cbgbt mentioned this pull request Mar 3, 2025

Update rust toolchain to nightly-2025-02-28 #471

Merged

ginglis13 mentioned this pull request May 3, 2025

nvidia-kmod: add advisories for tesla 535.247.01 driver bottlerocket-os/bottlerocket-kernel-kit#134

Merged

		grep v$(basename ${version})$ <(PAGER= git tag)
		if [ "$?" -ne 0 ]; then

	if find advisories -name '*.toml' -type f >/dev/null 2>&1 ; then
	if find ${BUILDSYS_ROOT_DIR}/advisories -name '*.toml' -type f >/dev/null 2>&1 ; then

twoliter: add check-advisories task to lint BRSAs #467

Are you sure you want to change the base?

twoliter: add check-advisories task to lint BRSAs #467

Uh oh!

Conversation

ginglis13 commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ginglis13 commented Feb 25, 2025

Uh oh!

ginglis13 commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ginglis13 commented Feb 25, 2025

Uh oh!

ginglis13 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

ginglis13 commented Feb 25, 2025

Uh oh!

ginglis13 commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ginglis13 commented Feb 28, 2025

Uh oh!

ginglis13 commented Mar 1, 2025

Uh oh!

cbgbt Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

cbgbt Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

ginglis13 Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

ginglis13 commented Mar 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ginglis13 commented Feb 24, 2025 •

edited

Loading

ginglis13 commented Feb 25, 2025 •

edited

Loading

ginglis13 commented Feb 26, 2025 •

edited

Loading