fix(app): reject malformed automation roots before startup

apps/vectorcraft/src/automation_args.rs

@@ -0,0 +1,90 @@
+//! Parse automation roots before starting any desktop services. A malformed flag must not
+//! silently remove a root and leave the control channel unconfined.
+
+use std::path::PathBuf;
+
+pub struct AutomationArgs {
+    pub read_root: Option<PathBuf>,
+    pub write_root: Option<PathBuf>,
+    read_given: bool,
+    write_given: bool,
+}
+
+impl AutomationArgs {
+    pub fn new(read_root: Option<PathBuf>, write_root: Option<PathBuf>) -> Self {
+        Self { read_root, write_root, read_given: false, write_given: false }
+    }
+
+    /// Returns whether this argument is a root flag. CLI values override environment defaults
+    /// once; a second occurrence is an error, including when the two spellings are mixed.
+    pub fn consume(&mut self, arg: &str, args: &mut impl Iterator<Item = String>) -> Result<bool, String> {
+        let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value)));
+        let (root, given) = match flag {
+            "--automation-read-root" => (&mut self.read_root, &mut self.read_given),
+            "--automation-write-root" => (&mut self.write_root, &mut self.write_given),
+            _ => return Ok(false),
+        };
+        if *given {
+            return Err(format!("{flag} given twice"));
+        }
+        let value = match inline {
+            Some(value) => value.to_owned(),
+            None => {
+                let value = args.next().ok_or_else(|| format!("{flag} needs a folder"))?;
+                if value.starts_with('-') {
+                    return Err(format!("{flag} needs a folder; use {flag}=<dir> for a folder starting with '-'"));
+                }
+                value
+            }
+        };
+        if value.is_empty() {
+            return Err(format!("{flag} needs a non-empty folder"));
+        }
+        *root = Some(value.into());
+        *given = true;
+        Ok(true)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn roots_override_environment_once_and_leave_other_arguments() {
+        let mut roots = AutomationArgs::new(Some("environment".into()), None);
+        let mut args = ["a folder".to_owned(), "drawing.svg".to_owned()].into_iter();
+        assert!(roots.consume("--automation-read-root", &mut args).unwrap());
+        assert!(roots.consume("--automation-write-root=out", &mut args).unwrap());
+        assert!(!roots.consume("--in-window-menus", &mut args).unwrap());
+        assert_eq!(roots.read_root, Some("a folder".into()));
+        assert_eq!(roots.write_root, Some("out".into()));
+        assert_eq!(args.next().as_deref(), Some("drawing.svg"));
+        assert!(roots.consume("--automation-read-root=other", &mut args).is_err());
+        assert_eq!(roots.read_root, Some("a folder".into()));
+    }
+
+    #[test]
+    fn invalid_operands_preserve_environment_roots() {
+        for flag in ["--automation-read-root", "--automation-write-root"] {
+            for operand in [None, Some(""), Some("--version"), Some("--control"), Some("-folder")] {
+                let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into()));
+                let mut args = operand.map(str::to_owned).into_iter();
+                assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}");
+                assert_eq!(roots.read_root, Some("read".into()));
+                assert_eq!(roots.write_root, Some("write".into()));
+            }
+            let mut roots = AutomationArgs::new(None, None);
+            assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err());
+        }
+    }
+
+    #[test]
+    fn explicit_dash_folder_and_relative_dash_folder_are_valid() {
+        let mut roots = AutomationArgs::new(None, None);
+        assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap());
+        assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap());
+        assert_eq!(roots.read_root, Some("-folder".into()));
+        assert_eq!(roots.write_root, Some("./-folder".into()));
+    }
+}

apps/vectorcraft/src/main.rs

@@ -21,6 +21,7 @@ compile_error!("windows7 requires --target x86_64-win7-windows-msvc; the ordinar
 #[cfg(all(target_vendor = "win7", not(feature = "windows7")))]
 compile_error!("the win7 target requires --no-default-features --features windows7");
 
+mod automation_args;
 mod clipboard;
 mod control_server;
 #[cfg(feature = "wgpu")]
@@ -585,7 +586,36 @@ fn native_options(custom_titlebar: bool) -> eframe::NativeOptions {
 }
 
 fn main() -> std::process::ExitCode {
-    // First, so every start-up warning is recorded (`logging`).
+    // Validate roots before logging, font scans, preferences or the window: a missing value
+    // must not clear an environment root and start the control channel without confinement.
+    let mut control_port: Option<u16> = std::env::var("VECTORCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok());
+    let root_env = |name: &str| std::env::var_os(name).filter(|v| !v.is_empty()).map(std::path::PathBuf::from);
+    let mut roots_args =
+        automation_args::AutomationArgs::new(root_env("VECTORCRAFT_AUTOMATION_READ_ROOT"), root_env("VECTORCRAFT_AUTOMATION_WRITE_ROOT"));
+    let mut files = Vec::new();
+    let mut in_window_menus = std::env::var_os("VECTORCRAFT_IN_WINDOW_MENUS").is_some_and(|v| !v.is_empty() && v != "0");
+    let mut args = std::env::args().skip(1);
+    while let Some(a) = args.next() {
+        match roots_args.consume(&a, &mut args) {
+            Ok(true) => continue,
+            Ok(false) => {}
+            Err(e) => {
+                eprintln!("vectorcraft: {e}");
+                return std::process::ExitCode::FAILURE;
+            }
+        }
+        match a.as_str() {
+            "--control" => control_port = args.next().and_then(|p| p.parse().ok()),
+            "--in-window-menus" => in_window_menus = true,
+            "--version" => {
+                println!("vectorcraft {}", env!("CARGO_PKG_VERSION"));
+                return std::process::ExitCode::SUCCESS;
+            }
+            _ => files.push(a),
+        }
+    }
+    let automation_args::AutomationArgs { read_root, write_root, .. } = roots_args;
+    // Before desktop services, so every start-up warning is recorded (`logging`).
     let logger = logging::install();
     vectorcraft_ui_egui::i18n::detect_system_lang_in_background();
     // Before the first font scan (the app's start): the fonts font services load (#579).
@@ -595,28 +625,6 @@ fn main() -> std::process::ExitCode {
     mac_fonts::install();
     // VectorCraft's own Fonts folder, which text.addFontFiles copies fonts into.
     prefs_dir::install_fonts();
-    let mut control_port: Option<u16> = std::env::var("VECTORCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok());
-    let root_env = |name: &str| std::env::var_os(name).filter(|v| !v.is_empty()).map(std::path::PathBuf::from);
-    let mut read_root = root_env("VECTORCRAFT_AUTOMATION_READ_ROOT");
-    let mut write_root = root_env("VECTORCRAFT_AUTOMATION_WRITE_ROOT");
-    let mut files = Vec::new();
-    let mut in_window_menus = std::env::var_os("VECTORCRAFT_IN_WINDOW_MENUS").is_some_and(|v| !v.is_empty() && v != "0");
-    let mut args = std::env::args().skip(1);
-    while let Some(a) = args.next() {
-        match a.as_str() {
-            "--control" => control_port = args.next().and_then(|p| p.parse().ok()),
-            "--automation-read-root" => read_root = args.next().map(std::path::PathBuf::from),
-            "--automation-write-root" => write_root = args.next().map(std::path::PathBuf::from),
-            _ if a.starts_with("--automation-read-root=") => read_root = a.split_once('=').map(|(_, d)| d.into()),
-            _ if a.starts_with("--automation-write-root=") => write_root = a.split_once('=').map(|(_, d)| d.into()),
-            "--in-window-menus" => in_window_menus = true,
-            "--version" => {
-                println!("vectorcraft {}", env!("CARGO_PKG_VERSION"));
-                return std::process::ExitCode::SUCCESS;
-            }
-            _ => files.push(a),
-        }
-    }
     // The log file lives in the settings directory, next to the preferences; opened after the
     // arguments, so `--version` leaves no file behind. Records logged until now are written to it
     // first. Runs without preferences (agents' test runs) log to standard error only, so they