From f32d38fe6aed676569f423c90533fcbecdb49797 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:09:33 +0000 Subject: [PATCH 1/3] feat(sdm): Add ensure_directory for privileged mkdir --- .../tests/cli/test_sudo_pass_cmd.py | 29 +++++++++++++++++++ .../src/storage_device_managers/__init__.py | 27 +++++++++++++++-- .../tests/test_mount_unmount.py | 29 +++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py b/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py index 8602007f..ec2ca0ab 100644 --- a/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py +++ b/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py @@ -57,6 +57,35 @@ def test_sudo_pass_cmd_is_used_in_open( ) +def test_open_uses_sudo_to_create_mount_dir( + runner: CliRunner, + encrypted_btrfs_device: cp.DeviceConfiguration, + mocker, + tmp_path: Path, +) -> None: + dest_dir = tmp_path / "mounts" + wrapped_config = cp.Configuration( + DeviceConfigurations=[encrypted_btrfs_device], + OpenDirectory=dest_dir, + ) + config_file = tmp_path / "config.json" + config_file.write_text(wrapped_config.model_dump_json()) + + ensure_directory = mocker.patch( + "storage_device_managers.ensure_directory", return_value=True + ) + mocker.patch( + "storage_device_managers.open_encrypted_device", + return_value=Path("/dev/mapper/test"), + ) + mocker.patch("storage_device_managers.mount_device") + + result = runner.invoke(app, ["open", "--config", str(config_file)]) + + assert result.exit_code == 0 + ensure_directory.assert_called_once_with(dest_dir / encrypted_btrfs_device.Name) + + def test_sudo_pass_cmd_is_used_in_backup( runner: CliRunner, encrypted_device: cp.DeviceConfiguration, diff --git a/projects/storage-device-managers/src/storage_device_managers/__init__.py b/projects/storage-device-managers/src/storage_device_managers/__init__.py index bbbb9125..717fac71 100644 --- a/projects/storage-device-managers/src/storage_device_managers/__init__.py +++ b/projects/storage-device-managers/src/storage_device_managers/__init__.py @@ -35,6 +35,7 @@ "close_decrypted_device", "decrypted_device", "encrypt_device", + "ensure_directory", "generate_passcmd", "get_filesystem", "get_mounted_devices", @@ -165,6 +166,27 @@ def decrypted_device(device: Path, pass_cmd: str) -> Iterator[Path]: ) +def ensure_directory(directory: Path) -> bool: + """Ensure a directory exists, creating it with root privileges if needed. + + Parameters: + ----------- + directory + directory that should exist + + Returns: + -------- + bool + ``True`` if the directory had to be created, ``False`` if it already + existed + """ + if directory.is_dir(): + return False + cmd: sh.StrPathList = ["sudo", "mkdir", "-p", directory] + sh.run_cmd(cmd=cmd) + return True + + @contextlib.contextmanager def mounted_device( device: Path, @@ -182,7 +204,8 @@ def mounted_device( the directory is left intact after the context exits. If `destination` is ``None`` (the default), a temporary directory is created for the mount and removed again upon exit. If `destination` is given but does not exist, - it is created before mounting **but not removed again** after unmounting. + it is created with root privileges before mounting **but not removed + again** after unmounting. If `compression` is provided, a mount option specifying the transparent file system compression is set. Compression is only supported for BtrFS @@ -207,7 +230,7 @@ def mounted_device( if is_mounted(device): unmount_device(device) if destination is not None: - destination.mkdir(parents=True, exist_ok=True) + ensure_directory(destination) ctx: contextlib.AbstractContextManager[Path] = ( _temporary_directory() if destination is None diff --git a/projects/storage-device-managers/tests/test_mount_unmount.py b/projects/storage-device-managers/tests/test_mount_unmount.py index 096e5ee3..ab50b25b 100644 --- a/projects/storage-device-managers/tests/test_mount_unmount.py +++ b/projects/storage-device-managers/tests/test_mount_unmount.py @@ -50,6 +50,21 @@ class MyCustomTestException(Exception): pass +def test_ensure_directory_uses_sudo_for_missing_directory( + mocker, tmp_path: Path +) -> None: + destination = tmp_path / "nested" / "mount" + run_cmd = mocker.patch("storage_device_managers.sh.run_cmd") + assert sdm.ensure_directory(destination) + run_cmd.assert_called_once_with(cmd=["sudo", "mkdir", "-p", destination]) + + +def test_ensure_directory_skips_existing_directory(mocker, tmp_path: Path) -> None: + run_cmd = mocker.patch("storage_device_managers.sh.run_cmd") + assert not sdm.ensure_directory(tmp_path) + run_cmd.assert_not_called() + + def test_mount_device( device_with_fs, tmp_path: Path, compression_kwargs: CompressionKwargsT ) -> None: @@ -180,3 +195,17 @@ def test_mounted_device_with_destination( assert str(device) not in sdm.get_mounted_devices() # A given destination should not be deleted after unmounting. assert destination.exists() + + +def test_mounted_device_uses_sudo_to_create_destination(mocker, tmp_path: Path) -> None: + device = tmp_path / "device" + destination = tmp_path / "nested" / "mount" + ensure_directory = mocker.patch( + "storage_device_managers.ensure_directory", return_value=True + ) + mocker.patch("storage_device_managers.is_mounted", return_value=False) + mocker.patch("storage_device_managers.mount_device") + mocker.patch("storage_device_managers.unmount_device") + with sdm.mounted_device(device, destination) as mount_dir: + assert mount_dir == destination + ensure_directory.assert_called_once_with(destination) From 8a256d69badc4ad5605fcd7ed774219528103b23 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 13:00:54 +0000 Subject: [PATCH 2/3] refactor: rewrite mocked tests without mocking --- .../tests/cli/test_sudo_pass_cmd.py | 29 ------------------- .../tests/test_mount_unmount.py | 26 +++-------------- 2 files changed, 4 insertions(+), 51 deletions(-) diff --git a/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py b/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py index ec2ca0ab..8602007f 100644 --- a/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py +++ b/projects/butter-backup/tests/cli/test_sudo_pass_cmd.py @@ -57,35 +57,6 @@ def test_sudo_pass_cmd_is_used_in_open( ) -def test_open_uses_sudo_to_create_mount_dir( - runner: CliRunner, - encrypted_btrfs_device: cp.DeviceConfiguration, - mocker, - tmp_path: Path, -) -> None: - dest_dir = tmp_path / "mounts" - wrapped_config = cp.Configuration( - DeviceConfigurations=[encrypted_btrfs_device], - OpenDirectory=dest_dir, - ) - config_file = tmp_path / "config.json" - config_file.write_text(wrapped_config.model_dump_json()) - - ensure_directory = mocker.patch( - "storage_device_managers.ensure_directory", return_value=True - ) - mocker.patch( - "storage_device_managers.open_encrypted_device", - return_value=Path("/dev/mapper/test"), - ) - mocker.patch("storage_device_managers.mount_device") - - result = runner.invoke(app, ["open", "--config", str(config_file)]) - - assert result.exit_code == 0 - ensure_directory.assert_called_once_with(dest_dir / encrypted_btrfs_device.Name) - - def test_sudo_pass_cmd_is_used_in_backup( runner: CliRunner, encrypted_device: cp.DeviceConfiguration, diff --git a/projects/storage-device-managers/tests/test_mount_unmount.py b/projects/storage-device-managers/tests/test_mount_unmount.py index ab50b25b..ac80ebcf 100644 --- a/projects/storage-device-managers/tests/test_mount_unmount.py +++ b/projects/storage-device-managers/tests/test_mount_unmount.py @@ -50,19 +50,15 @@ class MyCustomTestException(Exception): pass -def test_ensure_directory_uses_sudo_for_missing_directory( - mocker, tmp_path: Path -) -> None: +def test_ensure_directory_creates_missing_directory(tmp_path: Path) -> None: destination = tmp_path / "nested" / "mount" - run_cmd = mocker.patch("storage_device_managers.sh.run_cmd") + assert not destination.exists() assert sdm.ensure_directory(destination) - run_cmd.assert_called_once_with(cmd=["sudo", "mkdir", "-p", destination]) + assert destination.exists() -def test_ensure_directory_skips_existing_directory(mocker, tmp_path: Path) -> None: - run_cmd = mocker.patch("storage_device_managers.sh.run_cmd") +def test_ensure_directory_skips_existing_directory(tmp_path: Path) -> None: assert not sdm.ensure_directory(tmp_path) - run_cmd.assert_not_called() def test_mount_device( @@ -195,17 +191,3 @@ def test_mounted_device_with_destination( assert str(device) not in sdm.get_mounted_devices() # A given destination should not be deleted after unmounting. assert destination.exists() - - -def test_mounted_device_uses_sudo_to_create_destination(mocker, tmp_path: Path) -> None: - device = tmp_path / "device" - destination = tmp_path / "nested" / "mount" - ensure_directory = mocker.patch( - "storage_device_managers.ensure_directory", return_value=True - ) - mocker.patch("storage_device_managers.is_mounted", return_value=False) - mocker.patch("storage_device_managers.mount_device") - mocker.patch("storage_device_managers.unmount_device") - with sdm.mounted_device(device, destination) as mount_dir: - assert mount_dir == destination - ensure_directory.assert_called_once_with(destination) From 5bdf16c56e8702ba11ef24f7e80e9e751205c088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Sun, 5 Jul 2026 17:44:09 +0200 Subject: [PATCH 3/3] test: Move ensure_directory's tests to dedicated file Instead of squashing them into the mount_unmount helpers, the dedicated tests for ensure_directory are moved to a dedicated file. While at it, a new test was written which ensured correct functionality even for parent directories owned by root. --- .../tests/test_ensure_directory.py | 50 +++++++++++++++++++ .../tests/test_mount_unmount.py | 11 ---- 2 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 projects/storage-device-managers/tests/test_ensure_directory.py diff --git a/projects/storage-device-managers/tests/test_ensure_directory.py b/projects/storage-device-managers/tests/test_ensure_directory.py new file mode 100644 index 00000000..af36f7f3 --- /dev/null +++ b/projects/storage-device-managers/tests/test_ensure_directory.py @@ -0,0 +1,50 @@ +import typing as t +from pathlib import Path + +import pytest +import shell_interface as sh + +import storage_device_managers as sdm + + +@pytest.fixture +def root_owned_tmp_path(tmp_path: Path) -> t.Iterable[Path]: + """ + Create a temporary directory owned by root for testing + """ + root_owned_path = tmp_path / "root_owned" + root_owned_path.mkdir() + current_user = sh.get_user() + current_group = sh.get_group(current_user) + chown_to_root: sh.StrPathList = ["sudo", "chown", "root:root", root_owned_path] + chown_to_user: sh.StrPathList = [ + "sudo", + "chown", + f"{current_user}:{current_group}", + root_owned_path, + ] + sh.run_cmd(cmd=chown_to_root) + try: + yield root_owned_path + finally: + sh.run_cmd(cmd=chown_to_user) + + +def test_ensure_directory_creates_missing_directory(tmp_path: Path) -> None: + destination = tmp_path / "nested" / "mount" + assert not destination.exists() + assert sdm.ensure_directory(destination) + assert destination.exists() + + +def test_ensure_directory_creates_missing_directory_in_root_owned_path( + root_owned_tmp_path: Path, +) -> None: + destination = root_owned_tmp_path / "nested" / "mount" + assert not destination.exists() + assert sdm.ensure_directory(destination) + assert destination.exists() + + +def test_ensure_directory_skips_existing_directory(tmp_path: Path) -> None: + assert not sdm.ensure_directory(tmp_path) diff --git a/projects/storage-device-managers/tests/test_mount_unmount.py b/projects/storage-device-managers/tests/test_mount_unmount.py index ac80ebcf..096e5ee3 100644 --- a/projects/storage-device-managers/tests/test_mount_unmount.py +++ b/projects/storage-device-managers/tests/test_mount_unmount.py @@ -50,17 +50,6 @@ class MyCustomTestException(Exception): pass -def test_ensure_directory_creates_missing_directory(tmp_path: Path) -> None: - destination = tmp_path / "nested" / "mount" - assert not destination.exists() - assert sdm.ensure_directory(destination) - assert destination.exists() - - -def test_ensure_directory_skips_existing_directory(tmp_path: Path) -> None: - assert not sdm.ensure_directory(tmp_path) - - def test_mount_device( device_with_fs, tmp_path: Path, compression_kwargs: CompressionKwargsT ) -> None: