.github/workflows: add cargo clippy check

Signed-off-by: Kun Lai <[email protected]>

Author: imlk0

.github/workflows/clippy.yml

@@ -0,0 +1,52 @@
+name: Cargo Clippy
+
+on:
+  push:
+    branches:
+      - master
+    tags:
+      - "v*.*.*"
+  pull_request:
+    branches:
+      - 'master'
+
+jobs:
+  clippy:
+    strategy:
+      fail-fast: false
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        shell: bash
+    container:
+      image: alibaba-cloud-linux-3-registry.cn-hangzhou.cr.aliyuncs.com/alinux3/alinux3:latest
+    steps:
+      - name: Update yum mirror
+        run: |
+          set -e
+          set -x
+
+          # replace the mirror
+          sed -i -E 's|https?://mirrors.openanolis.cn/anolis/|https://mirrors.aliyun.com/anolis/|g' /etc/yum.repos.d/*.repo
+          sed -i -E 's|https?://mirrors.cloud.aliyuncs.com/|https://mirrors.aliyun.com/|g' /etc/yum.repos.d/*.repo
+
+          # install development tools
+          yum install -y autoconf automake binutils bison flex gcc gcc-c++ gdb glibc-devel libtool make pkgconf pkgconf-m4 pkgconf-pkg-config rpm-build rpm-sign strace asciidoc byacc ctags diffstat elfutils-libelf-devel git intltool patchutils perl-Fedora-VSP perl-Sys-Syslog perl-generators pesign source-highlight systemtap valgrind valgrind-devel cmake expect rpmdevtools rpmlint perl clang
+
+          # install rpmdevtools
+          yum install -y git yum-utils
+
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: dtolnay/[email protected]
+
+      - name: Install build dependencies
+        run: yum-builddep -y ./cryptpilot.spec
+
+      - name: Install Clippy
+        run: rustup component add clippy
+
+      - name: Cargo Clippy
+        run: cargo clippy --all-targets --all-features -- -D warnings

Makefile

@@ -117,3 +117,6 @@ shellcheck:
 	}
 	find . -name '*.sh' -exec shellcheck {} \;
 
+.PHONE: clippy
+clippy:
+	cargo clippy --all-targets --all-features -- -D warnings

clippy.toml

@@ -0,0 +1,6 @@
+disallowed-methods = [
+  {path = "std::process::Command::status", reason = "please use 'CheckCommandOutput::run()' instaed"},
+  {path = "std::process::Command::output", reason = "please use 'CheckCommandOutput::run()' instaed"},
+  {path = "tokio::process::Command::status", reason = "please use 'CheckCommandOutput::run()' instaed"},
+  {path = "tokio::process::Command::output", reason = "please use 'CheckCommandOutput::run()' instaed"},
+]

src/bin/gen-template/main.rs

@@ -274,7 +274,7 @@ where
     for (mut key, value) in table.iter_mut() {
         // extract docs
         let field_name = key.get();
-        let Ok(docs) = T::get_field_docs(&field_name) else {
+        let Ok(docs) = T::get_field_docs(field_name) else {
             // ignore fields not known to `T`
             continue;
         };
@@ -383,6 +383,6 @@ fn main() -> Result<()> {
         TemplateType::Fde => get_fde_config().as_annotated_toml()?,
     };
 
-    print!("{}", doc.to_string());
+    print!("{}", doc);
     Ok(())
 }

src/cmd/boot_service/initrd_state.rs

@@ -11,7 +11,7 @@ pub struct InitrdState {
     pub fde_config_bundle: FdeConfigBundle,
 }
 
-pub const CRYPTPILOT_INITRD_STATE_PATH: &'static str = "/var/run/cryptpilot/initrd_state.toml";
+pub const CRYPTPILOT_INITRD_STATE_PATH: &str = "/var/run/cryptpilot/initrd_state.toml";
 
 impl InitrdState {
     pub async fn save(&self) -> Result<()> {

src/cmd/boot_service/mod.rs

@@ -29,15 +29,15 @@ use crate::{
 
 use super::fde::disk::{FdeDisk as _, OnExternalFdeDisk};
 
-const ROOTFS_LOGICAL_VOLUME: &'static str = "/dev/mapper/system-rootfs";
-const ROOTFS_LAYER_NAME: &'static str = "rootfs";
-const ROOTFS_LAYER_DEVICE: &'static str = "/dev/mapper/rootfs";
-const ROOTFS_DECRYPTED_LAYER_DEVICE: &'static str = "/dev/mapper/rootfs_decrypted";
-const ROOTFS_DECRYPTED_LAYER_NAME: &'static str = "rootfs_decrypted";
-const ROOTFS_HASH_LOGICAL_VOLUME: &'static str = "/dev/mapper/system-rootfs_hash";
-const DATA_LOGICAL_VOLUME: &'static str = "/dev/mapper/system-data";
-const DATA_LAYER_NAME: &'static str = "data";
-const DATA_LAYER_DEVICE: &'static str = "/dev/mapper/data";
+const ROOTFS_LOGICAL_VOLUME: &str = "/dev/mapper/system-rootfs";
+const ROOTFS_LAYER_NAME: &str = "rootfs";
+const ROOTFS_LAYER_DEVICE: &str = "/dev/mapper/rootfs";
+const ROOTFS_DECRYPTED_LAYER_DEVICE: &str = "/dev/mapper/rootfs_decrypted";
+const ROOTFS_DECRYPTED_LAYER_NAME: &str = "rootfs_decrypted";
+const ROOTFS_HASH_LOGICAL_VOLUME: &str = "/dev/mapper/system-rootfs_hash";
+const DATA_LOGICAL_VOLUME: &str = "/dev/mapper/system-data";
+const DATA_LAYER_NAME: &str = "data";
+const DATA_LAYER_DEVICE: &str = "/dev/mapper/data";
 
 pub struct BootServiceCommand {
     pub boot_service_options: BootServiceOptions,
@@ -508,7 +508,7 @@ async fn setup_user_provided_volumes(boot_service_options: &BootServiceOptions)
         .await
         .get_volume_configs()
         .await?;
-    if volume_configs.len() == 0 {
+    if volume_configs.is_empty() {
         tracing::info!("The volume configs is empty, exit now");
         return Ok(());
     }
@@ -536,7 +536,7 @@ async fn setup_user_provided_volumes(boot_service_options: &BootServiceOptions)
             volume_config.volume,
             volume_config.dev
         );
-        match super::open::open_for_specific_volume(&volume_config).await {
+        match super::open::open_for_specific_volume(volume_config).await {
             Ok(_) => {
                 tracing::info!(
                     "The mapping for volume {} is active now",

src/cmd/close.rs

@@ -13,13 +13,13 @@ impl super::Command for CloseCommand {
         for volume in &self.close_options.volume {
             tracing::info!("Close volume {volume} now");
 
-            if !crate::fs::luks2::is_active(&volume) {
+            if !crate::fs::luks2::is_active(volume) {
                 tracing::info!("The mapping for {} is not active, nothing to do", volume);
                 continue;
             }
 
             tracing::info!("Removing mapping for {volume}");
-            crate::fs::luks2::close(&volume).await?;
+            crate::fs::luks2::close(volume).await?;
             tracing::info!("The volume {volume} is closed now");
         }
 

src/cmd/fde/disk.rs

@@ -67,13 +67,13 @@ impl MeasurementedBootComponents {
         let grub_authenticode_hashes = self
             .grub
             .iter()
-            .map(|bytes| calculate_authenticode_hash::<T>(&bytes))
+            .map(|bytes| calculate_authenticode_hash::<T>(bytes))
             .collect::<Result<Vec<_>>>()?;
 
         let shim_authenticode_hashes = self
             .shim
             .iter()
-            .map(|bytes| calculate_authenticode_hash::<T>(&bytes))
+            .map(|bytes| calculate_authenticode_hash::<T>(bytes))
             .collect::<Result<Vec<_>>>()?;
 
         Ok(BootComponentsHashValue {
@@ -217,11 +217,26 @@ pub trait FdeDisk: Send + Sync {
 
         for line in entry_content.lines() {
             if line.starts_with("linux ") {
-                kernel_path = line.splitn(2, ' ').nth(1).unwrap_or("").trim().to_string();
+                kernel_path = line
+                    .split_once(' ')
+                    .map(|x| x.1)
+                    .unwrap_or("")
+                    .trim()
+                    .to_string();
             } else if line.starts_with("options ") {
-                cmdline = line.splitn(2, ' ').nth(1).unwrap_or("").trim().to_string();
+                cmdline = line
+                    .split_once(' ')
+                    .map(|x| x.1)
+                    .unwrap_or("")
+                    .trim()
+                    .to_string();
             } else if line.starts_with("initrd ") {
-                initrd_path = line.splitn(2, ' ').nth(1).unwrap_or("").trim().to_string();
+                initrd_path = line
+                    .split_once(' ')
+                    .map(|x| x.1)
+                    .unwrap_or("")
+                    .trim()
+                    .to_string();
             }
         }
 
@@ -245,7 +260,7 @@ pub trait FdeDisk: Send + Sync {
 
         // Make kernel path absolute if needed
         if !kernel_path.is_empty() {
-            if kernel_path.starts_with("/") {
+            if kernel_path.starts_with('/') {
                 // Already absolute
             } else {
                 kernel_path = format!("/boot/{}", kernel_path);
@@ -254,7 +269,7 @@ pub trait FdeDisk: Send + Sync {
 
         // Make initrd path absolute if needed
         if !initrd_path.is_empty() {
-            if initrd_path.starts_with("/") {
+            if initrd_path.starts_with('/') {
                 // Already absolute
             } else {
                 initrd_path = format!("/boot/{}", initrd_path);
@@ -265,7 +280,7 @@ pub trait FdeDisk: Send + Sync {
 
         // Calculate SHA384 hashes
         let kernel = self
-            .read_file_on_disk(&kernel_path)
+            .read_file_on_disk(kernel_path)
             .await
             .with_context(|| format!("Failed to read kernel file at {:?}", kernel_path))?;
 
@@ -284,7 +299,7 @@ pub trait FdeDisk: Send + Sync {
                 // Extract partition number from device path
                 // For example, /dev/sda3 -> (hd0,gpt3) or (hd0,msdos3), /dev/nvme0n1p3 -> (hd0,gpt3) or (hd0,msdos3)
                 let boot_dev = self.get_boot_dev();
-                if let Some(partition_num) = boot_dev
+                if let Ok(partition_num) = boot_dev
                     .to_string_lossy()
                     .chars()
                     .rev()
@@ -294,7 +309,6 @@ pub trait FdeDisk: Send + Sync {
                     .rev()
                     .collect::<String>()
                     .parse::<u32>()
-                    .ok()
                 {
                     match partition_type {
                         PartitionTableType::Gpt => format!("(hd0,gpt{})", partition_num),
@@ -408,29 +422,22 @@ pub trait FdeDisk: Send + Sync {
 
         let mut entries = async_walkdir::WalkDir::new(efi_part_root_dir);
 
-        loop {
-            match entries.next().await {
-                Some(Ok(entry)) => {
-                    if matches!(entry.file_type().await.map(|e| e.is_file()), Ok(true)) {
-                        let file_name = entry.file_name().to_string_lossy().to_lowercase();
-
-                        if file_name == "grubx64.efi" {
-                            tracing::debug!(file=?entry.path(), "Found grubx64.efi");
-                            let mut buf = vec![];
-                            let mut file = File::open(entry.path()).await?;
-                            file.read_to_end(&mut buf).await?;
-                            let _ = grub_data.insert(buf);
-                        } else if file_name == "shimx64.efi" {
-                            tracing::debug!(file=?entry.path(), "Found shimx64.efi");
-                            let mut buf = vec![];
-                            let mut file = File::open(entry.path()).await?;
-                            file.read_to_end(&mut buf).await?;
-                            let _ = shim_data.insert(buf);
-                        }
-                    }
-                }
-                Some(Err(_)) | None => {
-                    break;
+        while let Some(Ok(entry)) = entries.next().await {
+            if matches!(entry.file_type().await.map(|e| e.is_file()), Ok(true)) {
+                let file_name = entry.file_name().to_string_lossy().to_lowercase();
+
+                if file_name == "grubx64.efi" {
+                    tracing::debug!(file=?entry.path(), "Found grubx64.efi");
+                    let mut buf = vec![];
+                    let mut file = File::open(entry.path()).await?;
+                    file.read_to_end(&mut buf).await?;
+                    let _ = grub_data.insert(buf);
+                } else if file_name == "shimx64.efi" {
+                    tracing::debug!(file=?entry.path(), "Found shimx64.efi");
+                    let mut buf = vec![];
+                    let mut file = File::open(entry.path()).await?;
+                    file.read_to_end(&mut buf).await?;
+                    let _ = shim_data.insert(buf);
                 }
             }
         }
@@ -445,8 +452,8 @@ pub trait FdeDisk: Send + Sync {
     fn get_efi_part_root_dir(&self) -> &Path;
 }
 
-const CRYPTPILOT_CONFIG_DIR_UNTRUSTED_IN_BOOT: &'static str = "cryptpilot/config";
-const METADATA_PATH_IN_BOOT: &'static str = "cryptpilot/metadata.toml";
+const CRYPTPILOT_CONFIG_DIR_UNTRUSTED_IN_BOOT: &str = "cryptpilot/config";
+const METADATA_PATH_IN_BOOT: &str = "cryptpilot/metadata.toml";
 
 async fn load_fde_config_bundle_from_dir(config_dir: &Path) -> Result<FdeConfigBundle> {
     Ok(FileSystemConfigSource::new(config_dir)
@@ -662,7 +669,7 @@ impl OnExternalFdeDisk {
 
         let content = String::from_utf8_lossy(&output);
         for line in content.lines() {
-            let fields: Vec<&str> = line.trim().split_whitespace().collect();
+            let fields: Vec<&str> = line.split_whitespace().collect();
             if fields.len() != 2 {
                 continue;
             }

src/cmd/fde/dump_config.rs

@@ -16,7 +16,7 @@ pub struct ConfigDumpCommand {
 impl super::super::Command for ConfigDumpCommand {
     async fn run(&self) -> Result<()> {
         let fde_disk: Box<dyn FdeDisk + Send> = match &self.disk {
-            Some(disk) => Box::new(OnExternalFdeDisk::new_from_disk(&disk).await?),
+            Some(disk) => Box::new(OnExternalFdeDisk::new_from_disk(disk).await?),
             None => Box::new(OnCurrentSystemFdeDisk::new().await?),
         };
 

src/cmd/fde/show_reference_value.rs

@@ -25,7 +25,7 @@ impl super::super::Command for ShowReferenceValueCommand {
     async fn run(&self) -> Result<()> {
         tracing::debug!("Get rootfs reference value");
         let fde_disk: Box<dyn FdeDisk + Send + Sync> = match &self.disk {
-            Some(disk) => Box::new(OnExternalFdeDisk::new_from_disk(&disk).await?),
+            Some(disk) => Box::new(OnExternalFdeDisk::new_from_disk(disk).await?),
             None => Box::new(OnCurrentSystemFdeDisk::new().await?),
         };