feat: split wrapper command on linux (#4427)

Rayzeq · web-flow · commit 6b5f8a41e798 · 2025-10-30T21:48:51.000Z
* feat: split wrapper command on linux * feat: use code from #3900 * feat: also use shlex on Windows * feat: add a version number to global settings * feat(app): add settings v2, where wrapper command are split
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -158,6 +158,7 @@ serde-xml-rs = "0.8.1"  # Also an XML (de)serializer, consider dropping yaserde
 sha1 = "0.10.6"
 sha1_smol = { version = "1.0.1", features = ["std"] }
 sha2 = "0.10.9"
+shlex = "1.3.0"
 spdx = "0.12.0"
 sqlx = { version = "0.8.6", default-features = false }
 sysinfo = { version = "0.37.2", default-features = false }
diff --git a/apps/app-frontend/src/helpers/settings.ts b/apps/app-frontend/src/helpers/settings.ts
@@ -67,6 +67,8 @@ export type AppSettings = {
 	skipped_update: string | null
 	pending_update_toast_for_version: string | null
 	auto_download_updates: boolean | null
+
+	version: number
 }
 
 // Get full settings object
diff --git a/packages/app-lib/.sqlx/query-07ea3a644644de61c4ed7c30ee711d29fd49f10534230b1b03097275a30cb50f.json b/packages/app-lib/.sqlx/query-07ea3a644644de61c4ed7c30ee711d29fd49f10534230b1b03097275a30cb50f.json
diff --git a/packages/app-lib/.sqlx/query-6e3fa492c085ebb8e7280dd4d55cdcf73da199ea6ac05ee3ee798ece80d877cf.json b/packages/app-lib/.sqlx/query-6e3fa492c085ebb8e7280dd4d55cdcf73da199ea6ac05ee3ee798ece80d877cf.json
diff --git a/packages/app-lib/.sqlx/query-a40e60da6dd1312d4a1ed52fa8fd2394e7ad21de1cb44cf8b93c4b1459cdc716.json b/packages/app-lib/.sqlx/query-a40e60da6dd1312d4a1ed52fa8fd2394e7ad21de1cb44cf8b93c4b1459cdc716.json
diff --git a/packages/app-lib/Cargo.toml b/packages/app-lib/Cargo.toml
@@ -73,6 +73,7 @@ serde_json = { workspace = true }
 serde_with = { workspace = true }
 sha1_smol = { workspace = true }
 sha2 = { workspace = true }
+shlex = { workspace = true }
 sqlx = { workspace = true, features = [
   "json",
   "macros",
diff --git a/packages/app-lib/migrations/20251002173041_settings-version.sql b/packages/app-lib/migrations/20251002173041_settings-version.sql
@@ -0,0 +1 @@
+ALTER TABLE settings ADD COLUMN version INTEGER NOT NULL DEFAULT 1;
diff --git a/packages/app-lib/src/api/profile/mod.rs b/packages/app-lib/src/api/profile/mod.rs
@@ -666,7 +666,14 @@ async fn run_credentials(
         .filter(|hook_command| !hook_command.is_empty());
     if let Some(hook) = pre_launch_hooks {
         // TODO: hook parameters
-        let mut cmd = hook.split(' ');
+        let mut cmd = shlex::split(hook)
+            .ok_or_else(|| {
+                crate::ErrorKind::LauncherError(format!(
+                    "Invalid pre-launch command: {hook}",
+                ))
+            })?
+            .into_iter();
+
         if let Some(command) = cmd.next() {
             let full_path = get_full_path(&profile.path).await?;
             let result = Command::new(command)
diff --git a/packages/app-lib/src/launcher/mod.rs b/packages/app-lib/src/launcher/mod.rs
@@ -567,7 +567,19 @@ pub async fn launch_minecraft(
     let args = version_info.arguments.clone().unwrap_or_default();
     let mut command = match wrapper {
         Some(hook) => {
-            let mut command = Command::new(hook);
+            let mut cmd = shlex::split(hook)
+                .ok_or_else(|| {
+                    crate::ErrorKind::LauncherError(format!(
+                        "Invalid wrapper command: {hook}",
+                    ))
+                })?
+                .into_iter();
+            let mut command = Command::new(cmd.next().ok_or(
+                crate::ErrorKind::LauncherError(
+                    "Empty wrapper command".to_owned(),
+                ),
+            )?);
+            command.args(cmd);
             command.arg(&java_version.path);
             command
         }
diff --git a/packages/app-lib/src/state/mod.rs b/packages/app-lib/src/state/mod.rs
@@ -89,6 +89,7 @@ impl State {
             let res = tokio::try_join!(
                 state.discord_rpc.clear_to_default(true),
                 Profile::refresh_all(),
+                Settings::migrate(&state.pool),
                 ModrinthCredentials::refresh_all(),
             );
 
diff --git a/packages/app-lib/src/state/process.rs b/packages/app-lib/src/state/process.rs
@@ -743,7 +743,14 @@ impl Process {
             // We do not wait on the post exist command to finish running! We let it spawn + run on its own.
             // This behaviour may be changed in the future
             if let Some(hook) = post_exit_command {
-                let mut cmd = hook.split(' ');
+                let mut cmd = shlex::split(&hook)
+                    .ok_or_else(|| {
+                        crate::ErrorKind::LauncherError(format!(
+                            "Invalid post-exit command: {hook}",
+                        ))
+                    })?
+                    .into_iter();
+
                 if let Some(command) = cmd.next() {
                     let mut command = Command::new(command);
                     command.args(cmd).current_dir(
diff --git a/packages/app-lib/src/state/profiles.rs b/packages/app-lib/src/state/profiles.rs
@@ -103,17 +103,19 @@ impl ProfileInstallStage {
 pub enum LauncherFeatureVersion {
     None,
     MigratedServerLastPlayTime,
+    MigratedLaunchHooks,
 }
 
 impl LauncherFeatureVersion {
-    pub const MOST_RECENT: Self = Self::MigratedServerLastPlayTime;
+    pub const MOST_RECENT: Self = Self::MigratedLaunchHooks;
 
     pub fn as_str(&self) -> &'static str {
         match *self {
             Self::None => "none",
             Self::MigratedServerLastPlayTime => {
                 "migrated_server_last_play_time"
             }
+            Self::MigratedLaunchHooks => "migrated_launch_hooks",
         }
     }
 
@@ -123,6 +125,7 @@ impl LauncherFeatureVersion {
             "migrated_server_last_play_time" => {
                 Self::MigratedServerLastPlayTime
             }
+            "migrated_launch_hooks" => Self::MigratedLaunchHooks,
             _ => Self::None,
         }
     }
@@ -785,6 +788,30 @@ impl Profile {
                 self.launcher_feature_version =
                     LauncherFeatureVersion::MigratedServerLastPlayTime;
             }
+            LauncherFeatureVersion::MigratedServerLastPlayTime => {
+                let quoter = shlex::Quoter::new().allow_nul(true);
+
+                // Previously split by spaces
+                if let Some(pre_launch) = self.hooks.pre_launch.as_ref() {
+                    self.hooks.pre_launch =
+                        Some(quoter.join(pre_launch.split(' ')).unwrap())
+                }
+
+                // Previously treated as complete path to command
+                if let Some(wrapper) = self.hooks.wrapper.as_ref() {
+                    self.hooks.wrapper =
+                        Some(quoter.quote(wrapper).unwrap().to_string())
+                }
+
+                // Previously split by spaces
+                if let Some(post_exit) = self.hooks.post_exit.as_ref() {
+                    self.hooks.post_exit =
+                        Some(quoter.join(post_exit.split(' ')).unwrap())
+                }
+
+                self.launcher_feature_version =
+                    LauncherFeatureVersion::MigratedLaunchHooks;
+            }
             LauncherFeatureVersion::MOST_RECENT => unreachable!(
                 "LauncherFeatureVersion::MOST_RECENT was not updated"
             ),
diff --git a/packages/app-lib/src/state/settings.rs b/packages/app-lib/src/state/settings.rs
@@ -1,6 +1,7 @@
 //! Theseus settings file
 
 use serde::{Deserialize, Serialize};
+use sqlx::{Pool, Sqlite};
 use std::collections::HashMap;
 
 // Types
@@ -42,6 +43,8 @@ pub struct Settings {
     pub skipped_update: Option<String>,
     pub pending_update_toast_for_version: Option<String>,
     pub auto_download_updates: Option<bool>,
+
+    pub version: usize,
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone, Copy, Eq, Hash, PartialEq)]
@@ -54,6 +57,8 @@ pub enum FeatureFlag {
 }
 
 impl Settings {
+    const CURRENT_VERSION: usize = 2;
+
     pub async fn get(
         exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
     ) -> crate::Result<Self> {
@@ -68,7 +73,8 @@ impl Settings {
                 mc_memory_max, mc_force_fullscreen, mc_game_resolution_x, mc_game_resolution_y, hide_on_process_start,
                 hook_pre_launch, hook_wrapper, hook_post_exit,
                 custom_dir, prev_custom_dir, migrated, json(feature_flags) feature_flags, toggle_sidebar,
-                skipped_update, pending_update_toast_for_version, auto_download_updates
+                skipped_update, pending_update_toast_for_version, auto_download_updates,
+                version
             FROM settings
             "
         )
@@ -126,6 +132,7 @@ impl Settings {
             pending_update_toast_for_version: res
                 .pending_update_toast_for_version,
             auto_download_updates: res.auto_download_updates.map(|x| x == 1),
+            version: res.version as usize,
         })
     }
 
@@ -140,6 +147,7 @@ impl Settings {
         let extra_launch_args = serde_json::to_string(&self.extra_launch_args)?;
         let custom_env_vars = serde_json::to_string(&self.custom_env_vars)?;
         let feature_flags = serde_json::to_string(&self.feature_flags)?;
+        let version = self.version as i64;
 
         sqlx::query!(
             "
@@ -183,7 +191,9 @@ impl Settings {
 
                 skipped_update = $29,
                 pending_update_toast_for_version = $30,
-                auto_download_updates = $31
+                auto_download_updates = $31,
+
+                version = $32
             ",
             max_concurrent_writes,
             max_concurrent_downloads,
@@ -216,12 +226,75 @@ impl Settings {
             self.skipped_update,
             self.pending_update_toast_for_version,
             self.auto_download_updates,
+            version,
         )
         .execute(exec)
         .await?;
 
         Ok(())
     }
+
+    pub async fn migrate(exec: &Pool<Sqlite>) -> crate::Result<()> {
+        let mut settings = Self::get(exec).await?;
+
+        if settings.version < Settings::CURRENT_VERSION {
+            tracing::info!(
+                "Migrating settings version {} to {:?}",
+                settings.version,
+                Settings::CURRENT_VERSION
+            );
+        }
+        while settings.version < Settings::CURRENT_VERSION {
+            if let Err(err) = settings.perform_migration() {
+                tracing::error!(
+                    "Failed to migrate settings from version {}: {}",
+                    settings.version,
+                    err
+                );
+                return Err(err);
+            }
+        }
+
+        settings.update(exec).await?;
+
+        Ok(())
+    }
+
+    pub fn perform_migration(&mut self) -> crate::Result<()> {
+        match self.version {
+            1 => {
+                let quoter = shlex::Quoter::new().allow_nul(true);
+
+                // Previously split by spaces
+                if let Some(pre_launch) = self.hooks.pre_launch.as_ref() {
+                    self.hooks.pre_launch =
+                        Some(quoter.join(pre_launch.split(' ')).unwrap())
+                }
+
+                // Previously treated as complete path to command
+                if let Some(wrapper) = self.hooks.wrapper.as_ref() {
+                    self.hooks.wrapper =
+                        Some(quoter.quote(wrapper).unwrap().to_string())
+                }
+
+                // Previously split by spaces
+                if let Some(post_exit) = self.hooks.post_exit.as_ref() {
+                    self.hooks.post_exit =
+                        Some(quoter.join(post_exit.split(' ')).unwrap())
+                }
+
+                self.version = 2;
+            }
+            version => {
+                return Err(crate::ErrorKind::OtherError(format!(
+                    "Invalid settings version: {version}"
+                ))
+                .into());
+            }
+        }
+
+        Ok(())
+    }
 }
 
 /// Theseus theme

Original file line number	Diff line number	Diff line change
`@@ -67,6 +67,8 @@ export type AppSettings = {`
`67`	`67`	`skipped_update: string \| null`
`68`	`68`	`pending_update_toast_for_version: string \| null`
`69`	`69`	`auto_download_updates: boolean \| null`
	`70`	`+`
	`71`	`+ version: number`
`70`	`72`	`}`
`71`	`73`
`72`	`74`	`// Get full settings object`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+ALTER TABLE settings ADD COLUMN version INTEGER NOT NULL DEFAULT 1;`