Skip to content

Commit

Permalink
Catch an edge case in expand._assert_local()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mssalvatore committed Sep 18, 2022
1 parent ba3995e commit 4249da1
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
5 changes: 4 additions & 1 deletion setuptools/config/expand.py
Expand Up @@ -41,6 +41,7 @@
Union,
cast
)
from pathlib import Path
from types import ModuleType

from distutils.errors import DistutilsOptionError
Expand Down Expand Up @@ -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)

Expand Down
9 changes: 9 additions & 0 deletions setuptools/tests/config/test_expand.py
@@ -1,4 +1,5 @@
import os
from pathlib import Path

import pytest

Expand Down Expand Up @@ -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"
Expand All @@ -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):
Expand Down

0 comments on commit 4249da1

Please sign in to comment.