Repository: ambv/black (python)

Issue:
Crash if there is a symlink to a resource outside of the root directory
Hello there, thank you for this great tool!

Black crashes when it finds a symbolic link pointing to a file or a directory outside of the root directory.

**Steps to reproduce**

- Create the following structure:
```
base
  |-- repo
  |     |-- dummy.py
  |     `-- resource -> ../resource
  `-- resource
```
- Initiate `base/repo` as a Git repository (so as to set the root directory);
- Call `black .` within `base/repo`;
- Expected behavior: *Black* ends as expected;
- Current behavior: throws an exception:
```
Traceback (most recent call last):
  File "/home/neraste/.virtualenvs/test_black/bin/black", line 11, in <module>
    sys.exit(main())
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/black.py", line 347, in main
    gen_python_files_in_dir(p, root, include_regex, exclude_regex, report)
  File "/home/neraste/.virtualenvs/test_black/lib/python3.6/site-packages/black.py", line 2942, in gen_python_files_in_dir
    normalized_path = "/" + child.resolve().relative_to(root).as_posix()
  File "/usr/lib64/python3.6/pathlib.py", line 872, in relative_to
    .format(str(self), str(formatted)))
ValueError: '/data/test_black/base/resource' does not start with '/data/test_black/base/repo'
```

As pointed out in the backtrace, this is due to the line 2942, where the resolved path of the files is made relative to the root directory. This is indeed impossible if the file is outside of the root directory. One can wonder if it makes sense to have an external link within a repository, but, well I am in this case.

Adding the conflicting file/folder to the exclude list does not work.

**Possible workaround**

A possible workaround could be to simply ignore this case and do not handle this simlinked resource:
```python
        try:
            normalized_path = "/" + child.resolve().relative_to(root).as_posix()

        except ValueError:
            if child.is_symlink():
                continue                                                        

            raise
```


**Additional information**

Operating system: GNU/Linux OpenSUSE Tumbleweed
Python version: 3.6
*Black* version: 18.6b2
Does also happen on master: yes


Pull request:
Ignore symbolic links pointing outside of the root directory
Within the structure of files to analyze, if there is a symbolic link pointing to a file or a directory outside of the root directory, this symlink is now ignored, as it appears to be out of the scope of the analyzed project.

This PR fixes #338.

Patch:
diff --git a/README.md b/README.md
index 0f22482..afcbf4c 100644
--- a/README.md
+++ b/README.md
@@ -684,8 +684,7 @@ to do this.
 ### Visual Studio Code
 
 Use the [Python extension](https://marketplace.visualstudio.com/items?itemName=ms-python.python)
-([instructions](https://code.visualstudio.com/docs/python/editing#_formatting))
-or [joslarson.black-vscode](https://marketplace.visualstudio.com/items?itemName=joslarson.black-vscode).
+([instructions](https://code.visualstudio.com/docs/python/editing#_formatting)).
 
 
 ### SublimeText 3
diff --git a/black.py b/black.py
index 9d9bada..e3b3882 100644
--- a/black.py
+++ b/black.py
@@ -2941,11 +2941,24 @@ def gen_python_files_in_dir(
     """Generate all files under `path` whose paths are not excluded by the
     `exclude` regex, but are included by the `include` regex.
 
+    Symbolic links pointing outside of the root directory are ignored.
+
     `report` is where output about exclusions goes.
     """
     assert root.is_absolute(), f"INTERNAL ERROR: `root` must be absolute but is {root}"
     for child in path.iterdir():
-        normalized_path = "/" + child.resolve().relative_to(root).as_posix()
+        try:
+            normalized_path = "/" + child.resolve().relative_to(root).as_posix()
+        except ValueError:
+            if child.is_symlink():
+                report.path_ignored(
+                    child,
+                    "is a symbolic link that points outside of the root directory",
+                )
+                continue
+
+            raise
+
         if child.is_dir():
             normalized_path += "/"
         exclude_match = exclude.search(normalized_path)


Test patch:
diff --git a/tests/test_black.py b/tests/test_black.py
index 84b7a61..3418df9 100644
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -11,7 +11,7 @@
 from tempfile import TemporaryDirectory
 from typing import Any, BinaryIO, Generator, List, Tuple, Iterator
 import unittest
-from unittest.mock import patch
+from unittest.mock import patch, MagicMock
 
 from click import unstyle
 from click.testing import CliRunner
@@ -1162,6 +1162,49 @@ def test_assert_equivalent_different_asts(self) -> None:
         with self.assertRaises(AssertionError):
             black.assert_equivalent("{}", "None")
 
+    def test_symlink_out_of_root_directory(self) -> None:
+        # prepare argumens
+        path = MagicMock()
+        root = THIS_DIR
+        child = MagicMock()
+        include = re.compile(black.DEFAULT_INCLUDES)
+        exclude = re.compile(black.DEFAULT_EXCLUDES)
+        report = black.Report()
+
+        # set the behavior of mock arguments
+        # child should behave like a symlink which resolved path is clearly
+        # outside of the root directory
+        path.iterdir.return_value = [child]
+        child.resolve.return_value = Path("/a/b/c")
+        child.is_symlink.return_value = True
+
+        # call the method
+        # it should not raise any error
+        list(black.gen_python_files_in_dir(path, root, include, exclude, report))
+
+        # check the call of the methods of the mock objects
+        path.iterdir.assert_called_once()
+        child.resolve.assert_called_once()
+        child.is_symlink.assert_called_once()
+
+        # set the behavior of mock arguments
+        # child should behave like a strange file which resolved path is clearly
+        # outside of the root directory
+        child.is_symlink.return_value = False
+
+        # call the method
+        # it should raise a ValueError
+        with self.assertRaises(ValueError):
+            list(black.gen_python_files_in_dir(path, root, include, exclude, report))
+
+        # check the call of the methods of the mock objects
+        path.iterdir.assert_called()
+        self.assertEqual(path.iterdir.call_count, 2)
+        child.resolve.assert_called()
+        self.assertEqual(child.resolve.call_count, 2)
+        child.is_symlink.assert_called()
+        self.assertEqual(child.is_symlink.call_count, 2)
+
 
 if __name__ == "__main__":
     unittest.main(module="test_black")