From 6120f5f1b8f7e2caaac24add5fd9d001f6bac5f6 Mon Sep 17 00:00:00 2001
From: Keith Brady <34693973+krcb197@users.noreply.github.com>
Date: Fri, 28 Apr 2023 09:54:14 +0100
Subject: [PATCH 1/2] New test for password logging

---
 tests/test_remote.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/test_remote.py b/tests/test_remote.py
index 5b83c85..7d17ae0 100644
--- a/tests/test_remote.py
+++ b/tests/test_remote.py
@@ -41,6 +41,28 @@ def test_checkout(self):
         """
         svn.remote.RemoteClient(self.test_svn_url).checkout('trial')
         self.assertTrue(os.path.exists('trial'))
+        
+        def test_password_logging(self):
+        """
+        Testing password are being masked out of the log and exception messages
+        :return:
+        """
+        with self.assertLogs(logger=None, level=logging.DEBUG) as cap_logging:
+            with self.assertRaises(svn.exception.SvnException) as cap_exec:
+                # this call will fail because it is trying to check out the file from the fake url
+                svn.remote.RemoteClient(self.test_fake_url,
+                                        username='fake_username',
+                                        password='fake_password').checkout('trial')
+
+        # make sure the password was present in the log but has been replaced
+        self.assertIn('--password', str(cap_exec.exception))
+        self.assertNotIn('fake_password', str(cap_exec.exception))
+
+        # there may be multiple log entries generated by test, need to check that any which include the '--password'
+        # command line argument do not feature the password in the clear
+        for log_line in cap_logging.output:
+            if '--password' in log_line:
+                self.assertNotIn('fake_password', log_line)
 
     def test_remove(self):
         with svn.test_support.temp_repo() as (_, rc):

From 86d47ac891ae757971166b2e0ea2ab21dded3017 Mon Sep 17 00:00:00 2001
From: Keith Brady <34693973+krcb197@users.noreply.github.com>
Date: Fri, 28 Apr 2023 09:56:58 +0100
Subject: [PATCH 2/2] Fix the password leak

Generate a "safe" version of the cmd, if the original includes a password with the password masked out, that is used in the logging.
---
 svn/common_base.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/svn/common_base.py b/svn/common_base.py
index b83f836..23f1f75 100644
--- a/svn/common_base.py
+++ b/svn/common_base.py
@@ -12,7 +12,16 @@ class CommonBase(object):
     def external_command(
             self, cmd, success_code=0, do_combine=False, return_binary=False,
             environment={}, wd=None, do_discard_stderr=True):
-        _LOGGER.debug("RUN: %s" % (cmd,))
+        
+        # the cmd may contain a user password that should not be logged, therefore we make a clean version of the
+        # comamnd for logging purposes
+        if '--password' in cmd:
+            safe_logging_comand = cmd.copy()
+            safe_logging_comand[cmd.index('--password') + 1] = '**************'
+        else:
+            safe_logging_comand = cmd
+        
+        _LOGGER.debug("RUN: %s" % (safe_logging_comand,))
 
         env = os.environ.copy()
 
@@ -49,7 +58,7 @@ def external_command(
 
             raise svn.exception.SvnException(
                 "Command failed with ({}): {}\nSTDOUT:\n\n{}\nSTDERR:\n\n{}".format(
-                return_code, cmd, stdout, stderr))
+                return_code, safe_logging_comand, stdout, stderr))
 
         if return_binary is True or do_combine is True:
             return stdout