diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 3e8ab969d..beb914109 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -40,8 +40,12 @@ impl Env for Box { #[derive(Debug, Default)] struct DirectorySpecificEnvironment { pub whitelisted_directories: Vec, - pub added_env_vars: HashMap>, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub overwritten_env_values: HashMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. + + //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. + pub added_env_vars: HashMap>, + + //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. + pub overwritten_env_values: HashMap>, } impl DirectorySpecificEnvironment { @@ -88,31 +92,27 @@ impl DirectorySpecificEnvironment { Ok(vars_to_add) } - //If the user has left directories which added env vars through .nurc, we clear those vars - //For each directory d in nurc_env_vars: - //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed + //If the user has left directories which added env vars through .nu, we clear those vars pub fn env_vars_to_delete(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; - let mut new_nurc_env_vars = HashMap::new(); - for (d, v) in self.added_env_vars.iter() { - let mut working_dir = Some(current_dir.as_path()); - while working_dir.is_some() { - if working_dir.unwrap() == d { - new_nurc_env_vars.insert(d.clone(), v.clone()); - break; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - } + let vars_to_delete = self.added_env_vars.iter().fold( + Vec::new(), + |mut vars_to_delete, (directory, env_vars)| { + let mut working_dir = Some(current_dir.as_path()); - let mut vars_to_delete = vec![]; - for (path, vals) in self.added_env_vars.iter() { - if !new_nurc_env_vars.contains_key(path) { - vars_to_delete.extend(vals.clone()); - } - } + while let Some(wdir) = working_dir { + if &wdir == directory { + return vars_to_delete; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + //only delete vars from directories we are not in + vars_to_delete.extend(env_vars.clone()); + vars_to_delete + }, + ); Ok(vars_to_delete) } diff --git a/post.org b/post.org index 2c304e37b..d3149410b 100644 --- a/post.org +++ b/post.org @@ -12,9 +12,11 @@ In order for a .nu-file to be read, the directory it is in must be listed in the ``` nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] ``` - -# The way it works now is that whenever you run the function `maintain_nurc_environment_vars`, the current directory is checked for a `.nu` file and if it exists the variables in it are added. This works. -# Of course, when you leave a directory the variables should be unset. I track this by having a map between directory and a vector of environment variables. If the user is not in the directory or one of its subdirectories, its environment variables are removed... And then for some reason they are re-added again? +This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless +the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download, +and now you suddenly have nushell picking up whatever environment variables they set. +This meant that the code necessarily become more involved than just looking for a .nu-file in the current directory and applying its variables, +but this extra code also allowed for some other nice features and behavior which is listed below. Behavior: - If you are in a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. @@ -24,8 +26,9 @@ Behavior: - TODO: What happens if you overwrite twice? Questions: -- ´add_env´ does not overwrite variables. Need ´add_env_force´? -- `ctx.get_env()` in `cli.rs` lacks access to the config, which is required. Is it ok to do it through the sync call instead? +`remove_env()` accesses the environment directly to remove variables. +Just using what I think was expected, envs.entries.remove(key), does not work. +If this is a problem, how can I make it work better with existing code? TODO: take care of situation where a directory overwrites an existing .nu conf.