From 6956f67f3239c81987c851a9a924c59030ef5729 Mon Sep 17 00:00:00 2001 From: Ross Andrews Date: Mon, 11 May 2026 20:04:46 -0500 Subject: [PATCH] bug --- CLAUDE.md | 46 ++++++++++++++++++++------------------ plans/phase7.md | 27 ++++++++++++++++++++++ src/parsing.rs | 59 +++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 102 insertions(+), 30 deletions(-) create mode 100644 plans/phase7.md diff --git a/CLAUDE.md b/CLAUDE.md index 73a230d..4f1f6db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,38 +12,42 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - `flate2` — gzip compression, paired with `tar` - `age` — encryption/decryption using SSH keys (RSA or Ed25519) loaded from disk - `clap` — command line argument parsing (derive API), used for subcommands and flags +- `ssh-key` — generating new Ed25519 SSH keys +- `rand` — `OsRng` entropy source for key generation `drawer` is a command line tool for managing projects. -A drawer is a single directory with related files in for a project. Drawer files always end -in the `.drawer` extension, like `foo.drawer`. To create a drawer file from a directory, we -first tar all files in the directory, and then encrypt the resulting tarball. To open a drawer -file, we decrypt it and expand the tarball to a directory. +A drawer is a single directory with related files for a project. Drawer files always end +in the `.drawer` extension, like `foo.drawer`. To create a drawer file from a directory, +the contents are tarred, gzip-compressed, and encrypted in memory before being written to disk. +To open a drawer file, it is decrypted and the tarball is expanded into a directory. -The `drawer` tool has two subcommands, `drawer open` and `drawer close`. Each of these commands -requires a key, the path to which can be given by either the `$DRAWER_KEY` environment variable -or a `-i` command line parameter. +The `drawer` tool has three subcommands. `open` and `close` require a key (an SSH private key), +the path to which can be given by either the `$DRAWER_KEY` environment variable or `-i`. -### Open subcommand +### open -The `drawer open` subcommand will decrypt and untar a drawer file into a path. It takes two arguments, -the name of a drawer file (required) and a path to expand it into (optional, defaults to a directory -under the current directory named the same as the drawer file). If the target path exists and is not -empty, print an error message, unless the `-f` flag is passed. +Decrypt and expand a drawer file into a directory. Takes a drawer file (required) and a target +path (optional, defaults to a directory named after the drawer file stem). Prints a message when +the target path is inferred. Fails if the target exists and is non-empty, unless `-f` is passed. -### Close subcommand +### close -The `drawer close` subcommand will create a drawer file from a directory and a key. The command -takes two arguments, the name of a drawer file (required) and the path to a directory (optional, -defaults to the current directory). If the target drawer file is inside the directory, print an -error message saying so. Otherwise, create a tarball (in memory) of the directory contents, encrypt -them (again in memory), and write the drawer file to disk. +Compress and encrypt a directory into a drawer file. Takes a drawer file (required) and a source +directory (optional, defaults to the directory named after the drawer file stem). Prints a message +when the source path is inferred. Fails if the drawer file would be written inside the source directory. + +### key + +Generate a new Ed25519 SSH key and write it to a file. Takes a filename (required). Fails if the +file already exists or the parent directory does not exist. ### Examples: -- `drawer close -i ~/.keys/mykey ~/Documents/blah.drawer ~/Documents/blah` (create a file, `~/Documents/blah.drawer`, -encrypted with `~/.keys/mykey`, containing the contents of `~/Documents/blah`) -- `drawer open ~/Documents/blah.drawer` (open `~/Documents/blah.drawer` into `./blah` using a key found at `$DRAWER_KEY`) +- `drawer close -i ~/.keys/mykey ~/Documents/blah.drawer ~/Documents/blah` — create `~/Documents/blah.drawer` from `~/Documents/blah` +- `drawer close -i ~/.keys/mykey ~/Documents/blah.drawer` — same, inferring source directory from drawer name +- `drawer open ~/Documents/blah.drawer` — open into `./blah` using `$DRAWER_KEY` +- `drawer key ~/.keys/mykey` — generate a new SSH key at `~/.keys/mykey` ## Implementation plan diff --git a/plans/phase7.md b/plans/phase7.md new file mode 100644 index 0000000..af0330f --- /dev/null +++ b/plans/phase7.md @@ -0,0 +1,27 @@ +# Phase 7 + +Supporting single-file (as opposed to folder) drawers has introduced a bug, which will require some refactoring to fix. +The basic problem is a confusion over where to open a single-file drawer to, the filename specified in the target path +(including the default name rules on that) or the filename in the tarball. + +The desired behavior is that if a target path is specified, it should be what's validated and used. If a target path is +not specified, the default should be: + +- For a single-file drawer, the filename of the single file in the drawer +- For a folder drawer, a folder named after the drawer file (without extension) + +In order to make this work, we'll need to refactor to move some of the validation into perform_open, since we can't +check whether the target path is valid (or even know what it is) until we first load and decrypt the drawer. + +Let's do the following: + +- `target_path` now needs to be an `Option` and we can get rid of `target_inferred` because `target_path` being +None means the same thing. +- Let's make a `default_target` method which returns what the default target would be for a given DrawerOperation, to +encapsulate these default rules (and put a nice comment above it explaining the rules). +- `validate_open` should no longer attempt to validate the target path. +- `perform_open` now needs to figure out a target path using the new `default_target` method (or `target_path` if present) +and can do the validation there. + +As an aside, is there any duplicated logic between `validate_open` and `validate_close` as far as checking the drawer file +path? Can any of that be extracted into a method? \ No newline at end of file diff --git a/src/parsing.rs b/src/parsing.rs index 9a61077..a69b732 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -122,6 +122,21 @@ fn resolve_key(key: Option) -> KeyType { EnvVar } +fn single_file_entry(data: &[u8]) -> Option { + let cursor = std::io::Cursor::new(data); + let gz = GzDecoder::new(cursor); + let mut archive = tar::Archive::new(gz); + let mut entries = archive.entries().ok()?; + let entry = entries.next()?.ok()?; + if !entry.header().entry_type().is_file() { + return None; + } + if entries.next().is_some() { + return None; + } + Some(entry.path().ok()?.into_owned()) +} + impl DrawerOperation { pub fn perform(&self) -> Result<(), DrawerError> { self.validate()?; @@ -138,11 +153,28 @@ impl DrawerOperation { let decrypted = self.decrypt(encrypted)?; - let cursor = std::io::Cursor::new(decrypted); - let gz = GzDecoder::new(cursor); - let mut archive = tar::Archive::new(gz); - archive.unpack(&self.target_path) - .map_err(|_| DrawerError::UntarFailed)?; + if let Some(entry_name) = single_file_entry(&decrypted) { + let output = if self.target_inferred { + PathBuf::from(entry_name.file_name().unwrap_or_default()) + } else { + self.target_path.clone() + }; + let cursor = std::io::Cursor::new(&decrypted); + let gz = GzDecoder::new(cursor); + let mut archive = tar::Archive::new(gz); + let mut entry = archive.entries() + .map_err(|_| DrawerError::UntarFailed)? + .next() + .ok_or(DrawerError::UntarFailed)? + .map_err(|_| DrawerError::UntarFailed)?; + entry.unpack(&output).map_err(|_| DrawerError::UntarFailed)?; + } else { + let cursor = std::io::Cursor::new(decrypted); + let gz = GzDecoder::new(cursor); + let mut archive = tar::Archive::new(gz); + archive.unpack(&self.target_path) + .map_err(|_| DrawerError::UntarFailed)?; + } Ok(()) } @@ -174,8 +206,14 @@ impl DrawerOperation { { let gz = GzEncoder::new(&mut buf, Compression::default()); let mut archive = tar::Builder::new(gz); - archive.append_dir_all(".", &self.target_path) - .map_err(|_| DrawerError::TarFailed)?; + if self.target_path.is_dir() { + archive.append_dir_all(".", &self.target_path) + .map_err(|_| DrawerError::TarFailed)?; + } else { + let name = self.target_path.file_name().unwrap_or_default(); + archive.append_path_with_name(&self.target_path, name) + .map_err(|_| DrawerError::TarFailed)?; + } let gz = archive.into_inner() .map_err(|_| DrawerError::TarFailed)?; gz.finish() @@ -268,6 +306,9 @@ impl DrawerOperation { return Err(DrawerError::DrawerFileNotFound(self.drawer_file.clone())); } if !self.force && self.target_path.exists() { + if self.target_path.is_file() { + return Err(DrawerError::TargetNotEmpty(self.target_path.clone())); + } let mut entries = std::fs::read_dir(&self.target_path) .map_err(|_| DrawerError::TargetInvalid(self.target_path.clone()))?; if entries.next().is_some() { @@ -292,10 +333,10 @@ impl DrawerOperation { } let meta = std::fs::metadata(&self.target_path) .map_err(|_| DrawerError::TargetInvalid(self.target_path.clone()))?; - if !meta.is_dir() { + if !meta.is_dir() && !meta.is_file() { return Err(DrawerError::TargetInvalid(self.target_path.clone())); } - if std::fs::read_dir(&self.target_path).is_err() { + if meta.is_dir() && std::fs::read_dir(&self.target_path).is_err() { return Err(DrawerError::TargetInvalid(self.target_path.clone())); } let canon_target = self.target_path.canonicalize()