Merge pull request #514 from jcastro/fix/oci-zfs-snapshot-mountpoints

fix(oci): preserve image defaults for ZFS snapshot backups

oci/remote/install_generic_stack.py

@@ -405,6 +405,13 @@ def attach_mounts(service, temporary):
                     run('cp','-a',str(target)+'/.',str(seed))
             else:
                 owner = (100000,100000,0o755)
+            if mount['type'] == 'managed-volume':
+                from oci_image_mountpoints import normalize
+                try:
+                    normalize(root, [mount['container_path']])
+                except (OSError, ValueError) as error:
+                    # Nothing was moved; the image keeps its files as before.
+                    print(f'Image defaults left in place for {mount["container_path"]}: {error}', file=sys.stderr)
         finally:
             run('pct','unmount',vmid)
         if mount['type']=='managed-volume':

oci/remote/install_oci.sh

@@ -521,6 +521,14 @@ capture_volume_seeds() {
     index=$((index + 1))
   done < <(jq -r '.mounts[]? | select(.type == "managed-volume") | .container_path' "$DEPLOYMENT_FILE")
 
+  if (( failed == 0 )); then
+    # Data seeds were captured first; prepare the raw image even when the
+    # transaction will reuse a persistent disk and therefore skipped seeding.
+    # The helper changes nothing when it refuses a path (a link, another
+    # filesystem); the image then installs as it did before.
+    oci_quiet python3 "$SCRIPT_DIR/oci_image_mountpoints.py" "$rootfs" "$DEPLOYMENT_FILE" \
+      || oci_log "Image defaults left in place: a volume mountpoint of the image is not a plain directory"
+  fi
   if (( mounted == 1 )); then
     oci_quiet pct unmount "$VMID" || failed=1
   fi

oci/remote/oci_image_mountpoints.py

@@ -0,0 +1,172 @@
+#!/usr/bin/env python3
+
+# ==========================================================
+# ProxMenux - A menu-driven script for Proxmox VE management
+# ==========================================================
+# Author      : jcastro
+# GitHub      : github.com/jcastro
+# Maintainer  : MacRimi
+# Copyright   : (c) 2026 MacRimi & contributors
+# License     : (GPL-3.0) (https://github.com/MacRimi/ProxMenux/blob/main/LICENSE)
+# Version     : 1.0
+# Last Updated: 22/10/2026
+# ==========================================================
+# Description:
+# Preserve original OCI image files outside managed-volume mountpoints
+# before attaching persistent disks, so ZFS snapshot backups can mount them.
+# Only operate on a stopped container's freshly extracted raw rootfs.
+# ==========================================================
+
+"""Keep raw OCI image defaults out of future managed-volume mountpoints.
+
+Call only on a stopped, freshly extracted rootfs, after capturing volume seeds
+and before attaching persistent disks. Reused disks are never opened here.
+"""
+import argparse
+import hashlib
+import json
+import os
+from pathlib import Path, PurePosixPath
+import re
+import stat
+
+ARCHIVE = '.proxmenux-image-defaults'
+MARKER = '.proxmenux-image-defaults-v1'
+FLAGS = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW
+
+
+def path_parts(value):
+    if (not isinstance(value, str) or not value.startswith('/') or value == '/'
+            or any(c in value for c in '\0\n\r')
+            or any(p in ('.', '..', '') for p in value.split('/')[1:])):
+        raise ValueError(f'Unsafe image mountpoint: {value!r}')
+    parts = PurePosixPath(value).parts[1:]
+    if parts[0] == ARCHIVE:
+        raise ValueError('The image-default archive is a reserved path')
+    return parts
+
+
+def mounted_paths():
+    def unescape(text):
+        return re.sub(r'\\([0-7]{3})', lambda m: chr(int(m[1], 8)), text)
+    return [Path(unescape(line.split()[4]))
+            for line in Path('/proc/self/mountinfo').read_text().splitlines()]
+
+
+def open_directory(root_fd, parts):
+    current = os.dup(root_fd)
+    try:
+        for component in parts:
+            child = os.open(component, FLAGS, dir_fd=current)
+            os.close(current)
+            current = child
+        return current
+    except BaseException:
+        os.close(current)
+        raise
+
+
+def normalize(root, paths, mounts=None):
+    root = Path(root)
+    if root.is_symlink() or not root.is_absolute() or root.resolve() != root:
+        raise ValueError('The raw rootfs must be a real absolute directory')
+    paths = sorted(set(paths), key=lambda p: (-len(path_parts(p)), p))
+    mounts = mounted_paths() if mounts is None else list(map(Path, mounts))
+    root_fd = os.open(root, FLAGS)
+    targets, result = [], []
+    try:
+        root_stat = os.fstat(root_fd)
+        # Validate every path before moving any entry. A mount on an ancestor
+        # or descendant means this is not exclusively the raw image anymore.
+        for value in paths:
+            parts = path_parts(value)
+            target = root.joinpath(*parts)
+            for mount in mounts:
+                if mount != root and mount.is_relative_to(root) and (mount == target or target.is_relative_to(mount)
+                                      or mount.is_relative_to(target)):
+                    raise ValueError(f'An attached filesystem intersects {value}')
+            try:
+                fd = open_directory(root_fd, parts)
+            except FileNotFoundError:
+                continue
+            except OSError as error:
+                raise ValueError(f'Image mountpoint is not a real directory: {value}') from error
+            if os.fstat(fd).st_dev != root_stat.st_dev:
+                os.close(fd)
+                raise ValueError(f'Image mountpoint crosses a filesystem: {value}')
+            targets.append((value, fd))
+        nonempty = [(p, fd) for p, fd in targets if os.listdir(fd)]
+        if not nonempty:
+            return []
+        existing_archive = False
+        try:
+            os.stat(ARCHIVE, dir_fd=root_fd, follow_symlinks=False)
+        except FileNotFoundError:
+            pass
+        else:
+            existing_archive = True
+        if not existing_archive:
+            os.mkdir(ARCHIVE, 0o755, dir_fd=root_fd)
+        archive_fd = os.open(ARCHIVE, FLAGS, dir_fd=root_fd)
+        try:
+            if existing_archive:
+                marker = os.open(MARKER, os.O_RDONLY | os.O_NOFOLLOW, dir_fd=archive_fd)
+                try:
+                    if (not stat.S_ISREG(os.fstat(marker).st_mode)
+                            or os.read(marker, 32) != b'proxmenux-image-defaults-v1\n'):
+                        raise ValueError('The image-default archive already exists')
+                finally:
+                    os.close(marker)
+            else:
+                os.fchown(archive_fd, root_stat.st_uid, root_stat.st_gid)
+                marker = os.open(MARKER, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644, dir_fd=archive_fd)
+                try:
+                    os.fchown(marker, root_stat.st_uid, root_stat.st_gid)
+                    os.write(marker, b'proxmenux-image-defaults-v1\n')
+                finally:
+                    os.close(marker)
+            # Validate all destinations before moving entries; never overwrite
+            # a prior saved directory (including one shipped by the image).
+            for value, _ in nonempty:
+                name = hashlib.sha256(value.encode()).hexdigest()
+                try:
+                    os.stat(name, dir_fd=archive_fd, follow_symlinks=False)
+                except FileNotFoundError:
+                    continue
+                raise ValueError(f'Image-default destination already exists: {value}')
+            for value, fd in nonempty:
+                name = hashlib.sha256(value.encode()).hexdigest()
+                os.mkdir(name, 0o700, dir_fd=archive_fd)
+                saved_fd = os.open(name, FLAGS, dir_fd=archive_fd)
+                try:
+                    owner = os.fstat(fd)
+                    os.fchown(saved_fd, owner.st_uid, owner.st_gid)
+                    os.fchmod(saved_fd, stat.S_IMODE(owner.st_mode))
+                    for child in os.listdir(fd):
+                        os.rename(child, child, src_dir_fd=fd, dst_dir_fd=saved_fd)
+                finally:
+                    os.close(saved_fd)
+                result.append({'target': value, 'saved': f'/{ARCHIVE}/{name}'})
+        finally:
+            os.close(archive_fd)
+        return result
+    finally:
+        for _, fd in targets:
+            os.close(fd)
+        os.close(root_fd)
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('rootfs')
+    parser.add_argument('deployment')
+    args = parser.parse_args()
+    deployment = json.loads(Path(args.deployment).read_text())
+    paths = [m['container_path'] for m in deployment.get('mounts', [])
+             if m.get('type') == 'managed-volume']
+    for item in normalize(Path(args.rootfs), paths):
+        print(f"Image defaults preserved: {item['target']} -> {item['saved']}")
+
+
+if __name__ == '__main__':
+    main()

oci/src/proxmenux_oci/installer.py

@@ -1588,6 +1588,7 @@ def _run_remote_install(
             shutil.copy2(project_root / 'remote' / 'haos_healthcheck.py', temporary / 'haos_healthcheck.py')
             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_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'):
                 shutil.copy2(project_root / 'remote' / helper, temporary / helper)
             if deployment_kind == 'generic-multi-lxc-stack':
@@ -1625,7 +1626,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_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_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'):
                 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_image_mountpoints.py

@@ -0,0 +1,133 @@
+import importlib.util
+import os
+from pathlib import Path
+import tempfile
+import unittest
+
+MODULE = Path(__file__).resolve().parents[1] / 'remote/oci_image_mountpoints.py'
+spec = importlib.util.spec_from_file_location('image_mountpoints', MODULE)
+image = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(image)
+
+
+class ImageMountpointTests(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory()
+        self.addCleanup(self.temp.cleanup)
+        self.root = Path(self.temp.name).resolve()
+        self.target = self.root / 'app/config'
+        self.target.mkdir(parents=True)
+
+    def normalize(self, paths=('/app/config',), mounts=()):
+        return image.normalize(self.root, paths, mounts)
+
+    def test_image_files_preserved_target_empty_and_metadata_unchanged(self):
+        (self.target / 'db').mkdir()
+        (self.target / 'DOCKER').touch()
+        data = self.target / 'defaults.json'
+        data.write_text('{"image-default":true}')
+        data.chmod(0o640)
+        self.target.chmod(0o750)
+        before = self.target.stat()
+        entries = self.normalize()
+        saved = self.root / entries[0]['saved'].lstrip('/')
+        self.assertEqual(list(self.target.iterdir()), [])
+        self.assertEqual((saved / 'defaults.json').read_bytes(), b'{"image-default":true}')
+        self.assertEqual((saved / 'defaults.json').stat().st_mode & 0o777, 0o640)
+        after = self.target.stat()
+        self.assertEqual((after.st_ino, after.st_uid, after.st_gid, after.st_mode),
+                         (before.st_ino, before.st_uid, before.st_gid, before.st_mode))
+        self.assertEqual(saved.stat().st_uid, before.st_uid)
+        self.assertEqual(saved.stat().st_mode & 0o777, 0o750)
+
+    def test_empty_and_missing_targets_do_not_create_archive(self):
+        self.assertEqual(self.normalize(('/app/config', '/absent')), [])
+        self.assertFalse((self.root / image.ARCHIVE).exists())
+
+    def test_link_inside_target_is_preserved_without_following(self):
+        (self.target / 'link').symlink_to('/outside')
+        saved = self.root / self.normalize()[0]['saved'].lstrip('/')
+        self.assertEqual(os.readlink(saved / 'link'), '/outside')
+
+    def test_symlink_target_and_symlink_ancestor_rejected(self):
+        for value in ('/link', '/link/config'):
+            with self.subTest(value=value):
+                link = self.root / 'link'
+                link.symlink_to(self.root / 'app')
+                with self.assertRaises(ValueError):
+                    self.normalize((value,))
+                link.unlink()
+
+    def test_all_paths_validated_before_changes(self):
+        (self.target / 'DOCKER').touch()
+        (self.root / 'unsafe').symlink_to('/tmp')
+        with self.assertRaises(ValueError):
+            self.normalize(('/app/config', '/unsafe'))
+        self.assertTrue((self.target / 'DOCKER').exists())
+
+    def test_attached_mounts_ancestors_and_descendants_rejected(self):
+        for mount in (self.target, self.root / 'app', self.target / 'db'):
+            with self.subTest(mount=mount), self.assertRaises(ValueError):
+                self.normalize(mounts=(mount,))
+
+    def test_other_mounted_volumes_are_not_touched(self):
+        other = self.root / 'persistent'
+        other.mkdir()
+        (other / 'proof').write_text('persistent data')
+        (self.target / 'DOCKER').touch()
+        self.normalize(mounts=(other,))
+        self.assertEqual((other / 'proof').read_text(), 'persistent data')
+
+    def test_host_root_mount_is_not_an_attached_guest_disk(self):
+        (self.target / 'DOCKER').touch()
+        self.assertTrue(self.normalize(mounts=('/', self.root)))
+
+    def test_archive_collision_preserves_everything(self):
+        (self.root / image.ARCHIVE).mkdir()
+        (self.target / 'proof').write_text('image data')
+        with self.assertRaises((ValueError, FileNotFoundError)):
+            self.normalize()
+        self.assertEqual((self.target / 'proof').read_text(), 'image data')
+
+    def test_unsafe_paths_rejected(self):
+        for path in ('/', 'relative', '/app/../data', '/app/./config', '/app//config',
+                     '/app/config/', '/app\nconfig', f'/{image.ARCHIVE}/data'):
+            with self.subTest(path=path), self.assertRaises(ValueError):
+                self.normalize((path,))
+
+    def test_reused_volume_is_not_an_input_and_is_not_modified(self):
+        with tempfile.TemporaryDirectory() as data:
+            proof = Path(data) / 'live-data'
+            proof.write_text('existing service data')
+            (self.target / 'new-image-default').write_text('v2')
+            self.normalize()
+            self.assertEqual(proof.read_text(), 'existing service data')
+
+    def test_multiple_stack_mounts_can_be_prepared_independently(self):
+        (self.target / 'DOCKER').touch()
+        self.normalize()
+        other = self.root / 'other'
+        other.mkdir()
+        (other / 'defaults').write_text('keep')
+        saved = self.root / self.normalize(('/other',))[0]['saved'].lstrip('/')
+        self.assertEqual((saved / 'defaults').read_text(), 'keep')
+        self.assertEqual(list(other.iterdir()), [])
+
+    def test_already_prepared_target_is_idempotent(self):
+        (self.target / 'DOCKER').touch()
+        self.normalize()
+        self.assertEqual(self.normalize(), [])
+
+
+    def test_installers_keep_installing_when_a_mountpoint_is_refused(self):
+        # A refused path changes nothing, so the image installs as it did before.
+        remote = MODULE.parent
+        script = (remote / 'install_oci.sh').read_text()
+        call = script[script.index('oci_image_mountpoints.py'):]
+        self.assertTrue(call.split('\n', 2)[1].strip().startswith('|| oci_log'))
+        stack = (remote / 'install_generic_stack.py').read_text()
+        block = stack[stack.index('normalize(root, [mount'):][:400]
+        self.assertIn('except (OSError, ValueError)', block)
+
+if __name__ == '__main__':
+    unittest.main()