Repository: asottile/all-repos (language: python)

Issue:
Setting output_dir to "." does rmtree to the current directory
I noticed running `all-repos-clone` with a configuration similar to the one below will remove all files and directories in the current directory.

```json
{
    "output_dir": ".",
    "source": "all_repos.source.github_org",
    "source_settings":  {
        "api_key": "redacted",
        "private": true,
        "org": "example"
    },
    "push": "all_repos.push.github_pull_request",
    "push_settings": {
        "api_key": "redacted",
        "username": "FirstLast"
    },
    "include": "prefix.*"
}
```

It is clear now the intent is to have `output_dir` be something other than the current directory.  However, I think a check to prevent removing the current directory should be considered.



Discussion / hints:
yeah `.` is kinda a bad decision there :P -- feel free to send a patch

Fix patch:
diff --git a/all_repos/config.py b/all_repos/config.py
index e9d74ee..c73e884 100644
--- a/all_repos/config.py
+++ b/all_repos/config.py
@@ -9,6 +9,8 @@
 from typing import NamedTuple
 from typing import Pattern
 
+REPOS_JSON_FILES = frozenset(('repos.json', 'repos_filtered.json'))
+
 
 class Config(NamedTuple):
     output_dir: str
@@ -45,6 +47,25 @@ def _check_permissions(filename: str) -> None:
         )
 
 
+def _check_output_dir(output_dir: str) -> None:
+    if os.path.exists(output_dir):
+        contents = set(os.listdir(output_dir))
+        if not contents:  # empty dir is ok!
+            return
+
+        if not (
+                contents >= REPOS_JSON_FILES and
+                all(
+                    os.path.isdir(os.path.join(output_dir, d))
+                    for d in contents - REPOS_JSON_FILES
+                )
+        ):
+            raise SystemExit(
+                'output_dir should only contain repos.json, '
+                'repos_filtered.json, and directories',
+            )
+
+
 def load_config(filename: str) -> Config:
     _check_permissions(filename)
     with open(filename) as f:
@@ -52,6 +73,7 @@ def load_config(filename: str) -> Config:
 
     output_dir = os.path.join(filename, '..', contents['output_dir'])
     output_dir = os.path.normpath(output_dir)
+    _check_output_dir(output_dir)
     source_module: Any = __import__(contents['source'], fromlist=['__trash'])
     source_settings = source_module.Settings(**contents['source_settings'])
     push_module: Any = __import__(contents['push'], fromlist=['__trash'])


Test patch:
diff --git a/tests/config_test.py b/tests/config_test.py
index 4b65a51..6a4d613 100644
--- a/tests/config_test.py
+++ b/tests/config_test.py
@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import os
+
 import pytest
 
 import all_repos.source.json_file
@@ -23,6 +25,23 @@ def test_load_config_too_permissive(file_config):
     )
 
 
+def test_load_config_output_dir_contains_file(file_config):
+    os.makedirs(file_config.output_dir, exist_ok=True)
+    file_config.output_dir.join('unsupported-file.txt').write('')
+    with pytest.raises(SystemExit) as excinfo:
+        load_config(file_config.cfg)
+    msg, = excinfo.value.args
+    assert msg == (
+        'output_dir should only contain repos.json, repos_filtered.json, '
+        'and directories'
+    )
+
+
+def test_load_config_empty_directory_is_ok(file_config):
+    os.makedirs(file_config.output_dir, exist_ok=True)
+    load_config(file_config.cfg)
+
+
 def test_get_cloned_repos(file_config):
     clone.main(('--config-filename', str(file_config.cfg)))
     cfg = load_config(file_config.cfg)