From cf11170a7d9f086866f3271e6080fbd41ece8549 Mon Sep 17 00:00:00 2001
From: Cleber Rosa <crosa@redhat.com>
Date: Mon, 14 Jan 2019 13:54:40 -0500
Subject: [PATCH] Tests' directory: don't create system wide directory

If one is running avocado as a privileged user, the "usable_ro_dir()"
function may create a system wide directory for hosting tests.  That's
confusing because, the following workflow is possible:

 1) Regular user creates test, say "foo.py" in his own test dir
 2) Privileged user run "avocado run does_not_exist.py", which by
    means of "usable_ro_dir()", may create a system wide location
    for tests.
 3) Regular user runs "avocado run foo.py", and then test is not found
    because "avocado.core.data_dir.get_test_dir()" gives precedence to
    the system test dir created earlier.

This is also a problem when running the selftests from within the
tree, when working as a privileged user.  And finally, as a general
rule, we should avoid creating directories in system-wide locations so
silently, so I do believe this pattern could be expanded to the
results and data directories too.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 avocado/core/data_dir.py       |  4 ++--
 avocado/utils/path.py          | 10 ++++++----
 selftests/unit/test_datadir.py |  4 +++-
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/avocado/core/data_dir.py b/avocado/core/data_dir.py
index 61f2634e..c1c89680 100755
--- a/avocado/core/data_dir.py
+++ b/avocado/core/data_dir.py
@@ -107,14 +107,14 @@ def get_test_dir():
     4) User default test dir (~/avocado/tests) is used
     """
     configured = _get_settings_dir('test_dir')
-    if utils_path.usable_ro_dir(configured):
+    if utils_path.usable_ro_dir(configured, False):
         return configured
 
     if settings.settings.intree:
         base_dir = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
         return os.path.join(base_dir, 'examples', 'tests')
 
-    if utils_path.usable_ro_dir(SYSTEM_TEST_DIR):
+    if utils_path.usable_ro_dir(SYSTEM_TEST_DIR, False):
         return SYSTEM_TEST_DIR
 
     if utils_path.usable_ro_dir(USER_TEST_DIR):
diff --git a/avocado/utils/path.py b/avocado/utils/path.py
index 083f8e24..562500d9 100644
--- a/avocado/utils/path.py
+++ b/avocado/utils/path.py
@@ -139,13 +139,14 @@ class PathInspector(object):
         return self.is_script(language='python')
 
 
-def usable_rw_dir(directory):
+def usable_rw_dir(directory, create=True):
     """
     Verify whether we can use this dir (read/write).
 
     Checks for appropriate permissions, and creates missing dirs as needed.
 
     :param directory: Directory
+    :param create: wether to create the directory
     """
     if os.path.isdir(directory):
         try:
@@ -155,7 +156,7 @@ def usable_rw_dir(directory):
             return True
         except OSError:
             pass
-    else:
+    elif create:
         try:
             init_dir(directory)
             return True
@@ -165,7 +166,7 @@ def usable_rw_dir(directory):
     return False
 
 
-def usable_ro_dir(directory):
+def usable_ro_dir(directory, create=True):
     """
     Verify whether dir exists and we can access its contents.
 
@@ -173,6 +174,7 @@ def usable_ro_dir(directory):
     least try to create one.
 
     :param directory: Directory
+    :param create: wether to create the directory
     """
     cwd = os.getcwd()
     if os.path.isdir(directory):
@@ -182,7 +184,7 @@ def usable_ro_dir(directory):
             return True
         except OSError:
             pass
-    else:
+    elif create:
         try:
             init_dir(directory)
             return True
diff --git a/selftests/unit/test_datadir.py b/selftests/unit/test_datadir.py
index 45bd82d6..acab970c 100644
--- a/selftests/unit/test_datadir.py
+++ b/selftests/unit/test_datadir.py
@@ -24,8 +24,10 @@ class DataDirTest(unittest.TestCase):
         a the path to a configuration file contain those same settings
         """
         base_dir = tempfile.mkdtemp(prefix='avocado_' + __name__)
+        test_dir = os.path.join(base_dir, 'tests')
+        os.mkdir(test_dir)
         mapping = {'base_dir': base_dir,
-                   'test_dir': os.path.join(base_dir, 'tests'),
+                   'test_dir': test_dir,
                    'data_dir': os.path.join(base_dir, 'data'),
                    'logs_dir': os.path.join(base_dir, 'logs')}
         temp_settings = ('[datadir.paths]\n'
-- 
GitLab