utils/mnt: fix mount permissions

This is a follow up to #1550 where we enabled a `rw` permissions mode,
which is not ideal since it would theoretically be possible to set both
`ro` and `rw` modes at the same time. This commit fixes the issue by only
allowing one option at a time.

Fixes #1588

osbuild/util/containers.py

@@ -4,7 +4,7 @@ import subprocess
 import tempfile
 from contextlib import contextmanager
 
-from osbuild.util.mnt import MountGuard
+from osbuild.util.mnt import MountGuard, MountPermissions
 
 
 def is_manifest_list(data):
@@ -123,7 +123,7 @@ def containers_storage_source(image, image_filepath, container_format):
     with MountGuard() as mg:
         # NOTE: the ostree.deploy.container needs explicit `rw` access to
         # the containers-storage store even when bind mounted.
-        mg.mount(image_filepath, storage_path, rw=True)
+        mg.mount(image_filepath, storage_path, permissions=MountPermissions.READ_WRITE)
 
         image_id = image["checksum"].split(":")[1]
         image_source = f"{container_format}:[{driver}@{storage_path}+/run/containers/storage]{image_id}"

osbuild/util/mnt.py

@@ -2,13 +2,20 @@
 """
 
 import contextlib
+import enum
 import subprocess
+from typing import Optional
+
+
+class MountPermissions(enum.Enum):
+    READ_WRITE = "rw"
+    READ_ONLY = "ro"
 
 
 def mount(source, target, bind=True, ro=True, private=True, mode="0755"):
     options = []
     if ro:
-        options += ["ro"]
+        options += [MountPermissions.READ_ONLY.value]
     if mode:
         options += [mode]
 
@@ -44,14 +51,14 @@ class MountGuard(contextlib.AbstractContextManager):
     def __init__(self):
         self.mounts = []
 
-    def mount(self, source, target, bind=True, ro=False, rw=False, mode="0755"):
+    def mount(self, source, target, bind=True, permissions: Optional[MountPermissions] = None, mode="0755"):
         options = []
         if bind:
             options += ["bind"]
-        if ro:
-            options += ["ro"]
-        if rw:
-            options += ["rw"]
+        if permissions:
+            if permissions not in list(MountPermissions):
+                raise ValueError(f"unknown filesystem permissions: {permissions}")
+            options += [permissions.value]
         if mode:
             options += [mode]
 

stages/org.osbuild.bootupd.gen-metadata

@@ -15,7 +15,7 @@ import subprocess
 import sys
 
 import osbuild.api
-from osbuild.util.mnt import MountGuard
+from osbuild.util.mnt import MountGuard, MountPermissions
 
 SCHEMA_2 = r"""
 "options": {
@@ -29,7 +29,7 @@ def main(tree):
         for source in ("/dev", "/sys", "/proc"):
             target = os.path.join(tree, source.lstrip("/"))
             os.makedirs(target, exist_ok=True)
-            mounter.mount(source, target, ro=True)
+            mounter.mount(source, target, permissions=MountPermissions.READ_ONLY)
 
         # Using a non-default sysroot is not supported by the generate-update-metadata command, so we need to chroot
         # into the tree and run against /

test/mod/test_util_mnt.py

@@ -21,6 +21,14 @@ def test_mount_guard_failure_msg(tmp_path):
     assert "special device /dev/invalid-src does not exist" in str(e.value)
 
 
+@pytest.mark.skipif(os.getuid() != 0, reason="root only")
+def test_mount_guard_incorrect_permissions_msg(tmp_path):
+    with pytest.raises(ValueError) as e:
+        with MountGuard() as mg:
+            mg.mount("/dev/invalid-src", tmp_path, permissions="abc")
+    assert "unknown filesystem permissions" in str(e.value)
+
+
 # This needs a proper refactor so that FileSystemMountService just uses
 # a common mount helper.
 class FakeFileSystemMountService(FileSystemMountService):