fix(icon): pass file paths as plain argv to the icon helper (WP-B2, #45)

additional/python/get_icon_path_for_file.py

@@ -7,11 +7,15 @@
 
 import jessentials
 
+# The path arrives as one argv element and is used as data, unchanged.
+#
+# It used to arrive quoted ("--file='/path'") because the caller ran the
+# helper through a shell, and this line stripped the quotes again. The
+# caller no longer adds them and no longer uses a shell, so stripping them
+# here would only corrupt a path that legitimately ends in a quote
+# character.
 file_path = jessentials.get_value_from_arguments("file")
 
-
-file_path = file_path.removeprefix("'").removesuffix("'")
-
 info = Gio.File.new_for_path(file_path).query_info('standard::icon', Gio.FileQueryInfoFlags.NONE, None)
 icon = info.get_icon()
 

additional/python/tests/test_icon_path_helper.py

@@ -0,0 +1,91 @@
+"""WP-B2 (#45): the icon helper must hand the path through unchanged.
+
+The icon loader used to pass the path to the Python helper as
+`--file='<path>'` — quotes embedded in the value — and the helper stripped
+them again with `removeprefix("'")` / `removesuffix("'")`. The two sides only
+agreed because the one caller happened to add exactly the quotes the one
+helper removed.
+
+The fix makes the contract explicit: the caller owns the encoding, the
+helper passes the value through. That is what these tests pin.
+
+Why it matters, concretely: the helper used to *rewrite* the value it was
+given. A caller that passes the plain path — which is what the icon loader
+does now — would have had a path ending in a quote character truncated by
+the old shim. The pair therefore has to change together, and the tests below
+hold both halves to that.
+
+The GTK stack is mocked: the helper only needs `gi` to exist, and the
+assertion is about which path reaches `Gio.File.new_for_path`.
+"""
+
+import contextlib
+import importlib.util
+import io
+import os
+import sys
+import unittest
+from unittest import mock
+
+SCRIPTS = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+HELPER = os.path.join(SCRIPTS, "get_icon_path_for_file.py")
+sys.path.insert(0, SCRIPTS)
+
+
+def load_helper(argv):
+    """Loads the helper with a mocked GTK stack, returns the mocked Gio."""
+    fake_gi = mock.MagicMock()
+    fake_repository = mock.MagicMock()
+    fake_gi.repository = fake_repository
+    sys.modules["gi"] = fake_gi
+    sys.modules["gi.repository"] = fake_repository
+
+    old_argv = sys.argv
+    sys.argv = argv
+    try:
+        spec = importlib.util.spec_from_file_location(
+            "get_icon_path_for_file_under_test", HELPER)
+        module = importlib.util.module_from_spec(spec)
+        # The helper prints the resolved icon path; that is not what this test
+        # is about, and the mock's repr would drown the test output.
+        with contextlib.redirect_stdout(io.StringIO()):
+            spec.loader.exec_module(module)
+    finally:
+        sys.argv = old_argv
+        sys.modules.pop("gi", None)
+        sys.modules.pop("gi.repository", None)
+    return fake_repository.Gio
+
+
+class IconPathHelper(unittest.TestCase):
+    def path_reaching_gio(self, file_value):
+        """Runs the helper with `--file=<file_value>`, returns that path."""
+        gio = load_helper(["get_icon_path_for_file.py", "--file=" + file_value])
+        gio.File.new_for_path.assert_called_once()
+        return gio.File.new_for_path.call_args[0][0]
+
+    def test_plain_path_arrives_unchanged(self):
+        path = "/home/user/Documents/report.txt"
+        self.assertEqual(self.path_reaching_gio(path), path)
+
+    def test_path_with_spaces_arrives_unchanged(self):
+        path = "/home/user/My Documents/annual report.txt"
+        self.assertEqual(self.path_reaching_gio(path), path)
+
+    def test_path_ending_in_a_quote_is_not_truncated(self):
+        # The old shim ran removesuffix("'") over the value, so a value that
+        # ends in a quote character lost it. The caller no longer adds
+        # quotes, which is exactly the case the old shim would have broken.
+        path = "/home/user/odd name'"
+        self.assertEqual(self.path_reaching_gio(path), path)
+
+    def test_surrounding_quotes_are_kept(self):
+        # The helper must not strip quotes: the value is data. Whether it
+        # carries quotes is the caller's decision — the icon loader passes
+        # the plain path, and adds no quotes of its own.
+        value = "'/tmp/foo'"
+        self.assertEqual(self.path_reaching_gio(value), value)
+
+
+if __name__ == "__main__":
+    unittest.main()

lib/services/icon_loader.dart

@@ -45,27 +45,48 @@ class IconLoader {
     }
     String iconPath = "not found";
 
+    // The path goes into the argv as data: no quoting around it, and no
+    // shell in between. The two belong together — the quotes existed only
+    // to survive the shell, and the Python helper had to strip them again.
+    //
+    // The shell was never a split risk (Dart quotes every argv element, so
+    // a path with a space stays one argument); it was an unnecessary
+    // intermediary, and `runInShell: false` is the pattern the rest of this
+    // file uses. What the quotes did cost: the helper had to rewrite the
+    // value it was given, which breaks a path that legitimately ends in a
+    // quote character. The helper now passes the value through unchanged,
+    // so both sides agree on the format.
+
     /// Load icon for folder
     if (appCode == "folder") {
-      iconPath = await Linux.runCommandWithCustomArguments("/usr/bin/python3", [
-        "${Linux.pythonScriptsFolder}/get_icon_path_for_file.py",
-        "--file='/tmp/'",
-      ]);
+      iconPath = await Linux.runCommandWithCustomArguments(
+          "/usr/bin/python3",
+          [
+            "${Linux.pythonScriptsFolder}/get_icon_path_for_file.py",
+            "--file=/tmp/",
+          ],
+          runInShell: false);
 
       /// Load icon for app
     } else if (appCode != "" && filePath == "") {
-      iconPath = await Linux.runCommandWithCustomArguments("/usr/bin/python3", [
-        "${Linux.pythonScriptsFolder}/get_icon_path_for_application.py",
-        "--icon=$appCode",
-        "--path-to-alternative-logos=${Linux.additionalFolder}logos/"
-      ]);
+      iconPath = await Linux.runCommandWithCustomArguments(
+          "/usr/bin/python3",
+          [
+            "${Linux.pythonScriptsFolder}/get_icon_path_for_application.py",
+            "--icon=$appCode",
+            "--path-to-alternative-logos=${Linux.additionalFolder}logos/"
+          ],
+          runInShell: false);
 
       /// Load icon for file type
     } else if (filePath != "") {
-      iconPath = await Linux.runCommandWithCustomArguments("/usr/bin/python3", [
-        "${Linux.pythonScriptsFolder}/get_icon_path_for_file.py",
-        "--file='$filePath'",
-      ]);
+      iconPath = await Linux.runCommandWithCustomArguments(
+          "/usr/bin/python3",
+          [
+            "${Linux.pythonScriptsFolder}/get_icon_path_for_file.py",
+            "--file=$filePath",
+          ],
+          runInShell: false);
     }
 
     iconPath = iconPath.trim();

security-fixplan-42-50.md

@@ -26,7 +26,7 @@
 |---|---|---|---|
 | #49 | confirmed (Triage+Skeptiker) | **Medium — top priority** | WP-S1 (V0.8.2.5) |
 | #44 | confirmed | Medium (Defense-in-Depth) | WP-S2 (V0.8.2.5) |
-| #45 | contested — Mechanik-Prämisse entkräftet | Low pending RV-1 | WP-B2 (V0.8.2) |
+| #45 | contested — Mechanik-Prämisse entkräftet | Low pending RV-1 | WP-B2 (V0.8.2) | ✅ erledigt — `4049874` |
 | #50 | contested — Mechanik-Prämisse entkräftet | Low pending RV-1 | WP-S3 (V0.8.2.5) |
 | #42 | **rejected** (Prämisse empirisch widerlegt) | Low (Härtung+Korrektheit) | WP-S3 (V0.8.2.5) |
 | #47 | confirmed | Low | WP-B1 (V0.8.2) |
@@ -169,6 +169,13 @@ Commit fließt in den V0.8.2-Commit „fix(launcher): …" oder eigener Commit
 
 ### WP-B2 — #45: Icon-Loader-Quote-Shim entfernen (nur als gekoppeltes Paar)
 
+**Status:** umgesetzt am 2026-10-23 — der Pfad geht als Daten-Element ins argv
+(`--file=<pfad>`, keine eingebetteten Quotes) und ohne Shell (`runInShell: false` an allen drei
+Aufrufstellen in `lib/services/icon_loader.dart`); der Quote-Strip-Shim in
+`additional/python/get_icon_path_for_file.py` ist entfernt, der Helfer reicht den Wert unverändert
+durch. TDD: `test/icon_loader_test.dart` (7 Tests) und
+`additional/python/tests/test_icon_path_helper.py` (4 Tests). Commit `4049874`.
+
 **Befund:** Sink `icon_loader.dart:64-68` (`--file='$filePath'`, drei Aufrufe :52/:59/:64) über
 runInShell:true-Default; Quote-Strip-Shim in `additional/python/get_icon_path_for_file.py`
 arbeitet unter Dart-Escaping korrekt. Entkräftet als Injection (RV-1 entscheidet final) —

test/icon_loader_test.dart

@@ -0,0 +1,124 @@
+// WP-B2 (#45): the icon loader must hand the path to the helper as plain
+// argv.
+//
+// The loader used to embed the path in a quoted string inside the argument
+// (`--file='<path>'`), and the Python helper stripped the quotes again. The
+// two only agreed because the one caller added exactly the quotes the one
+// helper removed.
+//
+// The fix makes the contract explicit: the caller owns the encoding, the
+// helper passes the value through. Two things follow from it:
+//
+// 1. the shell is no longer part of the path. It was never a split risk —
+//    Dart quotes every argv element, so a path with a space stayed one
+//    argument — but it was an unnecessary intermediary, and the repo's
+//    established pattern is `runInShell: false` (six call sites in
+//    `linux.dart`).
+// 2. the helper no longer rewrites the value. A caller that passes the
+//    plain path — which is what the loader does now — would have had a
+//    path ending in a quote character truncated by the old shim. The pair
+//    therefore has to change together.
+//
+// These tests pin both halves of that contract: the argv the loader builds,
+// and the `runInShell` flag it uses.
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:linux_assistant/helpers/command_helper.dart';
+import 'package:linux_assistant/services/icon_loader.dart';
+
+/// Records what the icon loader would have started instead of starting it.
+class _RecordingRunner implements CommandRunner {
+  final List<List<String>> argumentLists = <List<String>>[];
+  final List<bool> runInShellFlags = <bool>[];
+
+  @override
+  Future<CommandResult> run(
+    String command,
+    List<String> arguments, {
+    Map<String, String>? environment,
+    bool asRoot = false,
+    bool hostOnFlatpak = true,
+    bool runInShell = false,
+  }) async {
+    argumentLists.add(List<String>.of(arguments));
+    runInShellFlags.add(runInShell);
+    // "not found" keeps the loader on its default-icon branch, so the test
+    // never touches the file system for an icon.
+    return const CommandResult(true, "not found", "");
+  }
+}
+
+void main() {
+  late _RecordingRunner runner;
+
+  setUp(() {
+    runner = _RecordingRunner();
+    CommandHelper.runner = runner;
+    // The loader is a singleton with a widget cache; a cached entry would
+    // skip the command entirely and make the test pass for the wrong reason.
+    IconLoader().cache.clear();
+  });
+
+  tearDown(() {
+    CommandHelper.runner = CommandHelper.processRunner;
+    IconLoader().cache.clear();
+  });
+
+  group('IconLoader hands the path over as plain argv', () {
+    test('a file path arrives as one argument, without quotes', () async {
+      const String path = "/home/user/My Documents/annual report.txt";
+
+      await IconLoader().getIconForAppOrFile(path);
+
+      expect(runner.argumentLists, hasLength(1));
+      final List<String> arguments = runner.argumentLists.single;
+      expect(arguments, contains("--file=$path"));
+      // The path must not be split over several arguments.
+      expect(
+          arguments.where((String a) => a.startsWith("--file=")), hasLength(1));
+      // And it must not carry the quotes the old shim stripped again.
+      expect(arguments, isNot(contains("--file='$path'")));
+    });
+
+    test('a path ending in a quote character survives unchanged', () async {
+      const String path = "/home/user/odd name'.txt";
+
+      await IconLoader().getIconForAppOrFile(path);
+
+      expect(runner.argumentLists.single, contains("--file=$path"));
+    });
+
+    test('the folder lookup passes its path without quotes', () async {
+      await IconLoader().getIconForAppOrFile("folder");
+
+      expect(runner.argumentLists.single, contains("--file=/tmp/"));
+      expect(runner.argumentLists.single, isNot(contains("--file='/tmp/'")));
+    });
+
+    test('the application lookup passes its code without quotes', () async {
+      await IconLoader().getIconForAppOrFile("firefox");
+
+      expect(runner.argumentLists.single, contains("--icon=firefox"));
+    });
+  });
+
+  group('IconLoader does not run the helper through a shell', () {
+    test('the file lookup runs without a shell', () async {
+      await IconLoader().getIconForAppOrFile("/home/user/report.txt");
+
+      expect(runner.runInShellFlags.single, isFalse);
+    });
+
+    test('the folder lookup runs without a shell', () async {
+      await IconLoader().getIconForAppOrFile("folder");
+
+      expect(runner.runInShellFlags.single, isFalse);
+    });
+
+    test('the application lookup runs without a shell', () async {
+      await IconLoader().getIconForAppOrFile("firefox");
+
+      expect(runner.runInShellFlags.single, isFalse);
+    });
+  });
+}