From 3367eeaaa93672a5f805fb6efb203416724bbe15 Mon Sep 17 00:00:00 2001 From: Shtsh Date: Fri, 12 Apr 2024 14:43:16 +0200 Subject: [PATCH] Added user input validation --- Cargo.toml | 4 ++- README.md | 5 ++- src/commands/activate.rs | 21 ++++++------ src/errors.rs | 16 +++++++++ src/virtualenv.rs | 12 +++---- src/virtualenv/local.rs | 2 +- src/virtualenv/python.rs | 72 ++++++++++++++++++++++++++++++++++++++++ src/virtualenv/rsenv.rs | 70 ++++++++++++++++++++++++-------------- src/virtualenv/utils.rs | 19 +++++++---- 9 files changed, 171 insertions(+), 50 deletions(-) create mode 100644 src/virtualenv/python.rs diff --git a/Cargo.toml b/Cargo.toml index ddd4e44..ffabd2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rsvenv" -version = "0.2.0" +version = "0.3.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html @@ -12,6 +12,7 @@ error-stack = "0.4.1" glob = "0.3.1" itertools = "0.12.1" lazy_static = "1.4.0" +regex = "1.10.4" serde = { version = "1.0.197", features = ["serde_derive"] } serde_derive = "1.0.197" shellexpand = "3.1.0" @@ -19,6 +20,7 @@ simplelog = { version = "0.12.2", features = ["paris"] } sysinfo = "0.30.7" [dev-dependencies] +cfg-if = "1.0.0" mockall = "0.12.1" tempfile = "3.10.1" diff --git a/README.md b/README.md index 9e09fc5..554f9d7 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ rsvenv list Result will be something like this ``` Rsenv environments: - test_rsenv + 3.11.8/test_rsenv Pyenv environments: 3.12.1/envs/armis 3.9.9/envs/proxy_pac @@ -36,6 +36,9 @@ To create a new virtual environment ```bash rsvenv create path/to/python venv_name ``` +The created virtual environment name will be python_version/venv_name + + After this it is possible to use this venv in current directory ```bash rsvenv use venv_name diff --git a/src/commands/activate.rs b/src/commands/activate.rs index 89b7b49..1d69525 100644 --- a/src/commands/activate.rs +++ b/src/commands/activate.rs @@ -4,7 +4,7 @@ use simplelog::{debug, error}; use crate::{ errors::{CommandExecutionError, VirtualEnvError}, - virtualenv::{pyenv::Pyenv, rsenv::Rsenv, traits::VirtualEnvCompatible, VirtualEnvironment}, + virtualenv::{pyenv::Pyenv, rsenv::Rsenv, VirtualEnvironment}, }; #[derive(Debug, Parser)] @@ -13,17 +13,18 @@ pub struct Command { virtualenv: String, } -fn try_activate(f: &dyn VirtualEnvCompatible, venv: &String) -> Result<(), VirtualEnvError> { - if f.list().contains(venv) { - let venv_struct = VirtualEnvironment { kind: f }; - if let Err(e) = venv_struct.activate(Some(venv)) { +fn try_activate(v: VirtualEnvironment, venv: &String) -> Result<(), VirtualEnvError> { + if v.kind.list().contains(venv) { + if let Err(e) = v.activate(Some(venv)) { error!("{e}"); return Err(e); }; + return Ok(()); } - Err(Report::new(VirtualEnvError::NotVirtualEnv( - venv.to_string(), - ))) + Err( + Report::new(VirtualEnvError::NotVirtualEnv(venv.to_string())) + .attach_printable("{venv} is not a virtual environment"), + ) } impl Command { @@ -31,10 +32,10 @@ impl Command { if let Err(e) = VirtualEnvironment::deactivate(true) { debug!("{e}"); }; - if let Ok(_) = try_activate(&Rsenv, &self.virtualenv) { + if try_activate(VirtualEnvironment { kind: &Rsenv }, &self.virtualenv).is_ok() { return Ok(()); } - if let Ok(_) = try_activate(&Pyenv, &self.virtualenv) { + if try_activate(VirtualEnvironment { kind: &Pyenv }, &self.virtualenv).is_ok() { return Ok(()); } error!( diff --git a/src/errors.rs b/src/errors.rs index 3508610..acc32a6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -11,6 +11,7 @@ pub enum VirtualEnvError { CreatingError, IOError, ConfigurationError, + IncorrectName, } impl fmt::Display for VirtualEnvError { @@ -27,6 +28,7 @@ impl fmt::Display for VirtualEnvError { } VirtualEnvError::ConfigurationError => "Configuration error".to_owned(), VirtualEnvError::CreatingError => "Error while creating virtual environment".to_owned(), + VirtualEnvError::IncorrectName => "Incorrect virtual environment name".to_owned(), }; f.write_str(&data) } @@ -46,3 +48,17 @@ impl fmt::Display for CommandExecutionError { } impl Context for CommandExecutionError {} + +#[derive(Debug)] +pub enum PythonInterpreterError { + UnableToDetectVersion, + CreateVenvError, +} + +impl fmt::Display for PythonInterpreterError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Error running python interpreter") + } +} + +impl Context for PythonInterpreterError {} diff --git a/src/virtualenv.rs b/src/virtualenv.rs index 67317a9..9a7c3ab 100644 --- a/src/virtualenv.rs +++ b/src/virtualenv.rs @@ -1,5 +1,6 @@ pub mod local; pub mod pyenv; +mod python; pub mod rsenv; pub mod traits; mod utils; @@ -10,20 +11,19 @@ use simplelog::info; use std::io; use std::io::Write; -use crate::errors::VirtualEnvError; -use crate::virtualenv::utils::{get_current_dir, is_virtualenv}; - use self::local::Local; use self::pyenv::Pyenv; use self::rsenv::Rsenv; use self::traits::VirtualEnvCompatible; +use crate::errors::VirtualEnvError; +use crate::virtualenv::utils::{get_current_dir, is_virtualenv}; -pub struct VirtualEnvironment<'a> { +pub struct VirtualEnvironment { // Venv path - pub kind: &'a dyn VirtualEnvCompatible, + pub kind: &'static dyn VirtualEnvCompatible, } -impl VirtualEnvironment<'_> { +impl VirtualEnvironment { pub fn detect() -> Option { if Rsenv.relevant() { return Some(Self { kind: &Rsenv }); diff --git a/src/virtualenv/local.rs b/src/virtualenv/local.rs index f4cc6d9..a2824f4 100644 --- a/src/virtualenv/local.rs +++ b/src/virtualenv/local.rs @@ -23,7 +23,7 @@ impl VirtualEnvCompatible for Local { fn venv_name(&self) -> Result { let current_path = self.root_dir()?; for local_venv_path in ["venv", ".venv", "virtualenv", ".virtualenv"] { - if let Ok(_) = is_virtualenv(¤t_path.join(local_venv_path)) { + if is_virtualenv(¤t_path.join(local_venv_path)).is_ok() { return Ok(local_venv_path.to_string()); } } diff --git a/src/virtualenv/python.rs b/src/virtualenv/python.rs new file mode 100644 index 0000000..2e9cc8d --- /dev/null +++ b/src/virtualenv/python.rs @@ -0,0 +1,72 @@ +use std::{path::PathBuf, process, str::FromStr}; + +use error_stack::{Report, Result, ResultExt}; +use glob::Pattern; +use simplelog::info; + +use crate::errors::PythonInterpreterError; + +pub struct PythonInterpreter<'a> { + pub version: String, + pub interpreter: &'a String, +} + +impl<'a> PythonInterpreter<'a> { + pub fn new(interpreter: &'a String) -> Result { + let version = PythonInterpreter::detect_version(interpreter)?; + Ok(PythonInterpreter { + version, + interpreter, + }) + } + + pub fn create_venv(&self, path: &PathBuf) -> Result<(), PythonInterpreterError> { + info!( + "Executing {} -m venv {}", + self.interpreter, + &path.as_path().display() + ); + let status = process::Command::new(self.interpreter) + .arg("-m") + .arg("venv") + .arg(path) + .status() + .change_context(PythonInterpreterError::CreateVenvError)?; + + if status.code().unwrap_or_default() > 0 { + return Err( + Report::new(PythonInterpreterError::CreateVenvError).attach_printable(format!( + "Error creating venv {}: ", + path.as_path().display() + )), + ); + } + + Ok(()) + } + + fn detect_version(interpreter: &String) -> Result { + info!("Detecting python version"); + let output = process::Command::new(interpreter) + .arg("-c") + .arg(r#"import platform; print(platform.python_version())"#) + .output() + .change_context(PythonInterpreterError::UnableToDetectVersion) + .attach_printable_lazy(|| "{:?}")?; + if output.status.code().unwrap_or(1) != 0 { + return Err(Report::new(PythonInterpreterError::UnableToDetectVersion) + .attach_printable(format!("Python executable returned {}", output.status,))); + } + let mut version = String::from_utf8(output.stdout) + .change_context(PythonInterpreterError::UnableToDetectVersion) + .attach_printable_lazy(|| "unable to read version from stdout: {:?}")?; + + version = version.trim().into(); + if !Pattern::from_str("*.*.*").unwrap().matches(&version) { + return Err(Report::new(PythonInterpreterError::UnableToDetectVersion) + .attach_printable(format!("Unexpected python version {}", &version))); + } + + Ok(version) + } +} diff --git a/src/virtualenv/rsenv.rs b/src/virtualenv/rsenv.rs index 53e050e..5588bdd 100644 --- a/src/virtualenv/rsenv.rs +++ b/src/virtualenv/rsenv.rs @@ -2,15 +2,16 @@ use std::{ collections::HashSet, fs::{self, File}, path::{Path, PathBuf}, - process, }; use crate::{configuration::SETTINGS, errors::VirtualEnvError}; use error_stack::{Report, Result, ResultExt}; +use regex::Regex; use simplelog::{error, info}; use std::io::Write; use super::{ + python::PythonInterpreter, traits::VirtualEnvCompatible, utils::{get_current_dir, get_venvs_by_glob}, }; @@ -19,8 +20,25 @@ use super::{ pub struct Rsenv; impl Rsenv { + pub fn validate_name(name: &str) -> Result<(), VirtualEnvError> { + if Regex::new(r"^[\w._]*$").unwrap().is_match(name) + || Regex::new(r"^[\w._]*\/[\w._]*$").unwrap().is_match(name) + { + return Ok(()); + } + Err(Report::new(VirtualEnvError::IncorrectName).attach("name is invalid")) + } + pub fn create(&self, name: &String, python: &String) -> Result<(), VirtualEnvError> { - if self.list().contains(name) { + let interpreter = + PythonInterpreter::new(python).change_context(VirtualEnvError::CreatingError)?; + + let name_with_version = format!("{}/{}", &interpreter.version, name); + let existing = self.list(); + + Rsenv::validate_name(name)?; + if existing.contains(name) || existing.contains(&name_with_version) { + error!("Virtual environment {name} exists"); return Err(Report::new(VirtualEnvError::AlreadyExists( name.to_string(), ))); @@ -36,33 +54,16 @@ impl Rsenv { info!("Created root dir"); } - let venv_path = path.join(name); - info!( - "Executing {} -m venv {}", - python, - &venv_path.as_path().display() - ); - let status = process::Command::new(python) - .arg("-m") - .arg("venv") - .arg(venv_path) - .status() + let venv_path = path.join(&interpreter.version).join(name); + interpreter + .create_venv(&venv_path) .change_context(VirtualEnvError::CreatingError)?; - - if status.code().unwrap_or_default() > 0 { - return Err( - Report::new(VirtualEnvError::CreatingError).attach_printable(format!( - "Error creating venv {}: ", - path.join(name).as_path().display() - )), - ); - } - - info!("Created virtual environment {name}"); + info!("Created venv {name_with_version}"); Ok(()) } pub fn delete(&self, name: String) -> Result<(), VirtualEnvError> { + Rsenv::validate_name(&name)?; if !self.list().contains(&name) { error!("Virtual environment `{name}` is not found"); return Err(Report::new(VirtualEnvError::NotVirtualEnv(name.clone())) @@ -91,7 +92,7 @@ impl VirtualEnvCompatible for Rsenv { .change_context(VirtualEnvError::ConfigurationError) .attach_printable("unable to expand SETTINGS.path to the actual path")? .to_string(); - Ok(Path::new(&expanded).to_path_buf().join("versions")) + Ok(Path::new(&expanded).to_path_buf().join("venvs")) } fn list(&self) -> HashSet { @@ -135,3 +136,22 @@ impl VirtualEnvCompatible for Rsenv { Ok(()) } } + +#[cfg(test)] +mod tests { + + use super::*; + + #[test] + fn test_name_ok() { + assert!(Rsenv::validate_name(&String::from("good_name")).is_ok()); + assert!(Rsenv::validate_name(&String::from("Good_nAme")).is_ok()); + assert!(Rsenv::validate_name(&String::from("Good_nAme/asdfadsf")).is_ok()); + } + #[test] + fn test_bad_name() { + assert!(Rsenv::validate_name(&String::from("bad!name")).is_err()); + assert!(Rsenv::validate_name(&String::from("Good_nAme/asdfadsf/smth")).is_err()); + assert!(Rsenv::validate_name(&String::from("Good_nAme aa")).is_err()); + } +} diff --git a/src/virtualenv/utils.rs b/src/virtualenv/utils.rs index 691b219..268180e 100644 --- a/src/virtualenv/utils.rs +++ b/src/virtualenv/utils.rs @@ -1,16 +1,21 @@ -use std::{collections::HashSet, fs, path::PathBuf}; +use std::{ + collections::HashSet, + fs, + path::{Path, PathBuf}, +}; use error_stack::{Report, Result, ResultExt}; use crate::errors::VirtualEnvError; -pub fn is_virtualenv(path: &PathBuf) -> Result<(), VirtualEnvError> { +pub fn is_virtualenv(path: &Path) -> Result<(), VirtualEnvError> { if fs::metadata(path.join("bin").join("activate")).is_ok_and(|x| x.is_file()) { Ok(()) } else { Err(Report::new(VirtualEnvError::NotVirtualEnv( path.to_string_lossy().to_string(), - ))) + )) + .attach_printable("Is not a virtual environment")) } } @@ -28,12 +33,14 @@ pub fn get_venvs_by_glob(glob: String, dir: &PathBuf) -> Result, .flatten() { let value = path - .strip_prefix(&dir) + .strip_prefix(dir) .attach_printable("Unable to strip prefix") .change_context(VirtualEnvError::VenvBuildError)? .to_str(); - if is_virtualenv(&path).is_ok() && !value.is_none() { - result.insert(String::from(value.unwrap())); + if let Some(unwrapped) = value { + if is_virtualenv(&path).is_ok() { + result.insert(String::from(unwrapped)); + } } }