tweak the order of index priority (#2083)

Previously, `uv` would always prioritize the index given by
`--index-url`. It would then try any indexes after that given by zero
or more `--extra-index-url` flags. This differed from `pip` in that any
priority was given at all, where `pip` doesn't guarantee any priority
ordering of indexes.

We could go in the direction of mimicing `pip`'s behavior here, but it
at present has issues with dependency confusion attacks where packages
may get installed from indexes you don't control. More specifically,
there is an issue of different trust levels. See discussion in #171 and
[PEP-0708] for more on the security impact.

In contrast, `uv` will only select versions for a package from a single
index. That is, even if `foo` is in indexes `a` and `b`, it will
only consider the versions from the index that it checks first. This
probably helps with respect to dependency confusion attacks, but also
means that `uv` doesn't quite cover all of the same use cases as `pip`.

In this PR, we retain the notion of prioritizing indexes, but
tweak it so that PyPI is preferred last as opposed to first. Or
more precisely, the `--index-url` flag specifies a fallback index,
not the primary index, and is deprioritized beneath every index
specified by `--extra-index-url`. The ordering among indexes given by
`--extra-index-url` remains the same: earlier indexes are prioritized
over later indexes.

While this tweak likely won't hit all use cases, I believe it will
resolve some of the most common pain points without exacerbating
dependency confusion problems.

Ref #171, Fixes #1377, Fixes #1451, Fixes #1600

[PEP-0708]: https://peps.python.org/pep-0708/
This commit is contained in:
Andrew Gallant 2024-02-29 11:57:07 -05:00 committed by GitHub
parent 9a99aa7776
commit 5e351343da
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 138 additions and 17 deletions

View File

@ -288,12 +288,12 @@ impl Default for IndexUrls {
} }
impl<'a> IndexUrls { impl<'a> IndexUrls {
/// Return the primary [`IndexUrl`] entry. /// Return the fallback [`IndexUrl`] entry.
/// ///
/// If `--no-index` is set, return `None`. /// If `--no-index` is set, return `None`.
/// ///
/// If no index is provided, use the `PyPI` index. /// If no index is provided, use the `PyPI` index.
pub fn index(&'a self) -> Option<&'a IndexUrl> { fn index(&'a self) -> Option<&'a IndexUrl> {
if self.no_index { if self.no_index {
None None
} else { } else {
@ -305,7 +305,7 @@ impl<'a> IndexUrls {
} }
/// Return an iterator over the extra [`IndexUrl`] entries. /// Return an iterator over the extra [`IndexUrl`] entries.
pub fn extra_index(&'a self) -> impl Iterator<Item = &'a IndexUrl> + 'a { fn extra_index(&'a self) -> impl Iterator<Item = &'a IndexUrl> + 'a {
if self.no_index { if self.no_index {
Either::Left(std::iter::empty()) Either::Left(std::iter::empty())
} else { } else {
@ -314,13 +314,11 @@ impl<'a> IndexUrls {
} }
/// Return an iterator over all [`IndexUrl`] entries. /// Return an iterator over all [`IndexUrl`] entries.
///
/// If `no_index` was enabled, then this always returns an empty
/// iterator.
pub fn indexes(&'a self) -> impl Iterator<Item = &'a IndexUrl> + 'a { pub fn indexes(&'a self) -> impl Iterator<Item = &'a IndexUrl> + 'a {
self.index().into_iter().chain(self.extra_index()) self.extra_index().chain(self.index())
}
/// Return `true` if no index is configured.
pub fn no_index(&self) -> bool {
self.no_index
} }
} }

View File

@ -184,11 +184,12 @@ impl RegistryClient {
&self, &self,
package_name: &PackageName, package_name: &PackageName,
) -> Result<(IndexUrl, OwnedArchive<SimpleMetadata>), Error> { ) -> Result<(IndexUrl, OwnedArchive<SimpleMetadata>), Error> {
if self.index_urls.no_index() { let mut it = self.index_urls.indexes().peekable();
if it.peek().is_none() {
return Err(ErrorKind::NoIndex(package_name.as_ref().to_string()).into()); return Err(ErrorKind::NoIndex(package_name.as_ref().to_string()).into());
} }
for index in self.index_urls.indexes() { for index in it {
let result = self.simple_single_index(package_name, index).await?; let result = self.simple_single_index(package_name, index).await?;
return match result { return match result {

View File

@ -284,10 +284,25 @@ struct PipCompileArgs {
refresh_package: Vec<PackageName>, refresh_package: Vec<PackageName>,
/// The URL of the Python package index (by default: <https://pypi.org/simple>). /// The URL of the Python package index (by default: <https://pypi.org/simple>).
///
/// The index given by this flag is given lower priority than all other
/// indexes specified via the `--extra-index-url` flag.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, short, env = "UV_INDEX_URL")] #[clap(long, short, env = "UV_INDEX_URL")]
index_url: Option<IndexUrl>, index_url: Option<IndexUrl>,
/// Extra URLs of package indexes to use, in addition to `--index-url`. /// Extra URLs of package indexes to use, in addition to `--index-url`.
///
/// All indexes given via this flag take priority over the index
/// in `--index-url` (which defaults to PyPI). And when multiple
/// `--extra-index-url` flags are given, earlier values take priority.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, env = "UV_EXTRA_INDEX_URL")] #[clap(long, env = "UV_EXTRA_INDEX_URL")]
extra_index_url: Vec<IndexUrl>, extra_index_url: Vec<IndexUrl>,
@ -421,10 +436,25 @@ struct PipSyncArgs {
link_mode: install_wheel_rs::linker::LinkMode, link_mode: install_wheel_rs::linker::LinkMode,
/// The URL of the Python package index (by default: <https://pypi.org/simple>). /// The URL of the Python package index (by default: <https://pypi.org/simple>).
///
/// The index given by this flag is given lower priority than all other
/// indexes specified via the `--extra-index-url` flag.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, short, env = "UV_INDEX_URL")] #[clap(long, short, env = "UV_INDEX_URL")]
index_url: Option<IndexUrl>, index_url: Option<IndexUrl>,
/// Extra URLs of package indexes to use, in addition to `--index-url`. /// Extra URLs of package indexes to use, in addition to `--index-url`.
///
/// All indexes given via this flag take priority over the index
/// in `--index-url` (which defaults to PyPI). And when multiple
/// `--extra-index-url` flags are given, earlier values take priority.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, env = "UV_EXTRA_INDEX_URL")] #[clap(long, env = "UV_EXTRA_INDEX_URL")]
extra_index_url: Vec<IndexUrl>, extra_index_url: Vec<IndexUrl>,
@ -619,10 +649,25 @@ struct PipInstallArgs {
output_file: Option<PathBuf>, output_file: Option<PathBuf>,
/// The URL of the Python package index (by default: <https://pypi.org/simple>). /// The URL of the Python package index (by default: <https://pypi.org/simple>).
///
/// The index given by this flag is given lower priority than all other
/// indexes specified via the `--extra-index-url` flag.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, short, env = "UV_INDEX_URL")] #[clap(long, short, env = "UV_INDEX_URL")]
index_url: Option<IndexUrl>, index_url: Option<IndexUrl>,
/// Extra URLs of package indexes to use, in addition to `--index-url`. /// Extra URLs of package indexes to use, in addition to `--index-url`.
///
/// All indexes given via this flag take priority over the index
/// in `--index-url` (which defaults to PyPI). And when multiple
/// `--extra-index-url` flags are given, earlier values take priority.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, env = "UV_EXTRA_INDEX_URL")] #[clap(long, env = "UV_EXTRA_INDEX_URL")]
extra_index_url: Vec<IndexUrl>, extra_index_url: Vec<IndexUrl>,
@ -892,10 +937,25 @@ struct VenvArgs {
prompt: Option<String>, prompt: Option<String>,
/// The URL of the Python package index (by default: <https://pypi.org/simple>). /// The URL of the Python package index (by default: <https://pypi.org/simple>).
///
/// The index given by this flag is given lower priority than all other
/// indexes specified via the `--extra-index-url` flag.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, short, env = "UV_INDEX_URL")] #[clap(long, short, env = "UV_INDEX_URL")]
index_url: Option<IndexUrl>, index_url: Option<IndexUrl>,
/// Extra URLs of package indexes to use, in addition to `--index-url`. /// Extra URLs of package indexes to use, in addition to `--index-url`.
///
/// All indexes given via this flag take priority over the index
/// in `--index-url` (which defaults to PyPI). And when multiple
/// `--extra-index-url` flags are given, earlier values take priority.
///
/// Unlike `pip`, `uv` will stop looking for versions of a package as soon
/// as it finds it in an index. That is, it isn't possible for `uv` to
/// consider versions of the same package across multiple indexes.
#[clap(long, env = "UV_EXTRA_INDEX_URL")] #[clap(long, env = "UV_EXTRA_INDEX_URL")]
extra_index_url: Vec<IndexUrl>, extra_index_url: Vec<IndexUrl>,

View File

@ -3339,15 +3339,17 @@ fn emit_index_urls() -> Result<()> {
uv_snapshot!(context.compile() uv_snapshot!(context.compile()
.arg("requirements.in") .arg("requirements.in")
.arg("--emit-index-url") .arg("--emit-index-url")
.arg("--index-url")
.arg("https://test.pypi.org/simple/")
.arg("--extra-index-url") .arg("--extra-index-url")
.arg("https://test.pypi.org/simple/"), @r###" .arg("https://pypi.org/simple"), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
# This file was autogenerated by uv via the following command: # This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in --emit-index-url --extra-index-url https://test.pypi.org/simple/ # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in --emit-index-url --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple
--index-url https://pypi.org/simple --index-url https://test.pypi.org/simple/
--extra-index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple
black==23.10.1 black==23.10.1
click==8.1.7 click==8.1.7

View File

@ -36,14 +36,24 @@ fn decode_token(content: &[&str]) -> String {
/// Create a `pip install` command with options shared across scenarios. /// Create a `pip install` command with options shared across scenarios.
fn command(context: &TestContext) -> Command { fn command(context: &TestContext) -> Command {
let mut command = command_without_exclude_newer(context);
command.arg("--exclude-newer").arg(EXCLUDE_NEWER);
command
}
/// Create a `pip install` command with no `--exclude-newer` option.
///
/// One should avoid using this in tests to the extent possible because
/// it can result in tests failing when the index state changes. Therefore,
/// if you use this, there should be some other kind of mitigation in place.
/// For example, pinning package versions.
fn command_without_exclude_newer(context: &TestContext) -> Command {
let mut command = Command::new(get_bin()); let mut command = Command::new(get_bin());
command command
.arg("pip") .arg("pip")
.arg("install") .arg("install")
.arg("--cache-dir") .arg("--cache-dir")
.arg(context.cache_dir.path()) .arg(context.cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("VIRTUAL_ENV", context.venv.as_os_str()) .env("VIRTUAL_ENV", context.venv.as_os_str())
.current_dir(&context.temp_dir); .current_dir(&context.temp_dir);
command command
@ -802,6 +812,56 @@ fn install_no_index_version() {
context.assert_command("import flask").failure(); context.assert_command("import flask").failure();
} }
/// Install a package via --extra-index-url.
///
/// This is a regression test where previously `uv` would consult test.pypi.org
/// first, and if the package was found there, `uv` would not look at any other
/// indexes. We fixed this by flipping the priority order of indexes so that
/// test.pypi.org becomes the fallback (in this example) and the extra indexes
/// (regular PyPI) are checked first.
///
/// (Neither approach matches `pip`'s behavior, which considers versions of
/// each package from all indexes. `uv` stops at the first index it finds a
/// package in.)
///
/// Ref: <https://github.com/astral-sh/uv/issues/1600>
#[test]
fn install_extra_index_url_has_priority() {
let context = TestContext::new("3.12");
uv_snapshot!(command_without_exclude_newer(&context)
.arg("--index-url")
.arg("https://test.pypi.org/simple")
.arg("--extra-index-url")
.arg("https://pypi.org/simple")
// This tests what we want because BOTH of the following
// are true: `black` is on pypi.org and test.pypi.org, AND
// `black==24.2.0` is on pypi.org and NOT test.pypi.org. So
// this would previously check for `black` on test.pypi.org,
// find it, but then not find a compatible version. After
// the fix, `uv` will check pypi.org first since it is given
// priority via --extra-index-url.
.arg("black==24.2.0"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Downloaded 6 packages in [TIME]
Installed 6 packages in [TIME]
+ black==24.2.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==23.2
+ pathspec==0.12.1
+ platformdirs==4.2.0
"###
);
context.assert_command("import flask").failure();
}
/// Install a package from a public GitHub repository /// Install a package from a public GitHub repository
#[test] #[test]
#[cfg(feature = "git")] #[cfg(feature = "git")]