Bug/12187 v2 reliably detect Suricata startup

[KEIAHNY]

PR Type

Bug fix, Enhancement

---

Description

• Replace process scanning with PID file reading

• Verify readiness via Unix socket suricatasc

• Add CLI and config support for socket path

• Clean up stale PID files before execution

---

Diagram Walkthrough

flowchart TD
  Config["CLI / Config"] --> Init["Suriconf Init"]
  Init --> Exec["Execute Suricata"]
  Exec --> PID["Read PID File"]
  PID --> Socket["Check Socket Uptime"]
  Socket --> Ready["Suricata Ready"]

Loading

File Walkthrough

	Relevant files
Enhancement	argument.rs Add socket path CLI argument                                                          src/argument.rs Added path_to_socket argument to Commands::Suricata Enables CLI specification of Unix Socket path +4/-0      yaml.rs Extend Suriconf with socket configuration                                src/yaml.rs Added socket field to Suriconf struct Implemented find_socket parsing method Updated initialization to handle socket path +12/-0
Bug fix	suricata.rs Refactor Suricata startup detection logic                                src/suricata.rs Implemented wait_on_suricata_start using PID and socket Replaced check_process_name_for_suricata_main logic Added delete_pid_file and set_pid_file helpers Updated execute_suricata to verify startup reliability +67/-16
Configuration changes	suriconf.yaml Configure default socket path                                                        src/suriconf.yaml Added default socket path configuration Set value to /var/run/suricata.pid +1/-0

---

[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 Wrong default socket path The new socket config value is set to /var/run/suricata.pid, which is the PID file, not the Suricata Unix command socket (typically /var/run/suricata/suricata-command.socket). suricatasc will try to connect to a non-socket file and fail, so wait_on_suricata_start will report "Suricata could not start." even when Suricata is running, causing the process to be killed and the run to panic. This affects every user relying on the config default rather than the -s CLI flag. socket: /var/run/suricata.pid Possible pipe deadlock wait_on_suricata_start is called immediately after spawning Suricata, but the child's stderr/stdout pipes are not read until after this wait returns (up to ~20 seconds given the two 10-iteration, 1-second-sleep loops). Suricata can emit a large amount of startup output (e.g., rule loading warnings); once the OS pipe buffer fills, the child blocks on writing and never becomes ready, so the readiness check itself can hang the process it is waiting for. let suri_pid = match wait_on_suricata_start(&suriconf.socket) { Ok(pid) => pid, Err(e) => { let _ = child.kill(); panic!("{e}") } }; Inconsistent sudo invocation Suricata itself is spawned via sudo -n (non-interactive), but the suricatasc check in wait_on_suricata_start calls sudo without -n. If sudo requires a password in the target environment, .status() will block indefinitely waiting for terminal input instead of failing fast, leaving the readiness loop stuck. Use sudo -n for consistency. let output = Command::new("sudo") .arg("suricatasc") .arg("-c") .arg("uptime") .arg(socket) .status() .map_err(|e| format!("Unable to execute Suricata socket control tool: {e}"))?; Hardcoded PID file path The PID file location is hardcoded to /var/run/suricata.pid while the socket path is configurable. Since both are passed to/used by the same Suricata instance, a custom socket deployment (e.g., a different run directory) will still force the PID file to the fixed path, and delete_pid_file will remove or fail on that fixed location. Consider making the PID file path configurable alongside the socket, or deriving it from the same setting. pub fn delete_pid_file() -> std::io::Result<()> { match fs::remove_file(PIDFILE) { Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), result => result, } } pub fn set_pid_file(vec_of_sur_cmd: &mut Vec<String>) { vec_of_sur_cmd.push("--pidfile".to_string()); vec_of_sur_cmd.push(PIDFILE.to_string()); }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix socket path pointing to PID file The socket option points to the PID file path (/var/run/suricata.pid) instead of a Unix socket file such as /var/run/suricata/suricata.sock. With this value, suricatasc will try to talk to a PID file and startup detection will always fail. Point it at Suricata's actual Unix socket path. src/suriconf.yaml [12] -socket: /var/run/suricata.pid +socket: /var/run/suricata/suricata.sock Suggestion importance[1-10]: 8 __ Why: The socket value /var/run/suricata.pid is the PID file path, not a Unix socket path, so suricatasc in wait_on_suricata_start will never successfully connect, making the startup detection always fail. This is a real configuration bug that breaks the PR's core mechanism.	Medium
	Pass socket via --socket flag suricatasc does not accept the socket path as a positional argument; it must be passed via the --socket flag. As written, the command will fail to connect to the intended socket regardless of whether Suricata is up. Pass the path with --socket. src/suricata.rs [323-328] let output = Command::new("sudo") .arg("suricatasc") .arg("-c") .arg("uptime") + .arg("--socket") .arg(socket) .status() Suggestion importance[1-10]: 7 __ Why: suricatasc expects the socket path via the --socket flag, not as a positional argument, so passing it positionally would cause the command to fail or target the wrong socket even when Suricata is running. The fix is correct and directly affects the success of the startup wait logic.	Medium
	Configure Suricata's Unix socket path Suricata is never told to create its Unix socket at the configured suriconf.socket path, so it will use its default socket location and suricatasc will look in the wrong place. Add a --unix-socket argument (and corresponding unix-command enable) to the Suricata command line using the configured path. src/suricata.rs [226-229] -pub fn set_pid_file(vec_of_sur_cmd: &mut Vec<String>) { +pub fn set_pid_file(vec_of_sur_cmd: &mut Vec<String>, socket: &Path) { vec_of_sur_cmd.push("--pidfile".to_string()); vec_of_sur_cmd.push(PIDFILE.to_string()); + vec_of_sur_cmd.push("--unix-socket".to_string()); + vec_of_sur_cmd.push(socket.display().to_string()); } Suggestion importance[1-10]: 5 __ Why: The concern is legitimate: Suricata is never told to create its Unix socket at the configured suriconf.socket path in the constructed command line, so suricatasc may target a wrong location. However, Suricata's socket location may already be configured in its own yaml (referenced by suriconf.suri_configuration), so the issue is somewhat speculative.	Low
General	Verify pid belongs to Suricata The pid-file existence check does not guarantee the process it references is actually Suricata: a leftover/stale PID file recreated by any process could match, and /proc/{pid} existing only proves some process with that PID is alive. Consider verifying the process name (e.g., that the comm is suricata) before accepting the PID, to avoid operating on an unrelated process. src/suricata.rs [305-316] fn get_suricata_pid() -> Result<i32, String> { for _ in 0..10 { - ... - thread::sleep(Duration::from_millis(1000)); + if let Ok(content) = fs::read_to_string(PIDFILE) { + if let Ok(pid) = content.trim().parse::<i32>() { + if let Ok(stat) = fs::read_to_string(format!("/proc/{}/comm", pid)) { + if stat.trim() == "suricata" { + return Ok(pid); + } + } + } + } + thread::sleep(Duration::from_millis(1000)); + } + Err("Suricata process not found.".into()) } -Err("Suricata process not found.".into()) Suggestion importance[1-10]: 3 __ Why: The stale-PID-file concern is largely mitigated since delete_pid_file() is called before spawning Suricata, and the PR intentionally moved away from process-name matching. This is a reasonable defensive hardening but only a marginal improvement with a small race-window benefit.	Low

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