phase 2

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

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"),
+        }
+    }
+}

src/main.rs

@@ -1,106 +1,25 @@
-use std::ffi::OsString;
+use crate::parsing::parse_args;
-use std::path::PathBuf;
 
-use clap::{Parser, Subcommand};
+mod parsing;
-
+mod error;
-#[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<PathBuf>,
-        #[arg(short = 'i')]
-        key: Option<PathBuf>,
-        #[arg(short = 'f')]
-        force: bool,
-    },
-    Close {
-        drawer_file: PathBuf,
-        target_path: Option<PathBuf>,
-        #[arg(short = 'i')]
-        key: Option<PathBuf>,
-    },
-}
-
-pub fn parse_args<I, T>(args: I) -> Result<DrawerOperation, DrawerError>
-where
-    I: IntoIterator<Item = T>,
-    T: Into<OsString> + 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<PathBuf>) -> Result<PathBuf, DrawerError> {
-    if let Some(k) = key {
-        return Ok(k);
-    }
-    std::env::var("DRAWER_KEY")
-        .map(PathBuf::from)
-        .map_err(|_| DrawerError::NoKeyPath)
-}
 
 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() {
+    fn test_drawer_filenames() {
-        unsafe { std::env::set_var("DRAWER_KEY", "banana.key"); }
+        let op = parse_args(vec!["drawer", "open", "blah.notadrawer", "blah"]);
-        let op = parse_args(vec!["drawer", "open", "blah.drawer", "blah"]);
+        assert_eq!(op.unwrap().validate(), Err(DrawerError::DrawerFileInvalidExtension("blah.notadrawer".into())));
-        assert_eq!(*op.unwrap().key_path, *"banana.key");
+    }
 
-        unsafe { std::env::remove_var("DRAWER_KEY"); }
+    #[test]
-        let op = parse_args(vec!["drawer", "open", "blah.drawer", "blah"]);
+    fn test_missing_key() {
-        assert_eq!(op, Err(DrawerError::NoKeyPath))
+        let op = parse_args(vec!["drawer", "open", "blah.drawer", "-i", "notthere", "blah"]);
+        assert_eq!(op.unwrap().validate(), Err(DrawerError::KeyInvalid("notthere".into())));
     }
 }

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<PathBuf>,
+        #[arg(short = 'i')]
+        key: Option<PathBuf>,
+        #[arg(short = 'f')]
+        force: bool,
+    },
+    Close {
+        drawer_file: PathBuf,
+        target_path: Option<PathBuf>,
+        #[arg(short = 'i')]
+        key: Option<PathBuf>,
+    },
+}
+
+pub fn parse_args<I, T>(args: I) -> Result<DrawerOperation, DrawerError>
+where
+    I: IntoIterator<Item = T>,
+    T: Into<OsString> + 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<PathBuf>) -> 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(())
+    }
+}