fix: FileProvider.validate() leaked files outside root

insights/core/spec_factory.py

@@ -215,7 +215,22 @@ def __init__(self, relative_path, root="/", save_as=None, ds=None, ctx=None, cle
 
         self.validate()
 
+    def _is_inside_root(self):
+        """Checks that `self.relative_path` does not point outside `self.root`."""
+        resolved = os.path.realpath(self.path)
+
+        # pathlib.Path.is_relative_to() has been added only in Python 3.9
+        resolved_root = os.path.realpath(self.root)
+        if not resolved_root.endswith(os.sep):
+            resolved_root += os.sep
+
+        return resolved.startswith(resolved_root)
+
     def validate(self):
+        if not self._is_inside_root():
+            msg = "Relative path points outside the root: %s"
+            raise ValueError(msg % (self.path))
+
         # 1. No Such File
         if not os.path.exists(self.path):
             raise ContentException("%s does not exist." % self.path)
@@ -225,15 +240,10 @@ def validate(self):
             if self._filterable and not self._filters:
                 raise NoFilterException("Skipping %s due to no filters." % dr.get_name(self.ds))
             # 2.2 Customer Prohibits Collection
-            if not blacklist.allow_file("/" + self.relative_path):
-                log.warning("WARNING: Skipping file %s", "/" + self.relative_path)
+            if not blacklist.allow_file(os.sep + self.relative_path):
+                log.warning("WARNING: Skipping file %s", os.sep + self.relative_path)
                 raise BlacklistedSpec()
 
-        resolved = os.path.realpath(self.path)
-        if not resolved.startswith(os.path.realpath(self.root)):
-            msg = "Relative path points outside the root: %s -> %s."
-            raise Exception(msg % (self.path, resolved))
-
         if not os.access(self.path, os.R_OK):
             raise ContentException("Cannot access %s" % self.path)
 

insights/tests/core/spec_factory/test_file_provider.py

@@ -32,7 +32,8 @@ def create_file(relpath, content=""):
     create_file(SAMPLE_FILE)
     create_directory("sample/root")
     create_file("sample/root/access_denied.txt")
-    os.chmod(os.path.join(root, "sample/root/access_denied.txt"), 0)
+    os.chmod(os.path.join(root, "sample/root/access_denied.txt"), 0o000)
+    create_file("sample/root.txt")
     return root
 
 
@@ -48,7 +49,7 @@ def test_repr(sample_directory):
 
 
 @pytest.mark.skipif(
-    # GitHub workflows run Python 2.7 and 3.6 tests in containers, because these old Python
+    # GitHub workflows run Python 2.7 and 3.6 tests in containers because these old Python
     # versions are not available in GitHub-hosted runners anymore. The tests are executed as root.
     os.getuid() == 0,
     reason="Test must not be run as root. Root ignores file mode bits."
@@ -59,7 +60,11 @@ def test_repr(sample_directory):
         ("access_denied.txt", ContentException, "Cannot access"),
         ("no_such_file.txt", ContentException, "does not exist"),
         # file outside root that exists
-        ("../../sample_file.txt", Exception, "Relative path points outside the root"),
+        ("../../sample_file.txt", ValueError, "Relative path points outside the root"),
+        # file outside root that does not exist
+        ("../no_such_file.txt", ValueError, "Relative path points outside the root"),
+        # file outside root where root path is a prefix of the file path string-wise
+        ("../root.txt", ValueError, "Relative path points outside the root"),
     ]
 )
 def test_validate(sample_directory, relpath, exception_type, match):