Repository: iterative/dvc

Issue:
permissions are not set right for directories DVC creates
* Potentially affects shared cache group logic which can be a security issue?
* Affects all local commands that create directories which is also a security issue.

**For example: **

`dvc get https://github.com/iterative/dataset-registry get-started/data.xml -o data/data.xml`

creates an intermediate directory `data` with `0o777` permissions on Mac in `~` and `/tmp`:

```
drwxrwxrwx  3 ivan  wheel    96B Nov 13 12:47 data
```

It might be related to this [note](https://docs.python.org/3/library/os.html#os.makedirs): 

> Changed in version 3.7: The mode argument no longer affects the file permission bits of newly-created intermediate-level directories.

It also can be reproduced on Linux. More context here - https://discordapp.com/channels/485586884165107732/565699007037571084/644272213331279872


Discussion:

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1cc91e222..1a1fb7601 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -9,3 +9,8 @@ repos:
     hooks:
     - id: flake8
       language_version: python3
+-   repo: https://github.com/asottile/reorder_python_imports
+    rev: v1.8.0
+    hooks:
+    - id: reorder-python-imports
+      language_version: python3
diff --git a/.restyled.yaml b/.restyled.yaml
index d5eeec715..75c31e5ee 100644
--- a/.restyled.yaml
+++ b/.restyled.yaml
@@ -9,3 +9,13 @@ restylers:
     - "**/*.py"
     interpreters:
     - python
+  - name: reorder-python-imports
+    image: restyled/restyler-reorder-python-imports:v1.6.0
+    command:
+    - reorder-python-imports
+    - "--exit-zero-even-if-changed"
+    arguments: []
+    include:
+    - "**/*.py"
+    interpreters:
+    - python
diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py
index 864c9e5f5..a7e9ab5f4 100644
--- a/dvc/utils/__init__.py
+++ b/dvc/utils/__init__.py
@@ -152,9 +152,12 @@ def makedirs(path, exist_ok=False, mode=None):
         _makedirs(path, exist_ok=exist_ok)
         return
 
-    umask = os.umask(0)
+    # utilize umask to set proper permissions since Python 3.7 the `mode`
+    # `makedirs` argument no longer affects the file permission bits of
+    # newly-created intermediate-level directories.
+    umask = os.umask(0o777 - mode)
     try:
-        _makedirs(path, exist_ok=exist_ok, mode=mode)
+        _makedirs(path, exist_ok=exist_ok)
     finally:
         os.umask(umask)
 
diff --git a/dvc/utils/compat.py b/dvc/utils/compat.py
index 7d66dd090..4863e4a56 100644
--- a/dvc/utils/compat.py
+++ b/dvc/utils/compat.py
@@ -85,8 +85,6 @@ def _makedirs(name, mode=0o777, exist_ok=False):
             if e.errno != errno.EEXIST:
                 raise
         cdir = os.curdir
-        if isinstance(tail, bytes):
-            cdir = bytes(os.curdir, "ASCII")
         if tail == cdir:
             return
     try:


Test patch:
diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py
index 73f489e25..594fdc332 100644
--- a/tests/func/test_utils.py
+++ b/tests/func/test_utils.py
@@ -1,6 +1,10 @@
+# encoding: utf-8
 import filecmp
 import os
 import shutil
+import stat
+
+import pytest
 
 from dvc import utils
 from tests.basic_env import TestDvc
@@ -69,3 +73,18 @@ class TestUtils(TestDvc):
         )
 
         assert expected == utils.boxify("message")
+
+
+@pytest.mark.skipif(os.name == "nt", reason="Not supported for Windows.")
+def test_makedirs_permissions(tmpdir):
+    dir_mode = 0o755
+    os.chdir(str(tmpdir))
+    intermediate_dir = "тестовая-директория"
+    test_dir = os.path.join(intermediate_dir, "data")
+
+    assert not os.path.exists(intermediate_dir)
+
+    utils.makedirs(test_dir, mode=dir_mode)
+
+    assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode
+    assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode