From b2b1a422f76d0191331c3b82d91f4a79cbb6afb5 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 9 Sep 2026 01:07:54 +0200 Subject: [PATCH] prepare_subprocess_env: remove all passphrase-related env vars and BORGSTORE_REST_PASSWORD, #6480 Only BORG_PASSPHRASE was removed from the environment given to subprocesses (the command given in BORG_PASSCOMMAND, --paths-from-command / --content-from-command commands, tar filter commands, borg with-lock commands, fusermount/umount, fakeroot detection, ssh for legacy borg 1.x repos). Now also remove: - BORG_NEW_PASSPHRASE, BORG_OTHER_PASSPHRASE: secrets, like BORG_PASSPHRASE. - BORG_PASSCOMMAND, BORG_OTHER_PASSCOMMAND: often embed secrets or paths to them. - BORG_PASSPHRASE_FD, BORG_OTHER_PASSPHRASE_FD: the file descriptors are not inherited by the subprocess (Popen closes them), so the numbers would be dangling and misleading there. - BORGSTORE_REST_PASSWORD: borgstore's REST authentication password. The other borg env vars (BORG_REPO, BORG_KEY_FILE, ...) are not secrets and with-lock scripts may rely on them, so they are kept. Also document this in the BORG_PASSPHRASE help text and add tests. Co-Authored-By: Claude Fable 5.1 --- docs/changes.rst | 6 ++++ src/borg/archiver/help_cmd.py | 5 ++++ src/borg/helpers/process.py | 22 +++++++++++++-- src/borg/testsuite/helpers/process_test.py | 33 +++++++++++++++++++++- 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 1b0d115595..33a35a0f18 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -171,6 +171,12 @@ New features: - create/import-tar --json: report the deduplicated size of the new archive, #10335. It is also included in the archive_progress JSON output. +Fixes: + +- subprocess environment: also remove BORG_NEW_PASSPHRASE, BORG_OTHER_PASSPHRASE, BORG_PASSCOMMAND, + BORG_OTHER_PASSCOMMAND, BORG_PASSPHRASE_FD, BORG_OTHER_PASSPHRASE_FD and BORGSTORE_REST_PASSWORD + from the environment given to subprocesses (previously only BORG_PASSPHRASE was removed), #6480. + Version 2.0.0b24 (2026-09-02) ----------------------------- diff --git a/src/borg/archiver/help_cmd.py b/src/borg/archiver/help_cmd.py index acdcbfcf53..3638c2f2bd 100644 --- a/src/borg/archiver/help_cmd.py +++ b/src/borg/archiver/help_cmd.py @@ -614,6 +614,11 @@ class HelpMixIn: "More than one passphrase environment variable is set". The same applies to the ``BORG_OTHER_*`` variants (which are a separate, independent group). See also BORG_NEW_PASSPHRASE. + borg removes the passphrase-related variables (BORG_PASSPHRASE, BORG_NEW_PASSPHRASE, + BORG_PASSCOMMAND, BORG_PASSPHRASE_FD and their ``BORG_OTHER_*`` variants) as well as + BORGSTORE_REST_PASSWORD from the environment of the subprocesses it starts (like the + command given in BORG_PASSCOMMAND, ``--paths-from-command`` / ``--content-from-command`` + commands, tar filter commands and ``borg with-lock`` commands). BORG_PASSCOMMAND (and BORG_OTHER_PASSCOMMAND) When set, use the standard output of the command (trailing newlines are stripped) to answer the passphrase question for encrypted repositories. diff --git a/src/borg/helpers/process.py b/src/borg/helpers/process.py index a66e7afbf1..1e4da685ab 100644 --- a/src/borg/helpers/process.py +++ b/src/borg/helpers/process.py @@ -325,6 +325,23 @@ def is_terminal(fd=sys.stdout): return hasattr(fd, "isatty") and fd.isatty() and (not is_win32 or "ANSICON" in os.environ) +# Environment variables that prepare_subprocess_env() removes from the environment of subprocesses. +# The passphrases and the borgstore REST password are secrets, the commands given in BORG_PASSCOMMAND / +# BORG_OTHER_PASSCOMMAND often embed secrets or paths to them. The *_FD variables refer to file +# descriptors that the subprocess does not inherit (Popen closes them), so they would be dangling and +# misleading there. +SUBPROCESS_ENV_REMOVE = ( + "BORG_PASSPHRASE", + "BORG_NEW_PASSPHRASE", + "BORG_OTHER_PASSPHRASE", + "BORG_PASSCOMMAND", + "BORG_OTHER_PASSCOMMAND", + "BORG_PASSPHRASE_FD", + "BORG_OTHER_PASSPHRASE_FD", + "BORGSTORE_REST_PASSWORD", +) + + def prepare_subprocess_env(system, env=None): """ Prepare the environment for a subprocess we are going to create. @@ -356,8 +373,9 @@ def prepare_subprocess_env(system, env=None): lp = env.get(lp_key) if lp is not None and getattr(sys, "frozen", False) and hasattr(sys, "_MEIPASS"): env.pop(lp_key) - # security: do not give secrets to subprocess - env.pop("BORG_PASSPHRASE", None) + # security: do not give secrets (and things only meaningful in this process) to the subprocess + for name in SUBPROCESS_ENV_REMOVE: + env.pop(name, None) # for information, give borg version to the subprocess env["BORG_VERSION"] = __version__ return env diff --git a/src/borg/testsuite/helpers/process_test.py b/src/borg/testsuite/helpers/process_test.py index 7a9ff637c9..7de90fbc0d 100644 --- a/src/borg/testsuite/helpers/process_test.py +++ b/src/borg/testsuite/helpers/process_test.py @@ -1,7 +1,8 @@ import shutil import pytest -from ...helpers.process import popen_with_error_handling +from ... import __version__ +from ...helpers.process import popen_with_error_handling, prepare_subprocess_env, SUBPROCESS_ENV_REMOVE class TestPopenWithErrorHandling: @@ -25,3 +26,33 @@ def test_bad_syntax(self, cmd): def test_shell(self): with pytest.raises(AssertionError): popen_with_error_handling("", shell=True) + + +class TestPrepareSubprocessEnv: + @pytest.mark.parametrize("name", SUBPROCESS_ENV_REMOVE) + @pytest.mark.parametrize("system", (True, False)) + def test_removes_secrets_from_os_environ(self, monkeypatch, name, system): + monkeypatch.setenv(name, "secret") + assert name not in prepare_subprocess_env(system=system) + + def test_removes_secrets_from_given_env(self): + env = {name: "secret" for name in SUBPROCESS_ENV_REMOVE} + env["BORG_REPO"] = "/path/to/repo" + result = prepare_subprocess_env(system=True, env=env) + assert not set(result) & set(SUBPROCESS_ENV_REMOVE) + assert result["BORG_REPO"] == "/path/to/repo" + # the given env must not be modified + assert env["BORG_PASSPHRASE"] == "secret" + assert "BORG_VERSION" not in env + + def test_keeps_other_variables_and_sets_version(self, monkeypatch): + monkeypatch.setenv("BORG_REPO", "/path/to/repo") + monkeypatch.setenv("BORG_KEY_FILE", "/path/to/keyfile") + monkeypatch.setenv("BORGSTORE_REST_USERNAME", "username") + monkeypatch.setenv("HOME", "/home/user") + env = prepare_subprocess_env(system=True) + assert env["BORG_REPO"] == "/path/to/repo" + assert env["BORG_KEY_FILE"] == "/path/to/keyfile" + assert env["BORGSTORE_REST_USERNAME"] == "username" + assert env["HOME"] == "/home/user" + assert env["BORG_VERSION"] == __version__