Repository: timothycrosley/isort (python)

Issue:
isort can trash Python installations
Heya :)

This is a bit of a corner case, but it's happened to me more than once already...
So `isort` is prooobably not meant to be used on Python stdlib code, since it can be a little tricky with imports in places.

However, if you happen to have a virtualenv within your working directory, or a `.tox` directory containing those, `isort -rc .` may recurse down into them, like in the logs below.

This will be a problem in any environment where the python stdlib files are writable, as they are on MacOS when you've installed Python with Homebrew; running within a virtualenv doesn't matter, since these files are usually linked into it, not copied.

```
$ isort -rc .
Fixing ./.tox/flake8/bin/python-config
Fixing ./.tox/flake8/bin/activate_this.py
[...]
Fixing ./.tox/flake8/lib/python3.7/os.py
[...]
Fixing ./.tox/flake8/lib/python3.7/site.py
[...]
```

At this point, I hit ctrl+c, because this wasn't what I wanted, removed the `.tox` directory and tried to re-run `isort`:

```
$ rm -r .tox
$ isort -rc .
Fatal Python error: initsite: Failed to import the site module
Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site.py", line 73, in <module>
    import os
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/os.py", line 30, in <module>
    from os.path import (altsep, curdir, defpath, devnull, extsep, pardir, pathsep,
ModuleNotFoundError: No module named 'os.path'; 'os' is not a package
$
```
Oh no! Remembering I'd seen this before, I tried to run a non-virtualenv `python3`:
```
~ $ python3
Fatal Python error: initsite: Failed to import the site module
Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site.py", line 73, in <module>
    import os
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/os.py", line 30, in <module>
    from os.path import (altsep, curdir, defpath, devnull, extsep, pardir, pathsep,
ModuleNotFoundError: No module named 'os.path'; 'os' is not a package
```
It's dead, Jim 😞 

This is fixed by reinstalling Python (`brew reinstall python3` for me) to replace the isort-tainted stdlib modules with fresh ones.

I think it'd be worth it to add special-casing in `isort` to ignore (by default; could be overridden with some sort of footgun switch) files in:

* `*/.tox/`
* `*/lib/python*.*`
* any `site-packages` directory
* any `dist-packages` directory (Debian special)

Maybe some others?

Pull request description:
Refuse to work in system/.tox/venv/... directories by default
This PR makes sure isort won't wander into directories where it shouldn't. By @jdufresne's suggestion, this list is based on that of [Black's](https://github.com/ambv/black/blob/024c9cab55da7bd3236fd88759c9735d6149b464/black.py#L53-L55), with my addition of "lib/pythonX.X", which appears within venvs/site-packages/etc., but should never appear elsewhere.

This behavior can be disabled with the `--unsafe` switch.

Fixes #759

Patch:
diff --git a/isort/main.py b/isort/main.py
index 9356c6a3..8cba7b40 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -285,11 +285,16 @@ def create_parser():
                         help='Tells isort to ignore whitespace differences when --check-only is being used.')
     parser.add_argument('-y', '--apply', dest='apply', action='store_true',
                         help='Tells isort to apply changes recursively without asking')
+    parser.add_argument('--unsafe', dest='unsafe', action='store_true',
+                        help='Tells isort to look for files in standard library directories, etc. '
+                             'where it may not be safe to operate in')
     parser.add_argument('files', nargs='*', help='One or more Python source files that need their imports sorted.')
 
     arguments = {key: value for key, value in vars(parser.parse_args()).items() if value}
     if 'dont_order_by_type' in arguments:
         arguments['order_by_type'] = False
+    if arguments.pop('unsafe', False):
+        arguments['safety_excludes'] = False
     return arguments
 
 
diff --git a/isort/settings.py b/isort/settings.py
index 79ae6c4c..cb047158 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -27,6 +27,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 import fnmatch
 import io
 import os
+import re
 import posixpath
 import sys
 import warnings
@@ -48,6 +49,10 @@ except ImportError:
 MAX_CONFIG_SEARCH_DEPTH = 25  # The number of parent directories isort will look for a config file within
 DEFAULT_SECTIONS = ('FUTURE', 'STDLIB', 'THIRDPARTY', 'FIRSTPARTY', 'LOCALFOLDER')
 
+safety_exclude_re = re.compile(
+    r"/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|_build|buck-out|build|dist|lib/python[0-9].[0-9]+)/"
+)
+
 WrapModes = ('GRID', 'VERTICAL', 'HANGING_INDENT', 'VERTICAL_HANGING_INDENT', 'VERTICAL_GRID', 'VERTICAL_GRID_GROUPED',
              'VERTICAL_GRID_GROUPED_NO_COMMA', 'NOQA')
 WrapModes = namedtuple('WrapModes', WrapModes)(*range(len(WrapModes)))
@@ -145,7 +150,9 @@ default = {'force_to_top': [],
            'show_diff': False,
            'ignore_whitespace': False,
            'no_lines_before': [],
-           'no_inline_sort': False}
+           'no_inline_sort': False,
+           'safety_excludes': True,
+           }
 
 
 @lru_cache()
@@ -292,8 +299,13 @@ def _get_config_data(file_path, sections):
 
 def should_skip(filename, config, path='/'):
     """Returns True if the file should be skipped based on the passed in settings."""
+    normalized_path = posixpath.join(path.replace('\\', '/'), filename)
+
+    if config['safety_excludes'] and safety_exclude_re.search(normalized_path):
+        return True
+
     for skip_path in config['skip']:
-        if posixpath.abspath(posixpath.join(path.replace('\\', '/'), filename)) == posixpath.abspath(skip_path.replace('\\', '/')):
+        if posixpath.abspath(normalized_path) == posixpath.abspath(skip_path.replace('\\', '/')):
             return True
 
     position = os.path.split(filename)


Test patch:
diff --git a/test_isort.py b/test_isort.py
index 25ad87e7..732812a6 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -25,11 +25,12 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 from tempfile import NamedTemporaryFile
 import io
+import os
 import sys
 
 import pytest
 
-from isort import finders
+from isort import finders, main, settings
 from isort.isort import SortImports
 from isort.utils import exists_case_sensitive
 from isort.main import is_python_file
@@ -2592,3 +2593,19 @@ def test_monkey_patched_urllib():
         # Previous versions of isort monkey patched urllib which caused unusual
         # importing for other projects.
         from urllib import quote  # noqa: F401
+
+
+@pytest.mark.parametrize('enabled', (False, True))
+def test_safety_excludes(tmpdir, enabled):
+    tmpdir.join("victim.py").write("# ...")
+    tmpdir.mkdir(".tox").join("verysafe.py").write("# ...")
+    tmpdir.mkdir("lib").mkdir("python3.7").join("importantsystemlibrary.py").write("# ...")
+    config = dict(settings.default.copy(), safety_excludes=enabled)
+    skipped = []
+    file_names = set(os.path.relpath(f, str(tmpdir)) for f in main.iter_source_code([str(tmpdir)], config, skipped))
+    if enabled:
+        assert file_names == {'victim.py'}
+        assert len(skipped) == 2
+    else:
+        assert file_names == {'.tox/verysafe.py', 'lib/python3.7/importantsystemlibrary.py', 'victim.py'}
+        assert not skipped