Simplify check for symlinks that resolve outside root (#4221)

This PR does not change any behaviour.

There have been 1-2 issues about symlinks recently. Both over and under
resolving can cause problems. This makes a case where we resolve more
explicit and prevent a resolved path from leaking out via the return.
This commit is contained in:
Shantanu 2024-02-10 23:55:01 -08:00 committed by GitHub
parent dab37a6a11
commit a20100395c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 22 additions and 25 deletions

View File

@ -50,9 +50,9 @@
gen_python_files, gen_python_files,
get_gitignore, get_gitignore,
get_root_relative_path, get_root_relative_path,
normalize_path_maybe_ignore,
parse_pyproject_toml, parse_pyproject_toml,
path_is_excluded, path_is_excluded,
resolves_outside_root_or_cannot_stat,
wrap_stream_for_windows, wrap_stream_for_windows,
) )
from black.handle_ipynb_magics import ( from black.handle_ipynb_magics import (
@ -763,12 +763,9 @@ def get_sources(
) )
continue continue
normalized_path: Optional[str] = normalize_path_maybe_ignore( if resolves_outside_root_or_cannot_stat(path, root, report):
path, root, report
)
if normalized_path is None:
if verbose: if verbose:
out(f'Skipping invalid source: "{normalized_path}"', fg="red") out(f'Skipping invalid source: "{path}"', fg="red")
continue continue
if is_stdin: if is_stdin:
@ -780,7 +777,7 @@ def get_sources(
continue continue
if verbose: if verbose:
out(f'Found input source: "{normalized_path}"', fg="blue") out(f'Found input source: "{path}"', fg="blue")
sources.add(path) sources.add(path)
elif path.is_dir(): elif path.is_dir():
path = root / (path.resolve().relative_to(root)) path = root / (path.resolve().relative_to(root))

View File

@ -254,26 +254,24 @@ def get_gitignore(root: Path) -> PathSpec:
raise raise
def normalize_path_maybe_ignore( def resolves_outside_root_or_cannot_stat(
path: Path, path: Path,
root: Path, root: Path,
report: Optional[Report] = None, report: Optional[Report] = None,
) -> Optional[str]: ) -> bool:
"""Normalize `path`. May return `None` if `path` was ignored. """
Returns whether the path is a symbolic link that points outside the
`report` is where "path ignored" output goes. root directory. Also returns True if we failed to resolve the path.
""" """
try: try:
abspath = path if path.is_absolute() else Path.cwd() / path if sys.version_info < (3, 8, 6):
normalized_path = abspath.resolve() path = path.absolute() # https://bugs.python.org/issue33660
root_relative_path = get_root_relative_path(normalized_path, root, report) resolved_path = path.resolve()
return get_root_relative_path(resolved_path, root, report) is None
except OSError as e: except OSError as e:
if report: if report:
report.path_ignored(path, f"cannot be read because {e}") report.path_ignored(path, f"cannot be read because {e}")
return None return True
return root_relative_path
def get_root_relative_path( def get_root_relative_path(
@ -369,8 +367,7 @@ def gen_python_files(
report.path_ignored(child, "matches the --force-exclude regular expression") report.path_ignored(child, "matches the --force-exclude regular expression")
continue continue
normalized_path = normalize_path_maybe_ignore(child, root, report) if resolves_outside_root_or_cannot_stat(child, root, report):
if normalized_path is None:
continue continue
if child.is_dir(): if child.is_dir():

View File

@ -1760,12 +1760,15 @@ def test_bpo_33660_workaround(self) -> None:
return return
# https://bugs.python.org/issue33660 # https://bugs.python.org/issue33660
# Can be removed when we drop support for Python 3.8.5
root = Path("/") root = Path("/")
with change_directory(root): with change_directory(root):
path = Path("workspace") / "project" path = Path("workspace") / "project"
report = black.Report(verbose=True) report = black.Report(verbose=True)
normalized_path = black.normalize_path_maybe_ignore(path, root, report) resolves_outside = black.resolves_outside_root_or_cannot_stat(
self.assertEqual(normalized_path, "workspace/project") path, root, report
)
self.assertIs(resolves_outside, False)
def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None: def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None:
if system() != "Windows": if system() != "Windows":
@ -1778,13 +1781,13 @@ def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None:
os.system(f"mklink /J {junction_dir} {junction_target_outside_of_root}") os.system(f"mklink /J {junction_dir} {junction_target_outside_of_root}")
report = black.Report(verbose=True) report = black.Report(verbose=True)
normalized_path = black.normalize_path_maybe_ignore( resolves_outside = black.resolves_outside_root_or_cannot_stat(
junction_dir, root, report junction_dir, root, report
) )
# Manually delete for Python < 3.8 # Manually delete for Python < 3.8
os.system(f"rmdir {junction_dir}") os.system(f"rmdir {junction_dir}")
self.assertEqual(normalized_path, None) self.assertIs(resolves_outside, True)
def test_newline_comment_interaction(self) -> None: def test_newline_comment_interaction(self) -> None:
source = "class A:\\\r\n# type: ignore\n pass\n" source = "class A:\\\r\n# type: ignore\n pass\n"