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 --version flag to the genpolicy tool that prints the current version - Add version.rs.in template to store the version information - Update makefile to autogenerate version.rs from version.rs.in - Add license to Cargo.toml Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

genpolicy: add --version flag

Linux kernel generates a panic when the init process exits. The kernel is booted with panic=1, hence this leads to a vm reboot. When used as a service the kata-agent service has an ExecStop option which does a full sync and shuts down the vm. This patch mimicks this behavior when kata-agent is used as the init process. Fixes: kata-containers#9429 Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>

genpolicy: add support for cc-local-csi

agent: shutdown vm on exit when agent is used as init process

Add missing cache improvements specifically missing in containerd pull Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

…improvements genpolicy: add missing cache improvements

This patch adds support for the cc-azurefile-csi driver to the genpolicy. Signed-off-by: Archana Choudhary <archana1@microsoft.com>

This patch updates policy samples, required after adding support for cc-azurefile-csi driver in genpolicy. Signed-off-by: Archana Choudhary <archana1@microsoft.com>

genpolicy: add support for cc-azurefile-csi driver

This reverts commit 627be9b, that was insufficient. Waiting for blk devices used just the PCI device/slot index, but not the PCI segment/domain index. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Initialize the CLH Platform a single time. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Hotplug block devices on PCI segments >= 1. PCI segment 0 is used for the network interface, any disks present at Guest boot time, etc. Just bus 0 of each segment is used, and up to 31 devices can be hotplugged to each bus. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

This pod starts successfully when using default AKS-CC settings, and a permissive policy. When the Kata debug options are enabled, this pod fails to start while trying to hotplug image layer index 41. This bug is being investigated. The genpolicy tool should also try to create a smaller policy for this pod, because otherwise "kubectl apply" rejects the policy annotation as being too large. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Lock anyhow version to 1.0.58 because: - Versions between 1.0.59 - 1.0.76 have not been tested yet using Kata CI. However, those versions pass "make test" for the Kata Agent. - Versions 1.0.77 or newer fail during "make test" - see kata-containers#9538. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Implement Agent Policy using the regorus crate instead of the OPA daemon. The OPA daemon will be removed from the Guest rootfs in a future PR. Fixes: kata-containers#9388 Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Bump release version to 3.2.0-azl1.genpolicy0 Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

genpolicy: bump release version

Move pod-many-layers.yaml to needs_containerd_pull category Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

runtime: agent: use PCI segments 1+ for blk devices

agent: use regorus instead of opa

Since OPA binary was replaced by the regorus crate, we can finally stop building and shipping the binary. Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>

The PID needs to be initialized before calling isClhRunning. waitVMM() uses isClhRunning and is called by launchClh() just before returning from function. Fixes: kata-containers#9230 Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>

isClhRunning uses signal 0 to test whether the process is still alive or not. This doesn't work because the process is a direct child of the shim. Once it is dead the process becomes zombie. Since no one waits for it the process lingers until its parent dies and init reaps it. Hence sending signal 0 in isClhRunning will always return success whether the process is dead or not. This patch calls wait to reap the process, if it succeeds that means it is our child process, if not we send the signal. Fixes: kata-containers#9431 Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>

Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

clh: isClhRunning waits for full timeout when clh exits

rootfs: Stop building and shipping OPA

We've discussed this over and over. Let's try to get to an agreement here. I will use this issue to remove the mandatory Issue - PR dependency. Fixes: kata-containers#9500 Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>

ci: cherry-pick relaxed commit check from upstream

- subset of upstream commit 099b241 - should be straightforward to merge when sync-ing to upstream Signed-off-by: Manuel Huber <mahuber@microsoft.com>

- counterpart to upstream a131eec - unblocks build with rust v1.84 Signed-off-by: Manuel Huber <mahuber@microsoft.com>

- post-process to remove box_pointers annotation for generated files - while this unblocks the build, should look for better solution: - request/teach codegen to not add certain linter annotation - update ttrpc-codegen or other dependencies Signed-off-by: Manuel Huber <mahuber@microsoft.com>

Prepare for build with rust v1.84

Add clippy, nancy, binskim release checks to help stablize kata releases Signed-off-by: Mitch Zhu <mitchzhu@microsoft.com>

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).

Copilot

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

sprt and others added 30 commits April 17, 2024 20:40

samples: update genpolicy samples

5650c9b

Merge pull request #176 from microsoft/saulparedes/add_version_flag

b0a632c

genpolicy: add --version flag

Merge pull request #178 from sprt/gp-azurelocal

f467a04

genpolicy: add support for cc-local-csi

Merge pull request #179 from microsoft/saulparedes/sync_downstream

9e73d0b

agent: shutdown vm on exit when agent is used as init process

genpolicy: add missing cache improvements

a2207a3

Add missing cache improvements specifically missing in containerd pull Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Merge pull request #181 from microsoft/saulparedes/add_missing_cache_…

b4c814c

…improvements genpolicy: add missing cache improvements

genpolicy: add support for cc-azurefile-csi driver

0cb2324

This patch adds support for the cc-azurefile-csi driver to the genpolicy. Signed-off-by: Archana Choudhary <archana1@microsoft.com>

genpolicy: update policy samples

b5d68be

This patch updates policy samples, required after adding support for cc-azurefile-csi driver in genpolicy. Signed-off-by: Archana Choudhary <archana1@microsoft.com>

Merge pull request #180 from microsoft/archana1/azurefile-genpolicy

3d38906

genpolicy: add support for cc-azurefile-csi driver

Revert "runtime: agent: use up to 10 PCI segments (#61)"

823dcd2

This reverts commit 627be9b, that was insufficient. Waiting for blk devices used just the PCI device/slot index, but not the PCI segment/domain index. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: clh: clean-up merge from main

6a47c86

Initialize the CLH Platform a single time. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

agent: use regorus instead of opa

11f78ae

Implement Agent Policy using the regorus crate instead of the OPA daemon. The OPA daemon will be removed from the Guest rootfs in a future PR. Fixes: kata-containers#9388 Signed-off-by: Dan Mihai <dmihai@microsoft.com>

genpolicy: bump release version

0e79eef

Bump release version to 3.2.0-azl1.genpolicy0 Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Merge pull request #185 from microsoft/saulparedes/bump_release_version

7ea417b

genpolicy: bump release version

genpolicy: update sample location

cbb60ff

Move pod-many-layers.yaml to needs_containerd_pull category Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Merge pull request #183 from microsoft/danmihai1/hotplug7

99f1e83

runtime: agent: use PCI segments 1+ for blk devices

Merge pull request #184 from microsoft/danmihai1/msft-regorus

02f03b3

agent: use regorus instead of opa

rootfs: Stop building and shipping OPA

8df0459

Since OPA binary was replaced by the regorus crate, we can finally stop building and shipping the binary. Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>

clh: initialize clh pid before using it

304d016

The PID needs to be initialized before calling isClhRunning. waitVMM() uses isClhRunning and is called by launchClh() just before returning from function. Fixes: kata-containers#9230 Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>

genpolicy: update sample

9b43ba8

Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Merge pull request #182 from microsoft/saulparedes/wait_for_clh

597200d

clh: isClhRunning waits for full timeout when clh exits

Merge pull request #187 from microsoft/saulparedes/remove_opa2

dda2c28

rootfs: Stop building and shipping OPA

kata: Remove Issue - PR dependency

915a8fc

We've discussed this over and over. Let's try to get to an agreement here. I will use this issue to remove the mandatory Issue - PR dependency. Fixes: kata-containers#9500 Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>

Merge pull request #189 from microsoft/sprt/remove-fixes-check

9ce3226

ci: cherry-pick relaxed commit check from upstream

ms-mahuber and others added 4 commits February 14, 2025 20:35

powerpc64: Fix rust v1.84 build issue

be02780

- subset of upstream commit 099b241 - should be straightforward to merge when sync-ing to upstream Signed-off-by: Manuel Huber <mahuber@microsoft.com>

agent: config: Remove supports_seccomp

5163038

- counterpart to upstream a131eec - unblocks build with rust v1.84 Signed-off-by: Manuel Huber <mahuber@microsoft.com>

Merge pull request #313 from microsoft/kata/rust184

4779365

Prepare for build with rust v1.84

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 <mitchzhu@microsoft.com>

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

danmihai1 force-pushed the msft-main branch from d5ec820 to b900faf Compare April 3, 2025 21:04

Redent0r force-pushed the msft-main branch from 7954edb to 9746966 Compare June 5, 2025 17:21

Redent0r force-pushed the msft-main branch from f4b3c21 to 57499f2 Compare June 17, 2025 17:08

ms-mahuber force-pushed the msft-main branch from 57499f2 to c04bfdc Compare June 25, 2025 22:33

ms-mahuber force-pushed the msft-main branch from c04bfdc to 5586d27 Compare July 25, 2025 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

miz060 commented Feb 24, 2025 •

edited

Loading

Uh oh!

ms-mahuber Feb 25, 2025

Uh oh!

ms-mahuber Feb 25, 2025

Uh oh!

ms-mahuber Feb 25, 2025

Uh oh!

ms-mahuber Feb 25, 2025

Uh oh!

sprt Feb 25, 2025

Uh oh!

ms-mahuber Feb 25, 2025

Uh oh!

Redent0r commented Feb 25, 2025

Uh oh!

sprt left a comment

Uh oh!

sprt Feb 25, 2025

Uh oh!

sprt Feb 25, 2025

Uh oh!

sprt Feb 25, 2025

Uh oh!

sprt Feb 25, 2025

Uh oh!

sprt Feb 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

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

Uh oh!

Conversation

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

Uh oh!

Merge Checklist

Summary

Test Methodology

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Redent0r commented Feb 25, 2025

Uh oh!

sprt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

Uh oh!

miz060 commented Feb 24, 2025 •

edited

Loading