Subject: Re: [bug#75688] [PATCH 0/2] Introduce GUIX_LIBRARY_PATH to replace harmful environment variables

From: 宋文武 <[email-masked]>
Date: Tue, 21 Jan 2025 18:44:59 +0800

宋文武 <[email-masked]> writes:

>> But I don't see why we should use an intermetate GUIX_LIBRARY_PATH to
>> compute all the other correct paths; this should be left to the search
>> paths, in my opinion (as it's simpler, cleaner).
>
> Well, lesser variables give a "clean" feeling, it's mostly aesthetic,
> especially in GNOME and KDE, mixed with wrappers, there are a lot
> variables "noise" in the environment.
>
> Also think a litte more, as we need patching software anyway, I hope to
> modify the logic `guix_build_library_path` to also include paths from
> what wrappers currently provides, to drop wrappers.  That logic should
> be simpler to implement in one place than for every specified
> variable.

Okay, I changed the plan, since use only one env like GUIX_LIBRARY_PATH
would lead to some form of 'stats storm', and the logic is not harder
for patching different variables. If we get rid of wrapper scripts, the
"noise" will only contains profile paths, so are acceptable..

Now I get this patch for glib:

1. add a `gchar **g_build_guix_search_path_dirs (const gchar *variable)`
   function to GLIB, which handle .VARIABLE file in addition to the env
   value.  Those .VARIABLE files sit at the same directory of
   executable, would be used to replace wrapper script.  As said early,
   wrapper scripts leaks environment variables could cause incompatable
   problems.

2. Use `g_build_guix_search_path_dirs` for GUIX_GIO_EXTRA_MODULES,
   GUIX_GSETTINGS_SCHEMA_DIR, and etc. (later).
   
3. (later) Modify wrap-program usages to use .VARIABLE files to get rid
   of environment variables leaks.

diff --git a/gio/giomodule.c b/gio/giomodule.c
index 76c2028..49b02bb 100644
--- a/gio/giomodule.c
+++ b/gio/giomodule.c
@@ -1330,6 +1330,13 @@ _g_io_modules_ensure_loaded (void)
       g_io_modules_scan_all_in_directory_with_scope (module_dir, scope);
       g_free (module_dir);
 
+      /* GUIX: Load gio modules from GUIX_GIO_EXTRA_MODULES */
+      gchar **guix_giomodule_dirs = g_build_guix_search_path_dirs ("GUIX_GIO_EXTRA_MODULES");
+      for (int i = 0; guix_giomodule_dirs[i] != NULL; i++) {
+        g_io_modules_scan_all_in_directory_with_scope (guix_giomodule_dirs[i], scope);
+      }
+      g_strfreev (guix_giomodule_dirs);
+
       g_io_module_scope_free (scope);
 
       /* Initialize types from built-in "modules" */
diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c
index e8ccc8c..d7ff8f4 100644
--- a/gio/gsettingsschema.c
+++ b/gio/gsettingsschema.c
@@ -369,6 +369,13 @@ initialise_schema_sources (void)
           g_strfreev (extra_schema_dirs);
         }
 
+      /* GUIX: Load schemas from GUIX_GSETTINGS_SCHEMA_DIR. */
+      char **guix_schema_dirs = g_build_guix_search_path_dirs ("GUIX_GSETTINGS_SCHEMA_DIR");
+      i = g_strv_length(guix_schema_dirs);
+      while (i--)
+        try_prepend_dir (guix_schema_dirs[i]);
+      g_strfreev (guix_schema_dirs);
+
       g_once_init_leave (&initialised, TRUE);
     }
 }
diff --git a/glib/gutils.c b/glib/gutils.c
index 8628a56..0139d42 100644
--- a/glib/gutils.c
+++ b/glib/gutils.c
@@ -2849,6 +2849,46 @@ g_get_system_config_dirs (void)
   return system_config_dirs;
 }
 
+gchar **
+g_build_guix_search_path_dirs (const gchar *variable)
+{
+  gchar **dirs = NULL;
+  char *value = NULL;
+  GStrvBuilder *builder = g_strv_builder_new ();
+
+#if defined(__linux__) || defined(__gnu_hurd__)
+  /* First add paths from the .VARIABLE file, which can be used to replace wrapper script. */
+  gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
+  gchar *var_path = g_strjoin(NULL, exe_path, ".", variable, NULL);
+  if (g_file_get_contents (var_path, &value, NULL, NULL)) {
+      dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
+      g_strv_builder_addv (builder, (const gchar **) dirs);
+      g_strfreev (dirs);
+      g_free (value);
+  }
+  g_free (exe_path);
+  g_free (var_path);
+#endif
+
+  /* Then add paths from the environment variable. */
+  gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
+  if (is_setuid) /* we don't want to access arbitrary files when running as setuid. */
+    value = NULL;
+  else
+    value = g_strdup (g_getenv (variable));
+
+  if (value && value[0]) {
+      dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
+      g_strv_builder_addv (builder, (const gchar **) dirs);
+      g_strfreev (dirs);
+  }
+  g_free (value);
+
+  dirs = g_strv_builder_end (builder);
+  g_strv_builder_unref (builder);
+  return dirs;
+}
+
 /**
  * g_nullify_pointer:
  * @nullify_location: (not nullable): the memory address of the pointer.
diff --git a/glib/gutils.h b/glib/gutils.h
index efc6914..710cf27 100644
--- a/glib/gutils.h
+++ b/glib/gutils.h
@@ -36,6 +36,9 @@
 
 G_BEGIN_DECLS
 
+GLIB_AVAILABLE_IN_ALL
+gchar **g_build_guix_search_path_dirs (const gchar *variable);
+
 GLIB_AVAILABLE_IN_ALL
 const gchar *         g_get_user_name        (void);
 GLIB_AVAILABLE_IN_ALL

A similiar patch would needed for qtbase, or maybe I can use glib API
directly from any Qt application? (haven't tried)  Does this level of
patching seems okay?

Thanks!