workflows: Add clippy, nancy, and binskim release checks #319

miz060 · 2025-02-24T22:35:41Z

Merge Checklist

Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
- Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
The upstream/missing label (or upstream/not-needed) has been set on the PR.

Summary

Introduces

a Clippy GitHub CI PR gate to enforce Rust code quality.
a Nancy GitHub CI PR gate to enforce Go dependency security.
a Binskim GitHub CI PR gate to enforce binary hardening.

These checks will also be added as part of kata release process later in kata conformance test pipeline through test containers.

Existing upstream kata ci static checks include below tools, so there is no duplication:

Go: golangci-lint
Shell scripts: shellcheck and syntax validation (bash -n)
JSON: jq
YAML: yamllint
Dockerfiles: hadolint
C/C++ files: Checks SPDX license headers and copyright statements
XML files: xmllint

Test Methodology

Test runs:

Nancy: https://github.com/microsoft/kata-containers/actions/runs/13509385513/job/37746262440?pr=319
Clippy: https://github.com/microsoft/kata-containers/actions/runs/13509385510/job/37746262539?pr=319 (error to be addressed after upstream code update
BinSkim: https://github.com/microsoft/kata-containers/actions/runs/13509385497/job/37746262355?pr=319

Add clippy, nancy, binskim release checks to help stablize kata releases Signed-off-by: Mitch Zhu <[email protected]>

ms-mahuber · 2025-02-25T00:38:17Z

.github/workflows/clippy.yaml

+          echo "✅ Clippy check passed for kata overlay."
+
+      - name: Run Clippy on tardev-snapshotter
+        working-directory: src/tardev-snapshotter


do we wan to also scan utarfs?

ms-mahuber · 2025-02-25T00:39:12Z

.github/workflows/binskim.yaml

+        run: |
+          dotnet new console -n TempConsoleApp
+          cd TempConsoleApp
+          echo "Installing BinSkim version 1.9.5"


is there a pattern to install the latest stable version?

ms-mahuber · 2025-02-25T00:39:46Z

.github/workflows/binskim.yaml

+          echo "Building kata pod sandboxing binaries"
+          pushd tools/osbuilder/node-builder/azure-linux
+          # Adapt build script for ubuntu environment
+          sed -i 's|^OS_VERSION=.*|OS_VERSION="3.0"|' common.sh


we don't need this. If you code doesn't work for Ubuntu, we can pass OS_VERSION as a variable for make package

ms-mahuber · 2025-02-25T00:40:27Z

.github/workflows/binskim.yaml

+
+          # Prepare go binaries for binskim
+          pushd src/runtime
+          strip --strip-unneeded containerd-shim-kata-v2


should stripping be something we should generally be doing when we build?

It might remove information needed when debugging, no?

ms-mahuber · 2025-02-25T00:41:44Z

.github/workflows/binskim.yaml

@@ -0,0 +1,128 @@
+name: Release Binary Hardening checks


should we add a codeql file as well for the tarfs module, similar to https://github.com/kata-containers/kata-containers/pull/10930/files ?

Redent0r · 2025-02-25T18:24:48Z

For the clippy check, the agent makefile already has a check target that runs standard_rust_check. And standard_rust_check runs clippy. I'm wondering if it's better to add a clippy.yaml vs running make check on the things we want to clippy check (and make each of them required CI checks). Running make check seems to be the upstream approach. cc @sprt

sprt

IMO we're adding quite a bit of complexity when we could simply add all these checks to the static-checks.yaml.

sprt · 2025-02-25T18:23:27Z

.github/workflows/binskim.yaml

+      - name: Validate BinSkim results
+        run: |
+          # Validate pod sandboxing binaries
+          for result in artifacts/vanilla/*_binskim_result; do
+            if [ ! -f "$result" ]; then
+              echo "❌ Error: $result was not generated."
+              exit 1
+            fi
+            echo "Validating: pod sandboxing ${result}" 
+            cat "$result"
+
+            if grep -qi "fail" "$result"; then
+              echo "❌ Error: Failures detected in pod sandboxing binary: $result"
+              exit 1
+            fi
+            echo "--------------------------- End-------------------------"
+          done
+          echo "✅ All pod sandboxing binaries passed BinSkim."
+
+          # Validate confpod binaries
+          for result in artifacts/confpods/*_binskim_result; do
+            if [ ! -f "$result" ]; then
+              echo "❌ Error: $result was not generated."
+              exit 1
+            fi
+            echo "Validating: conf pod ${result}" 
+            cat "$result"
+
+            if grep -qi "fail" "$result"; then
+              echo "❌ Error: Failures detected in Confidential Pod binary: $result"
+              exit 1
+            fi
+            echo "--------------------------- End-------------------------"
+          done
+          echo "✅ All confpod binaries passed BinSkim."


These should be independent steps so one binary failing doesn't block other results. We should probably rework this step using matrix:.

sprt · 2025-02-25T18:25:23Z

.github/workflows/clippy.yaml

+
+jobs:
+  clippy:
+    name: Run Clippy on Rust Components


The upstream static checks (static-checks.yaml) already run clippy - we should leverage those?

sprt · 2025-02-25T18:26:13Z

.github/workflows/clippy.yaml

+      - name: Run Clippy on agent
+        working-directory: src/agent
+        run: |
+          echo "Running Clippy on kata agent..."
+          if ! cargo clippy -- -D warnings; then
+            echo "❌ Clippy check failed for kata agent."
+            exit 1
+          fi
+          echo "✅ Clippy check passed for kata agent."
+
+      - name: Run Clippy on overlay
+        working-directory: src/overlay
+        run: |
+          echo "Running Clippy on kata overlay..."
+          if ! cargo clippy -- -D warnings; then
+            echo "❌ Clippy check failed for kata overlay."
+            exit 1
+          fi
+          echo "✅ Clippy check passed for kata overlay."
+
+      - name: Run Clippy on tardev-snapshotter
+        working-directory: src/tardev-snapshotter
+        run: |
+          echo "Running Clippy on tardev-snapshotter..."
+          if ! cargo clippy -- -D warnings; then
+            echo "❌ Clippy check failed for tardev-snapshotter."
+            exit 1
+          fi
+          echo "✅ Clippy check passed for tardev-snapshotter."


These steps should be independent too.

And no need for the output complexity, simply running cargo clippy -- -D warnings will return an exit code already.

sprt · 2025-02-25T18:29:09Z

.github/workflows/nancy.yaml

+      - name: Verify Nancy Installation
+        run: |
+          echo "Checking Nancy installation..."
+          nancy --help || echo "Nancy installed successfully!"
+


Redundant

Suggested change

- name: Verify Nancy Installation

run: |

echo "Checking Nancy installation..."

nancy --help || echo "Nancy installed successfully!"

sprt · 2025-02-25T18:31:18Z

.github/workflows/clippy.yaml

+      - name: Install Rust Toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy


We should install Rust the way that upstream does it (install_rust.sh).

Regardless, we might not want to pick up Rust's latest version here, as it could break us. We should probably fetch the version from versions.yaml (and ensure the version that's there is the one we want).

PR Overview

This PR introduces new GitHub CI workflows for release checks that enforce binary hardening, dependency security, and Rust code quality.

Adds a binskim workflow to analyze compiled binaries for security hardening.
Adds a nancy workflow to scan Go dependencies for vulnerabilities.
Adds a clippy workflow to run Rust linter checks on source components.

Reviewed Changes

File	Description
.github/workflows/binskim.yaml	Adds steps to set up and run BinSkim on both kata pod and confpod binaries.
.github/workflows/nancy.yaml	Sets up a Nancy vulnerability scan on Go dependencies.
.github/workflows/clippy.yaml	Configures Clippy runs on Rust components using specified toolchains.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

.github/workflows/binskim.yaml:73

The binary 'containerd-shim-kata-v2' is being stripped twice (once at line 47 and again at line 73), which may be redundant. Consider removing one of these calls if it is not required for the intended binary preparation.

strip --strip-unneeded containerd-shim-kata-v2

miz060 force-pushed the mitchzhu/sdl branch 2 times, most recently from 4706b68 to d6648a0 Compare February 24, 2025 22:39

workflows: Add clippy, nancy, and binskim release checks

e63c06c

Add clippy, nancy, binskim release checks to help stablize kata releases Signed-off-by: Mitch Zhu <[email protected]>

miz060 force-pushed the mitchzhu/sdl branch from d6648a0 to e63c06c Compare February 24, 2025 22:43

miz060 changed the title ~~Add clippy, nancy, and binskim release checks~~ workflows: Add clippy, nancy, and binskim release checks Feb 24, 2025

miz060 added upstream/missing PRs that are yet to be upstreamed upstream/not-needed PRs that will not be upstreamed (e.g. internal) and removed upstream/missing PRs that are yet to be upstreamed labels Feb 24, 2025

miz060 marked this pull request as ready for review February 24, 2025 23:01

miz060 requested review from a team as code owners February 24, 2025 23:01

sprt self-requested a review February 24, 2025 23:04

ms-mahuber reviewed Feb 25, 2025

View reviewed changes

sprt reviewed Feb 25, 2025

View reviewed changes

christopherco requested a review from Copilot March 7, 2025 03:34

Copilot AI reviewed Mar 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

workflows: Add clippy, nancy, and binskim release checks #319

workflows: Add clippy, nancy, and binskim release checks #319

miz060 commented Feb 24, 2025 •

edited

Loading

ms-mahuber Feb 25, 2025

ms-mahuber Feb 25, 2025

ms-mahuber Feb 25, 2025

ms-mahuber Feb 25, 2025

sprt Feb 25, 2025

ms-mahuber Feb 25, 2025

Redent0r commented Feb 25, 2025

sprt left a comment

sprt Feb 25, 2025

sprt Feb 25, 2025

sprt Feb 25, 2025

sprt Feb 25, 2025

sprt Feb 25, 2025

workflows: Add clippy, nancy, and binskim release checks #319

Are you sure you want to change the base?

workflows: Add clippy, nancy, and binskim release checks #319

Conversation

miz060 commented Feb 24, 2025 • edited Loading

Merge Checklist

Summary

Test Methodology

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Redent0r commented Feb 25, 2025

sprt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

miz060 commented Feb 24, 2025 •

edited

Loading