diff mbox series

[bug#70446,v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively. Adjust bubblewrap wrapper to add user profile.

Message ID a02a0f70b92b794acefc4029b019d271da1d4ebc.1713413172.git.abhi@quic.us
State New
Headers show
Series [bug#70446,v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively. Adjust bubblewrap wrapper to add user profile. | expand

Commit Message

Abhishek Cherath April 18, 2024, 4:06 a.m. UTC 
* gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
Add ~/.guix-profile to bubblewrap gtk sandbox
* gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
'configure-bubblewrap-store-directory' phase, also supply locale
and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
template.

Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
---
apparently the space on the second line of the patch is significant,
doesn't apply otherwise

 .../webkitgtk-adjust-bubblewrap-paths.patch   | 28 +++++++++++++++++--
 gnu/packages/webkit.scm                       | 11 +++++++-
 2 files changed, 35 insertions(+), 4 deletions(-)


base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282

Comments

Liliana Marie Prikler April 19, 2024, 6:53 p.m. UTC | #1 
Am Donnerstag, dem 18.04.2024 um 00:06 -0400 schrieb Abhishek Cherath:
> * gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
> Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
> Add ~/.guix-profile to bubblewrap gtk sandbox
> * gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
> 'configure-bubblewrap-store-directory' phase, also supply locale
> and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
> template.
> 
> Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
> ---
> apparently the space on the second line of the patch is significant,
> doesn't apply otherwise
Wrapping the entire user profile looks evil.  Why?
Abhishek Cherath April 19, 2024, 8:24 p.m. UTC | #2 
Could just add the locale and dri dir, but afaik the user profile is just stuff in the store, right? And the thing has access to the whole store anyhow, so no change, right?
Abhishek Cherath April 19, 2024, 8:33 p.m. UTC | #3 
Will say, I thought it was kinda odd to begin with that it has access to the whole store, though. 

On 19 April 2024 4:24:56 pm GMT-04:00, Abhishek Cherath <abhi@quic.us> wrote:
>Could just add the locale and dri dir, but afaik the user profile is just stuff in the store, right? And the thing has access to the whole store anyhow, so no change, right?
Liliana Marie Prikler April 19, 2024, 9:19 p.m. UTC | #4 
Am Freitag, dem 19.04.2024 um 16:24 -0400 schrieb Abhishek Cherath:
> Could just add the locale and dri dir, but afaik the user profile is
> just stuff in the store, right? And the thing has access to the whole
> store anyhow, so no change, right?
The user dir *is* just stuff in the store, but it is particularly stuff
in the store that's linked to the currently logged-in user.  That is,
you're giving the sandbox extra information by exposing it, and I don't
think it'd be solely (or even largely) useful for beneficial purposes.

Cheers
Abhishek Cherath April 19, 2024, 9:59 p.m. UTC | #5 
That makes sense. I've modified the patch and sent a v3.

I only used the profile path instead of the specific paths because it's the first thing I got working, and I figured there wasn't really anything sensitive in the profile anyway.
diff mbox series

Patch

diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
index 18ddb645ad..c81916279e 100644
--- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
+++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
@@ -1,11 +1,21 @@ 
 Share /gnu/store in the BubbleWrap container and remove FHS mounts.
+Also share user profile directory.
 
 This is a Guix-specific patch not meant to be upstreamed.
 diff --git a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
-index f0a5e4b05dff..88b11f806968 100644
+index 99395d6..3604730 100644
 --- a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
 +++ b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
-@@ -854,27 +854,12 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
+@@ -765,6 +765,8 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
+         return adoptGRef(g_subprocess_launcher_spawnv(launcher, argv, error));
+ 
+     const char* runDir = g_get_user_runtime_dir();
++    const char* homeDir = g_get_home_dir();
++    char* profileDir = g_strconcat(homeDir, "/.guix-profile", NULL);
+     Vector<CString> sandboxArgs = {
+         "--die-with-parent",
+         "--unshare-uts",
+@@ -786,28 +788,24 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
          "--ro-bind", "/sys/dev", "/sys/dev",
          "--ro-bind", "/sys/devices", "/sys/devices",
  
@@ -33,6 +43,18 @@  index f0a5e4b05dff..88b11f806968 100644
 +
 +        // Bind mount the store inside the WebKitGTK sandbox.
 +        "--ro-bind", "@storedir@", "@storedir@",
++
++        // Bind mount the guix profile directory
++        "--ro-bind", profileDir, profileDir,
++
++        // This is needed for locales if not in profile
++        "--ro-bind-try", "@localedir@", "@localedir@",
++
++        // This is needed for video hardware acceleration (va-api)
++        // via /lib/dri if not in profile
++        "--ro-bind-try", "@dridir@", "@dridir@",
      };
++    free(profileDir);
  
-     if (launchOptions.processType == ProcessLauncher::ProcessType::DBusProxy) {
+     if (enableDebugPermissions()) {
+         const char* dataDir = g_get_user_data_dir();
diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index bf24a65e83..a0d04f31d3 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -8,6 +8,7 @@ 
 ;;; Copyright © 2019 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2022, 2023 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2024 Abhishek Cherath <abhi@quic.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -190,7 +191,15 @@  (define-public webkitgtk
               (let ((store-directory (%store-directory)))
                 (substitute*
                     "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
-                  (("@storedir@") store-directory)))))
+                  (("@storedir@") store-directory)
+                  ;; this adds access to drivers for va-api
+                  ;; for hardware accelerated video
+                  (("@dridir@") "/run/current-system/profile/lib/dri")
+                  ;; this silences gtk locale errors
+                  ;; Unfortunately, simply bind mounting /run/current-system
+                  ;; does not work since it leads to weird issues
+                  ;; with symlinks that confuse bubblewrap.
+                  (("@localedir@") "/run/current-system/locale")))))
           (add-after 'unpack 'do-not-disable-new-dtags
             ;; Ensure the linker uses new dynamic tags as this is what Guix
             ;; uses and validates in the validate-runpath phase.