From e4c7b90589a8ffe9461f8ad68a2dafa2e92b64a6 Mon Sep 17 00:00:00 2001 From: Scott Sanderson Date: Sat, 8 Feb 2025 11:49:15 -0500 Subject: [PATCH 1/3] Detect infinite recursion in uv run. Handle potential infinite recursion if `uv run` recursively invokes `uv run`. This can happen if the shebang line of a script includes `uv run` without passing --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 it increments the counter. Closes https://github.com/astral-sh/uv/issues/11220. --- crates/uv-cli/src/lib.rs | 5 ++++ crates/uv-static/src/env_vars.rs | 11 ++++++++ crates/uv/src/commands/project/run.rs | 39 +++++++++++++++++++++++++++ crates/uv/src/lib.rs | 1 + crates/uv/src/settings.rs | 9 +++++++ crates/uv/tests/it/run.rs | 36 +++++++++++++++++++++++++ 6 files changed, 101 insertions(+) diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index d6db185559bf..d90be269b8c5 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -2932,6 +2932,11 @@ 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 recursively invoke itself before + /// giving up. + #[arg(long, hide = true, env = EnvVars::UV_RUN_MAX_RECURSION_DEPTH)] + pub max_recursion_depth: Option, } #[derive(Args)] diff --git a/crates/uv-static/src/env_vars.rs b/crates/uv-static/src/env_vars.rs index aa35eb162ce3..f63d3a36eae8 100644 --- a/crates/uv-static/src/env_vars.rs +++ b/crates/uv-static/src/env_vars.rs @@ -614,4 +614,15 @@ 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 recursively invoked itself. Used to + /// guard against infinite recursion when uv run is in a script shebang + /// line. + #[attr_hidden] + pub const UV_RUN_RECURSION_DEPTH: &'static str = "UV_RUN_RECURSION_DEPTH"; + + /// Maximum number of times that uv run will invoke itself before giving + /// up. + #[attr_hidden] + pub const UV_RUN_MAX_RECURSION_DEPTH: &'static str = "UV_RUN_MAX_RECURSION_DEPTH"; } diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 6729466f5139..72ba7b200b2b 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::collections::BTreeMap; +use std::env::VarError; use std::ffi::OsString; use std::fmt::Write; use std::io::Read; @@ -92,7 +93,18 @@ pub(crate) async fn run( env_file: Vec, no_env_file: bool, preview: PreviewMode, + max_recursion_depth: u32, ) -> anyhow::Result { + // 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!( + "Exiting because `uv run` invoked itself recursively {max_recursion_depth} times. If you are running a script with `uv run` as the shebang line, you may need to include --script in the arguments to uv.", + ); + } + // 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; @@ -1080,6 +1092,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()); @@ -1483,3 +1501,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 { + 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::() + .with_context(|| format!("invalid value for {}", EnvVars::UV_RUN_RECURSION_DEPTH)) +} diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs index e63eee833b00..03361b44b13f 100644 --- a/crates/uv/src/lib.rs +++ b/crates/uv/src/lib.rs @@ -1512,6 +1512,7 @@ async fn run_project( args.env_file, args.no_env_file, globals.preview, + args.max_recursion_depth, )) .await } diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs index 6d108ee97155..c1a48d15201d 100644 --- a/crates/uv/src/settings.rs +++ b/crates/uv/src/settings.rs @@ -299,9 +299,16 @@ pub(crate) struct RunSettings { pub(crate) settings: ResolverInstallerSettings, pub(crate) env_file: Vec, 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) -> 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), } } } diff --git a/crates/uv/tests/it/run.rs b/crates/uv/tests/it/run.rs index c88b0ada7072..faa44a96ef77 100644 --- a/crates/uv/tests/it/run.rs +++ b/crates/uv/tests/it/run.rs @@ -4120,3 +4120,39 @@ fn run_without_overlay() -> Result<()> { Ok(()) } + +/// See: +#[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: Exiting because `uv run` invoked itself recursively 5 times. If you are running a script with `uv run` as the shebang line, you may need to include --script in the arguments to uv. + "###); + + Ok(()) +} From e7b1d82cf1bc08a9aa4adde643c4b20c8ed48b23 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 12 Feb 2025 07:43:33 -0600 Subject: [PATCH 2/3] Review --- crates/uv-cli/src/lib.rs | 8 ++++++-- crates/uv-static/src/env_vars.rs | 9 ++++----- crates/uv/src/commands/project/run.rs | 7 ++++++- crates/uv/tests/it/run.rs | 4 +++- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index d90be269b8c5..61e25473c1d4 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -2933,8 +2933,12 @@ pub struct RunArgs { #[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 recursively invoke itself before - /// giving up. + /// 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, } diff --git a/crates/uv-static/src/env_vars.rs b/crates/uv-static/src/env_vars.rs index f63d3a36eae8..69dfbcd399a4 100644 --- a/crates/uv-static/src/env_vars.rs +++ b/crates/uv-static/src/env_vars.rs @@ -615,14 +615,13 @@ 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 recursively invoked itself. Used to - /// guard against infinite recursion when uv run is in a script shebang - /// line. + /// 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"; - /// Maximum number of times that uv run will invoke itself before giving - /// up. + /// 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"; } diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 72ba7b200b2b..e6b142e69aea 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -101,7 +101,12 @@ pub(crate) async fn run( let recursion_depth = read_recursion_depth_from_environment_variable()?; if recursion_depth > max_recursion_depth { bail!( - "Exiting because `uv run` invoked itself recursively {max_recursion_depth} times. If you are running a script with `uv run` as the shebang line, you may need to include --script in the arguments to uv.", + 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(), ); } diff --git a/crates/uv/tests/it/run.rs b/crates/uv/tests/it/run.rs index faa44a96ef77..de05df460dd1 100644 --- a/crates/uv/tests/it/run.rs +++ b/crates/uv/tests/it/run.rs @@ -4151,7 +4151,9 @@ fn detect_infinite_recursion() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: Exiting because `uv run` invoked itself recursively 5 times. If you are running a script with `uv run` as the shebang line, you may need to include --script in the arguments to uv. + 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(()) From 12bd203ec55baed89d9da6c36d57f69323311928 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 12 Feb 2025 12:47:43 -0600 Subject: [PATCH 3/3] Lint --- crates/uv/src/commands/project/run.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index e6b142e69aea..2ea7b2dd92c0 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -101,10 +101,10 @@ pub(crate) async fn run( let recursion_depth = read_recursion_depth_from_environment_variable()?; if recursion_depth > max_recursion_depth { bail!( - r#" + 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."#, +hint: If you are running a script with `{}` in the shebang, you may need to include the `{}` flag.", "uv run".green(), "--script".green(), );