From 4249da1ecfaca9541d64f0d3c7f08fb22e73e3b9 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Sun, 18 Sep 2022 19:30:03 -0400 Subject: [PATCH 1/2] Catch an edge case in expand._assert_local() Using str.startswith() has an edge case where someone can access files outside the root directory. For example, consider the case where the root directory is "/home/user/my-package" but some secrets are stored in "/home/user/my-package-secrets". Evaluating a check that "/home/user/my-package-secrets".startswith("/home/user/my-package") will return True, but the statement's intention is that no file outside of "/home/user/my-package" can be accessed. Using pathlib.Path.resolve() and pathlib.Path.parents eliminates this edge case. --- setuptools/config/expand.py | 5 ++++- setuptools/tests/config/test_expand.py | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/setuptools/config/expand.py b/setuptools/config/expand.py index 384504d879..1497b20ca0 100644 --- a/setuptools/config/expand.py +++ b/setuptools/config/expand.py @@ -41,6 +41,7 @@ Union, cast ) +from pathlib import Path from types import ModuleType from distutils.errors import DistutilsOptionError @@ -150,7 +151,9 @@ def _read_file(filepath: Union[bytes, _Path]) -> str: def _assert_local(filepath: _Path, root_dir: str): - if not os.path.abspath(filepath).startswith(root_dir): + # NOTE: Path.resolve() will raise RuntimeError if an infinite loop is + # encountered along the resolution path of root_dir or file_path. + if Path(root_dir).resolve() not in Path(filepath).resolve().parents: msg = f"Cannot access {filepath!r} (or anything outside {root_dir!r})" raise DistutilsOptionError(msg) diff --git a/setuptools/tests/config/test_expand.py b/setuptools/tests/config/test_expand.py index 523779a8ed..87e00438d5 100644 --- a/setuptools/tests/config/test_expand.py +++ b/setuptools/tests/config/test_expand.py @@ -1,4 +1,5 @@ import os +from pathlib import Path import pytest @@ -45,6 +46,10 @@ def test_read_files(tmp_path, monkeypatch): } write_files(files, dir_) + secrets = Path(str(dir_) + "secrets") + secrets.mkdir(exist_ok=True) + write_files({"secrets.txt": "secret keys"}, secrets) + with monkeypatch.context() as m: m.chdir(dir_) assert expand.read_files(list(files)) == "a\nb\nc" @@ -53,6 +58,10 @@ def test_read_files(tmp_path, monkeypatch): with pytest.raises(DistutilsOptionError, match=cannot_access_msg): expand.read_files(["../a.txt"]) + cannot_access_secrets_msg = r"Cannot access '.*secrets\.txt'" + with pytest.raises(DistutilsOptionError, match=cannot_access_secrets_msg): + expand.read_files(["../dir_secrets/secrets.txt"]) + # Make sure the same APIs work outside cwd assert expand.read_files(list(files), dir_) == "a\nb\nc" with pytest.raises(DistutilsOptionError, match=cannot_access_msg): From 063aecddad96472c905edddaf8fe1ed03a37df18 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 22 Sep 2022 20:16:31 -0400 Subject: [PATCH 2/2] Use abspath() instead of resolve() in expand._assert_local() 4249da1ecf uses `pathlib.Path.resolve()` instead of `os.path.abspath()` to canonicalize path names. `resolve()` resolves symlinks, whereas `abspath()` does not. `resolve()` can also raise a `RuntimeError` if infinite loops are discovered while resolving the path. There is some concern that using `resolve()` would not be backwards compatible. This commit switches back to `abspath()` but still uses `Path.parents` to avoid the edge case. See PR #3595 for more details. --- setuptools/config/expand.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/setuptools/config/expand.py b/setuptools/config/expand.py index 1497b20ca0..7a100d69c9 100644 --- a/setuptools/config/expand.py +++ b/setuptools/config/expand.py @@ -151,9 +151,7 @@ def _read_file(filepath: Union[bytes, _Path]) -> str: def _assert_local(filepath: _Path, root_dir: str): - # NOTE: Path.resolve() will raise RuntimeError if an infinite loop is - # encountered along the resolution path of root_dir or file_path. - if Path(root_dir).resolve() not in Path(filepath).resolve().parents: + if Path(os.path.abspath(root_dir)) not in Path(os.path.abspath(filepath)).parents: msg = f"Cannot access {filepath!r} (or anything outside {root_dir!r})" raise DistutilsOptionError(msg)