mirror of
https://github.com/astral-sh/uv.git
synced 2025-08-04 02:48:17 +00:00
Detect infinite recursion in uv run. (#11386)
<!-- Thank you for contributing to uv! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Handle potential infinite recursion if `uv run` recursively invokes `uv run`. This can happen if the shebang line of a script includes `uv run`, but does not pass `--script`. Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which contains a counter of the number of times that uv run has been recursively invoked. If unset, it defaults to zero, and each time uv run starts a subprocess we increment the counter, erroring if the value is greater than a configurable (but not currently exposed or documented) threshold. Closes https://github.com/astral-sh/uv/issues/11220. ## Test Plan I've added a snapshot test to `uv/crates/uv/tests/it/run` that tests the end-to-end recursion detection flow. I've currently made it a unix-only test because I'm not sure offhand how uv run will interact with shebang lines on windows. --------- Co-authored-by: Zanie Blue <contact@zanie.dev>
This commit is contained in:
parent
81966c43dc
commit
7154800e0c
6 changed files with 111 additions and 0 deletions
|
@ -2932,6 +2932,15 @@ pub struct RunArgs {
|
|||
/// By default, environment modifications are omitted, but enabled under `--verbose`.
|
||||
#[arg(long, env = EnvVars::UV_SHOW_RESOLUTION, value_parser = clap::builder::BoolishValueParser::new(), hide = true)]
|
||||
pub show_resolution: bool,
|
||||
|
||||
/// Number of times that `uv run` will allow recursive invocations.
|
||||
///
|
||||
/// The current recursion depth is tracked by environment variable. If environment variables are
|
||||
/// cleared, uv will fail to detect the recursion depth.
|
||||
///
|
||||
/// If uv reaches the maximum recursion depth, it will exit with an error.
|
||||
#[arg(long, hide = true, env = EnvVars::UV_RUN_MAX_RECURSION_DEPTH)]
|
||||
pub max_recursion_depth: Option<u32>,
|
||||
}
|
||||
|
||||
#[derive(Args)]
|
||||
|
|
|
@ -623,4 +623,14 @@ impl EnvVars {
|
|||
|
||||
/// Enables fetching files stored in Git LFS when installing a package from a Git repository.
|
||||
pub const UV_GIT_LFS: &'static str = "UV_GIT_LFS";
|
||||
|
||||
/// Number of times that `uv run` has been recursively invoked. Used to guard against infinite
|
||||
/// recursion, e.g., when `uv run`` is used in a script shebang.
|
||||
#[attr_hidden]
|
||||
pub const UV_RUN_RECURSION_DEPTH: &'static str = "UV_RUN_RECURSION_DEPTH";
|
||||
|
||||
/// Number of times that `uv run` will allow recursive invocations, before exiting with an
|
||||
/// error.
|
||||
#[attr_hidden]
|
||||
pub const UV_RUN_MAX_RECURSION_DEPTH: &'static str = "UV_RUN_MAX_RECURSION_DEPTH";
|
||||
}
|
||||
|
|
|
@ -1,4 +1,5 @@
|
|||
use std::borrow::Cow;
|
||||
use std::env::VarError;
|
||||
use std::ffi::OsString;
|
||||
use std::fmt::Write;
|
||||
use std::io::Read;
|
||||
|
@ -91,7 +92,23 @@ pub(crate) async fn run(
|
|||
env_file: Vec<PathBuf>,
|
||||
no_env_file: bool,
|
||||
preview: PreviewMode,
|
||||
max_recursion_depth: u32,
|
||||
) -> anyhow::Result<ExitStatus> {
|
||||
// Check if max recursion depth was exceeded. This most commonly happens
|
||||
// for scripts with a shebang line like `#!/usr/bin/env -S uv run`, so try
|
||||
// to provide guidance for that case.
|
||||
let recursion_depth = read_recursion_depth_from_environment_variable()?;
|
||||
if recursion_depth > max_recursion_depth {
|
||||
bail!(
|
||||
r"
|
||||
`uv run` was recursively invoked {recursion_depth} times which exceeds the limit of {max_recursion_depth}.
|
||||
|
||||
hint: If you are running a script with `{}` in the shebang, you may need to include the `{}` flag.",
|
||||
"uv run".green(),
|
||||
"--script".green(),
|
||||
);
|
||||
}
|
||||
|
||||
// These cases seem quite complex because (in theory) they should change the "current package".
|
||||
// Let's ban them entirely for now.
|
||||
let mut requirements_from_stdin: bool = false;
|
||||
|
@ -1034,6 +1051,12 @@ pub(crate) async fn run(
|
|||
)?;
|
||||
process.env(EnvVars::PATH, new_path);
|
||||
|
||||
// Increment recursion depth counter.
|
||||
process.env(
|
||||
EnvVars::UV_RUN_RECURSION_DEPTH,
|
||||
(recursion_depth + 1).to_string(),
|
||||
);
|
||||
|
||||
// Ensure `VIRTUAL_ENV` is set.
|
||||
if interpreter.is_virtualenv() {
|
||||
process.env(EnvVars::VIRTUAL_ENV, interpreter.sys_prefix().as_os_str());
|
||||
|
@ -1464,3 +1487,24 @@ fn is_python_zipapp(target: &Path) -> bool {
|
|||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Read and parse recursion depth from the environment.
|
||||
///
|
||||
/// Returns Ok(0) if `EnvVars::UV_RUN_RECURSION_DEPTH` is not set.
|
||||
///
|
||||
/// Returns an error if `EnvVars::UV_RUN_RECURSION_DEPTH` is set to a value
|
||||
/// that cannot ber parsed as an integer.
|
||||
fn read_recursion_depth_from_environment_variable() -> anyhow::Result<u32> {
|
||||
let envvar = match std::env::var(EnvVars::UV_RUN_RECURSION_DEPTH) {
|
||||
Ok(val) => val,
|
||||
Err(VarError::NotPresent) => return Ok(0),
|
||||
Err(e) => {
|
||||
return Err(e)
|
||||
.with_context(|| format!("invalid value for {}", EnvVars::UV_RUN_RECURSION_DEPTH))
|
||||
}
|
||||
};
|
||||
|
||||
envvar
|
||||
.parse::<u32>()
|
||||
.with_context(|| format!("invalid value for {}", EnvVars::UV_RUN_RECURSION_DEPTH))
|
||||
}
|
||||
|
|
|
@ -1516,6 +1516,7 @@ async fn run_project(
|
|||
args.env_file,
|
||||
args.no_env_file,
|
||||
globals.preview,
|
||||
args.max_recursion_depth,
|
||||
))
|
||||
.await
|
||||
}
|
||||
|
|
|
@ -299,9 +299,16 @@ pub(crate) struct RunSettings {
|
|||
pub(crate) settings: ResolverInstallerSettings,
|
||||
pub(crate) env_file: Vec<PathBuf>,
|
||||
pub(crate) no_env_file: bool,
|
||||
pub(crate) max_recursion_depth: u32,
|
||||
}
|
||||
|
||||
impl RunSettings {
|
||||
// Default value for UV_RUN_MAX_RECURSION_DEPTH if unset. This is large
|
||||
// enough that it's unlikely a user actually needs this recursion depth,
|
||||
// but short enough that we detect recursion quickly enough to avoid OOMing
|
||||
// or hanging for a long time.
|
||||
const DEFAULT_MAX_RECURSION_DEPTH: u32 = 100;
|
||||
|
||||
/// Resolve the [`RunSettings`] from the CLI and filesystem configuration.
|
||||
#[allow(clippy::needless_pass_by_value)]
|
||||
pub(crate) fn resolve(args: RunArgs, filesystem: Option<FilesystemOptions>) -> Self {
|
||||
|
@ -344,6 +351,7 @@ impl RunSettings {
|
|||
show_resolution,
|
||||
env_file,
|
||||
no_env_file,
|
||||
max_recursion_depth,
|
||||
} = args;
|
||||
|
||||
let install_mirrors = filesystem
|
||||
|
@ -403,6 +411,7 @@ impl RunSettings {
|
|||
env_file,
|
||||
no_env_file,
|
||||
install_mirrors,
|
||||
max_recursion_depth: max_recursion_depth.unwrap_or(Self::DEFAULT_MAX_RECURSION_DEPTH),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -4202,3 +4202,41 @@ fn run_without_overlay() -> Result<()> {
|
|||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// See: <https://github.com/astral-sh/uv/issues/11220>
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn detect_infinite_recursion() -> Result<()> {
|
||||
use crate::common::get_bin;
|
||||
use indoc::formatdoc;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let context = TestContext::new("3.12");
|
||||
|
||||
let test_script = context.temp_dir.child("main");
|
||||
test_script.write_str(&formatdoc! { r#"
|
||||
#!{uv} run
|
||||
|
||||
print("Hello, world!")
|
||||
"#, uv = get_bin().display()})?;
|
||||
|
||||
fs_err::set_permissions(test_script.path(), PermissionsExt::from_mode(0o0744))?;
|
||||
|
||||
let mut cmd = std::process::Command::new(test_script.as_os_str());
|
||||
|
||||
// Set the max recursion depth to a lower amount to speed up testing.
|
||||
cmd.env("UV_RUN_MAX_RECURSION_DEPTH", "5");
|
||||
|
||||
uv_snapshot!(context.filters(), cmd, @r###"
|
||||
success: false
|
||||
exit_code: 2
|
||||
----- stdout -----
|
||||
|
||||
----- stderr -----
|
||||
error: `uv run` was recursively invoked 6 times which exceeds the limit of 5.
|
||||
|
||||
hint: If you are running a script with `uv run` in the shebang, you may need to include the `--script` flag.
|
||||
"###);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue