Eagerly reject unsupported Git schemes by konstin · Pull Request #11514 · astral-sh/uv

konstin · 2025-02-14T16:40:03Z

Initially, we were limiting Git schemes to HTTPS and SSH as only supported schemes. We lost this validation in #3429. This incidentally allowed file schemes, which apparently work with Git out of the box.

A caveat for this is that in tool.uv.sources, we parse the git field always as URL. This caused a problem with #11425: repo = { git = 'c:\path\to\repo', rev = "xxxxx" } was parsed as a URL where c: is the scheme, causing a bad error message down the line.

This PR:

Puts Git URL validation back in place. It bans everything but HTTPS, SSH, and file URLs. This could be a breaking change, if users were using a git transport protocol were not aware of, even though never intentionally supported.
Allows file: URL in Git: This seems to be supported by Git and we were supporting it albeit unintentionally, so it's reasonable to continue to support it.
It does not allow relative paths in the git field in tool.uv.sources. Absolute file URLs are supported, whether we want relative file URLs for Git too should be discussed separately.

Closes #3429: We reject the input with a proper error message, while hinting the user towards file:. If there's still desire for relative path support, we can keep it open.

charliermarsh · 2025-02-14T16:47:16Z

@konstin -- I'm wondering if we should instead put this in the GitUrl constructors?

Initially, we were limiting Git schemes to HTTPS and SSH as only supported schemes. We lost this validation in #3429. This incidentally allow file schemes, which apparently work with Git out of the box. A caveat for this is that in `tool.uv.sources`, we parse the `git` field always as URL. This caused a problem with #11425: `repo = { git = 'c:\path\to\repo', rev = "xxxxx" }` was parsed as a URL where `c:` is the scheme, causing a bad error message down the line. This PR: * Puts Git URL validation back in place * Allows `file:` URL in Git: This seems to be supported by Git and we were supporting it albeit unintentionally, so it's reasonable to continue to support it. * It does _not_ allow relative paths in the `git` field in `tool.uv.sources`. Absolute file URLs are supported, whether we want relative file URLs for Git too should be discussed separately. Closes #3429: We reject the input with a proper error message, while hinting the user towards `file:`. If there's still desire for a relative path, we can reopen.

konstin · 2025-02-17T16:49:13Z

Refactored RequirementSource::Git for catching this directly in GitUrl without adding more error paths, same errors being caught still.

charliermarsh · 2025-02-18T01:59:25Z

crates/uv/tests/it/pip_install.rs

+#[test]
+fn unknown_git_schema() {
+    let context = TestContext::new("3.12");
+    // Reverse direction: Check that we switch back to the workspace package with `--upgrade`.


Stray comment, copy-pasted from above.

charliermarsh · 2025-02-18T02:01:02Z

crates/uv/tests/it/pip_install.rs

 }
+
+#[test]
+fn unknown_git_schema() {


I think this should be scheme like the others.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.6.0` -> `0.6.3` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.6.3`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#063) [Compare Source](astral-sh/uv@0.6.2...0.6.3) ##### Enhancements - Allow quotes around command-line options in `requirement.txt files` ([#11644](astral-sh/uv#11644)) - Initialize PEP 723 script in `uv lock --script` ([#11717](astral-sh/uv#11717)) ##### Configuration - Accept multiple `.env` files in `UV_ENV_FILE` ([#11665](astral-sh/uv#11665)) ##### Performance - Reduce overhead in converting resolutions ([#11660](astral-sh/uv#11660)) - Use `SmallString` on `Hashes` ([#11756](astral-sh/uv#11756)) - Use a `Box` for `Yanked` on `File` ([#11755](astral-sh/uv#11755)) - Use a `SmallString` for the `Yanked` enum ([#11715](astral-sh/uv#11715)) - Use boxed slices for hash vector ([#11714](astral-sh/uv#11714)) - Use install concurrency for bytecode compilation too ([#11615](astral-sh/uv#11615)) ##### Bug fixes - Avoid installing duplicate dependencies across conflicting groups ([#11653](astral-sh/uv#11653)) - Check subdirectory existence after cache heal ([#11719](astral-sh/uv#11719)) - Include uppercase platforms for Windows wheels ([#11681](astral-sh/uv#11681)) - Respect existing PEP 723 script settings in `uv add` ([#11716](astral-sh/uv#11716)) - Reuse refined interpreter to create tool environment ([#11680](astral-sh/uv#11680)) - Skip removed directories during bytecode compilation ([#11633](astral-sh/uv#11633)) - Support conflict markers in `uv export` ([#11643](astral-sh/uv#11643)) - Treat lockfile as outdated if (empty) extras are added ([#11702](astral-sh/uv#11702)) - Display path separators as backslashes on Windows ([#11667](astral-sh/uv#11667)) - Display the built file name instead of the canonicalized name in `uv build` ([#11593](astral-sh/uv#11593)) - Fix message when there are no buildable packages ([#11722](astral-sh/uv#11722)) - Re-allow HTTP schemes for Git dependencies ([#11687](astral-sh/uv#11687)) ##### Documentation - Add anchor links to arguments and options in the CLI reference ([#11754](astral-sh/uv#11754)) - Add link to environment marker specification ([#11748](astral-sh/uv#11748)) - Fix missing a closing bracket in the `cache-keys` setting ([#11669](astral-sh/uv#11669)) - Remove the last edited date from documentation pages ([#11753](astral-sh/uv#11753)) - Fix readme typo ([#11742](astral-sh/uv#11742)) ### [`v0.6.2`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#062) [Compare Source](astral-sh/uv@0.6.1...0.6.2) ##### Enhancements - Add support for constraining build dependencies with `tool.uv.build-constraint-dependencies` ([#11585](astral-sh/uv#11585)) - Sort dependency group keys when adding new group ([#11591](astral-sh/uv#11591)) ##### Performance - Use an `Arc` for index URLs ([#11586](astral-sh/uv#11586)) ##### Bug fixes - Allow use of x86-64 Python on ARM Windows ([#11625](astral-sh/uv#11625)) - Fix an issue where conflict markers could instigate a very large lock file ([#11293](astral-sh/uv#11293)) - Fix duplicate packages with multiple conflicting extras declared ([#11513](astral-sh/uv#11513)) - Respect color settings for log messages ([#11604](astral-sh/uv#11604)) - Eagerly reject unsupported Git schemes ([#11514](astral-sh/uv#11514)) ##### Documentation - Add documentation for specifying Python versions in tool commands ([#11598](astral-sh/uv#11598)) ### [`v0.6.1`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#061) [Compare Source](astral-sh/uv@0.6.0...0.6.1) ##### Enhancements - Allow users to mark platforms as "required" for wheel coverage ([#10067](astral-sh/uv#10067)) - Warn for builds in non-build and workspace root pyproject.toml ([#11394](astral-sh/uv#11394)) ##### Bug fixes - Add `--all` to `uvx --reinstall` message ([#11535](astral-sh/uv#11535)) - Fallback to `GET` on HTTP 400 when attempting to use range requests for wheel download ([#11539](astral-sh/uv#11539)) - Prefer local variants in preference selection ([#11546](astral-sh/uv#11546)) - Respect verbatim executable name in `uvx` ([#11524](astral-sh/uv#11524)) ##### Documentation - Add documentation for required environments ([#11542](astral-sh/uv#11542)) - Note that `main.py` used to be `hello.py` ([#11519](astral-sh/uv#11519)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Initially, we were limiting Git schemes to HTTPS and SSH as only supported schemes. We lost this validation in astral-sh#3429. This incidentally allowed file schemes, which apparently work with Git out of the box. A caveat for this is that in tool.uv.sources, we parse the git field always as URL. This caused a problem with astral-sh#11425: repo = { git = 'c:\path\to\repo', rev = "xxxxx" } was parsed as a URL where c: is the scheme, causing a bad error message down the line. This PR: * Puts Git URL validation back in place. It bans everything but HTTPS, SSH, and file URLs. This could be a breaking change, if users were using a git transport protocol were not aware of, even though never intentionally supported. * Allows file: URL in Git: This seems to be supported by Git and we were supporting it albeit unintentionally, so it's reasonable to continue to support it. * It does not allow relative paths in the git field in tool.uv.sources. Absolute file URLs are supported, whether we want relative file URLs for Git too should be discussed separately. Closes astral-sh#3429: We reject the input with a proper error message, while hinting the user towards file:. If there's still desire for relative path support, we can keep it open. --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

konstin added the error messages Messaging when something goes wrong label Feb 14, 2025

konstin force-pushed the konsti/invalid-git-urls branch from 692e3e5 to bfdb89d Compare February 17, 2025 10:58

konstin force-pushed the konsti/invalid-git-urls branch from bfdb89d to 782fdd1 Compare February 17, 2025 11:05

#[cfg(feature = "git")]

06d1d3e

charliermarsh reviewed Feb 18, 2025

View reviewed changes

charliermarsh approved these changes Feb 18, 2025

View reviewed changes

Tweaks

91e7fcd

charliermarsh force-pushed the konsti/invalid-git-urls branch from 74ca969 to c2e5871 Compare February 18, 2025 02:05

charliermarsh added the no-build Disable building binaries in CI label Feb 18, 2025

Merge branch 'main' into konsti/invalid-git-urls

0ba33ac

charliermarsh force-pushed the konsti/invalid-git-urls branch from c2e5871 to 0ba33ac Compare February 18, 2025 02:05

charliermarsh enabled auto-merge (squash) February 18, 2025 02:14

charliermarsh merged commit 29c2be3 into main Feb 18, 2025
82 checks passed

charliermarsh deleted the konsti/invalid-git-urls branch February 18, 2025 02:14

konstin mentioned this pull request Feb 18, 2025

Reject git+file paths in requirement definitions #11425

Open

BrewTestBot mentioned this pull request Feb 19, 2025

uv 0.6.2 Homebrew/homebrew-core#208313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eagerly reject unsupported Git schemes#11514

Eagerly reject unsupported Git schemes#11514
charliermarsh merged 4 commits intomainfrom
konsti/invalid-git-urls

konstin commented Feb 14, 2025 •

edited

Loading

Uh oh!

charliermarsh commented Feb 14, 2025

Uh oh!

konstin commented Feb 17, 2025

Uh oh!

charliermarsh Feb 18, 2025

Uh oh!

charliermarsh Feb 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

konstin commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charliermarsh commented Feb 14, 2025

Uh oh!

konstin commented Feb 17, 2025

Uh oh!

charliermarsh Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

charliermarsh Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

konstin commented Feb 14, 2025 •

edited

Loading