Improved game executable launcher, and added argument validation

Now uses proper shell escaping and parsing for all executable arguments, for more robust game launching. All existing game JSON files with the old string values will automatically be converted to the new format on app launch. The executable parsing uses the "shlex" library, which guarantees accurate parsing.

We now also use direct process launching (without any intermediary shell) by default, but the old "shell"-based launch method still exists in the code via an alternative flag in `run_command.py` (if we ever need to restore it for some reason).

Furthermore, if the user attempts to manually write an improperly escaped argument into the game's details (such as missing closing quotation marks), the GUI will now alert the user that their executable argument is invalid, along with telling them the exact reason why it's invalid.

src/utils/bottles_parser.py

@@ -112,9 +112,10 @@ def bottles_parser(parent_widget, action):
             continue
 
         values["name"] = game["name"]
-        values["executable"] = "xdg-open " + shlex.quote(
+        values["executable"] = [
+            "xdg-open",
             f'bottles:run/{game["bottle"]["name"]}/{game["name"]}'
-        )
+        ]
         values["hidden"] = False
         values["source"] = "bottles"
         values["added"] = current_time

src/utils/create_details_window.py

@@ -19,6 +19,7 @@
 
 import json
 import os
+import shlex
 import time
 
 from gi.repository import Adw, GdkPixbuf, Gio, GLib, GObject, Gtk
@@ -52,7 +53,7 @@ def create_details_window(parent_widget, game_id=None):
         )
         name = Gtk.Entry.new_with_buffer(Gtk.EntryBuffer.new(games[game_id].name, -1))
         executable = Gtk.Entry.new_with_buffer(
-            Gtk.EntryBuffer.new((games[game_id].executable), -1)
+            Gtk.EntryBuffer.new(shlex.join(games[game_id].executable), -1)
         )
         apply_button = Gtk.Button.new_with_label(_("Apply"))
 
@@ -201,6 +202,21 @@ def create_details_window(parent_widget, game_id=None):
         final_developer = developer.get_buffer().get_text()
         final_executable = executable.get_buffer().get_text()
 
+        try:
+            # Attempt to parse using shell parsing rules (doesn't verify executable existence).
+            final_executable_split = shlex.split(
+                final_executable, comments=False, posix=True
+            )
+        except Exception as e:
+            create_dialog(
+                window,
+                _("Couldn't Add Game")
+                if game_id is None
+                else _("Couldn't Apply Preferences"),
+                f'{_("Executable")}: {e}.',  # e = Shell parsing error message. Not translatable.
+            )
+            return
+
         if game_id is None:
             if final_name == "":
                 create_dialog(
@@ -252,7 +268,7 @@ def create_details_window(parent_widget, game_id=None):
 
         values["name"] = final_name
         values["developer"] = final_developer or None
-        values["executable"] = final_executable
+        values["executable"] = final_executable_split
 
         path = os.path.join(
             os.path.join(

src/utils/get_games.py

@@ -19,6 +19,9 @@
 
 import json
 import os
+import shlex
+
+from .game_data_to_json import game_data_to_json
 
 
 def get_games(game_ids=None):
@@ -39,8 +42,30 @@ def get_games(game_ids=None):
         game_files = os.listdir(games_dir)
 
     for game in game_files:
-        with open(os.path.join(games_dir, game), "r") as open_file:
+        with open(os.path.join(games_dir, game), "r+") as open_file:
             data = json.loads(open_file.read())
+
+            # Convert any outdated JSON values to our newest data format.
+            needs_rewrite = False
+            if "executable" in data and isinstance(data["executable"], str):
+                needs_rewrite = True
+                try:
+                    # Use shell parsing to determine what the individual components are.
+                    executable_split = shlex.split(
+                        data["executable"], comments=False, posix=True
+                    )
+                except:
+                    # Fallback: Split once at earliest space (1 part if no spaces, else 2 parts).
+                    executable_split = data["executable"].split(" ", 1)
+                data["executable"] = executable_split
+
+            if needs_rewrite:
+                open_file.seek(0)
+                open_file.truncate()
+                open_file.write(game_data_to_json(data))
+
             open_file.close()
-        games[data["game_id"]] = data
+
+            games[data["game_id"]] = data
+
     return games

src/utils/heroic_parser.py

@@ -129,9 +129,9 @@ def heroic_parser(parent_widget, action):
                 values["name"] = game["title"]
                 values["developer"] = game["developer"]
                 values["executable"] = (
-                    f"start heroic://launch/{app_name}"
+                    ["start", f"heroic://launch/{app_name}"]
                     if os.name == "nt"
-                    else f"xdg-open heroic://launch/{app_name}"
+                    else ["xdg-open", f"heroic://launch/{app_name}"]
                 )
                 values["hidden"] = False
                 values["source"] = "heroic_epic"
@@ -195,9 +195,9 @@ def heroic_parser(parent_widget, action):
                     break
 
             values["executable"] = (
-                f"start heroic://launch/{app_name}"
+                ["start", f"heroic://launch/{app_name}"]
                 if os.name == "nt"
-                else f"xdg-open heroic://launch/{app_name}"
+                else ["xdg-open", f"heroic://launch/{app_name}"]
             )
             values["hidden"] = False
             values["source"] = "heroic_gog"
@@ -230,9 +230,9 @@ def heroic_parser(parent_widget, action):
 
             values["name"] = item["title"]
             values["executable"] = (
-                f"start heroic://launch/{app_name}"
+                ["start", f"heroic://launch/{app_name}"]
                 if os.name == "nt"
-                else f"xdg-open heroic://launch/{app_name}"
+                else ["xdg-open", f"heroic://launch/{app_name}"]
             )
             values["hidden"] = False
             values["source"] = "heroic_sideload"

src/utils/run_command.py

@@ -18,6 +18,7 @@
 # SPDX-License-Identifier: GPL-3.0-or-later
 
 import os
+import shlex
 import subprocess
 import sys
 
@@ -25,15 +26,33 @@ from gi.repository import Gio
 
 
 def run_command(executable):
-    subprocess.Popen(
+    use_shell = False
-        [f"flatpak-spawn --host {executable}"]
+    if not use_shell:
-        if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges"
+        # The host environment is automatically passed through by Popen.
-        else executable.split()
+        subprocess.Popen(
-        if os.name == "nt"
+            ["flatpak-spawn", "--host", *executable]  # Flatpak
-        else [executable],
+            if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges"
-        shell=True,
+            else executable  # Windows
-        start_new_session=True,
+            if os.name == "nt"
-        creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0,
+            else executable,  # Linux/Others
-    )
+            shell=False,  # If true, the extra arguments would incorrectly be given to the shell instead.
+            start_new_session=True,
+            creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0,
+        )
+    else:
+        # When launching as a shell, we must pass 1 string with the exact command
+        # line exactly as we would type it in a shell (with escaped arguments).
+        subprocess.Popen(
+            shlex.join(
+                ["flatpak-spawn", "--host", *executable]  # Flatpak
+                if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges"
+                else executable  # Windows
+                if os.name == "nt"
+                else executable  # Linux/Others
+            ),
+            shell=True,
+            start_new_session=True,
+            creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0,
+        )
     if Gio.Settings.new("hu.kramo.Cartridges").get_boolean("exit-after-launch"):
         sys.exit()

src/utils/steam_parser.py

@@ -64,9 +64,9 @@ def get_game(task, datatypes, current_time, parent_widget, appmanifest, steam_di
         return
 
     values["executable"] = (
-        f'start steam://rungameid/{values["appid"]}'
+        ["start", f'steam://rungameid/{values["appid"]}']
         if os.name == "nt"
-        else f'xdg-open steam://rungameid/{values["appid"]}'
+        else ["xdg-open", f'steam://rungameid/{values["appid"]}']
     )
     values["hidden"] = False
     values["source"] = "steam"