Bug/12187 v1 reliably detect Suricata startup

[KEIAHNY]

PR Type

Bug fix, Enhancement

---

Description

• Replace process scanning with PID file reading

• Add socket availability check via suricatasc

• Pass --pidfile argument to Suricata command

• Ensure Suricata is fully ready before proceeding

---

Diagram Walkthrough

flowchart LR
  A["Start Suricata"] --> B["Wait for PID File"]
  B --> C["Verify Socket Ready"]
  C --> D["Proceed Execution"]

Loading

File Walkthrough

	Relevant files
Bug fix	suricata.rs Refactor Suricata startup detection logic                                src/suricata.rs Added --pidfile argument to Suricata command Replaced process scanning with PID file parsing Implemented wait_on_suricata_start for socket verification Removed all_processes dependency for startup detection +42/-15

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing pidfile path The --pidfile argument is pushed to vec_of_sur_cmd without a corresponding path value. Suricata expects --pidfile <path> or --pidfile=<path>. Without the path, Suricata will either fail to start or misinterpret the next argument (from get_capture_mode) as the pidfile path. Meanwhile, get_suricata_pid hardcodes the read path as /var/run/suricata.pid, so even if Suricata uses a default, it may not match this path, causing the startup detection to always fail and panic. vec_of_sur_cmd.push("--pidfile".to_string()); Socket check unreliable suricatasc -c uptime is invoked without specifying a socket path (e.g., via --socket). If the Suricata configuration uses a non-default socket location, suricatasc will fail even though Suricata is running, causing wait_on_suricata_start to return an error and panic. The socket path should be derived from the configuration and passed to suricatasc to ensure the check is valid. let output = Command::new("sudo") .arg("suricatasc") .arg("-c") .arg("uptime") .status() .expect("Unable to execute Suricata socket control tool.");

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add missing pidfile path argument The --pidfile flag requires a path argument. Without it, Suricata will either fail to start or consume the next argument (added by get_capture_mode) as the pidfile path, breaking both settings. Add the path /var/run/suricata.pid to match the location read by get_suricata_pid. src/suricata.rs [39] vec_of_sur_cmd.push("--pidfile".to_string()); +vec_of_sur_cmd.push("/var/run/suricata.pid".to_string()); Suggestion importance[1-10]: 9 __ Why: The --pidfile flag requires a path argument, and without it Suricata may either fail to start or consume the next argument from get_capture_mode as the path. This is critical because get_suricata_pid reads from /var/run/suricata.pid, so the path must match.	High
	Kill child process before panic on failure When wait_on_suricata_start() fails, the already-spawned child process is not killed before panicking. Since Rust's Child drop does not kill the process by default, this leaks a running Suricata process. Call child.kill() before panicking to clean up properly. src/suricata.rs [76-81] let suri_pid = match wait_on_suricata_start() { Ok(pid) => pid, Err(e) => { + let _ = child.kill(); panic!("{e}") } }; Suggestion importance[1-10]: 7 __ Why: When wait_on_suricata_start() fails, the already-spawned child process is not killed before panicking, which leaks a running Suricata process. Calling child.kill() before the panic is a good cleanup practice, though it only affects the error path.	Medium
General	Propagate error instead of panicking on command failure Using .expect() on the suricatasc command execution causes a panic if the tool is unavailable, instead of propagating the error. Since wait_on_suricata_start returns Result, use ? with map_err to handle the error gracefully and allow the caller to decide how to respond. src/suricata.rs [300-305] let output = Command::new("sudo") .arg("suricatasc") .arg("-c") .arg("uptime") .status() - .expect("Unable to execute Suricata socket control tool."); + .map_err(|e| format!("Unable to execute Suricata socket control tool: {e}"))?; Suggestion importance[1-10]: 6 __ Why: Using .expect() causes a panic if suricatasc is unavailable, whereas the function returns a Result. Propagating the error with ? and map_err is a better practice, but the impact is moderate since it only changes error handling behavior.	Low

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.