Refactor env_vars_to_delete

This commit is contained in:
Sam Hedin 2020-06-06 03:31:30 +02:00
parent 3a278b38da
commit fb6eb1924f
2 changed files with 31 additions and 28 deletions

View File

@ -40,8 +40,12 @@ impl Env for Box<dyn Env> {
#[derive(Debug, Default)] #[derive(Debug, Default)]
struct DirectorySpecificEnvironment { struct DirectorySpecificEnvironment {
pub whitelisted_directories: Vec<PathBuf>, pub whitelisted_directories: Vec<PathBuf>,
pub added_env_vars: HashMap<PathBuf, Vec<String>>, //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<PathBuf, Vec<(String, String)>>, //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<PathBuf, Vec<String>>,
//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<PathBuf, Vec<(String, String)>>,
} }
impl DirectorySpecificEnvironment { impl DirectorySpecificEnvironment {
@ -88,31 +92,27 @@ impl DirectorySpecificEnvironment {
Ok(vars_to_add) Ok(vars_to_add)
} }
//If the user has left directories which added env vars through .nurc, we clear those vars //If the user has left directories which added env vars through .nu, 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
pub fn env_vars_to_delete(&mut self) -> std::io::Result<Vec<String>> { pub fn env_vars_to_delete(&mut self) -> std::io::Result<Vec<String>> {
let current_dir = std::env::current_dir()?; let current_dir = std::env::current_dir()?;
let mut new_nurc_env_vars = HashMap::new(); let vars_to_delete = self.added_env_vars.iter().fold(
for (d, v) in self.added_env_vars.iter() { Vec::new(),
let mut working_dir = Some(current_dir.as_path()); |mut vars_to_delete, (directory, env_vars)| {
while working_dir.is_some() { let mut working_dir = Some(current_dir.as_path());
if working_dir.unwrap() == d {
new_nurc_env_vars.insert(d.clone(), v.clone());
break;
} else {
working_dir = working_dir.unwrap().parent();
}
}
}
let mut vars_to_delete = vec![]; while let Some(wdir) = working_dir {
for (path, vals) in self.added_env_vars.iter() { if &wdir == directory {
if !new_nurc_env_vars.contains_key(path) { return vars_to_delete;
vars_to_delete.extend(vals.clone()); } 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) Ok(vars_to_delete)
} }

View File

@ -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"] nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"]
``` ```
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 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. the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download,
# 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? 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: Behavior:
- If you are in a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. - 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? - TODO: What happens if you overwrite twice?
Questions: Questions:
- ´add_env´ does not overwrite variables. Need ´add_env_force´? `remove_env()` accesses the environment directly to remove variables.
- `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? 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. TODO: take care of situation where a directory overwrites an existing .nu conf.