fix: Code review bug fixes and cleanup (#55)

* fix: Log GID instead of UID in user GID tracing field

The tracing statement for `user.gid` was reading from `user.uid`
instead of `user.gid`, causing the wrong value to be reported.

* refactor: Use idiomatic empty check for disk collection

Replace `into_iter().next().is_none()` with `list().is_empty()`
for clarity, and use `list().iter()` for the actual collection.

* fix: Remove stale .source() call with unused result

This was likely a debugging leftover — the error source chain is
already captured via the `successors` iterator below.

* fix: Fix typo "proess" -> "process" in error message

* fix: Replace unwrap() calls with error logging in collection loop

JSON serialization and file write can fail at runtime (e.g. disk
full). Log the error and continue the loop instead of crashing,
since this tool may run continuously for hours.

* fix: Use tokio::time::sleep instead of std::thread::sleep

std::thread::sleep blocks the entire tokio worker thread.
Since main is already async, use the non-blocking alternative.

* fix: Handle DNS resolver initialization failure gracefully

In a container debugging tool, broken DNS config (/etc/resolv.conf)
is a likely scenario to diagnose. Log the error and skip DNS lookups
instead of panicking.

* fix: Wrap network collector in ComponentResult for consistent error handling

The network collector silently swallowed interface listing errors by
returning empty data. Now it returns Result so the orchestrator wraps
it in ComponentResult, matching the pattern used by other fallible
collectors. Errors appear in JSON output instead of being silently
lost.

* refactor: Use BTreeMap for deterministic JSON key ordering

HashMap produces non-deterministic JSON output, making it hard to
diff containerdebug output across runs. BTreeMap sorts keys
consistently.

* fix: Bump rustls-webpki to 0.103.10 to negate RUSTSEC-2026-0049

* chore: Remove undetected, ignored advisories

---------

Co-authored-by: Techassi <git@techassi.dev>

Author: lfrancke

deny.toml

@@ -14,31 +14,6 @@ targets = [
 
 [advisories]
 yanked = "deny"
-ignore = [
-    # https://rustsec.org/advisories/RUSTSEC-2023-0071
-    # "rsa" crate: Marvin Attack: potential key recovery through timing sidechannel
-    #
-    # No patch is yet available, however work is underway to migrate to a fully constant-time implementation
-    # So we need to accept this, as of SDP 25.3 we are not using the rsa crate to create certificates used in production
-    # setups.
-    #
-    # https://github.com/RustCrypto/RSA/issues/19 is the tracking issue
-    "RUSTSEC-2023-0071",
-
-    # https://rustsec.org/advisories/RUSTSEC-2024-0436
-    # The "paste" crate is no longer maintained because the owner states that the implementation is
-    # finished. There are at least two (forked) alternatives which state to be maintained. They'd
-    # need to be vetted before a potential switch. Additionally, they'd need to be in a maintained
-    # state for a couple of years to provide any benefit over using "paste".
-    #
-    # This crate is only used in a single place in the xtask package inside the declarative
-    # "write_crd" macro. The impact of vulnerabilities, if any, should be fairly minimal.
-    #
-    # See thread: https://users.rust-lang.org/t/paste-alternatives/126787/4
-    #
-    # This can only be removed again if we decide to use a different crate.
-    "RUSTSEC-2024-0436",
-]
 
 [bans]
 multiple-versions = "allow"

src/error.rs

@@ -31,7 +31,6 @@ impl<T> ComponentResult<T> {
                     error = &err as &dyn std::error::Error,
                     "error reported by {component}, ignoring...",
                 );
-                err.source();
                 ComponentResult::Err {
                     inner: ComponentError {
                         message: err.to_string(),

src/main.rs

@@ -68,13 +68,28 @@ async fn main() {
         if !next_run_sleep.is_zero() {
             tracing::info!(?next_run, "scheduling next run...");
         }
-        std::thread::sleep(next_run_sleep);
+        tokio::time::sleep(next_run_sleep).await;
 
         let system_information = SystemInformation::collect(&mut collect_ctx).await;
 
-        let serialized = serde_json::to_string_pretty(&system_information).unwrap();
-        if let Some(output_path) = &opts.output {
-            std::fs::write(output_path, &serialized).unwrap();
+        match serde_json::to_string_pretty(&system_information) {
+            Ok(serialized) => {
+                if let Some(output_path) = &opts.output
+                    && let Err(err) = std::fs::write(output_path, &serialized)
+                {
+                    tracing::error!(
+                        path = %output_path.display(),
+                        error = &err as &dyn std::error::Error,
+                        "failed to write JSON output file"
+                    );
+                }
+            }
+            Err(err) => {
+                tracing::error!(
+                    error = &err as &dyn std::error::Error,
+                    "failed to serialize system information"
+                );
+            }
         }
 
         match opts.loop_interval {

src/system_information/disk.rs

@@ -12,10 +12,10 @@ impl Disk {
     #[tracing::instrument(name = "Disk::collect_all")]
     pub fn collect_all() -> Vec<Self> {
         let disks = sysinfo::Disks::new_with_refreshed_list();
-        if disks.into_iter().next().is_none() {
+        if disks.list().is_empty() {
             tracing::info!("no disks found");
         }
-        disks.into_iter().map(Self::from).collect()
+        disks.list().iter().map(Self::from).collect()
     }
 }
 

src/system_information/mod.rs

@@ -15,7 +15,7 @@ pub struct SystemInformation {
     pub os: Option<os::OperatingSystem>,
     pub current_user: Option<ComponentResult<user::User>>,
     pub disks: Option<Vec<disk::Disk>>,
-    pub network: Option<network::SystemNetworkInfo>,
+    pub network: Option<ComponentResult<network::SystemNetworkInfo>>,
     // TODO:
     //  Current time
     //  SElinux/AppArmor
@@ -70,7 +70,10 @@ impl SystemInformation {
                 user::User::collect_current(&ctx.system),
             )),
             disks: Some(disk::Disk::collect_all()),
-            network: Some(network::SystemNetworkInfo::collect().await),
+            network: Some(ComponentResult::report_from_result(
+                "SystemNetworkInfo::collect",
+                network::SystemNetworkInfo::collect().await,
+            )),
             // ..Default::default()
         };
 

src/system_information/network.rs

@@ -3,73 +3,86 @@ use hickory_resolver::{
 };
 use local_ip_address::list_afinet_netifas;
 use serde::Serialize;
+use snafu::{ResultExt, Snafu};
 use std::{
-    collections::{BTreeSet, HashMap},
+    collections::{BTreeMap, BTreeSet},
     net::IpAddr,
     sync::LazyLock,
     time::Duration,
 };
 use tokio::task::JoinSet;
 
-static GLOBAL_DNS_RESOLVER: LazyLock<TokioResolver> = LazyLock::new(|| {
-    let (resolver_config, mut resolver_opts) =
-        read_system_conf().expect("failed to read system resolv config");
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(display("failed to list network interfaces"))]
+    ListInterfaces { source: local_ip_address::Error },
+}
+
+static GLOBAL_DNS_RESOLVER: LazyLock<Option<TokioResolver>> = LazyLock::new(|| {
+    let (resolver_config, mut resolver_opts) = match read_system_conf() {
+        Ok(conf) => conf,
+        Err(err) => {
+            tracing::error!(
+                error = &err as &dyn std::error::Error,
+                "failed to read system DNS config, DNS lookups will be skipped"
+            );
+            return None;
+        }
+    };
     resolver_opts.timeout = Duration::from_secs(5);
 
-    TokioResolver::builder_with_config(resolver_config, TokioConnectionProvider::default())
-        .with_options(resolver_opts)
-        .build()
+    Some(
+        TokioResolver::builder_with_config(resolver_config, TokioConnectionProvider::default())
+            .with_options(resolver_opts)
+            .build(),
+    )
 });
 
 /// Captures all system network information, including network interfaces,
 /// and the results of reverse and forward DNS lookups.
 #[derive(Debug, Serialize)]
 pub struct SystemNetworkInfo {
-    pub interfaces: HashMap<String, Vec<IpAddr>>,
-    pub reverse_lookups: HashMap<IpAddr, Vec<String>>,
-    pub forward_lookups: HashMap<String, Vec<IpAddr>>,
+    pub interfaces: BTreeMap<String, Vec<IpAddr>>,
+    pub reverse_lookups: BTreeMap<IpAddr, Vec<String>>,
+    pub forward_lookups: BTreeMap<String, Vec<IpAddr>>,
 }
 
 impl SystemNetworkInfo {
     #[tracing::instrument(name = "SystemNetworkInfo::collect")]
-    pub async fn collect() -> SystemNetworkInfo {
-        let interfaces = match list_afinet_netifas() {
-            Ok(netifs) => {
-                let mut interface_map = std::collections::HashMap::new();
+    pub async fn collect() -> Result<SystemNetworkInfo, Error> {
+        let netifs = list_afinet_netifas().context(ListInterfacesSnafu)?;
+        let mut interfaces = BTreeMap::new();
 
-                // Iterate over the network interfaces and group them by name
-                // An interface may appear multiple times if it has multiple IP addresses (e.g. IPv4 and IPv6)
-                for (name, ip_addr) in netifs {
-                    tracing::info!(
-                        network.interface.name = name,
-                        network.interface.address = %ip_addr,
-                        "found network interface"
-                    );
-                    interface_map
-                        .entry(name)
-                        .or_insert_with(Vec::new)
-                        .push(ip_addr);
-                }
-                interface_map
-            }
-            Err(error) => {
-                tracing::error!(
-                    error = &error as &dyn std::error::Error,
-                    "failed to list network interfaces"
-                );
-                HashMap::new()
-            }
-        };
+        // Iterate over the network interfaces and group them by name
+        // An interface may appear multiple times if it has multiple IP addresses (e.g. IPv4 and IPv6)
+        for (name, ip_addr) in netifs {
+            tracing::info!(
+                network.interface.name = name,
+                network.interface.address = %ip_addr,
+                "found network interface"
+            );
+            interfaces
+                .entry(name)
+                .or_insert_with(Vec::new)
+                .push(ip_addr);
+        }
 
         let ips: BTreeSet<IpAddr> = interfaces.values().flatten().copied().collect();
         tracing::info!(network.addresses.ip = ?ips, "ip addresses");
 
-        let mut reverse_lookups = JoinSet::new();
+        let Some(resolver) = GLOBAL_DNS_RESOLVER.as_ref() else {
+            return Ok(SystemNetworkInfo {
+                interfaces,
+                reverse_lookups: BTreeMap::new(),
+                forward_lookups: BTreeMap::new(),
+            });
+        };
+
+        let mut reverse_lookup_tasks = JoinSet::new();
         for ip in ips {
-            reverse_lookups
-                .spawn(async move { (ip, GLOBAL_DNS_RESOLVER.reverse_lookup(ip).await) });
+            reverse_lookup_tasks.spawn(async move { (ip, resolver.reverse_lookup(ip).await) });
         }
-        let reverse_lookups: HashMap<IpAddr, Vec<String>> = reverse_lookups
+        let reverse_lookups: BTreeMap<IpAddr, Vec<String>> = reverse_lookup_tasks
             .join_all()
             .await
             .into_iter()
@@ -96,16 +109,12 @@ impl SystemNetworkInfo {
         let hostname_set: BTreeSet<String> = reverse_lookups.values().flatten().cloned().collect();
         tracing::info!(network.addresses.hostname = ?hostname_set, "hostnames");
 
-        let mut forward_lookups = JoinSet::new();
+        let mut forward_lookup_tasks = JoinSet::new();
         for hostname in hostname_set {
-            forward_lookups.spawn(async move {
-                (
-                    hostname.clone(),
-                    GLOBAL_DNS_RESOLVER.lookup_ip(hostname).await,
-                )
-            });
+            forward_lookup_tasks
+                .spawn(async move { (hostname.clone(), resolver.lookup_ip(hostname).await) });
         }
-        let forward_lookups: HashMap<String, Vec<IpAddr>> = forward_lookups
+        let forward_lookups: BTreeMap<String, Vec<IpAddr>> = forward_lookup_tasks
             .join_all()
             .await
             .into_iter()
@@ -126,10 +135,10 @@ impl SystemNetworkInfo {
             })
             .collect();
 
-        SystemNetworkInfo {
+        Ok(SystemNetworkInfo {
             interfaces,
             reverse_lookups,
             forward_lookups,
-        }
+        })
     }
 }

src/system_information/user.rs

@@ -8,7 +8,7 @@ use crate::error::SysinfoError;
 pub enum Error {
     #[snafu(display("failed to get pid of the current process"))]
     GetCurrentPid { source: SysinfoError },
-    #[snafu(display("current pid {pid} could not be resolved to a proess"))]
+    #[snafu(display("current pid {pid} could not be resolved to a process"))]
     ResolveCurrentProcess { pid: Pid },
 }
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -53,7 +53,7 @@ impl User {
         tracing::info!(
             user.name,
             user.uid = user.uid.as_ref().map(|uid| format!("{uid:?}")),
-            user.gid = user.uid.as_ref().map(|gid| format!("{gid:?}")),
+            user.gid = user.gid.as_ref().map(|gid| format!("{gid:?}")),
             "current user"
         );
         Ok(user)