diff --git a/crates/didbot-agentd/src/shut.rs b/crates/didbot-agentd/src/shut.rs index 78610804..0b83f671 100644 --- a/crates/didbot-agentd/src/shut.rs +++ b/crates/didbot-agentd/src/shut.rs @@ -79,16 +79,21 @@ pub(crate) fn temporary_for(path: &Path) -> PathBuf { } /// Writes `bytes` beside `path` and syncs them, without replacing `path`. +/// +/// Whatever sits at the temporary path is unlinked first and the file is then +/// created fresh, the shape `crates/didbot-tls/src/storage.rs` uses. That is +/// what keeps two things true. `rename(2)` carries the source's mode, so a +/// temporary left at a loose mode by a crash would widen the file it +/// replaces; and `create_new` refuses a symlink, so a temporary somebody +/// planted cannot point this write at a file of their choosing. pub(crate) fn stage(path: &Path, bytes: &[u8]) -> io::Result { let tmp = temporary_for(path); - let mut file = open_shut() - .write(true) - .create(true) - .truncate(true) - .open(&tmp)?; - // The temporary may be left over from a crash under a looser mode; the - // open above does not change an existing file's mode, so set it. - file.set_permissions(std::fs::Permissions::from_mode(FILE_MODE))?; + match std::fs::remove_file(&tmp) { + Ok(()) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + let mut file = open_shut().write(true).create_new(true).open(&tmp)?; file.write_all(bytes)?; file.sync_all()?; Ok(Staged { @@ -122,3 +127,45 @@ fn sync_parent(path: &Path) -> io::Result<()> { let dir = path.parent().unwrap_or(Path::new(".")); File::open(dir)?.sync_all() } + +#[cfg(test)] +mod tests { + use super::*; + use crate::scratch::Scratch; + + fn mode(path: &Path) -> u32 { + mode_of(&std::fs::metadata(path).unwrap()) + } + + #[test] + fn a_symlink_planted_at_the_temporary_does_not_take_the_write() { + let scratch = Scratch::new("shut-symlink"); + std::fs::create_dir_all(&scratch.0).unwrap(); + let elsewhere = scratch.0.join("elsewhere"); + std::fs::write(&elsewhere, b"bystander").unwrap(); + std::fs::set_permissions(&elsewhere, std::fs::Permissions::from_mode(0o644)).unwrap(); + + let path = scratch.0.join("secret"); + std::os::unix::fs::symlink(&elsewhere, temporary_for(&path)).unwrap(); + write(&path, b"a refresh token").unwrap(); + + assert_eq!(std::fs::read(&elsewhere).unwrap(), b"bystander"); + assert_eq!(mode(&elsewhere), 0o644, "not chmodded either"); + assert_eq!(std::fs::read(&path).unwrap(), b"a refresh token"); + assert_eq!(mode(&path), FILE_MODE); + } + + #[test] + fn a_temporary_left_at_a_loose_mode_does_not_widen_the_file_it_replaces() { + let scratch = Scratch::new("shut-loose-temporary"); + std::fs::create_dir_all(&scratch.0).unwrap(); + let path = scratch.0.join("secret"); + let tmp = temporary_for(&path); + std::fs::write(&tmp, b"what a crash left").unwrap(); + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o666)).unwrap(); + + write(&path, b"a refresh token").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"a refresh token"); + assert_eq!(mode(&path), FILE_MODE); + } +}