Explicitly close file descriptors when invoking LVM commands (#1068).

Reviewed-on: https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/1074

borgmatic/execute.py

@@ -288,6 +288,7 @@ def execute_command(
     borg_local_path=None,
     borg_exit_codes=None,
     run_to_completion=True,
+    close_fds=False,  # Necessary for passing credentials via anonymous pipe.
 ):
     '''
     Execute the given command (a sequence of command/argument strings) and log its output at the
@@ -315,8 +316,7 @@ def execute_command(
         shell=shell,
         env=environment,
         cwd=working_directory,
-        # Necessary for passing credentials via anonymous pipe.
-        close_fds=False,
+        close_fds=close_fds,
     )
     if not run_to_completion:
         return process
@@ -340,6 +340,7 @@ def execute_command_and_capture_output(
     working_directory=None,
     borg_local_path=None,
     borg_exit_codes=None,
+    close_fds=False,  # Necessary for passing credentials via anonymous pipe.
 ):
     '''
     Execute the given command (a sequence of command/argument strings), capturing and returning its
@@ -365,8 +366,7 @@ def execute_command_and_capture_output(
             shell=shell,
             env=environment,
             cwd=working_directory,
-            # Necessary for passing credentials via anonymous pipe.
-            close_fds=False,
+            close_fds=close_fds,
         )
     except subprocess.CalledProcessError as error:
         if (
@@ -390,6 +390,7 @@ def execute_command_with_processes(
     working_directory=None,
     borg_local_path=None,
     borg_exit_codes=None,
+    close_fds=False,  # Necessary for passing credentials via anonymous pipe.
 ):
     '''
     Execute the given command (a sequence of command/argument strings) and log its output at the
@@ -425,8 +426,7 @@ def execute_command_with_processes(
             shell=shell,
             env=environment,
             cwd=working_directory,
-            # Necessary for passing credentials via anonymous pipe.
-            close_fds=False,
+            close_fds=close_fds,
         )
     except (subprocess.CalledProcessError, OSError):
         # Something has gone wrong. So vent each process' output buffer to prevent it from hanging.

borgmatic/hooks/data_source/btrfs.py

@@ -32,7 +32,8 @@ def get_subvolume_mount_points(findmnt_command):
             'btrfs',
             '--json',
             '--list',  # Request a flat list instead of a nested subvolume hierarchy.
-        )
+        ),
+        close_fds=True,
     )
 
     try:
@@ -59,6 +60,7 @@ def get_subvolume_property(btrfs_command, subvolume_path, property_name):
             subvolume_path,
             property_name,
         ),
+        close_fds=True,
     )
 
     try:
@@ -227,6 +229,7 @@ def snapshot_subvolume(btrfs_command, subvolume_path, snapshot_path):  # pragma:
             snapshot_path,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -298,6 +301,7 @@ def delete_snapshot(btrfs_command, snapshot_path):  # pragma: no cover
             snapshot_path,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 

borgmatic/hooks/data_source/lvm.py

@@ -50,7 +50,8 @@ def get_logical_volumes(lsblk_command, patterns=None):
                     'name,path,mountpoint,type',
                     '--json',
                     '--list',
-                )
+                ),
+                close_fds=True,
             )
         )
     except json.JSONDecodeError as error:
@@ -109,6 +110,7 @@ def snapshot_logical_volume(
             logical_volume_device,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -129,6 +131,7 @@ def mount_snapshot(mount_command, snapshot_device, snapshot_mount_path):  # prag
             snapshot_mount_path,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -277,6 +280,7 @@ def unmount_snapshot(umount_command, snapshot_mount_path):  # pragma: no cover
     borgmatic.execute.execute_command(
         tuple(umount_command.split(' ')) + (snapshot_mount_path,),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -291,6 +295,7 @@ def remove_snapshot(lvremove_command, snapshot_device_path):  # pragma: no cover
             snapshot_device_path,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -318,7 +323,8 @@ def get_snapshots(lvs_command, snapshot_name=None):
                     'lv_name,lv_path',
                     '--select',
                     'lv_attr =~ ^s',  # Filter to just snapshots.
-                )
+                ),
+                close_fds=True,
             )
         )
     except json.JSONDecodeError as error:

borgmatic/hooks/data_source/zfs.py

@@ -53,7 +53,8 @@ def get_datasets_to_backup(zfs_command, patterns):
             'filesystem',
             '-o',
             f'name,mountpoint,canmount,{BORGMATIC_USER_PROPERTY}',
-        )
+        ),
+        close_fds=True,
     )
 
     try:
@@ -131,7 +132,8 @@ def get_all_dataset_mount_points(zfs_command):
             'filesystem',
             '-o',
             'mountpoint',
-        )
+        ),
+        close_fds=True,
     )
 
     return tuple(
@@ -158,6 +160,7 @@ def snapshot_dataset(zfs_command, full_snapshot_name):  # pragma: no cover
             full_snapshot_name,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -180,6 +183,7 @@ def mount_snapshot(mount_command, full_snapshot_name, snapshot_mount_path):  # p
             snapshot_mount_path,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -310,6 +314,7 @@ def unmount_snapshot(umount_command, snapshot_mount_path):  # pragma: no cover
     borgmatic.execute.execute_command(
         tuple(umount_command.split(' ')) + (snapshot_mount_path,),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -325,6 +330,7 @@ def destroy_snapshot(zfs_command, full_snapshot_name):  # pragma: no cover
             full_snapshot_name,
         ),
         output_log_level=logging.DEBUG,
+        close_fds=True,
     )
 
 
@@ -342,7 +348,8 @@ def get_all_snapshots(zfs_command):
             'snapshot',
             '-o',
             'name',
-        )
+        ),
+        close_fds=True,
     )
 
     return tuple(line.rstrip() for line in list_output.splitlines())

tests/unit/hooks/data_source/test_lvm.py

@@ -214,6 +214,7 @@ def test_snapshot_logical_volume_with_percentage_snapshot_name_uses_lvcreate_ext
             '/dev/snap',
         ),
         output_log_level=object,
+        close_fds=True,
     )
 
     module.snapshot_logical_volume('lvcreate', 'snap', '/dev/snap', '10%ORIGIN')
@@ -233,6 +234,7 @@ def test_snapshot_logical_volume_with_non_percentage_snapshot_name_uses_lvcreate
             '/dev/snap',
         ),
         output_log_level=object,
+        close_fds=True,
     )
 
     module.snapshot_logical_volume('lvcreate', 'snap', '/dev/snap', '10TB')