From 99c4d02a70516e5993a4ccad0ce5799c04b2edf4 Mon Sep 17 00:00:00 2001 From: rfxfxfx Date: Sat, 29 Aug 2026 00:10:10 +0800 Subject: [PATCH 1/2] fix(cli): enforce 0600 on an existing private validator key file `OpenOptions::mode(0o600)` applies only when the file is created. If the key path already exists with looser permissions, the mode is silently ignored and the private validator key is written into a world-readable file. This is reachable: `arc init --overwrite` writes a fresh key over an existing path (`cmd/init.rs:53`), so a key file restored from a backup or left behind by an older version keeps its original mode. Call `set_permissions(0o600)` after opening so an existing file is tightened before the key is written to it. Adds two tests: one covering the existing create path, and one that pre-creates a 0644 file and asserts it is tightened to 0600. Co-Authored-By: Claude Opus 5 --- crates/malachite-cli/src/file.rs | 58 ++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/crates/malachite-cli/src/file.rs b/crates/malachite-cli/src/file.rs index 5f49e3cb..88539df2 100644 --- a/crates/malachite-cli/src/file.rs +++ b/crates/malachite-cli/src/file.rs @@ -45,14 +45,21 @@ fn save(path: &Path, data: &str) -> Result<(), Error> { // Create file with secure permissions (0600) on Unix systems #[cfg(unix)] let mut f = { - use std::os::unix::fs::OpenOptionsExt; - fs::OpenOptions::new() + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + let f = fs::OpenOptions::new() .write(true) .create(true) .truncate(true) .mode(0o600) // Set permissions at creation time .open(path) - .map_err(|_| Error::OpenFile(path.to_path_buf()))? + .map_err(|_| Error::OpenFile(path.to_path_buf()))?; + // `mode()` above applies only when the file is created. If the path already + // exists with looser permissions, it is silently ignored and the private key + // would be written into a world-readable file. Enforce 0600 explicitly so an + // existing file is tightened before the key is written to it. + f.set_permissions(fs::Permissions::from_mode(0o600)) + .map_err(|_| Error::OpenFile(path.to_path_buf()))?; + f }; #[cfg(not(unix))] @@ -68,3 +75,48 @@ fn save(path: &Path, data: &str) -> Result<(), Error> { Ok(()) } + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use arc_consensus_types::signing::PrivateKey; + use rand::rngs::OsRng; + use std::os::unix::fs::PermissionsExt; + use tempfile::tempdir; + + fn mode_of(path: &Path) -> u32 { + fs::metadata(path).unwrap().permissions().mode() & 0o777 + } + + #[test] + fn save_priv_validator_key_creates_file_with_0600() { + let dir = tempdir().unwrap(); + let key_file = dir.path().join("priv_validator_key.json"); + + save_priv_validator_key(&key_file, &PrivateKey::generate(OsRng)).unwrap(); + + assert_eq!(mode_of(&key_file), 0o600); + } + + #[test] + fn save_priv_validator_key_tightens_existing_loose_permissions() { + let dir = tempdir().unwrap(); + let key_file = dir.path().join("priv_validator_key.json"); + + // Simulate a key file left world-readable by a backup restore or an + // older version. `OpenOptions::mode()` is ignored for existing files, + // so without an explicit set_permissions the key would be written into + // this 0644 file. + fs::write(&key_file, "{}").unwrap(); + fs::set_permissions(&key_file, fs::Permissions::from_mode(0o644)).unwrap(); + assert_eq!(mode_of(&key_file), 0o644); + + save_priv_validator_key(&key_file, &PrivateKey::generate(OsRng)).unwrap(); + + assert_eq!( + mode_of(&key_file), + 0o600, + "existing file should be tightened to 0600 before the key is written" + ); + } +} From f8334fa3361c366c57d7369454dd7b8bcc76a442 Mon Sep 17 00:00:00 2001 From: rfxfxfx Date: Mon, 31 Aug 2026 19:40:00 +0800 Subject: [PATCH 2/2] test(cli): use a plain #[cfg(test)] module so clippy allows unwrap CI clippy failed with `used unwrap() on a Result value` in the new tests. The workspace denies `clippy::unwrap_used`, and `clippy.toml` relaxes it with `allow-unwrap-in-tests = true`. That relaxation only applies to items clippy recognises as test code, which requires a plain `#[cfg(test)]` module. The new tests were gated `#[cfg(all(test, unix))]`, so clippy did not treat them as tests and the deny applied. Switch to `#[cfg(test)] mod tests` with `#[cfg(unix)]` on the individual helper and tests, matching the convention already used in cmd/init.rs and cmd/start.rs. No change to what is tested. Co-Authored-By: Claude Opus 5 --- crates/malachite-cli/src/file.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/malachite-cli/src/file.rs b/crates/malachite-cli/src/file.rs index 88539df2..e9078b41 100644 --- a/crates/malachite-cli/src/file.rs +++ b/crates/malachite-cli/src/file.rs @@ -76,18 +76,22 @@ fn save(path: &Path, data: &str) -> Result<(), Error> { Ok(()) } -#[cfg(all(test, unix))] +#[cfg(test)] mod tests { use super::*; use arc_consensus_types::signing::PrivateKey; use rand::rngs::OsRng; - use std::os::unix::fs::PermissionsExt; use tempfile::tempdir; + /// Returns the permission bits of `path` on Unix systems. + #[cfg(unix)] fn mode_of(path: &Path) -> u32 { - fs::metadata(path).unwrap().permissions().mode() & 0o777 + use std::os::unix::fs::PermissionsExt; + let metadata = fs::metadata(path).unwrap(); + metadata.permissions().mode() & 0o777 } + #[cfg(unix)] #[test] fn save_priv_validator_key_creates_file_with_0600() { let dir = tempdir().unwrap(); @@ -98,8 +102,11 @@ mod tests { assert_eq!(mode_of(&key_file), 0o600); } + #[cfg(unix)] #[test] fn save_priv_validator_key_tightens_existing_loose_permissions() { + use std::os::unix::fs::PermissionsExt; + let dir = tempdir().unwrap(); let key_file = dir.path().join("priv_validator_key.json");