From 6a39fc4b5441282a8078bebe648c337ef562c056 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 14:42:52 +0900 Subject: [PATCH 01/16] validate basetemp argument --- src/_pytest/main.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 61eb7ca74c2..1b789d8ff3a 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -12,6 +12,8 @@ from typing import Sequence from typing import Tuple from typing import Union +import pathlib +import argparse import attr import py @@ -167,6 +169,7 @@ def pytest_addoption(parser): "--basetemp", dest="basetemp", default=None, + type=validate_basetemp, metavar="dir", help=( "base temporary directory for this test run." @@ -174,6 +177,16 @@ def pytest_addoption(parser): ), ) +def validate_basetemp(path): + # GH 7119 + cwd = pathlib.Path.cwd() + given_path = pathlib.Path(path).resolve() + + if given_path == cwd or str(given_path) in str(cwd): + msg = "basetemp should not be '' or . or any parent folder of the cwd" + raise argparse.ArgumentTypeError(msg) + return str(given_path) + def wrap_session( config: Config, doit: Callable[[Config, "Session"], Optional[Union[int, ExitCode]]] From 7f65c353271cf7e4b15451507aa9d236bc81953e Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 14:43:22 +0900 Subject: [PATCH 02/16] added issue in changelog --- changelog/7119.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/7119.bugfix.rst diff --git a/changelog/7119.bugfix.rst b/changelog/7119.bugfix.rst new file mode 100644 index 00000000000..6428681b037 --- /dev/null +++ b/changelog/7119.bugfix.rst @@ -0,0 +1 @@ +Raise exception if ``basetemp`` argument is empty string or . or parent folder of current working directory. \ No newline at end of file From 882fdebfc4415ead811fd77af616c2f222f67b31 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 14:43:40 +0900 Subject: [PATCH 03/16] added author --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index b59ebc2a27f..b21422a969d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -221,6 +221,7 @@ Pedro Algarvio Philipp Loose Pieter Mulder Piotr Banaszkiewicz +Prashant Anand Pulkit Goyal Punyashloka Biswal Quentin Pradet From 4339fd7506946ac6cccce107b2b1452a016a432f Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 15:01:13 +0900 Subject: [PATCH 04/16] run pre-commit --- src/_pytest/main.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1b789d8ff3a..f6cb0dea010 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -1,8 +1,10 @@ """ core implementation of testing process: init, session, runtest loop. """ +import argparse import fnmatch import functools import importlib import os +import pathlib import sys from typing import Callable from typing import Dict @@ -12,8 +14,6 @@ from typing import Sequence from typing import Tuple from typing import Union -import pathlib -import argparse import attr import py @@ -177,6 +177,7 @@ def pytest_addoption(parser): ), ) + def validate_basetemp(path): # GH 7119 cwd = pathlib.Path.cwd() From e18b2f735d4bcc44c4f2c798c83e50062e299aa9 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 15:06:00 +0900 Subject: [PATCH 05/16] added newline at end of 7119.bugfix.rst --- changelog/7119.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/7119.bugfix.rst b/changelog/7119.bugfix.rst index 6428681b037..49d05921e09 100644 --- a/changelog/7119.bugfix.rst +++ b/changelog/7119.bugfix.rst @@ -1 +1 @@ -Raise exception if ``basetemp`` argument is empty string or . or parent folder of current working directory. \ No newline at end of file +Raise exception if ``basetemp`` argument is empty string or . or parent folder of current working directory. From 9aa3a647e1b6ece075e41bcd008bd1f546046fca Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 16:18:28 +0900 Subject: [PATCH 06/16] fixed basetemp validation --- src/_pytest/main.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index f6cb0dea010..d02415a581c 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -181,12 +181,11 @@ def pytest_addoption(parser): def validate_basetemp(path): # GH 7119 cwd = pathlib.Path.cwd() - given_path = pathlib.Path(path).resolve() - if given_path == cwd or str(given_path) in str(cwd): + if path == "" or path == "." or path in str(cwd): msg = "basetemp should not be '' or . or any parent folder of the cwd" raise argparse.ArgumentTypeError(msg) - return str(given_path) + return path def wrap_session( From 58ea074ef00355329ad72850c755b7cabbe2f37f Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 18:02:25 +0900 Subject: [PATCH 07/16] fixed basetemp validation --- src/_pytest/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index d02415a581c..e1f6c87e8fc 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -182,7 +182,7 @@ def validate_basetemp(path): # GH 7119 cwd = pathlib.Path.cwd() - if path == "" or path == "." or path in str(cwd): + if path == "" or path == "." or str(cwd).startswith(path): msg = "basetemp should not be '' or . or any parent folder of the cwd" raise argparse.ArgumentTypeError(msg) return path From 8f70e6294bd7dd5f66976b34b1789490be6678e9 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Wed, 6 May 2020 18:33:38 +0900 Subject: [PATCH 08/16] added tests for validate_basetemp --- testing/test_main.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/testing/test_main.py b/testing/test_main.py index 07aca3a1e24..7eefd47ada5 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -1,7 +1,9 @@ +import argparse from typing import Optional import pytest from _pytest.config import ExitCode +from _pytest.main import validate_basetemp from _pytest.pytester import Testdir @@ -75,3 +77,10 @@ def pytest_sessionfinish(): assert result.ret == ExitCode.NO_TESTS_COLLECTED assert result.stdout.lines[-1] == "collected 0 items" assert result.stderr.lines == ["Exit: exit_pytest_sessionfinish"] + + +@pytest.mark.parametrize("basetemp", ["", "."]) +def test_validate_basetemp(testdir, basetemp): + msg = "basetemp should not be '' or . or any parent folder of the cwd" + with pytest.raises(argparse.ArgumentTypeError, match=msg): + validate_basetemp(basetemp) From deab886195c780f8f1e42156985f62a0316402ce Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 07:52:20 +0900 Subject: [PATCH 09/16] import pathlib from _pytest Co-authored-by: Bruno Oliveira --- src/_pytest/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index e1f6c87e8fc..44bf4a92604 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -4,7 +4,7 @@ import functools import importlib import os -import pathlib +from _pytest.pathlib import Path import sys from typing import Callable from typing import Dict From 35ac4e52bc85cc886408dd0ffd0618f65b69288a Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 07:52:51 +0900 Subject: [PATCH 10/16] add type hints Co-authored-by: Ran Benita --- src/_pytest/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 44bf4a92604..97d22479e6a 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -178,7 +178,7 @@ def pytest_addoption(parser): ) -def validate_basetemp(path): +def validate_basetemp(path: str) -> str: # GH 7119 cwd = pathlib.Path.cwd() From d8f37f6c40d37e42a9cb1d37165807021891c59c Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 07:53:30 +0900 Subject: [PATCH 11/16] update error message Co-authored-by: Ran Benita --- src/_pytest/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 97d22479e6a..69481ed1fe2 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -183,7 +183,7 @@ def validate_basetemp(path: str) -> str: cwd = pathlib.Path.cwd() if path == "" or path == "." or str(cwd).startswith(path): - msg = "basetemp should not be '' or . or any parent folder of the cwd" + msg = "basetemp must not be empty, the current working directory or any parent directory of it" raise argparse.ArgumentTypeError(msg) return path From ec090f4e1a3c3c779596cb5c1521578b3d6170ef Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 08:21:46 +0900 Subject: [PATCH 12/16] use pathlib.Path.parents to check for ancestors --- src/_pytest/main.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 69481ed1fe2..d00d286bade 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -4,7 +4,6 @@ import functools import importlib import os -from _pytest.pathlib import Path import sys from typing import Callable from typing import Dict @@ -28,6 +27,7 @@ from _pytest.config import UsageError from _pytest.fixtures import FixtureManager from _pytest.outcomes import exit +from _pytest.pathlib import Path from _pytest.reports import CollectReport from _pytest.runner import collect_one_node from _pytest.runner import SetupState @@ -180,11 +180,29 @@ def pytest_addoption(parser): def validate_basetemp(path: str) -> str: # GH 7119 - cwd = pathlib.Path.cwd() + msg = "basetemp must not be empty, the current working directory or any parent directory of it" - if path == "" or path == "." or str(cwd).startswith(path): - msg = "basetemp must not be empty, the current working directory or any parent directory of it" + # empty path + if not path: raise argparse.ArgumentTypeError(msg) + + def is_ancestor(base: Path, query: Path) -> bool: + """ return True if query is an ancestor of base, else False.""" + if base == query: + return True + for parent in base.parents: + if parent == query: + return True + return False + + # check if path is an ancestor of cwd + if is_ancestor(Path.cwd(), Path(path).absolute()): + raise argparse.ArgumentTypeError(msg) + + # check symlinks for ancestors + if is_ancestor(Path.cwd().resolve(), Path(path).resolve()): + raise argparse.ArgumentTypeError(msg) + return path From e7280ce131022b85b81b81cf10692f7d67677c12 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 08:22:18 +0900 Subject: [PATCH 13/16] updated error message in test --- testing/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_main.py b/testing/test_main.py index 7eefd47ada5..d380afa5b52 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -81,6 +81,6 @@ def pytest_sessionfinish(): @pytest.mark.parametrize("basetemp", ["", "."]) def test_validate_basetemp(testdir, basetemp): - msg = "basetemp should not be '' or . or any parent folder of the cwd" + msg = "basetemp must not be empty, the current working directory or any parent directory of it" with pytest.raises(argparse.ArgumentTypeError, match=msg): validate_basetemp(basetemp) From 53a05a443327384f264283f46ce06db1b58fdba7 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Sat, 9 May 2020 08:42:32 +0900 Subject: [PATCH 14/16] updated validate basetemp tests and added integration tests --- testing/test_main.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/testing/test_main.py b/testing/test_main.py index d380afa5b52..ee8349a9f33 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -79,8 +79,22 @@ def pytest_sessionfinish(): assert result.stderr.lines == ["Exit: exit_pytest_sessionfinish"] -@pytest.mark.parametrize("basetemp", ["", "."]) -def test_validate_basetemp(testdir, basetemp): +@pytest.mark.parametrize("basetemp", ["foo", "foo/bar"]) +def test_validate_basetemp_ok(tmp_path, basetemp, monkeypatch): + monkeypatch.chdir(str(tmp_path)) + validate_basetemp(tmp_path / basetemp) + + +@pytest.mark.parametrize("basetemp", ["", ".", ".."]) +def test_validate_basetemp_fails(tmp_path, basetemp, monkeypatch): + monkeypatch.chdir(str(tmp_path)) msg = "basetemp must not be empty, the current working directory or any parent directory of it" with pytest.raises(argparse.ArgumentTypeError, match=msg): + if basetemp: + basetemp = tmp_path / basetemp validate_basetemp(basetemp) + + +def test_validate_basetemp_integration(testdir): + result = testdir.runpytest("--basetemp=.") + result.stderr.fnmatch_lines("*basetemp must not be*") From cb4cd4065eb4147b59914d03caabbb4a8e3e5c61 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Mon, 11 May 2020 00:06:08 +0900 Subject: [PATCH 15/16] Update changelog Co-authored-by: Ran Benita --- changelog/7119.bugfix.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog/7119.bugfix.rst b/changelog/7119.bugfix.rst index 49d05921e09..6cef9883633 100644 --- a/changelog/7119.bugfix.rst +++ b/changelog/7119.bugfix.rst @@ -1 +1,2 @@ -Raise exception if ``basetemp`` argument is empty string or . or parent folder of current working directory. +Exit with an error if the ``--basetemp`` argument is empty, the current working directory or parent directory of it. +This is done to protect against accidental data loss, as any directory passed to this argument is cleared. From 5ba5b1f81454edb2b0fd29cda75ea55f42728a05 Mon Sep 17 00:00:00 2001 From: Prashant Anand Date: Mon, 11 May 2020 00:07:29 +0900 Subject: [PATCH 16/16] updated changelog filename --- changelog/{7119.bugfix.rst => 7119.improvement.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{7119.bugfix.rst => 7119.improvement.rst} (100%) diff --git a/changelog/7119.bugfix.rst b/changelog/7119.improvement.rst similarity index 100% rename from changelog/7119.bugfix.rst rename to changelog/7119.improvement.rst