Skip to content

Commit 1347527

Browse files
zoza1982claude
andauthored
fix(vault): create the vault directory before writing (fixes "io error" on first run) (#127)
On a fresh install the platform config dir (e.g. ~/.config/cairn) does not exist until something writes there. Vault creation — triggered the first time a credential is saved, e.g. adding an SSH host with a password — failed with a raw "io error" because `NamedTempFile::new_in` cannot create its temp file in a missing directory. Both vault write paths (`atomic_create` and `atomic_write`) now ensure the parent directory exists first via a shared `ensure_parent_dir` helper, which sets a directory it creates to owner-only 0700 on Unix (the vault file itself is already 0600). A pre-existing user directory is left untouched. Regression test: `Vault::create` into a path whose parent is missing now succeeds and yields a 0700 directory. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 562295b commit 1347527

2 files changed

Lines changed: 67 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
9191

9292
### Fixed
9393

94+
- **Creating the vault no longer fails with "io error" on a fresh install.** The vault's config
95+
directory (e.g. `~/.config/cairn`) does not exist until something writes there, and vault creation
96+
(triggered the first time you save a credential — e.g. adding an SSH host with a password) failed
97+
because the atomic write couldn't create its temp file in a missing directory. Vault writes now
98+
create the parent directory first (owner-only `0700` on Unix). Regression test added.
99+
94100
- **Panes now root at the OS filesystem root** (`/` on Unix, drive root on Windows) so `..`
95101
navigation is unrestricted all the way up. Cairn still opens at the launch directory, but
96102
the user can navigate above it without hitting an artificial ceiling. The `LocalVfs` base is

crates/cairn-vault/src/lib.rs

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,37 @@ fn parse(bytes: &[u8]) -> Result<Parsed<'_>, VaultError> {
404404
})
405405
}
406406

407+
/// Resolve `path`'s parent directory, creating it (and any missing ancestors) if absent.
408+
///
409+
/// The vault file's containing directory (the platform config dir, e.g. `~/.config/cairn`) does
410+
/// not exist on a fresh install, and `NamedTempFile::new_in` fails with a raw I/O error ("io
411+
/// error") if it is missing — so both write paths must ensure it exists first. A directory we
412+
/// create here is set owner-only (`0700`) on Unix, since it holds the encrypted vault; a directory
413+
/// the user already set up is left untouched.
414+
fn ensure_parent_dir(path: &Path) -> Result<PathBuf, VaultError> {
415+
let dir = path
416+
.parent()
417+
.unwrap_or_else(|| Path::new("."))
418+
.to_path_buf();
419+
// Sampled before the create so the chmod below only tightens a dir we made. Bound under
420+
// `cfg(unix)` because that block is its only reader — otherwise it's unused on Windows and
421+
// trips `-D warnings`.
422+
#[cfg(unix)]
423+
let newly_created = !dir.exists();
424+
std::fs::create_dir_all(&dir)?;
425+
#[cfg(unix)]
426+
if newly_created {
427+
use std::os::unix::fs::PermissionsExt;
428+
// Best-effort: never fail the write over a permission tweak on a dir we just made.
429+
let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700));
430+
}
431+
Ok(dir)
432+
}
433+
407434
fn atomic_write(path: &Path, bytes: &[u8]) -> Result<(), VaultError> {
408435
use std::io::Write;
409-
let dir = path.parent().unwrap_or_else(|| Path::new("."));
410-
let mut tmp = tempfile::NamedTempFile::new_in(dir)?;
436+
let dir = ensure_parent_dir(path)?;
437+
let mut tmp = tempfile::NamedTempFile::new_in(&dir)?;
411438
tmp.write_all(bytes)?;
412439
tmp.as_file().sync_all()?;
413440
tmp.persist(path).map_err(|e| VaultError::Io(e.error))?;
@@ -423,8 +450,8 @@ fn atomic_write(path: &Path, bytes: &[u8]) -> Result<(), VaultError> {
423450
/// receive the same sentinel as the pre-flight check, not a raw I/O error.
424451
fn atomic_create(path: &Path, bytes: &[u8]) -> Result<(), VaultError> {
425452
use std::io::Write;
426-
let dir = path.parent().unwrap_or_else(|| Path::new("."));
427-
let mut tmp = tempfile::NamedTempFile::new_in(dir)?;
453+
let dir = ensure_parent_dir(path)?;
454+
let mut tmp = tempfile::NamedTempFile::new_in(&dir)?;
428455
tmp.write_all(bytes)?;
429456
tmp.as_file().sync_all()?;
430457
tmp.persist_noclobber(path).map_err(|e| {
@@ -564,6 +591,36 @@ mod tests {
564591
));
565592
}
566593

594+
/// Regression: on a fresh install the platform config dir (e.g. `~/.config/cairn`) does not
595+
/// exist yet, and `Vault::create` used to fail with a raw "io error" because
596+
/// `NamedTempFile::new_in` can't create a temp file in a nonexistent directory. The create path
597+
/// must now create the parent directory itself (owner-only `0700` on Unix).
598+
#[test]
599+
fn create_succeeds_when_the_parent_directory_is_missing() {
600+
let base = tempfile::tempdir().unwrap();
601+
let path = base
602+
.path()
603+
.join("does")
604+
.join("not")
605+
.join("exist")
606+
.join("vault.cvlt");
607+
assert!(!path.parent().unwrap().exists());
608+
let vault = Vault::create_with_params(&path, &pass("pw"), KdfParams::fast_for_tests())
609+
.expect("create must succeed into a missing dir");
610+
assert!(path.exists(), "vault file must be written");
611+
// The created vault directory is owner-only (0700) on Unix.
612+
#[cfg(unix)]
613+
{
614+
use std::os::unix::fs::PermissionsExt;
615+
let mode = std::fs::metadata(path.parent().unwrap())
616+
.unwrap()
617+
.permissions()
618+
.mode();
619+
assert_eq!(mode & 0o777, 0o700, "created vault dir must be 0700");
620+
}
621+
drop(vault);
622+
}
623+
567624
/// Regression test for the create-path clobber window (Fix 2): `atomic_create` must return
568625
/// `VaultError::AlreadyExists` and leave the existing file byte-for-byte intact.
569626
///

0 commit comments

Comments
 (0)