Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vcs: fix parsing of basic auth http(s) credentials #791

Merged
merged 3 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 29 additions & 119 deletions src/poetry/core/vcs/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,47 @@


PROTOCOL = r"\w+"
USER = r"[a-zA-Z0-9_.-]+"
# https://url.spec.whatwg.org/#forbidden-host-code-point
URL_RESTRICTED = r"[^/\?#:@<>\[\]\|]"
USER = rf"{URL_RESTRICTED}+"
USER_AUTH_HTTP = rf"((?P<username>{USER})(:(?P<password>{URL_RESTRICTED}*))?)"
RESOURCE = r"[a-zA-Z0-9_.-]+"
PORT = r"\d+"
PATH = r"[%\w~.\-\+/\\\$]+"
NAME = r"[%\w~.\-]+"
REV = r"[^@#]+?"
SUBDIR = r"[\w\-/\\]+"
PATTERN_SUFFIX = (
r"(?:"
rf"#(?:egg=.+?&subdirectory=|subdirectory=)(?P<subdirectory>{SUBDIR})"
r"|"
r"#egg=?.+"
r"|"
rf"[@#](?P<rev>{REV})(?:[&#](?:(?:egg=.+?&subdirectory=|subdirectory=)(?P<rev_subdirectory>{SUBDIR})|egg=.+?))?"
r")?"
r"$"
)

PATTERNS = [
re.compile(
r"^(git\+)?"
r"(?P<protocol>https?|git|ssh|rsync|file)://"
r"(?P<protocol>git|ssh|rsync|file)://"
rf"(?:(?P<user>{USER})@)?"
rf"(?P<resource>{RESOURCE})?"
rf"(:(?P<port>{PORT}))?"
rf"(?P<pathname>[:/\\]({PATH}[/\\])?"
rf"((?P<name>{NAME}?)(\.git|[/\\])?)?)"
r"(?:"
rf"#(?:egg=.+?&subdirectory=|subdirectory=)(?P<subdirectory>{SUBDIR})"
r"|"
r"#egg=?.+"
r"|"
rf"[@#](?P<rev>{REV})(?:[&#](?:(?:egg=.+?&subdirectory=|subdirectory=)(?P<rev_subdirectory>{SUBDIR})|egg=.+?))?"
r")?"
r"$"
rf"{PATTERN_SUFFIX}"
),
re.compile(
r"^(git\+)?"
r"(?P<protocol>https?)://"
rf"(?:(?P<user>{USER_AUTH_HTTP})@)?"
rf"(?P<resource>{RESOURCE})?"
rf"(:(?P<port>{PORT}))?"
rf"(?P<pathname>[:/\\]({PATH}[/\\])?"
rf"((?P<name>{NAME}?)(\.git|[/\\])?)?)"
rf"{PATTERN_SUFFIX}"
),
re.compile(
r"(git\+)?"
Expand All @@ -45,44 +61,23 @@
rf"(:(?P<port>{PORT}))?"
rf"(?P<pathname>({PATH})"
rf"(?P<name>{NAME})(\.git|/)?)"
r"(?:"
rf"#(?:egg=.+?&subdirectory=|subdirectory=)(?P<subdirectory>{SUBDIR})"
r"|"
r"#egg=?.+"
r"|"
rf"[@#](?P<rev>{REV})(?:[&#](?:(?:egg=.+?&subdirectory=|subdirectory=)(?P<rev_subdirectory>{SUBDIR})|egg=.+?))?"
r")?"
r"$"
rf"{PATTERN_SUFFIX}"
),
re.compile(
rf"^(?:(?P<user>{USER})@)?"
rf"(?P<resource>{RESOURCE})"
rf"(:(?P<port>{PORT}))?"
rf"(?P<pathname>([:/]{PATH}/)"
rf"(?P<name>{NAME})(\.git|/)?)"
r"(?:"
rf"#(?:egg=.+?&subdirectory=|subdirectory=)(?P<subdirectory>{SUBDIR})"
r"|"
r"#egg=?.+"
r"|"
rf"[@#](?P<rev>{REV})(?:[&#](?:(?:egg=.+?&subdirectory=|subdirectory=)(?P<rev_subdirectory>{SUBDIR})|egg=.+?))?"
r")?"
r"$"
rf"{PATTERN_SUFFIX}"
),
re.compile(
rf"((?P<user>{USER})@)?"
rf"(?P<resource>{RESOURCE})"
r"[:/]{{1,2}}"
rf"(?P<pathname>({PATH})"
rf"(?P<name>{NAME})(\.git|/)?)"
r"(?:"
rf"#(?:egg=.+?&subdirectory=|subdirectory=)(?P<subdirectory>{SUBDIR})"
r"|"
r"#egg=?.+"
r"|"
rf"[@#](?P<rev>{REV})(?:[&#](?:(?:egg=.+?&subdirectory=|subdirectory=)(?P<rev_subdirectory>{SUBDIR})|egg=.+?))?"
r")?"
r"$"
rf"{PATTERN_SUFFIX}"
),
]

Expand Down Expand Up @@ -262,67 +257,6 @@ def version(self) -> tuple[int, int, int]:
return (0, 0, 0)
return int(version.group(1)), int(version.group(2)), int(version.group(3))

def clone(self, repository: str, dest: Path) -> str:
self._check_parameter(repository)
cmd = [
"clone",
"--filter=blob:none",
"--recurse-submodules",
"--",
repository,
str(dest),
]
# Blobless clones introduced in Git 2.17
if self.version < (2, 17):
cmd.remove("--filter=blob:none")
return self.run(*cmd)

def checkout(self, rev: str, folder: Path | None = None) -> str:
args = []
if folder is None and self._work_dir:
folder = self._work_dir

if folder:
args += [
"--git-dir",
(folder / ".git").as_posix(),
"--work-tree",
folder.as_posix(),
]

self._check_parameter(rev)

args += ["checkout", "--recurse-submodules", rev]

return self.run(*args)

def rev_parse(self, rev: str, folder: Path | None = None) -> str:
args = []
if folder is None and self._work_dir:
folder = self._work_dir

self._check_parameter(rev)

# We need "^0" (an alternative to "^{commit}") to ensure that the
# commit SHA of the commit the tag points to is returned, even in
# the case of annotated tags.
#
# We deliberately avoid the "^{commit}" syntax itself as on some
# platforms (cygwin/msys to be specific), the braces are interpreted
# as special characters and would require escaping, while on others
# they should not be escaped.
args += ["rev-parse", rev + "^0"]

return self.run(*args, folder=folder)

def get_current_branch(self, folder: Path | None = None) -> str:
if folder is None and self._work_dir:
folder = self._work_dir

output = self.run("symbolic-ref", "--short", "HEAD", folder=folder)

return output.strip()

def get_ignored_files(self, folder: Path | None = None) -> list[str]:
args = []
if folder is None and self._work_dir:
Expand All @@ -341,23 +275,6 @@ def get_ignored_files(self, folder: Path | None = None) -> list[str]:

return output.strip().split("\n")

def remote_urls(self, folder: Path | None = None) -> dict[str, str]:
output = self.run(
"config", "--get-regexp", r"remote\..*\.url", folder=folder
).strip()

urls = {}
for url in output.splitlines():
name, url = url.split(" ", 1)
urls[name.strip()] = url.strip()

return urls

def remote_url(self, folder: Path | None = None) -> str:
urls = self.remote_urls(folder=folder)

return urls.get("remote.origin.url", urls[next(iter(urls.keys()))])

def run(self, *args: Any, **kwargs: Any) -> str:
folder = kwargs.pop("folder", None)
if folder:
Expand All @@ -376,10 +293,3 @@ def run(self, *args: Any, **kwargs: Any) -> str:
.decode()
.strip()
)

def _check_parameter(self, parameter: str) -> None:
"""
Checks a git parameter to avoid unwanted code execution.
"""
if parameter.strip().startswith("-"):
raise GitError(f"Invalid Git parameter: {parameter}")
52 changes: 36 additions & 16 deletions tests/vcs/test_vcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from poetry.core.utils._compat import WINDOWS
from poetry.core.vcs import get_vcs
from poetry.core.vcs.git import Git
from poetry.core.vcs.git import GitError
from poetry.core.vcs.git import GitUrl
from poetry.core.vcs.git import ParsedUrl
from poetry.core.vcs.git import _reset_executable
Expand Down Expand Up @@ -273,6 +272,42 @@ def test_normalize_url(url: str, normalized: GitUrl) -> None:
None,
),
),
(
"git+https://username:@github.com/sdispater/pendulum",
ParsedUrl(
"https",
"github.com",
"/sdispater/pendulum",
"username:",
None,
"pendulum",
None,
),
),
(
"git+https://username:password@github.com/sdispater/pendulum",
ParsedUrl(
"https",
"github.com",
"/sdispater/pendulum",
"username:password",
None,
"pendulum",
None,
),
),
(
"git+https://username+suffix:password@github.com/sdispater/pendulum",
ParsedUrl(
"https",
"github.com",
"/sdispater/pendulum",
"username+suffix:password",
None,
"pendulum",
None,
),
),
(
"git+https://github.com/sdispater/pendulum#7a018f2d075b03a73409e8356f9b29c9ad4ea2c5",
ParsedUrl(
Expand Down Expand Up @@ -440,21 +475,6 @@ def test_parse_url_should_fail() -> None:
ParsedUrl.parse(url)


def test_git_clone_raises_error_on_invalid_repository() -> None:
with pytest.raises(GitError):
Git().clone("-u./payload", Path("foo"))


def test_git_checkout_raises_error_on_invalid_repository() -> None:
with pytest.raises(GitError):
Git().checkout("-u./payload")


def test_git_rev_parse_raises_error_on_invalid_repository() -> None:
with pytest.raises(GitError):
Git().rev_parse("-u./payload")


@pytest.mark.skipif(
not WINDOWS,
reason=(
Expand Down
Loading