From adfd77a98319f3d803c039329a9fa12cb73eaeef Mon Sep 17 00:00:00 2001 From: menzelj Date: Sun, 16 Aug 2026 18:53:25 +0000 Subject: [PATCH] Fix NFS uploads against root_squash exports (0.40.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploading a backup extracted the tar straight into the NFS mount via put_archive, and the daemon chowns every entry while extracting — an export with root_squash refuses that ("failed to Lchown ... for UID 0, GID 0: operation not permitted"), so the upload died with a docker 500 even though plain writes to the share work (which is why the destination test passed). The helper container now unpacks into its own filesystem and copies the file into the mount with cat, which never chowns. Restores hit the same wall when a volume or bind folder lives on a squashed mount, so import_path/import_volume fall back to a copy-through-staging when (and only when) the failure is a chown denial — local restores keep preserving ownership. NFS file names are validated against the same safe charset as the subdir parts, since both are interpolated into the helper's shell commands. Verified against a real root_squash NFS export: test/upload/list/download/delete round trip, byte-identical download, restore into an NFS-backed volume via the fallback, and ownership still preserved (1000:1000, 0600) on a local volume. Co-Authored-By: Claude Opus 5 --- .../services/backup_destination_service.py | 24 +++++-- backend/services/stack_assets_service.py | 62 ++++++++++++++++--- backend/version.py | 2 +- frontend/package.json | 2 +- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/backend/services/backup_destination_service.py b/backend/services/backup_destination_service.py index c375594..8bcaff4 100644 --- a/backend/services/backup_destination_service.py +++ b/backend/services/backup_destination_service.py @@ -221,7 +221,11 @@ _NFS_VOLUME_PREFIX = "stackpilot-nfs-dest-" def _nfs_check_name(name: str) -> None: - if not name or "/" in name or name in (".", "..") or name.startswith(".."): + import re + + # Same safe charset as the subdir parts: the name is interpolated into the + # helper container's shell commands. + if not name or not re.fullmatch(r"[A-Za-z0-9._-]+", name) or name.startswith("."): raise DestinationError(f"Invalid backup file name '{name}'") @@ -308,7 +312,7 @@ def _nfs_run(volume: str, command: list[str]) -> str: raise DestinationError(f"NFS mount failed: {exc}") from exc -def _nfs_helper(volume: str): +def _nfs_helper(volume: str, command: list[str] | str = "true"): """A created (not started) helper container for archive I/O on /nfs.""" import docker.errors @@ -322,7 +326,7 @@ def _nfs_helper(volume: str): return safe_call( client.containers.create, settings.BACKUP_HELPER_IMAGE, - command="true", + command=command, volumes={volume: {"bind": "/nfs", "mode": "rw"}}, ) except (docker.errors.APIError, DockerError) as exc: @@ -337,13 +341,23 @@ def _nfs_upload(dest: BackupDestination, cfg: dict, local_path: str, filename: s target = _nfs_target(cfg) # Creates the subdir if needed AND fails early with a clear mount error. _nfs_run(volume, ["mkdir", "-p", target]) - container = _nfs_helper(volume) + # Unpack into the container's own filesystem, then copy the file across: + # extracting straight into the NFS mount makes the daemon chown the file, + # which a root_squash export refuses ("failed to Lchown ... for UID 0"). + container = _nfs_helper( + volume, ["sh", "-c", f"cat '/tmp/{filename}' > '{target}/{filename}'"] + ) try: with tempfile.TemporaryFile() as tmp: with tarfile.open(fileobj=tmp, mode="w") as tar: tar.add(local_path, arcname=filename) tmp.seek(0) - container.put_archive(target, tmp) + container.put_archive("/tmp", tmp) + container.start() + status = container.wait(timeout=3600).get("StatusCode", 1) + if status != 0: + err = (container.logs(stdout=True, stderr=True) or b"").decode("utf-8", "replace") + raise DestinationError(f"NFS upload failed: {err.strip() or f'exit {status}'}") except docker.errors.APIError as exc: raise DestinationError(f"NFS upload failed: {exc}") from exc finally: diff --git a/backend/services/stack_assets_service.py b/backend/services/stack_assets_service.py index 8362a31..ced1216 100644 --- a/backend/services/stack_assets_service.py +++ b/backend/services/stack_assets_service.py @@ -94,6 +94,54 @@ def _split(path: str) -> tuple[str, str]: return os.path.dirname(clean) or "/", os.path.basename(clean) +def _is_chown_error(exc: Exception) -> bool: + text = str(exc) + return "chown" in text.lower() and "not permitted" in text.lower() + + +def _put_archive(container, dest: str, src_file: str, volumes: dict) -> None: + """Unpack an archive into a container path, coping with squashed mounts. + + The daemon restores ownership while extracting, which an NFS/CIFS export + with ``root_squash`` refuses. In that case the archive is unpacked into a + throwaway container's own filesystem first and the files are then copied + across — ownership cannot be preserved there, but the restore completes + instead of failing outright. + """ + with open(src_file, "rb") as fh: + try: + container.put_archive(dest, fh) + return + except Exception as exc: # noqa: BLE001 + if not _is_chown_error(exc): + raise + logger.warning("%s rejects ownership changes; restoring without it", dest) + client = get_client() + staging = _create_helper_with( + client, volumes, ["sh", "-c", f"cp -R /tmp/. '{dest}/'"] + ) + try: + with open(src_file, "rb") as fh: + staging.put_archive("/tmp", fh) + staging.start() + status = staging.wait(timeout=3600).get("StatusCode", 1) + if status != 0: + err = (staging.logs(stdout=True, stderr=True) or b"").decode("utf-8", "replace") + raise AssetError(err.strip() or f"copy failed (exit {status})") + finally: + _remove(staging) + + +def _create_helper_with(client, volumes: dict, command: list[str]): + ensure_helper_image(client) + return safe_call( + client.containers.create, + settings.BACKUP_HELPER_IMAGE, + command=command, + volumes=volumes, + ) + + def inspect_paths(paths: list[str]) -> dict[str, dict]: """Classify host paths ({path: {"kind", "size"}}) via one helper container. @@ -179,12 +227,12 @@ def import_path(source: str, kind: str, src_file: str) -> None: ensure_helper_image(client) if kind == "file": parent, _base = _split(source) - container = _create_helper(client, {parent: {"bind": "/dst", "mode": "rw"}}) + mounts = {parent: {"bind": "/dst", "mode": "rw"}} else: - container = _create_helper(client, {source: {"bind": "/dst", "mode": "rw"}}) + mounts = {source: {"bind": "/dst", "mode": "rw"}} + container = _create_helper(client, mounts) try: - with open(src_file, "rb") as fh: - container.put_archive("/dst", fh) + _put_archive(container, "/dst", src_file, mounts) finally: _remove(container) @@ -225,10 +273,10 @@ def import_volume(full_name: str, labels: dict, src_file: str, wipe: bool = True volumes={full_name: {"bind": "/v", "mode": "rw"}}, remove=True, ) - container = _create_helper(client, {full_name: {"bind": "/v", "mode": "rw"}}) + mounts = {full_name: {"bind": "/v", "mode": "rw"}} + container = _create_helper(client, mounts) try: - with open(src_file, "rb") as fh: - container.put_archive("/v", fh) + _put_archive(container, "/v", src_file, mounts) finally: _remove(container) diff --git a/backend/version.py b/backend/version.py index 0cfe496..c57694b 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.40.1" +APP_VERSION = "0.40.2" diff --git a/frontend/package.json b/frontend/package.json index 00078ec..a8a22c2 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.40.1", + "version": "0.40.2", "type": "module", "scripts": { "dev": "vite",