From 60f037d20d253abe883ac9b8bf12f1f8d3a5fc05 Mon Sep 17 00:00:00 2001 From: Ross Andrews Date: Sat, 9 May 2026 17:22:50 -0500 Subject: [PATCH] phase 2 --- CLAUDE.md | 16 +++++ src/error.rs | 29 +++++++++ src/main.rs | 134 +++++++++--------------------------------- src/parsing.rs | 156 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 106 deletions(-) create mode 100644 src/error.rs create mode 100644 src/parsing.rs diff --git a/CLAUDE.md b/CLAUDE.md index 278cee2..0df9e93 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,6 +74,22 @@ Certain things should cause the function to return `Err`: - Passing any flags but `-f` or `-i` - Not passing `-i` when the `$DRAWER_KEY` environment variable is undefined +### Phase 2 + +Phase 2 is about error handling. We want to add some variants to `DrawerError` and a `validate` method to `DrawerOperation` +that will catch various problems and ensure that we can do the operation correctly. Here are some error cases in no particular order: + +- If the command is `Open` and the drawer file does not exist. +- If the command is `Close` and the target path either does not exist, is not a directory, or is not readable. +- If the `key` is `EnvVar` but the `$DRAWER_KEY` environment variable is not set, or is not set to a path that exists and is readable. +- If the `key` is `Path()` but the path does not exist or is not readable. +- If the command is `Open` and the target is a non-empty directory that exists, and the `force` flag is false +- If the command is `Close` and the target contains the drawer file (at any level; the target is any parent of the drawer file's path) + +The `validate` method should take `&self` and return `Result<(), DrawerError>`. Let's have `main`, which is currently +creating a `DrawerOperation` and not using it, also call `validate` and print out the error if there is one. This means +we also need to `impl Display` on `DrawerError`. + ## Commands ```bash diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..186a5d6 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,29 @@ +use std::fmt; +use std::path::PathBuf; + +#[derive(Debug, PartialEq)] +pub enum DrawerError { + ParseError(String), + NoKeyPath, + DrawerFileInvalidExtension(PathBuf), + DrawerFileNotFound(PathBuf), + TargetInvalid(PathBuf), + KeyInvalid(PathBuf), + TargetNotEmpty(PathBuf), + DrawerInsideTarget, +} + +impl fmt::Display for DrawerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DrawerError::ParseError(s) => write!(f, "{s}"), + DrawerError::NoKeyPath => write!(f, "no key specified: set $DRAWER_KEY or use -i"), + DrawerError::DrawerFileInvalidExtension(p) => write!(f, "drawer file must have a .drawer extension: {}", p.display()), + DrawerError::DrawerFileNotFound(p) => write!(f, "drawer file not found: {}", p.display()), + DrawerError::TargetInvalid(p) => write!(f, "target path does not exist, is not a directory, or is not readable: {}", p.display()), + DrawerError::KeyInvalid(p) => write!(f, "key not found or not readable: {}", p.display()), + DrawerError::TargetNotEmpty(p) => write!(f, "target is not empty: {} (use -f to force)", p.display()), + DrawerError::DrawerInsideTarget => write!(f, "drawer file is inside the target directory"), + } + } +} diff --git a/src/main.rs b/src/main.rs index 1d78a4d..6796f70 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,106 +1,25 @@ -use std::ffi::OsString; -use std::path::PathBuf; +use crate::parsing::parse_args; -use clap::{Parser, Subcommand}; - -#[derive(Debug, Copy, Clone, PartialEq)] -pub enum OperationType { - Open, - Close, -} - -#[derive(Debug, PartialEq)] -pub struct DrawerOperation { - pub operation_type: OperationType, - pub drawer_file: PathBuf, - pub key_path: PathBuf, - pub target_path: PathBuf, - pub force: bool, -} - -#[derive(Debug, PartialEq)] -pub enum DrawerError { - ParseError(String), - NoKeyPath, -} - -#[derive(Parser)] -#[command(name = "drawer")] -struct Cli { - #[command(subcommand)] - command: Command, -} - -#[derive(Subcommand)] -enum Command { - Open { - drawer_file: PathBuf, - target_path: Option, - #[arg(short = 'i')] - key: Option, - #[arg(short = 'f')] - force: bool, - }, - Close { - drawer_file: PathBuf, - target_path: Option, - #[arg(short = 'i')] - key: Option, - }, -} - -pub fn parse_args(args: I) -> Result -where - I: IntoIterator, - T: Into + Clone, -{ - let cli = Cli::try_parse_from(args) - .map_err(|e| DrawerError::ParseError(e.to_string()))?; - - match cli.command { - Command::Open { drawer_file, target_path, key, force } => { - let key_path = resolve_key(key)?; - let target = target_path.unwrap_or_else(|| { - PathBuf::from(drawer_file.file_stem().unwrap_or_default()) - }); - Ok(DrawerOperation { - operation_type: OperationType::Open, - drawer_file, - key_path, - target_path: target, - force, - }) - } - Command::Close { drawer_file, target_path, key } => { - let key_path = resolve_key(key)?; - let target = target_path.unwrap_or_else(|| PathBuf::from(".")); - Ok(DrawerOperation { - operation_type: OperationType::Close, - drawer_file, - key_path, - target_path: target, - force: false, - }) - } - } -} - -fn resolve_key(key: Option) -> Result { - if let Some(k) = key { - return Ok(k); - } - std::env::var("DRAWER_KEY") - .map(PathBuf::from) - .map_err(|_| DrawerError::NoKeyPath) -} +mod parsing; +mod error; fn main() { - let _ = parse_args(std::env::args_os()); + match parse_args(std::env::args_os()) { + Err(e) => eprintln!("{e}"), + Ok(op) => { + if let Err(e) = op.validate() { + eprintln!("{e}"); + } + } + } } #[cfg(test)] mod tests { - use crate::OperationType::*; + use crate::error::DrawerError; + use crate::parsing::DrawerOperation; + use crate::parsing::KeyType::*; + use crate::parsing::OperationType::*; use super::*; #[test] @@ -110,7 +29,7 @@ mod tests { DrawerOperation { operation_type: Close, drawer_file: "blah.drawer".into(), - key_path: "foo.key".into(), + key: Path("foo.key".into()), target_path: "blah".into(), force: false, } @@ -124,7 +43,7 @@ mod tests { DrawerOperation { operation_type: Open, drawer_file: "blah.drawer".into(), - key_path: "foo.key".into(), + key: Path("foo.key".into()), target_path: "blah".into(), force: false, } @@ -132,17 +51,20 @@ mod tests { } #[test] - fn test_missing_key() { + fn test_key_env_variable() { + let op = parse_args(vec!["drawer", "open", "blah.drawer", "blah"]); + assert_eq!(op.unwrap().key, EnvVar) } #[test] - fn test_key_env_variable() { - unsafe { std::env::set_var("DRAWER_KEY", "banana.key"); } - let op = parse_args(vec!["drawer", "open", "blah.drawer", "blah"]); - assert_eq!(*op.unwrap().key_path, *"banana.key"); + fn test_drawer_filenames() { + let op = parse_args(vec!["drawer", "open", "blah.notadrawer", "blah"]); + assert_eq!(op.unwrap().validate(), Err(DrawerError::DrawerFileInvalidExtension("blah.notadrawer".into()))); + } - unsafe { std::env::remove_var("DRAWER_KEY"); } - let op = parse_args(vec!["drawer", "open", "blah.drawer", "blah"]); - assert_eq!(op, Err(DrawerError::NoKeyPath)) + #[test] + fn test_missing_key() { + let op = parse_args(vec!["drawer", "open", "blah.drawer", "-i", "notthere", "blah"]); + assert_eq!(op.unwrap().validate(), Err(DrawerError::KeyInvalid("notthere".into()))); } } \ No newline at end of file diff --git a/src/parsing.rs b/src/parsing.rs new file mode 100644 index 0000000..e27dcf5 --- /dev/null +++ b/src/parsing.rs @@ -0,0 +1,156 @@ +use std::ffi::OsString; +use std::io; +use std::path::{Path, PathBuf}; +use clap::{Parser, Subcommand}; +use crate::error::DrawerError; +use crate::parsing::KeyType::*; + +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum OperationType { + Open, + Close, +} + +#[derive(Debug, PartialEq)] +pub struct DrawerOperation { + pub operation_type: OperationType, + pub drawer_file: PathBuf, + pub key: KeyType, + pub target_path: PathBuf, + pub force: bool, +} + +#[derive(Debug, PartialEq)] +pub enum KeyType { + EnvVar, Path(PathBuf) +} + +#[derive(Parser)] +#[command(name = "drawer")] +struct Cli { + #[command(subcommand)] + command: Command, +} + +#[derive(Subcommand)] +enum Command { + Open { + drawer_file: PathBuf, + target_path: Option, + #[arg(short = 'i')] + key: Option, + #[arg(short = 'f')] + force: bool, + }, + Close { + drawer_file: PathBuf, + target_path: Option, + #[arg(short = 'i')] + key: Option, + }, +} + +pub fn parse_args(args: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + let cli = Cli::try_parse_from(args) + .map_err(|e| DrawerError::ParseError(e.to_string()))?; + + match cli.command { + Command::Open { drawer_file, target_path, key, force } => { + let key = resolve_key(key); + let target = target_path.unwrap_or_else(|| { + PathBuf::from(drawer_file.file_stem().unwrap_or_default()) + }); + Ok(DrawerOperation { + operation_type: OperationType::Open, + drawer_file, + key, + target_path: target, + force, + }) + } + Command::Close { drawer_file, target_path, key } => { + let key = resolve_key(key); + let target = target_path.unwrap_or_else(|| PathBuf::from(".")); + Ok(DrawerOperation { + operation_type: OperationType::Close, + drawer_file, + key, + target_path: target, + force: false, + }) + } + } +} + +fn resolve_key(key: Option) -> KeyType { + if let Some(k) = key { + return Path(k); + } + EnvVar +} + +impl DrawerOperation { + pub fn validate(&self) -> Result<(), DrawerError> { + if self.drawer_file.extension().and_then(|e| e.to_str()) != Some("drawer") { + return Err(DrawerError::DrawerFileInvalidExtension(self.drawer_file.clone())); + } + self.validate_key()?; + match self.operation_type { + OperationType::Open => self.validate_open(), + OperationType::Close => self.validate_close(), + } + } + + fn validate_key(&self) -> Result<(), DrawerError> { + let key_path = match &self.key { + EnvVar => { + let val = std::env::var("DRAWER_KEY").map_err(|_| DrawerError::NoKeyPath)?; + PathBuf::from(val) + } + Path(p) => p.clone(), + }; + match std::fs::File::open(&key_path) { + Ok(_) => Ok(()), + Err(_) => Err(DrawerError::KeyInvalid(key_path)), + } + } + + fn validate_open(&self) -> Result<(), DrawerError> { + if !self.drawer_file.exists() { + return Err(DrawerError::DrawerFileNotFound(self.drawer_file.clone())); + } + if !self.force && self.target_path.exists() { + let mut entries = std::fs::read_dir(&self.target_path) + .map_err(|_| DrawerError::TargetInvalid(self.target_path.clone()))?; + if entries.next().is_some() { + return Err(DrawerError::TargetNotEmpty(self.target_path.clone())); + } + } + Ok(()) + } + + fn validate_close(&self) -> Result<(), DrawerError> { + let meta = std::fs::metadata(&self.target_path) + .map_err(|_| DrawerError::TargetInvalid(self.target_path.clone()))?; + if !meta.is_dir() { + return Err(DrawerError::TargetInvalid(self.target_path.clone())); + } + if std::fs::read_dir(&self.target_path).is_err() { + return Err(DrawerError::TargetInvalid(self.target_path.clone())); + } + let canon_target = self.target_path.canonicalize() + .unwrap_or_else(|_| self.target_path.clone()); + let drawer_parent = self.drawer_file.parent().unwrap_or(Path::new(".")); + let canon_drawer = drawer_parent.canonicalize() + .unwrap_or_else(|_| drawer_parent.to_path_buf()) + .join(self.drawer_file.file_name().unwrap_or_default()); + if canon_drawer.starts_with(&canon_target) { + return Err(DrawerError::DrawerInsideTarget); + } + Ok(()) + } +}