Merge pull request #3051 from ThomasWaldmann/fix-spawn-passcommand

fix subprocess environments

src/borg/archiver.py

@@ -64,7 +64,7 @@ from .helpers import ProgressIndicatorPercent
 from .helpers import basic_json_data, json_print
 from .helpers import replace_placeholders
 from .helpers import ChunkIteratorFileWrapper
-from .helpers import popen_with_error_handling
+from .helpers import popen_with_error_handling, prepare_subprocess_env
 from .helpers import dash_open
 from .helpers import umount
 from .nanorst import rst_to_terminal
@@ -771,10 +771,12 @@ class Archiver:
             # The decision whether to close that or not remains the same.
             filterout = tarstream
             filterout_close = tarstream_close
+            env = prepare_subprocess_env(system=True)
             # There is no deadlock potential here (the subprocess docs warn about this), because
             # communication with the process is a one-way road, i.e. the process can never block
             # for us to do something while we block on the process for something different.
-            filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout, log_prefix='--tar-filter: ')
+            filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout,
+                                                   log_prefix='--tar-filter: ', env=env)
             if not filterproc:
                 return EXIT_ERROR
             # Always close the pipe, otherwise the filter process would not notice when we are done.
@@ -1480,9 +1482,10 @@ class Archiver:
         # we can only do this for local repositories (with .io), though:
         if hasattr(repository, 'io'):
             repository.io.close_segment()
+        env = prepare_subprocess_env(system=True)
         try:
             # we exit with the return code we get from the subprocess
-            return subprocess.call([args.command] + args.args)
+            return subprocess.call([args.command] + args.args, env=env)
         finally:
             # we need to commit the "no change" operation we did to the manifest
             # because it created a new segment file in the repository. if we would

src/borg/crypto/key.py

@@ -23,6 +23,7 @@ from ..helpers import yes
 from ..helpers import get_keys_dir, get_security_dir
 from ..helpers import get_limited_unpacker
 from ..helpers import bin_to_hex
+from ..helpers import prepare_subprocess_env
 from ..item import Key, EncryptedKey
 from ..platform import SaveFile
 
@@ -425,8 +426,10 @@ class Passphrase(str):
     def env_passcommand(cls, default=None):
         passcommand = os.environ.get('BORG_PASSCOMMAND', None)
         if passcommand is not None:
+            # passcommand is a system command (not inside pyinstaller env)
+            env = prepare_subprocess_env(system=True)
             try:
-                passphrase = subprocess.check_output(shlex.split(passcommand), universal_newlines=True)
+                passphrase = subprocess.check_output(shlex.split(passcommand), universal_newlines=True, env=env)
             except (subprocess.CalledProcessError, FileNotFoundError) as e:
                 raise PasscommandFailure(e)
             return cls(passphrase.rstrip('\n'))

src/borg/helpers/fs.py

@@ -6,6 +6,8 @@ import subprocess
 import sys
 import textwrap
 
+from .process import prepare_subprocess_env
+
 from ..constants import *  # NOQA
 
 
@@ -154,7 +156,8 @@ def dash_open(path, mode):
 
 
 def umount(mountpoint):
+    env = prepare_subprocess_env(system=True)
     try:
-        return subprocess.call(['fusermount', '-u', mountpoint])
+        return subprocess.call(['fusermount', '-u', mountpoint], env=env)
     except FileNotFoundError:
-        return subprocess.call(['umount', mountpoint])
+        return subprocess.call(['umount', mountpoint], env=env)

src/borg/helpers/process.py

@@ -6,6 +6,8 @@ import signal
 import subprocess
 import sys
 
+from .. import __version__
+
 from ..logger import create_logger
 logger = create_logger()
 
@@ -115,3 +117,29 @@ def popen_with_error_handling(cmd_line: str, log_prefix='', **kwargs):
 
 def is_terminal(fd=sys.stdout):
     return hasattr(fd, 'isatty') and fd.isatty() and (sys.platform != 'win32' or 'ANSICON' in os.environ)
+
+
+def prepare_subprocess_env(system, env=None):
+    """
+    Prepare the environment for a subprocess we are going to create.
+
+    :param system: True for preparing to invoke system-installed binaries,
+                   False for stuff inside the pyinstaller environment (like borg, python).
+    :param env: optionally give a environment dict here. if not given, default to os.environ.
+    :return: a modified copy of the environment
+    """
+    env = dict(env if env is not None else os.environ)
+    if system:
+        # a pyinstaller binary's bootloader modifies LD_LIBRARY_PATH=/tmp/_ME...,
+        # but we do not want that system binaries (like ssh or other) pick up
+        # (non-matching) libraries from there.
+        # thus we install the original LDLP, before pyinstaller has modified it:
+        lp_key = 'LD_LIBRARY_PATH'
+        lp_orig = env.get(lp_key + '_ORIG')  # pyinstaller >= 20160820 has this
+        if lp_orig is not None:
+            env[lp_key] = lp_orig
+    # security: do not give secrets to subprocess
+    env.pop('BORG_PASSPHRASE', None)
+    # for information, give borg version to the subprocess
+    env['BORG_VERSION'] = __version__
+    return env

src/borg/remote.py

@@ -29,6 +29,7 @@ from .helpers import replace_placeholders
 from .helpers import sysinfo
 from .helpers import format_file_size
 from .helpers import truncate_and_unlink
+from .helpers import prepare_subprocess_env
 from .logger import create_logger, setup_logging
 from .repository import Repository
 from .version import parse_version, format_version
@@ -530,21 +531,12 @@ class RemoteRepository:
         self.server_version = parse_version('1.0.8')  # fallback version if server is too old to send version information
         self.p = None
         testing = location.host == '__testsuite__'
+        # when testing, we invoke and talk to a borg process directly (no ssh).
+        # when not testing, we invoke the system-installed ssh binary to talk to a remote borg.
+        env = prepare_subprocess_env(system=not testing)
         borg_cmd = self.borg_cmd(args, testing)
-        env = dict(os.environ)
         if not testing:
             borg_cmd = self.ssh_cmd(location) + borg_cmd
-            # pyinstaller binary modifies LD_LIBRARY_PATH=/tmp/_ME... but we do not want
-            # that the system's ssh binary picks up (non-matching) libraries from there.
-            # thus we install the original LDLP, before pyinstaller has modified it:
-            lp_key = 'LD_LIBRARY_PATH'
-            lp_orig = env.get(lp_key + '_ORIG')  # pyinstaller >= 20160820 has this
-            if lp_orig is not None:
-                env[lp_key] = lp_orig
-            else:
-                env.pop(lp_key, None)
-        env.pop('BORG_PASSPHRASE', None)  # security: do not give secrets to subprocess
-        env['BORG_VERSION'] = __version__
         logger.debug('SSH command line: %s', borg_cmd)
         self.p = Popen(borg_cmd, bufsize=0, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env)
         self.stdin_fd = self.p.stdin.fileno()

src/borg/xattr.py

@@ -10,7 +10,7 @@ from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_
 from ctypes.util import find_library
 from distutils.version import LooseVersion
 
-from .helpers import Buffer
+from .helpers import Buffer, prepare_subprocess_env
 
 
 try:
@@ -88,7 +88,9 @@ if sys.platform.startswith('linux'):
     preloads = re.split("[ :]", LD_PRELOAD)
     for preload in preloads:
         if preload.startswith("libfakeroot"):
-            fakeroot_version = LooseVersion(subprocess.check_output(['fakeroot', '-v']).decode('ascii').split()[-1])
+            env = prepare_subprocess_env(system=True)
+            fakeroot_output = subprocess.check_output(['fakeroot', '-v'], env=env)
+            fakeroot_version = LooseVersion(fakeroot_output.decode('ascii').split()[-1])
             if fakeroot_version >= LooseVersion("1.20.2"):
                 # 1.20.2 has been confirmed to have xattr support
                 # 1.18.2 has been confirmed not to have xattr support