From 00ebb0b2f9a6cd69cbb1a6c591d9c45bdfc136a4 Mon Sep 17 00:00:00 2001 From: jcastro <190036+jcastro@users.noreply.github.com> Date: Thu, 22 Oct 2026 10:27:00 +0200 Subject: [PATCH] fix(oci): preserve image defaults before attaching managed volumes --- oci/remote/install_generic_stack.py | 3 + oci/remote/install_oci.sh | 5 + oci/remote/oci_image_mountpoints.py | 172 ++++++++++++++++++++++++++++ oci/src/proxmenux_oci/installer.py | 3 +- oci/tests/test_image_mountpoints.py | 123 ++++++++++++++++++++ 5 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 oci/remote/oci_image_mountpoints.py create mode 100644 oci/tests/test_image_mountpoints.py diff --git a/oci/remote/install_generic_stack.py b/oci/remote/install_generic_stack.py index 2eb88b7e..102ca7f5 100644 --- a/oci/remote/install_generic_stack.py +++ b/oci/remote/install_generic_stack.py @@ -405,6 +405,9 @@ 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 + normalize(root, [mount['container_path']]) finally: run('pct','unmount',vmid) if mount['type']=='managed-volume': diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index 0251776e..f0981c00 100755 --- a/oci/remote/install_oci.sh +++ b/oci/remote/install_oci.sh @@ -521,6 +521,11 @@ 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. + oci_quiet python3 "$SCRIPT_DIR/oci_image_mountpoints.py" "$rootfs" "$DEPLOYMENT_FILE" || failed=1 + fi if (( mounted == 1 )); then oci_quiet pct unmount "$VMID" || failed=1 fi diff --git a/oci/remote/oci_image_mountpoints.py b/oci/remote/oci_image_mountpoints.py new file mode 100644 index 00000000..bd7acfa7 --- /dev/null +++ b/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() diff --git a/oci/src/proxmenux_oci/installer.py b/oci/src/proxmenux_oci/installer.py index ce067425..c0b42bfa 100644 --- a/oci/src/proxmenux_oci/installer.py +++ b/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') diff --git a/oci/tests/test_image_mountpoints.py b/oci/tests/test_image_mountpoints.py new file mode 100644 index 00000000..3273aab0 --- /dev/null +++ b/oci/tests/test_image_mountpoints.py @@ -0,0 +1,123 @@ +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(), []) + + +if __name__ == '__main__': + unittest.main()