Qt 6: Make sure macOS sandboxing is reenabled

[The-Compiler]

See pyinstaller/pyinstaller#6903 and #7252 (comment)

[hpfmn]

Is here anything I could help with to progress with this issue?

[The-Compiler]

@hpfmn This needs to be fixed in PyInstaller, maybe @rokm can say more.

[rokm]

We need to implement support for symlinks in PyInstaller so we can satisfy PyInstaller's current assumptions about library locations and stop butchering Qt .framework bundles. Until this is done and the structure of those .framework bundles is properly preserved, it's either disabled sandbox or non-functioning executable (unless, perhaps behavior changed in 6.4.0...).

Or, you can use PySide6 < Qt 6.3.1 and undo the effect of runtime hook from pyinstaller/pyinstaller#6903 by resetting QTWEBENGINE_DISABLE_SANDBOX environment variable at the start of your program.

[rokm]

Actually, until this is sorted out on PyInstaller's side as described above, you could fix it yourself as a post-processing step in your build process. What you need to do is, for each .framework bundle that is collected into PySide6/Qt/lib directory (within top-level directory of onedir build, or Contents/MacOS directory of onedir app bundle), you should remove the corresponding copy of the shared library that has been collected into top-level application directory (or Contents/MacOS), and replace it with a symlink to the library in PySide6/Qt/lib/{QtModuleName}.framework/Versions/A/{QtModuleName}.

Below is an example program and a fix-up python script:

Test program - program.py

# program.py
import sys
import signal
import os

if 'QTWEBENGINE_DISABLE_SANDBOX' in os.environ:
    print("Re-enabling sandbox...")
    del os.environ['QTWEBENGINE_DISABLE_SANDBOX']

from PySide6 import QtWidgets, QtWebEngineWidgets, QtCore

app = QtWidgets.QApplication(sys.argv)

signal.signal(signal.SIGINT, signal.SIG_DFL)

view = QtWebEngineWidgets.QWebEngineView()
view.setUrl(QtCore.QUrl.fromUserInput("http://www.google.com"))
view.show()

sys.exit(app.exec())

Fix-up script - fixup_qtwebengine.py

# fixup_qtwebengine.py
import pathlib
import sys

if len(sys.argv) != 2:
    print(f"Usage {sys.argv[0]} <path-to-app-bundle>")
    sys.exit(1)

app_bundle = pathlib.Path(sys.argv[1])
assert app_bundle.name.endswith('.app'), "The given path does not end with an .app"

print(f"Attempting to fix up QtWebEngine in .app bundle: {app_bundle}")

top_level = app_bundle / "Contents" / "MacOS"
qt_libdir = top_level / "PySide6" / "Qt" / "lib"

# For collected Qt .framework bundles, replace the copy of lib in top-level directory
# with symlink to the copy from the .framework bundle
for framework in qt_libdir.iterdir():
    if not framework.is_dir():
        continue
    if not framework.match("**/*.framework"):
        continue

    # Infer shared lib name from .framework name
    lib_name = framework.name[:-len('.framework')]
    framework_lib_file = framework / "Versions" / "A" / lib_name

    assert framework_lib_file.is_file(), "Framework lib file does not exist!"

    # Check if library exists in top-level directory
    lib_file = top_level / lib_name
    if not lib_file.is_file():
        continue

    rel_framework_lib_file = framework_lib_file.relative_to(top_level)

    print(f"Replacing {lib_file!s} with symbolic link to {framework_lib_file!s}; rel path: {rel_framework_lib_file}")
    lib_file.unlink()
    lib_file.symlink_to(rel_framework_lib_file)

# Remove redundant QtWebEngine files - these are actually just symlinks to
# counterparts in Resources
for file in top_level.glob("qtwebengine_*.pak"):
    print(f"Removing {file}")
    file.unlink()

# Remove redundant QtWebEngine files in Resources (the useful copy is
# located within the collected .framework bundle)
resources_dir = app_bundle / "Contents" / "Resources"
for file in resources_dir.glob("qtwebengine_*.pak"):
    print(f"Removing {file}")
    file.unlink()

Freeze the program in onedir windowed mode:

pyinstaller --clean --noconfirm --windowed program.py 

Trying to run the resulting app bundle:

./dist/program.app/Contents/MacOS/program

crashes the QtWebEngineProcess helper processes due to sandbox errors:

...
sandbox initialization failed: empty subpath pattern
Nov 16 17:57:22  QtWebEngineProcess[8754] <Error>: SeatbeltExecServer: Failed to initialize sandbox: -1 empty subpath pattern
sandbox initialization failed: empty subpath pattern
Nov 16 17:57:24  QtWebEngineProcess[8756] <Error>: SeatbeltExecServer: Failed to initialize sandbox: -1 empty subpath pattern

Fix up the bundle using the script:

python fixup_qtwebengine.py dist/program.app/

And now

./dist/program.app/Contents/MacOS/program

runs as expected.

Don't forget to re-sign the bundle.

[toofar]

PyInstaller has a new release coming up that should address this. It fixes some issues and hacks around collecting stuff (especially around Qt wrappers it seems) and packaging stuff for macOS. The release notes for 5.13.0 (the previous release) says that those changes can be breaking and will only be included in a 6.0.0 release.

That has me a bit worried since we want to re-enable webengine (actually chromium) sandbox on mac for the 3.0 release. We do have a WIP PR up to enable that on our side but since there is an upstream fix now it would be good to look at that. I've hacked up a branch to do the nightly build jobs with an updated PyInstaller (from their develop branch) to have an early look at any issues. It's failing 🥲

macOS:

• edit: I've disabled patching for now is currently failing in patch_mac_app with FileNotFound for dist/qutebrowser.app/Contents/MacOS/PyQt5/Qt5/lib/QtWebEngineCore.framework/Resources (same error for PyQt5 and 6 with with different numbers in the path).
• it's building now but the smoke test is failing because the path we need to load resources from has changed, https://pyinstaller.org/en/stable/runtime-information.html might help

windows:

• edit: building now and smoke test is passing builds are crashing in the smoke test with no log output. Not sure why, there are some "lib not found" warnings in the pyinstaller run but those are present in the current (working) PyQt5 nightly builds too.
• I got tricked by the workflow file having jobs with --debug arguments which caused the stderr to be swallowed!

I've changed the build_release tox job to pass PYINSTALLER_COMPILE_BOOTLOADER through for now to tell pyinstaller to rebuild it's bootloader. Seems to work transparently but I guess we don't want to keep this there after the new release comes out.

As to how we can check that sandboxing is re-enabled on macOS, since the chrome://sandbox/ page apparently doesn't work there, I think there are three options:

1. try to run an exploit that the sandbox would prevent
2. look at the CLI args of a rendered process for the --no-sandbox arg, since that's all QTWEBENGINE_DISABLE_SANDBOX=1 does
3. look for the info log message Sandboxing disabled by user. that webengine logs when that env var is set

We are already doing (3) but have removed it temporarily for macOS. So just removing that should be fine 🤞. Checking a renderer CLI args in a "unit" test probably wouldn't be too hard either.

[hpfmn]

You can check if the sandbox is enabled in macOS activity monitor. You can right click on the column names and add the "sandbox" column. The qutebrowser's main process should not be sandboxed but all child processes. It's the same with chrome.

[rokm]

macOS is currently failing in patch_mac_app with FileNotFound for dist/qutebrowser.app/Contents/MacOS/PyQt5/Qt5/lib/QtWebEngineCore.framework/Resources (same error for PyQt5 and 6 with with different numbers in the path).

The contents are now distributed between Contents/Frameworks and Contents/Resources. Contents/MacOS contains only the program executable.

Both windows builds are crashing in the smoke test with no log output. Not sure why, there are some "lib not found" warnings in the pyinstaller run but those are present in the current (working) PyQt5 nightly builds too.

For "regular" onedir builds, the layout has also changed - everything but the executable is put into a sub-directory (although that's a different PR).

As a side note, for the develop branch to actually work, you need to rebuild the bootloader yourself (or force it to be re-built during installation, by setting PYINSTALLER_COMPILE_BOOTLOADER=1 environment variable). Otherwise the search locations will not match the new layout (this goes both for macOS .app bundles and for regular onedir builds).

[toofar]

As a side note, for the develop branch to actually work, you need to rebuild the bootloader yourself (or force it to be re-built during installation, by setting PYINSTALLER_COMPILE_BOOTLOADER=1 environment variable). Otherwise the search locations will not match the new layout (this goes both for macOS .app bundles and for regular onedir builds).

Aha, thanks for the info! I'll try to see if I can get that working on CI. Is that a normal step for using develop or just because stuff moved recently and the boatloader hasn't been rebuilt and commited back to the repo yet?

[rokm]

As a side note, for the develop branch to actually work, you need to rebuild the bootloader yourself (or force it to be re-built during installation, by setting PYINSTALLER_COMPILE_BOOTLOADER=1 environment variable). Otherwise the search locations will not match the new layout (this goes both for macOS .app bundles and for regular onedir builds).

Aha, thanks for the info! I'll try to see if I can get that working on CI. Is that a normal step for using develop or just because stuff moved recently and the boatloader hasn't been rebuilt and commited back to the repo yet?

We usually rebuild and commit bootloaders only when we make new release, so if you want to use develop, it is safest to always rebuild the bootloader yourself (i.e., you can get away without it only if there are no changes in bootloader code since the last release).

[toofar]

Looks like the smoke test is working with pyinstaller develop now 🎉. And that includes removing the allowance for "Sandboxing disabled by user", so it should be sandboxing now. The relevant production code change was just:

diff --git a/qutebrowser/utils/resources.py b/qutebrowser/utils/resources.py
index 44e4687368a..b299e4d7e2e 100644
--- a/qutebrowser/utils/resources.py
+++ b/qutebrowser/utils/resources.py
@@ -52,7 +52,16 @@ def _path(filename: str) -> _ResourceType:
     if hasattr(sys, 'frozen'):
         # For PyInstaller, where we can't store resource files in a qutebrowser/ folder
         # because the executable is already named "qutebrowser" (at least on macOS).
-        return pathlib.Path(sys.executable).parent / filename
+        return pathlib.Path(__file__).parent.parent.parent / filename
 
     return importlib_resources.files(qutebrowser) / filename

To follow the recommended way of finding data files. (The comment above the changed line is probably no-longer true. But probably you want to call it frozen-assets or something anyway? Like why risk the conflict at all?) There's still some work to do to clean up our build scripts, and I would like to arrange a bit of manual testing (esp for the mac build), but this seems pretty de-risked now in my opinion.

I'm not sure how to suggest going about manual testing. If you install these versions you should not open your normal qutebrowser profile with them, wait until a proper release comes out. You could install them and open a temp basedir for testing (find the executable and open it with -T on the command line) the re-install the last release afterwards. So you can open your normal profile again. Sounds cumbersome. You should be able to install as an alternate user or in a VM too...

[The-Compiler]

@toofar PyInstaller seems to support importlib.resources nowadays - while you're at it, any chance you could try what happens if you drop that if hasattr(sys, 'frozen'): entirely?

[toofar]

Yep, looks like that works (at least as per the smoke tests) after adjusting the data paths to match the source build:

diff --git i/misc/qutebrowser.spec w/misc/qutebrowser.spec
index c74171543fb8..a74a1a1082a8 100644
--- i/misc/qutebrowser.spec
+++ w/misc/qutebrowser.spec
@@ -64,17 +64,17 @@ INFO_PLIST_UPDATES = {

 def get_data_files():
     data_files = [
-        ('../qutebrowser/html', 'html'),
-        ('../qutebrowser/img', 'img'),
-        ('../qutebrowser/icons', 'icons'),
-        ('../qutebrowser/javascript', 'javascript'),
-        ('../qutebrowser/html/doc', 'html/doc'),
-        ('../qutebrowser/git-commit-id', '.'),
-        ('../qutebrowser/config/configdata.yml', 'config'),
+        ('../qutebrowser/html', 'qutebrowser/html'),
+        ('../qutebrowser/img', 'qutebrowser/img'),
+        ('../qutebrowser/icons', 'qutebrowser/icons'),
+        ('../qutebrowser/javascript', 'qutebrowser/javascript'),
+        ('../qutebrowser/html/doc', 'qutebrowser/html/doc'),
+        ('../qutebrowser/git-commit-id', 'qutebrowser/git-commit-id'),
+        ('../qutebrowser/config/configdata.yml', 'qutebrowser/config'),
     ]

     if os.path.exists(os.path.join('qutebrowser', '3rdparty', 'pdfjs')):
-        data_files.append(('../qutebrowser/3rdparty/pdfjs', '3rdparty/pdfjs'))
+        data_files.append(('../qutebrowser/3rdparty/pdfjs', 'qutebrowser/3rdparty/pdfjs'))
     else:
         print("Warning: excluding pdfjs as it's not present!")

[toofar]

You can check if the sandbox is enabled in macOS activity monitor. You can right click on the column names and add the "sandbox" column. The qutebrowser's main process should not be sandboxed but all child processes. It's the same with chrome.

Alright, seems to work on a mac I managed to scrounge up. I'll work on cleaning up my branch this week and than I guess we'll hope a PyInstaller release comes out before the other tasks in the milestone get done (probably 2-3 weeks).