fix(cli): enforce 0600 on an existing private validator key file

[0xrlawrence]

Problem

OpenOptions::mode(0o600) in crates/malachite-cli/src/file.rs 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 rather than theoretical. cmd/init.rs:53 guards with:

if priv_validator_key_file.exists() && !overwrite { ... }

so init --overwrite against an existing key file (restored from a backup, or left by an older version) writes a fresh private key while keeping the file's original mode.

Fix

Call set_permissions(0o600) after opening, so an existing file is tightened before the key is written to it. Creation behaviour is unchanged.

Tests

Adds two tests to file.rs:

• save_priv_validator_key_creates_file_with_0600 covers the existing create path
• save_priv_validator_key_tightens_existing_loose_permissions pre-creates a 0644 file and asserts it becomes 0600

The second fails without the fix and passes with it.

running 2 tests
test file::tests::save_priv_validator_key_tightens_existing_loose_permissions ... ok
test file::tests::save_priv_validator_key_creates_file_with_0600 ... ok

test result: ok. 2 passed; 0 failed; 0 ignored

Existing 0600 assertions in cmd/init.rs and cmd/start.rs only cover newly created files, which is why this gap was not caught.

Notes

Unix-only, matching the existing #[cfg(unix)] structure. The parent directory is still created with the default create_dir_all mode; tightening that to 0700 felt out of scope here since the directory holds non-secret config too, but happy to add it if you would prefer.

🤖 Generated with Claude Code

[osr21]

Verified the claims independently against main — the diagnosis, reachability, and scoping all check out:

• The gap is real. save() on main relies solely on OpenOptions::mode(0o600), which Rust documents as applying only when the file is created — for an existing path it's silently ignored. Confirmed cmd/init.rs:53 guards with exists() && !overwrite, so init --overwrite against a pre-existing loose-permission key file (backup restore, older version) is a genuine reproduction path, not a theoretical one.
• No collateral scope. save() has exactly one caller in the crate: save_priv_validator_key. Forcing 0600 here can't accidentally tighten non-secret files (genesis, config) because none of them go through this code path.
• Ordering is correct. set_permissions runs before write_all, so the key bytes never hit disk while the file is still world-readable. And since chmod operates on the inode, any hardlinks to the file are tightened too.
• Tests compile as-is. tempfile is already a dev-dependency and rand/arc-consensus-types are regular deps of malachite-cli, so no Cargo.toml change is needed — and the second test genuinely fails without the fix, since mode()-ignored-on-existing-file is documented OpenOptionsExt behavior.

One residual gap worth noting (fine as a follow-up rather than blocking this): chmod does not revoke file descriptors that are already open, and truncate(true) reuses the same inode. So if a local process opened the world-readable key file before init --overwrite runs, it still reads the freshly written key through its existing FD even after the tighten-to-0600. The complete pattern for secret files is: create a sibling temp file with O_EXCL + mode(0o600), write the key there, then atomically rename() over the target. That yields a new inode (stale FDs see only the old content), closes this window entirely, and gets crash-atomicity for free — a half-written key file can't survive a crash mid-write. This PR is still a strict improvement without it; the FD-holding attacker is a narrower threat than the world-readable file it fixes.

Two minor observations, take or leave:

1. The set_permissions failure is mapped to Error::OpenFile — slightly mislabeled for someone debugging from the error message alone, though adding a variant may not be worth the churn.
2. Agree that parent-dir 0700 is out of scope here given the directory holds non-secret config; the temp-file-plus-rename follow-up above would be the more valuable next hardening step anyway.

The fix is minimal, correctly ordered, well-tested, and matches the crate's existing #[cfg(unix)] structure.

[melekes]

nice catch 👍 thanks