diff --git a/hive-c0re/src/forge/users.rs b/hive-c0re/src/forge/users.rs index 4bbdc059..cd511bb5 100644 --- a/hive-c0re/src/forge/users.rs +++ b/hive-c0re/src/forge/users.rs @@ -147,14 +147,9 @@ async fn mint_token(name: &str, scopes: &str) -> Result { /// by hive-c0re itself (under `/var/lib/hyperhive/`), so a direct /// write is both correct and necessary (no priv round-trip). async fn mint_and_persist_core_token(path: &Path) -> Result<()> { - use std::os::unix::fs::PermissionsExt; let token = mint_token("core", CORE_TOKEN_SCOPES).await?; - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).ok(); - } - std::fs::write(path, format!("{token}\n")) + crate::secret_file::write_secret_0600(path, format!("{token}\n").as_bytes()) .with_context(|| format!("write core token to {}", path.display()))?; - let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)); tracing::info!(path = %path.display(), "forge: persisted core access token"); Ok(()) } diff --git a/hive-c0re/src/lifecycle/agent_identity.rs b/hive-c0re/src/lifecycle/agent_identity.rs index 604b59b8..5a2aa079 100644 --- a/hive-c0re/src/lifecycle/agent_identity.rs +++ b/hive-c0re/src/lifecycle/agent_identity.rs @@ -121,13 +121,12 @@ async fn collect(agent: &str, dir: &Path) -> Result<(), Skipped> { /// Write the two private files, replacing whatever was there. /// -/// `0600` from the moment each file exists rather than after a `chmod`: the -/// key is written through a handle opened with that mode, so there is no -/// instant where it sits on disk world-readable. +/// `0600` from the moment each file exists rather than after a `chmod` +/// ([`crate::secret_file::write_secret_0600`]), so there is no instant where +/// the key sits on disk world-readable. fn write_identity(dir: &Path, credential: &mtls::Credential) -> anyhow::Result<()> { use anyhow::Context as _; - use std::io::Write as _; - use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + use std::os::unix::fs::PermissionsExt as _; std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?; // The directory too: its own mode is what stops anything but `hive-core` @@ -139,19 +138,7 @@ fn write_identity(dir: &Path, credential: &mtls::Credential) -> anyhow::Result<( (CERT_CREDENTIAL, credential.cert.as_bytes()), (KEY_CREDENTIAL, credential.key.as_bytes()), ] { - let file = dir.join(name); - let mut handle = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(0o600) - .open(&file) - .with_context(|| format!("open {}", file.display()))?; - // The error carries the path and never the bytes — the one line in - // this function where a value could otherwise be interpolated. - handle - .write_all(bytes) - .with_context(|| format!("write {}", file.display()))?; + crate::secret_file::write_secret_0600(&dir.join(name), bytes)?; } // Public material, and `0644` says so — it is the same bundle every host diff --git a/hive-c0re/src/main.rs b/hive-c0re/src/main.rs index 51dbc06d..06eb05eb 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -29,6 +29,7 @@ mod meta; mod migrate; mod paths; mod priv_client; +mod secret_file; mod server; mod snapshot_push; mod socket_server; diff --git a/hive-c0re/src/matrix.rs b/hive-c0re/src/matrix.rs index 020306d7..143aaeda 100644 --- a/hive-c0re/src/matrix.rs +++ b/hive-c0re/src/matrix.rs @@ -194,14 +194,8 @@ fn sender_source<'a>(stored: Option<&'a str>, on_disk: Option<&str>) -> SenderSo /// directory if it is not there. The file's mode is the only thing keeping an /// unprivileged reader off the hive's matrix credential. fn persist_sender_token(path: &std::path::Path, access_token: &str) -> Result<()> { - use std::os::unix::fs::PermissionsExt; - - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).ok(); - } - std::fs::write(path, format!("{access_token}\n")) + crate::secret_file::write_secret_0600(path, format!("{access_token}\n").as_bytes()) .with_context(|| format!("matrix: write the sender token to {}", path.display()))?; - let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)); tracing::info!(path = %path.display(), "matrix: provisioned the sender token"); Ok(()) } @@ -308,15 +302,9 @@ pub fn read_sender_token() -> Result { /// Persist the hive Space room id to [`hive_space_room_id_path()`] (0600). fn persist_space_room_id(room_id: &str) -> Result<()> { - use std::os::unix::fs::PermissionsExt; let path = hive_space_room_id_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).ok(); - } - std::fs::write(&path, format!("{room_id}\n")) - .with_context(|| format!("matrix: write space room_id to {}", path.display()))?; - let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); - Ok(()) + crate::secret_file::write_secret_0600(&path, format!("{room_id}\n").as_bytes()) + .with_context(|| format!("matrix: write space room_id to {}", path.display())) } /// Scan the admin account's joined rooms for the canonical hive Space: the @@ -449,15 +437,9 @@ pub async fn ensure_hive_space(client: &reqwest::Client, sender_token: &str) -> /// Persist the hive chat room id to [`hive_chat_room_id_path()`] (0600). fn persist_chat_room_id(room_id: &str) -> Result<()> { - use std::os::unix::fs::PermissionsExt; let path = hive_chat_room_id_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).ok(); - } - std::fs::write(&path, format!("{room_id}\n")) - .with_context(|| format!("matrix: write chat room_id to {}", path.display()))?; - let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); - Ok(()) + crate::secret_file::write_secret_0600(&path, format!("{room_id}\n").as_bytes()) + .with_context(|| format!("matrix: write chat room_id to {}", path.display())) } /// Scan the admin account's joined rooms for the canonical hive chat room: diff --git a/hive-c0re/src/paths.rs b/hive-c0re/src/paths.rs index c72e2a88..7660ae26 100644 --- a/hive-c0re/src/paths.rs +++ b/hive-c0re/src/paths.rs @@ -295,8 +295,14 @@ fn move_if_legacy(old: &Path, new: &Path) { if !old.exists() || new.exists() { return; } - if let Some(parent) = new.parent() { - let _ = std::fs::create_dir_all(parent); + if let Some(parent) = new.parent() + && let Err(e) = std::fs::create_dir_all(parent) + { + tracing::warn!( + dir = %parent.display(), + error = ?e, + "relocate: create parent dir failed" + ); } match std::fs::rename(old, new) { Ok(()) => tracing::info!( diff --git a/hive-c0re/src/secret_file.rs b/hive-c0re/src/secret_file.rs new file mode 100644 index 00000000..f13f51e0 --- /dev/null +++ b/hive-c0re/src/secret_file.rs @@ -0,0 +1,79 @@ +//! [`write_secret_0600`]: the one way hive-c0re puts a credential on disk. + +use std::io::Write as _; +use std::os::unix::fs::OpenOptionsExt as _; +use std::path::Path; + +use anyhow::{Context as _, Result}; + +/// Write `contents` to `path` as a `0600` file, creating the parent directory. +/// +/// The bytes go to a sibling temp file opened `0600` and are renamed over +/// `path`, so the file is never readable by anyone else for an instant, and a +/// reader (or a crash) never sees half of it. Errors carry paths, never +/// `contents`. +pub fn write_secret_0600(path: &Path, contents: &[u8]) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?; + } + let mut tmp_name = path + .file_name() + .with_context(|| format!("{} has no file name", path.display()))? + .to_owned(); + tmp_name.push(".tmp"); + let tmp = path.with_file_name(tmp_name); + // A temp file left by a crashed earlier write would fail `create_new`. + let _ = std::fs::remove_file(&tmp); + let mut f = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&tmp) + .with_context(|| format!("create {}", tmp.display()))?; + f.write_all(contents) + .and_then(|()| f.sync_all()) + .with_context(|| format!("write {}", tmp.display()))?; + std::fs::rename(&tmp, path).with_context(|| format!("rename onto {}", path.display())) +} + +#[cfg(test)] +mod tests { + use super::write_secret_0600; + use std::os::unix::fs::PermissionsExt as _; + + fn mode(path: &std::path::Path) -> u32 { + std::fs::metadata(path).expect("stat").permissions().mode() & 0o777 + } + + #[test] + fn creates_parent_and_writes_0600() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("sub").join("token"); + write_secret_0600(&path, b"secret\n").expect("writes"); + assert_eq!(std::fs::read(&path).expect("read"), b"secret\n"); + assert_eq!(mode(&path), 0o600); + assert!(!dir.path().join("sub").join("token.tmp").exists()); + } + + /// An existing file at a looser mode is replaced, not rewritten in place: + /// the new file is 0600 and holds only the new bytes. + #[test] + fn replaces_an_existing_looser_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("token"); + std::fs::write(&path, "a much longer old value\n").expect("seed"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).expect("chmod"); + write_secret_0600(&path, b"new\n").expect("writes"); + assert_eq!(std::fs::read(&path).expect("read"), b"new\n"); + assert_eq!(mode(&path), 0o600); + } + + #[test] + fn a_stale_temp_file_does_not_block_the_write() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("token"); + std::fs::write(dir.path().join("token.tmp"), "stale").expect("seed"); + write_secret_0600(&path, b"new\n").expect("writes"); + assert_eq!(std::fs::read(&path).expect("read"), b"new\n"); + } +} diff --git a/hive-c0re/src/workers/auto_update.rs b/hive-c0re/src/workers/auto_update.rs index 65a474b7..ac31b48f 100644 --- a/hive-c0re/src/workers/auto_update.rs +++ b/hive-c0re/src/workers/auto_update.rs @@ -208,8 +208,10 @@ pub async fn ensure_root_agent(coord: &Arc) -> Result<()> { if let Err(e) = coord.power.set(MANAGER_NAME, crate::power::Wanted::Up) { tracing::warn!(error = ?e, "agent_power: set manager wanted=up failed"); } - if let Some(rev) = current_rev { - let _ = std::fs::write(crate::paths::applied_rev_marker(MANAGER_NAME), &rev); + if let Some(rev) = current_rev + && let Err(e) = std::fs::write(crate::paths::applied_rev_marker(MANAGER_NAME), &rev) + { + tracing::warn!(name = MANAGER_NAME, error = ?e, "write rev marker failed"); } Ok(()) } diff --git a/hive-c0re/src/workers/knowledge.rs b/hive-c0re/src/workers/knowledge.rs index 0edf056a..bdccebe2 100644 --- a/hive-c0re/src/workers/knowledge.rs +++ b/hive-c0re/src/workers/knowledge.rs @@ -109,10 +109,24 @@ pub async fn pull(coord: &Coordinator) -> Result<()> { // place this repo stored a token in a named remote. Harmless to repeat // once the URL is already clean. let plain = crate::forge::forge_git_url(&format!("{ORG}/{REPO}")); - let _ = crate::lifecycle::git_command() + // Logs the exit status only: git's stderr may echo the old, credentialed URL. + match crate::lifecycle::git_command() .args(["-C", LOCAL_DIR, "remote", "set-url", "origin", &plain]) .output() - .await; + .await + { + Ok(out) if out.status.success() => {} + Ok(out) => tracing::warn!( + remote = "origin", + status = %out.status, + "knowledge: git remote set-url failed; a credentialed origin URL may remain" + ), + Err(e) => tracing::warn!( + remote = "origin", + error = ?e, + "knowledge: spawning git remote set-url failed" + ), + } // Discard any local drift before pulling — both tracked (`reset --hard`) // and untracked (`clean -fd`). Nothing in this codebase writes to // `LOCAL_DIR` after the initial clone — this working tree exists to mirror