From 078ba5aabeeb9541285c50e7ffa69fab745a879c Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Mon, 15 Apr 2024 21:09:58 +0200 Subject: [PATCH] Disallow setting the `PWD` via `load-env` input (#12522) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixes #12520 # User-Facing Changes Breaking change: Any operation parsing input with `PWD` to set the environment will now fail with `ShellError::AutomaticEnvVarSetManually` Furthermore transactions containing the special env-vars will be rejected before executing any modifications. Prevoiusly this was changing valid variables before while leaving valid variables after the violation untouched. ## `PWD` handling. Now failing ``` {PWD: "/trolling"} | load-env ``` already failing ``` load-env {PWD: "/trolling"} ``` ## Error management ``` > load-env {MY_VAR1: foo, PWD: "/trolling", MY_VAR2: bar} Error: nu::shell::automatic_env_var_set_manually × PWD cannot be set manually. ╭─[entry #1:1:2] 1 │ load-env {MY_VAR1: foo, PWD: "/trolling", MY_VAR2: bar} · ────┬─── · ╰── cannot set 'PWD' manually ╰──── help: The environment variable 'PWD' is set automatically by Nushell and cannot be set manually. ``` ### Before: ``` > $env.MY_VAR1 foo > $env.MY_VAR2 Error: nu::shell::name_not_found .... ``` ### After: ``` > $env.MY_VAR1 Error: nu::shell::name_not_found .... > $env.MY_VAR2 Error: nu::shell::name_not_found .... ``` # After Submitting We need to check if any integrations rely on this hack. --- crates/nu-command/src/env/load_env.rs | 69 ++++++++++----------------- 1 file changed, 25 insertions(+), 44 deletions(-) diff --git a/crates/nu-command/src/env/load_env.rs b/crates/nu-command/src/env/load_env.rs index 9e622a5803..a8a4fe1c47 100644 --- a/crates/nu-command/src/env/load_env.rs +++ b/crates/nu-command/src/env/load_env.rs @@ -1,4 +1,4 @@ -use nu_engine::{command_prelude::*, current_dir}; +use nu_engine::command_prelude::*; #[derive(Clone)] pub struct LoadEnv; @@ -37,53 +37,34 @@ impl Command for LoadEnv { let arg: Option = call.opt(engine_state, stack, 0)?; let span = call.head; - match arg { - Some(record) => { - for (env_var, rhs) in record { - let env_var_ = env_var.as_str(); - if ["FILE_PWD", "CURRENT_FILE", "PWD"].contains(&env_var_) { - return Err(ShellError::AutomaticEnvVarSetManually { - envvar_name: env_var, - span: call.head, - }); - } - stack.add_env_var(env_var, rhs); - } - Ok(PipelineData::empty()) - } + let record = match arg { + Some(record) => record, None => match input { - PipelineData::Value(Value::Record { val, .. }, ..) => { - for (env_var, rhs) in val.into_owned() { - let env_var_ = env_var.as_str(); - if ["FILE_PWD", "CURRENT_FILE"].contains(&env_var_) { - return Err(ShellError::AutomaticEnvVarSetManually { - envvar_name: env_var, - span: call.head, - }); - } - - if env_var == "PWD" { - let cwd = current_dir(engine_state, stack)?; - let rhs = rhs.coerce_into_string()?; - let rhs = nu_path::expand_path_with(rhs, cwd, true); - stack.add_env_var( - env_var, - Value::string(rhs.to_string_lossy(), call.head), - ); - } else { - stack.add_env_var(env_var, rhs); - } - } - Ok(PipelineData::empty()) + PipelineData::Value(Value::Record { val, .. }, ..) => val.into_owned(), + _ => { + return Err(ShellError::UnsupportedInput { + msg: "'load-env' expects a single record".into(), + input: "value originated from here".into(), + msg_span: span, + input_span: input.span().unwrap_or(span), + }) } - _ => Err(ShellError::UnsupportedInput { - msg: "'load-env' expects a single record".into(), - input: "value originated from here".into(), - msg_span: span, - input_span: input.span().unwrap_or(span), - }), }, + }; + + for prohibited in ["FILE_PWD", "CURRENT_FILE", "PWD"] { + if record.contains(prohibited) { + return Err(ShellError::AutomaticEnvVarSetManually { + envvar_name: prohibited.to_string(), + span: call.head, + }); + } } + + for (env_var, rhs) in record { + stack.add_env_var(env_var, rhs); + } + Ok(PipelineData::empty()) } fn examples(&self) -> Vec {