fix(oci): stage private images for the mapped Proxmox reader

.github/workflows/test-features.yml

@@ -387,3 +387,13 @@ jobs:
         run: |
           npm ci --prefix .github/ci/offline-node --legacy-peer-deps --ignore-scripts --no-audit --no-fund
           NODE_PATH="$PWD/.github/ci/offline-node/node_modules" node AppImage/tests/pbs-navigation.cjs
+
+  native-image-import:
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+      - name: Exercise private images with isolated mapped Linux readers
+        run: sudo /usr/bin/python3 -B -m unittest discover -s oci/tests -p test_native_import_access.py -v

oci/README.md

@@ -111,6 +111,8 @@ Selecting an application, or giving a definition, opens the same sequence.
 4. **Resolve.** The registry is queried, the architecture of the host is selected and the digest the tag points to at that moment is fixed.
 5. **Download and verify.** Skopeo downloads the image as an OCI archive. Every layer is checked against its digest before anything is created.
 6. **Build.** `pct create` builds the rootfs from the archive and imports `Entrypoint`, `Cmd`, `Env`, `User`, `WorkingDir` and `StopSignal` from the image.
+
+   Proxmox reads the image inside the container's user namespace. Readable template-storage archives use the existing path without a copy. If a verified archive or its parents are private, the native create wrapper makes a temporary root-owned copy under `/var/tmp`, readable only by the mapped root group, and removes it after creation, failure or a catchable interruption. The cached image, storage directories, registry credentials and caller permissions stay unchanged. Reflinks are used where available; otherwise this uncommon path needs temporary space for one image copy. Only the `pct create` child receives the normal Proxmox creation umask.
 7. **Connect.** The volumes are added as mount points, then the devices, the environment, the network and the security settings the oci-compose declares.
 8. **Prepare.** Each new volume is made ready for the first start: `lost+found` is removed and the directory is given to the user the application writes with. The files the oci-compose asks to write are written.
 9. **Start and check.** The container starts, an address is waited for, the settings that need the running application are applied, and the service is checked. A failed first start is reported as a failed installation.

oci/remote/oci_native_import.py

@@ -0,0 +1,136 @@
+#!/usr/bin/env python3
+"""Give Proxmox's mapped OCI reader access without publishing a private cache.
+
+Ordinary readable templates use the existing zero-copy path. An inaccessible
+archive gets a short-lived copy (reflink where supported), owned by root and
+readable only by the mapped root group. Never chmod/chown the source image,
+its parents, registry credentials, or the caller's process umask.
+"""
+from contextlib import contextmanager
+import errno
+import ctypes
+import fcntl
+import os
+from pathlib import Path
+import shutil
+import signal
+import stat
+import subprocess
+import sys
+import tempfile
+
+STAGING_ROOT = Path('/var/tmp')  # Deliberately independent of a private TMPDIR.
+FICLONE = 0x40049409
+
+
+def reader_ids(options):
+    """Fresh pct creates use PVE's standard idmap, not a restored custom map."""
+    unprivileged = False
+    for index, option in enumerate(options):
+        if option == '--unprivileged':
+            unprivileged = index + 1 == len(options) or options[index + 1] not in ('0', 'false')
+        elif option.startswith('--unprivileged='):
+            unprivileged = option.partition('=')[2] not in ('0', 'false')
+    return (100000, 100000) if unprivileged else (0, 0)
+
+
+def _allowed(path, uid, gid, required):
+    info = path.lstat()
+    if stat.S_ISLNK(info.st_mode):
+        return False
+    shift = 6 if info.st_uid == uid else 3 if info.st_gid == gid else 0
+    return bool((info.st_mode >> shift) & required)
+
+
+def readable(path, uid, gid):
+    """Conservative DAC check: an ACL or symlink may make staging unnecessary,
+    but never justify publishing the cache or broadening a storage directory."""
+    path = Path(os.path.abspath(path))
+    if uid == 0:
+        return path.is_file()
+    return (_allowed(path, uid, gid, 4)
+            and all(_allowed(parent, uid, gid, 1) for parent in path.parents))
+
+
+def _copy(source, target):
+    source_fd = os.open(source, os.O_RDONLY | os.O_NOFOLLOW)
+    try:
+        if not stat.S_ISREG(os.fstat(source_fd).st_mode):
+            raise OSError(errno.EINVAL, os.strerror(errno.EINVAL), str(source))
+        with os.fdopen(source_fd, 'rb', closefd=False) as incoming, target.open('xb') as outgoing:
+            try:
+                fcntl.ioctl(outgoing.fileno(), FICLONE, incoming.fileno())
+            except OSError as error:
+                if error.errno not in (errno.EXDEV, errno.EOPNOTSUPP, errno.ENOTTY, errno.EINVAL, errno.ENOSYS):
+                    raise
+                shutil.copyfileobj(incoming, outgoing, length=1024 * 1024)
+    finally:
+        os.close(source_fd)
+
+
+def trusted_staging_root(uid, gid):
+    base = STAGING_ROOT
+    info = base.lstat()
+    ancestors = (base, *base.parents)
+    if (not stat.S_ISDIR(info.st_mode) or info.st_uid != 0
+            or any(p.lstat().st_uid != 0 or
+                   (p.lstat().st_mode & 0o022 and not p.lstat().st_mode & stat.S_ISVTX)
+                   for p in ancestors)
+            or not all(_allowed(p, uid, gid, 1) for p in ancestors)):
+        raise PermissionError(errno.EACCES, os.strerror(errno.EACCES), str(base))
+    return base
+
+
+@contextmanager
+def import_archive(source, original, uid, gid):
+    source = Path(source)
+    if readable(source, uid, gid):
+        yield original
+        return
+    base = trusted_staging_root(uid, gid)
+    # mkdtemp is 0700 even if the caller's umask is permissive. Publish group
+    # traversal only after the root-owned, read-only copy is complete.
+    directory = Path(tempfile.mkdtemp(prefix='proxmenux-oci-import-', dir=base))
+    try:
+        target = directory / 'image.tar'
+        _copy(source.resolve(strict=True), target)
+        os.chown(target, 0, gid)
+        target.chmod(0o440)
+        os.chown(directory, 0, gid)
+        directory.chmod(0o750)
+        yield str(target)
+    finally:
+        shutil.rmtree(directory)
+
+
+def create(vmid, original, source, options):
+    uid, gid = reader_ids(options)
+    with import_archive(source, original, uid, gid) as archive:
+        result = subprocess.run(['pct', 'create', vmid, archive, *options],
+                                close_fds=True, umask=0o022)
+        return 128 - result.returncode if result.returncode < 0 else result.returncode
+
+
+def main():
+    # A catchable interruption must unwind the staging context. subprocess.run
+    # terminates its child when the handler raises; no background pct is orphaned.
+    for caught in (signal.SIGTERM, signal.SIGHUP, signal.SIGQUIT):
+        signal.signal(caught, lambda signum, frame: sys.exit(128 + signum))
+    if sys.platform == 'linux':
+        parent = os.getppid()
+        libc = ctypes.CDLL(None, use_errno=True)
+        if libc.prctl(1, signal.SIGTERM, 0, 0, 0) != 0:  # PR_SET_PDEATHSIG
+            raise OSError(ctypes.get_errno(), os.strerror(ctypes.get_errno()))
+        if os.getppid() != parent:
+            return 143
+    try:
+        return create(sys.argv[1], sys.argv[2], sys.argv[3], sys.argv[4:])
+    except KeyboardInterrupt:
+        return 130
+    except OSError as error:
+        print(error, file=sys.stderr)
+        return 1
+
+
+if __name__ == '__main__':
+    raise SystemExit(main())

oci/remote/oci_ui.sh

@@ -127,7 +127,7 @@ oci_create_container() {
     total_mib=$(python3 "$VERIFY_OCI_ARCHIVE" --extracted-mib "$path" 2>/dev/null || printf '0')
     [[ $total_mib =~ ^[0-9]+$ ]] || total_mib=0
   fi
-  pct create "$vmid" "$archive" "$@" >>"$log" 2>&1 &
+  python3 "${SCRIPT_DIR}/oci_native_import.py" "$vmid" "$archive" "$path" "$@" >>"$log" 2>&1 &
   pid=$!
   while kill -0 "$pid" 2>/dev/null; do
     if mountpoint -q "$rootfs" 2>/dev/null; then

oci/src/proxmenux_oci/installer.py

@@ -1611,7 +1611,7 @@ def _run_remote_install(
             shutil.copy2(project_root / 'remote' / 'oci_installation_state.py', temporary / 'oci_installation_state.py')
             shutil.copy2(project_root / 'remote' / 'oci_instances.py', temporary / 'oci_instances.py')
             shutil.copy2(project_root / 'remote' / 'oci_image_mountpoints.py', temporary / 'oci_image_mountpoints.py')
-            for helper in ('oci_carried_record.py', 'oci_compose_jobs.py', 'oci_stack_public_routes.py', 'oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
+            for helper in ('oci_carried_record.py', 'oci_compose_jobs.py', 'oci_stack_public_routes.py', 'oci_ui.sh', 'oci_ui.py', 'oci_native_import.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
                 shutil.copy2(project_root / 'remote' / helper, temporary / helper)
             if deployment_kind == 'generic-multi-lxc-stack':
                 shutil.copy2(project_root / 'remote' / 'install_generic_stack.py', temporary / 'install_generic_stack.py')
@@ -1648,7 +1648,7 @@ def _run_remote_install(
             archive.add(project_root / 'remote' / 'haos_healthcheck.py', arcname='haos_healthcheck.py')
             archive.add(project_root / 'remote' / 'oci_installation_state.py', arcname='oci_installation_state.py')
             archive.add(project_root / 'remote' / 'oci_instances.py', arcname='oci_instances.py')
-            for helper in ('oci_carried_record.py', 'oci_compose_jobs.py', 'oci_stack_public_routes.py', 'oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_image_mountpoints.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
+            for helper in ('oci_carried_record.py', 'oci_compose_jobs.py', 'oci_stack_public_routes.py', 'oci_ui.sh', 'oci_ui.py', 'oci_native_import.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_image_mountpoints.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
                 archive.add(project_root / 'remote' / helper, arcname=helper)
             if deployment_kind == 'generic-multi-lxc-stack':
                 archive.add(project_root / 'remote' / 'install_generic_stack.py', arcname='install_generic_stack.py')

oci/tests/test_native_import_access.py

@@ -0,0 +1,181 @@
+"""Native mapped readers may read an image without exposing its private cache."""
+import errno
+import importlib.util
+import os
+from pathlib import Path
+import signal
+import stat
+import subprocess
+import sys
+import tempfile
+import time
+import types
+import unittest
+from unittest.mock import patch
+
+ROOT = Path(__file__).resolve().parents[1]
+spec = importlib.util.spec_from_file_location('native_import_access', ROOT / 'remote/oci_native_import.py')
+imports = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(imports)
+
+
+class PrivateImageImportTests(unittest.TestCase):
+    def setUp(self):
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.root = Path(self.directory.name)
+        self.source = self.root / 'private-image.tar'
+        self.source.write_bytes(b'OCI image fixture\x00private layers')
+        self.source.chmod(0o600)
+
+    def staging(self):
+        return patch.object(imports, 'trusted_staging_root', return_value=self.root)
+
+    def test_normal_readable_image_uses_original_storage_volume_without_copy(self):
+        with patch.object(imports, 'readable', return_value=True), \
+             patch.object(imports, '_copy', side_effect=AssertionError('must stay zero-copy')):
+            with imports.import_archive(self.source, 'local:vztmpl/public.tar', 100000, 100000) as path:
+                self.assertEqual(path, 'local:vztmpl/public.tar')
+
+    def test_private_copy_preserves_source_and_limits_mapped_reader_group(self):
+        before = self.source.stat()
+        with self.staging(), patch.object(imports.os, 'chown') as chown, \
+             patch.object(imports, 'readable', return_value=False):
+            with imports.import_archive(self.source, str(self.source), 100000, 100000) as path:
+                staged = Path(path)
+                self.assertEqual(staged.read_bytes(), self.source.read_bytes())
+                self.assertEqual(stat.S_IMODE(staged.stat().st_mode), 0o440)
+                self.assertEqual(stat.S_IMODE(staged.parent.stat().st_mode), 0o750)
+                chown.assert_any_call(staged, 0, 100000)
+                chown.assert_any_call(staged.parent, 0, 100000)
+            self.assertFalse(staged.parent.exists())
+        after = self.source.stat()
+        self.assertEqual((before.st_uid, before.st_gid, before.st_mode),
+                         (after.st_uid, after.st_gid, after.st_mode))
+
+    def test_copy_error_cleans_stage_and_never_invokes_pct(self):
+        with self.staging(), patch.object(imports, 'readable', return_value=False), \
+             patch.object(imports, '_copy', side_effect=OSError(errno.ENOSPC, 'fixture')), \
+             patch.object(imports.subprocess, 'run') as run:
+            with self.assertRaises(OSError):
+                imports.create('9161', str(self.source), str(self.source), ['--unprivileged', '1'])
+            run.assert_not_called()
+        self.assertEqual(list(self.root.iterdir()), [self.source])
+
+    def test_pct_failure_and_interrupt_both_clean_stage(self):
+        for error in (None, KeyboardInterrupt()):
+            with self.staging(), patch.object(imports.os, 'chown'), \
+                 patch.object(imports, 'readable', return_value=False), \
+                 patch.object(imports.subprocess, 'run', side_effect=error,
+                              return_value=types.SimpleNamespace(returncode=23)) as run:
+                if error:
+                    with self.assertRaises(KeyboardInterrupt):
+                        imports.create('9161', str(self.source), str(self.source), ['--unprivileged', '1'])
+                else:
+                    self.assertEqual(imports.create('9161', str(self.source), str(self.source),
+                                                    ['--unprivileged', '1']), 23)
+                self.assertEqual(run.call_args.kwargs['umask'], 0o022)
+            self.assertEqual(list(self.root.iterdir()), [self.source])
+
+    def test_standard_map_and_privileged_profile_initial_unprivileged_create(self):
+        for options in (['--unprivileged', '1'], ['--unprivileged=1'], ['--unprivileged', 'true']):
+            self.assertEqual(imports.reader_ids(options), (100000, 100000))
+        self.assertEqual(imports.reader_ids(['--unprivileged', '0']), (0, 0))
+        installer = (ROOT / 'remote/install_oci.sh').read_text()
+        self.assertIn('CREATE_UNPRIVILEGED_FLAG=1', installer)
+        self.assertIn('--unprivileged "$CREATE_UNPRIVILEGED_FLAG"', installer)
+
+    def test_derived_numeric_user_image_is_staged_without_mutating_its_source(self):
+        derived = self.root / 'image.user-databases.123.tar'
+        derived.write_bytes(b'derived OCI image')
+        derived.chmod(0o600)
+        with self.staging(), patch.object(imports.os, 'chown'), \
+             patch.object(imports, 'readable', return_value=False):
+            with imports.import_archive(derived, str(derived), 100000, 100000) as path:
+                self.assertEqual(Path(path).read_bytes(), derived.read_bytes())
+        self.assertEqual(stat.S_IMODE(derived.stat().st_mode), 0o600)
+
+    def test_reflink_unavailable_falls_back_to_an_identical_copy(self):
+        target = self.root / 'copy.tar'
+        with patch.object(imports.fcntl, 'ioctl', side_effect=OSError(errno.EOPNOTSUPP, 'fixture')):
+            imports._copy(self.source, target)
+        self.assertEqual(target.read_bytes(), self.source.read_bytes())
+
+    def test_local_and_ssh_installer_stage_the_native_helper(self):
+        import ast
+        tree = ast.parse((ROOT / 'src/proxmenux_oci/installer.py').read_text())
+        function = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == '_run_remote_install')
+        lists = [n.iter for n in ast.walk(function) if isinstance(n, ast.For)
+                 and isinstance(n.iter, ast.Tuple)
+                 and any(isinstance(v, ast.Constant) and v.value == 'oci_ui.sh' for v in n.iter.elts)]
+        self.assertEqual(len(lists), 2)
+        for values in lists:
+            self.assertIn('oci_native_import.py', [v.value for v in values.elts])
+        self.assertIn('oci_native_import.py', (ROOT / 'remote/oci_ui.sh').read_text())
+
+
+@unittest.skipUnless(sys.platform == 'linux' and os.geteuid() == 0,
+                     'isolated mapped UID/GID proof runs as root in Linux CI')
+class MappedReaderProof(unittest.TestCase):
+    def reader(self, path, uid, gid):
+        return subprocess.run([sys.executable, '-c',
+            'import os,sys; os.setgroups([]); os.setgid(int(sys.argv[2])); '
+            'os.setuid(int(sys.argv[1])); print(open(sys.argv[3],"rb").read().decode())',
+            str(uid), str(gid), str(path)], capture_output=True, text=True)
+
+    def test_mapped_reader_only_private_source_and_caller_state_are_preserved(self):
+        with tempfile.TemporaryDirectory(dir='/var/tmp') as folder:
+            root = Path(folder)
+            root.chmod(0o700)
+            source = root / 'private.tar'
+            source.write_text('private image contents')
+            source.chmod(0o600)
+            self.assertNotEqual(self.reader(source, 100000, 100000).returncode, 0)
+            previous = os.umask(0o077)
+            try:
+                with imports.import_archive(source, str(source), 100000, 100000) as staged:
+                    path = Path(staged)
+                    self.assertEqual(self.reader(path, 100000, 100000).stdout.strip(), 'private image contents')
+                    self.assertNotEqual(self.reader(path, 100001, 100001).returncode, 0)
+                    self.assertFalse(path.stat().st_mode & 0o022)
+                    self.assertEqual(path.stat().st_uid, 0)
+                self.assertFalse(path.parent.exists())
+                record = root / 'record.json'
+                record.write_text('private credentials fixture')
+                self.assertEqual(stat.S_IMODE(record.stat().st_mode), 0o600)
+                self.assertEqual(os.umask(0o077), 0o077)
+                self.assertEqual(stat.S_IMODE(source.stat().st_mode), 0o600)
+                self.assertEqual(stat.S_IMODE(root.stat().st_mode), 0o700)
+            finally:
+                os.umask(previous)
+
+    def test_sigterm_removes_staging_and_preserves_native_output_and_status(self):
+        with tempfile.TemporaryDirectory(dir='/var/tmp') as folder:
+            root = Path(folder)
+            source = root / 'private.tar'; source.write_text('private image'); source.chmod(0o600)
+            pct = root / 'pct'
+            pct.write_text('#!/bin/sh\nmkdir "$FIXTURE/pve-tmp"\ntouch "$FIXTURE/pve-tmp/pct.conf"\nprintf "%s\\n" "$3" >"$MARKER"\necho native-output\nexec sleep 60\n')
+            pct.chmod(0o700)
+            marker = root / 'marker'
+            env = dict(os.environ, PATH=str(root) + ':' + os.environ['PATH'], MARKER=str(marker), FIXTURE=str(root))
+            process = subprocess.Popen([sys.executable, str(ROOT / 'remote/oci_native_import.py'),
+                '9161', str(source), str(source), '--unprivileged', '1'], env=env,
+                stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, umask=0o077)
+            try:
+                for _ in range(100):
+                    if marker.exists(): break
+                    time.sleep(0.05)
+                self.assertTrue(marker.exists())
+                staged = Path(marker.read_text().strip())
+                self.assertTrue(staged.exists())
+                self.assertEqual(stat.S_IMODE((root / "pve-tmp").stat().st_mode), 0o755)
+                self.assertEqual(stat.S_IMODE((root / "pve-tmp/pct.conf").stat().st_mode), 0o644)
+                process.send_signal(signal.SIGTERM)
+                # Do not leave a shell sleep grandchild holding capture pipes open.
+                output, error = process.communicate(timeout=5)
+                self.assertIn("native-output", output)
+                self.assertEqual(process.returncode, 143)
+                self.assertFalse(staged.parent.exists())
+            finally:
+                if process.poll() is None: process.kill(); process.wait()
+                process.stdout.close(); process.stderr.close()