Simplify apivfs_cmd() and chroot_cmd()

We move the setpgid logic to run(), avoiding the need to pass a tools
argument to chroot_cmd() and apivfs_cmd().

We also try to remove as much logic from these functions as possible.
Since we can't really assume that any logic we execute during the
function will still hold true in the sandbox, so it's best to delay
any logic execution until we're already in the sandbox (using the
--ro-bind-try options of bubblewrap).

We also rework the /etc/resolv.conf handling to simply make sure that
/run/systemd/resolve exists in the chroot since if /etc/resolv.conf
points to /run it'll almost certainly be to
/run/systemd/resolv/stub-resolv.conf.

mkosi/__init__.py

@@ -442,7 +442,6 @@ def run_prepare_scripts(context: Context, build: bool) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=True,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -517,7 +516,6 @@ def run_build_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -587,7 +585,6 @@ def run_postinst_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",
@@ -648,7 +645,6 @@ def run_finalize_scripts(context: Context) -> None:
             chroot = chroot_cmd(
                 context.root,
                 resolve=context.config.with_network,
-                tools=context.config.tools(),
                 options=[
                     "--bind", "/work", "/work",
                     "--chdir", "/work/src",

mkosi/distributions/gentoo.py

@@ -26,7 +26,7 @@
 
 def invoke_emerge(context: Context, packages: Sequence[str] = (), apivfs: bool = True) -> None:
     run(
-        apivfs_cmd(context.root, tools=context.config.tools()) + [
+        apivfs_cmd(context.root) + [
             # We can't mount the stage 3 /usr using `options`, because bwrap isn't available in the stage 3
             # tarball which is required by apivfs_cmd(), so we have to mount /usr from the tarball later
             # using another bwrap exec.
@@ -161,7 +161,6 @@ def install(cls, context: Context) -> None:
 
         chroot = chroot_cmd(
             stage3,
-            tools=context.config.tools(),
             options=["--bind", context.cache_dir / "repos", "/var/db/repos"],
         )
 

mkosi/installer/__init__.py

@@ -41,12 +41,12 @@ def clean_package_manager_metadata(context: Context) -> None:
 
 def package_manager_scripts(context: Context) -> dict[str, list[PathString]]:
     return {
-        "pacman": apivfs_cmd(context.root, tools=context.config.tools()) + pacman_cmd(context),
-        "zypper": apivfs_cmd(context.root, tools=context.config.tools()) + zypper_cmd(context),
-        "dnf"   : apivfs_cmd(context.root, tools=context.config.tools()) + dnf_cmd(context),
-        "rpm"   : apivfs_cmd(context.root, tools=context.config.tools()) + rpm_cmd(context),
+        "pacman": apivfs_cmd(context.root) + pacman_cmd(context),
+        "zypper": apivfs_cmd(context.root) + zypper_cmd(context),
+        "dnf"   : apivfs_cmd(context.root) + dnf_cmd(context),
+        "rpm"   : apivfs_cmd(context.root) + rpm_cmd(context),
     } | {
-        command: apivfs_cmd(context.root, tools=context.config.tools()) + apt_cmd(context, command) for command in (
+        command: apivfs_cmd(context.root) + apt_cmd(context, command) for command in (
             "apt",
             "apt-cache",
             "apt-cdrom",

mkosi/installer/apt.py

@@ -113,7 +113,7 @@ def invoke_apt(
                     *finalize_crypto_mounts(tools=context.config.tools()),
                     *mounts,
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

mkosi/installer/dnf.py

@@ -139,7 +139,7 @@ def invoke_dnf(context: Context, command: str, packages: Iterable[str], apivfs:
                     context.cache_dir / "lib" / dnf_subdir(context),
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

mkosi/installer/pacman.py

@@ -99,7 +99,7 @@ def invoke_pacman(
                     "--bind", context.cache_dir / "cache/pacman/pkg", context.cache_dir / "cache/pacman/pkg",
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

mkosi/installer/zypper.py

@@ -87,7 +87,7 @@ def invoke_zypper(
                     "--bind", context.cache_dir / "cache/zypp", context.cache_dir / "cache/zypp",
                     *finalize_crypto_mounts(tools=context.config.tools()),
                 ],
-            ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else [])
+            ) + (apivfs_cmd(context.root) if apivfs else [])
         ),
         env=context.config.environment,
     )

mkosi/run.py

@@ -289,6 +289,12 @@ def preexec() -> None:
         if preexec_fn:
             preexec_fn()
 
+    if (
+        sandbox and
+        subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0
+    ):
+        cmdline = ["setpgid", "--foreground", "--"] + cmdline
+
     try:
         # subprocess.run() will use SIGKILL to kill processes when an exception is raised.
         # We'd prefer it to use SIGTERM instead but since this we can't configure which signal
@@ -373,6 +379,13 @@ def preexec() -> None:
         if preexec_fn:
             preexec_fn()
 
+    if (
+        foreground and
+        sandbox and
+        subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0
+    ):
+        cmdline = ["setpgid", "--foreground", "--"] + cmdline
+
     try:
         with subprocess.Popen(
             sandbox + cmdline,

mkosi/sandbox.py

@@ -7,7 +7,6 @@
 from pathlib import Path
 from typing import Optional
 
-from mkosi.run import find_binary
 from mkosi.types import PathString
 from mkosi.util import INVOKING_USER, flatten, one_zero
 
@@ -33,18 +32,12 @@ def finalize_passwd_mounts(root: Path) -> list[PathString]:
     """
     If passwd or a related file exists in the apivfs directory, bind mount it over the host files while we
     run the command, to make sure that the command we run uses user/group information from the apivfs
-    directory instead of from the host. If the file doesn't exist yet, mount over /dev/null instead.
+    directory instead of from the host.
     """
     options: list[PathString] = []
 
     for f in ("passwd", "group", "shadow", "gshadow"):
-        if not (Path("/etc") / f).exists():
-            continue
-        p = root / "etc" / f
-        if p.exists():
-            options += ["--bind", p, f"/etc/{f}"]
-        else:
-            options += ["--bind", "/dev/null", f"/etc/{f}"]
+        options += ["--ro-bind-try", root / "etc" / f, f"/etc/{f}"]
 
     return options
 
@@ -161,14 +154,11 @@ def sandbox_cmd(
 
     cmdline += ["sh", "-c", f"{shm} && exec $0 \"$@\""]
 
-    if setpgid := find_binary("setpgid", root=tools):
-        cmdline += [setpgid, "--foreground", "--"]
-
     return cmdline
 
 
-def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]:
-    cmdline: list[PathString] = [
+def apivfs_cmd(root: Path) -> list[PathString]:
+    return [
         "bwrap",
         "--dev-bind", "/", "/",
         "--tmpfs", root / "run",
@@ -178,38 +168,30 @@ def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]:
         "--dev", root / "dev",
         # APIVFS generally means chrooting is going to happen so unset TMPDIR just to be safe.
         "--unsetenv", "TMPDIR",
-    ]
-
-    if (root / "etc/machine-id").exists():
         # Make sure /etc/machine-id is not overwritten by any package manager post install scripts.
-        cmdline += ["--ro-bind", root / "etc/machine-id", root / "etc/machine-id"]
-
-    cmdline += finalize_passwd_mounts(root)
-
-    if setpgid := find_binary("setpgid", root=tools):
-        cmdline += [setpgid, "--foreground", "--"]
-
-    chmod = f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'}"
-    # Make sure anything running in the root directory thinks it's in a container. $container can't always be
-    # accessed so we write /run/host/container-manager as well which is always accessible.
-    container = f"mkdir {root}/run/host && echo mkosi >{root}/run/host/container-manager"
-
-    cmdline += ["sh", "-c", f"{chmod} && {container} && exec $0 \"$@\""]
-
-    return cmdline
+        "--ro-bind-try", root / "etc/machine-id", root / "etc/machine-id",
+        *finalize_passwd_mounts(root),
+        "sh", "-c",
+        f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'} && "
+        f"chmod 755 {root / 'run'} && "
+        # Make sure anything running in the root directory thinks it's in a container. $container can't always be
+        # accessed so we write /run/host/container-manager as well which is always accessible.
+        f"mkdir -m 755 {root}/run/host && echo mkosi >{root}/run/host/container-manager && "
+        "exec $0 \"$@\"",
+    ]
 
 
-def chroot_cmd(
-    root: Path,
-    *,
-    resolve: bool = False,
-    tools: Path = Path("/"),
-    options: Sequence[PathString] = (),
-) -> list[PathString]:
+def chroot_cmd(root: Path, *, resolve: bool = False, options: Sequence[PathString] = ()) -> list[PathString]:
     cmdline: list[PathString] = [
         "sh", "-c",
+        f"trap 'rm -rf {root / 'work'}' EXIT && "
+        # /etc/resolv.conf can be a dangling symlink to /run/systemd/resolve/stub-resolv.conf. Bubblewrap tries to call
+        # mkdir() on each component of the path which means it will try to call
+        # mkdir(/run/systemd/resolve/stub-resolv.conf) which will fail unless /run/systemd/resolve exists already so
+        # we make sure that it already exists.
+        f"mkdir -p -m 755 {root / 'work'} {root / 'run/systemd'} {root / 'run/systemd/resolve'} && "
         # No exec here because we need to clean up the /work directory afterwards.
-        f"trap 'rm -rf {root / 'work'}' EXIT && mkdir -p {root / 'work'} && chown 777 {root / 'work'} && $0 \"$@\"",
+        f"$0 \"$@\"",
         "bwrap",
         "--dev-bind", root, "/",
         "--setenv", "container", "mkosi",
@@ -218,20 +200,8 @@ def chroot_cmd(
     ]
 
     if resolve:
-        p = Path("etc/resolv.conf")
-        if (root / p).is_symlink():
-            # For each component in the target path, bubblewrap will try to create it if it doesn't exist
-            # yet. If a component in the path is a dangling symlink, bubblewrap will end up calling
-            # mkdir(symlink) which obviously fails if multiple components of the dangling symlink path don't
-            # exist yet. As a workaround, we resolve the symlink ourselves so that bubblewrap will correctly
-            # create all missing components in the target path.
-            p = p.parent / (root / p).readlink()
-
-        cmdline += ["--ro-bind", "/etc/resolv.conf", Path("/") / p]
-
-    cmdline += [*options]
+        cmdline += ["--ro-bind-try", "/etc/resolv.conf", "/etc/resolv.conf"]
 
-    if setpgid := find_binary("setpgid", root=root):
-        cmdline += [setpgid, "--foreground", "--"]
+    cmdline += options
 
-    return apivfs_cmd(root, tools=tools) + cmdline
+    return apivfs_cmd(root) + cmdline