Route the last two atomic writes through fs_util

src/commands/trust.rs

@@ -1,9 +1,11 @@
 use std::collections::BTreeMap;
 use std::fs::{self, OpenOptions};
 use std::io::Write;
+use std::os::unix::fs::OpenOptionsExt;
 use std::path::{Path, PathBuf};
 
 use anyhow::{Context, Result};
+use bootroot::fs_util;
 use bootroot::openbao::OpenBaoClient;
 use bootroot::trust_bootstrap::CA_BUNDLE_PEM_KEY;
 use serde::{Deserialize, Serialize};
@@ -16,6 +18,12 @@ use crate::state::ServiceEntry;
 pub(crate) const SERVICE_TRUST_KV_SUFFIX: &str = "trust";
 const ROTATION_STATE_FILENAME: &str = "rotation-state.json";
 
+/// Mode for `rotation-state.json`. Only the rotation command writes
+/// and reads it, so it is created owner-only rather than at whatever
+/// the umask happens to be — and the same mode on both write paths,
+/// so the file does not change permissions depending on which one ran.
+const ROTATION_STATE_MODE: u32 = 0o600;
+
 /// Describes which CA components are included in the rotation.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub(crate) enum RotationMode {
@@ -58,10 +66,13 @@ pub(crate) fn create_rotation_state(
     let mut file = OpenOptions::new()
         .write(true)
         .create_new(true)
+        .mode(ROTATION_STATE_MODE)
         .open(&path)
         .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?;
     file.write_all(json.as_bytes())
         .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?;
+    file.sync_all()
+        .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?;
     Ok(())
 }
 
@@ -74,18 +85,8 @@ pub(crate) fn update_rotation_state(
     let path = rotation_state_path(state_dir);
     let json = serde_json::to_string_pretty(state)
         .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?;
-    let temp_path = state_dir.join(format!(
-        "{ROTATION_STATE_FILENAME}.tmp.{}.{}",
-        std::process::id(),
-        std::time::SystemTime::now()
-            .duration_since(std::time::SystemTime::UNIX_EPOCH)
-            .map_or(0, |d| d.as_nanos())
-    ));
-    fs::write(&temp_path, json.as_bytes())
-        .with_context(|| messages.error_write_file_failed(&temp_path.display().to_string()))?;
-    fs::rename(&temp_path, &path)
-        .with_context(|| messages.error_write_file_failed(&path.display().to_string()))?;
-    Ok(())
+    fs_util::atomic_write_blocking(&path, json.as_bytes(), ROTATION_STATE_MODE)
+        .with_context(|| messages.error_write_file_failed(&path.display().to_string()))
 }
 
 /// Loads `rotation-state.json` if it exists. Returns `None` if absent.

src/fast_poll.rs

@@ -103,6 +103,11 @@ fn log_poll_outcomes(kind: &str, outcomes: &[PollApplyOutcome], needs_relogin: &
 /// renewals but failed to be acknowledged back to `OpenBao`. Missing
 /// entries in `last_reissue_seen_version` mean "never seen a reissue
 /// request".
+/// Mode for the fast-poll state file. It is the agent's own resume
+/// record — nobody else reads it — so it is created owner-only rather
+/// than at whatever the umask happens to be.
+const STATE_FILE_MODE: u32 = 0o600;
+
 #[derive(Debug, Default, Serialize, Deserialize, Clone)]
 pub(crate) struct FastPollState {
     #[serde(default)]
@@ -190,7 +195,9 @@ impl FastPollState {
         }
     }
 
-    /// Persists the state file atomically via a `<path>.tmp` rename.
+    /// Persists the state file atomically, owner-only and flushed: it
+    /// is what the next tick resumes from, so it has to survive a
+    /// power loss rather than merely replace cleanly.
     pub(crate) async fn save(&self, path: &Path) -> Result<()> {
         if let Some(parent) = path.parent()
             && !parent.as_os_str().is_empty()
@@ -202,27 +209,11 @@ impl FastPollState {
                 )
             })?;
         }
-        let tmp_path: PathBuf = {
-            let mut os = path.as_os_str().to_owned();
-            os.push(".tmp");
-            PathBuf::from(os)
-        };
         let body =
             serde_json::to_string_pretty(self).context("Failed to serialize fast-poll state")?;
-        fs::write(&tmp_path, body).await.with_context(|| {
-            format!(
-                "Failed to write fast-poll state tmp file at {}",
-                tmp_path.display()
-            )
-        })?;
-        fs::rename(&tmp_path, path).await.with_context(|| {
-            format!(
-                "Failed to rename {} -> {}",
-                tmp_path.display(),
-                path.display()
-            )
-        })?;
-        Ok(())
+        fs_util::atomic_write(path, body.as_bytes(), STATE_FILE_MODE)
+            .await
+            .with_context(|| format!("Failed to write fast-poll state at {}", path.display()))
     }
 }
 

src/fs_util.rs

@@ -279,60 +279,73 @@ async fn write_owned_impl(path: &Path, contents: &[u8], mode: u32, replace: bool
 /// permissioned, chowned (when ownership preservation is needed), or
 /// renamed.
 pub async fn atomic_write(path: &Path, contents: &[u8], mode: u32) -> Result<()> {
+    // Owned once, to move into the blocking task. The blocking half
+    // borrows, so this is the only copy either path makes.
+    let dest = path.to_path_buf();
+    let payload = contents.to_vec();
+    tokio::task::spawn_blocking(move || atomic_write_blocking(&dest, &payload, mode))
+        .await
+        .context("Atomic write task panicked")?
+}
+
+/// The blocking half of [`atomic_write`], for callers that are not async.
+///
+/// Same guarantees, same order: staged in the destination's directory,
+/// written, `sync_all`ed, permissioned, ownership-preserved, renamed.
+/// Callers in an async context use [`atomic_write`] instead, which runs
+/// this on a blocking thread.
+///
+/// # Errors
+/// Returns an error under the same conditions as [`atomic_write`].
+pub fn atomic_write_blocking(path: &Path, contents: &[u8], mode: u32) -> Result<()> {
     let parent = path
         .parent()
         .filter(|p| !p.as_os_str().is_empty())
         .map_or_else(|| std::path::PathBuf::from("."), Path::to_path_buf);
-    let dest = path.to_path_buf();
-    let payload = contents.to_vec();
-    tokio::task::spawn_blocking(move || -> Result<()> {
-        // Capture the existing destination's uid/gid (if any) so the
-        // rename does not strip operator-meaningful ownership. Missing
-        // file -> None; do not chown the staged file in that case so a
-        // fresh create keeps process default ownership.
-        let existing_owner = std::fs::metadata(&dest).ok().map(|m| (m.uid(), m.gid()));
-
-        let mut tmp = tempfile::NamedTempFile::new_in(&parent)
-            .with_context(|| format!("Failed to create temp file in {}", parent.display()))?;
-        tmp.as_file_mut()
-            .write_all(&payload)
-            .with_context(|| format!("Failed to write temp file for {}", dest.display()))?;
-        tmp.as_file_mut()
-            .sync_all()
-            .with_context(|| format!("Failed to fsync temp file for {}", dest.display()))?;
-        std::fs::set_permissions(tmp.path(), std::fs::Permissions::from_mode(mode)).with_context(
-            || {
-                format!(
-                    "Failed to set mode {mode:o} on temp file for {}",
-                    dest.display()
-                )
-            },
-        )?;
-        if let Some((dest_uid, dest_gid)) = existing_owner {
-            let tmp_meta = std::fs::metadata(tmp.path())
-                .with_context(|| format!("Failed to stat temp file for {}", dest.display()))?;
-            if tmp_meta.uid() != dest_uid || tmp_meta.gid() != dest_gid {
-                std::os::unix::fs::chown(tmp.path(), Some(dest_uid), Some(dest_gid)).with_context(
-                    || {
-                        format!(
-                            "Failed to preserve existing uid={dest_uid} gid={dest_gid} on {}",
-                            dest.display()
-                        )
-                    },
-                )?;
-            }
-        }
-        tmp.persist(&dest).map_err(|e| {
-            anyhow::anyhow!(
-                "Failed to rename temp file to {}: {}",
-                dest.display(),
-                e.error
+    // Capture the existing destination's uid/gid (if any) so the
+    // rename does not strip operator-meaningful ownership. Missing
+    // file -> None; do not chown the staged file in that case so a
+    // fresh create keeps process default ownership.
+    let existing_owner = std::fs::metadata(path).ok().map(|m| (m.uid(), m.gid()));
+
+    let mut tmp = tempfile::NamedTempFile::new_in(&parent)
+        .with_context(|| format!("Failed to create temp file in {}", parent.display()))?;
+    tmp.as_file_mut()
+        .write_all(contents)
+        .with_context(|| format!("Failed to write temp file for {}", path.display()))?;
+    tmp.as_file_mut()
+        .sync_all()
+        .with_context(|| format!("Failed to fsync temp file for {}", path.display()))?;
+    std::fs::set_permissions(tmp.path(), std::fs::Permissions::from_mode(mode)).with_context(
+        || {
+            format!(
+                "Failed to set mode {mode:o} on temp file for {}",
+                path.display()
             )
-        })?;
-        Ok(())
-    })
-    .await
-    .context("Atomic write task panicked")?
+        },
+    )?;
+    if let Some((dest_uid, dest_gid)) = existing_owner {
+        let tmp_meta = std::fs::metadata(tmp.path())
+            .with_context(|| format!("Failed to stat temp file for {}", path.display()))?;
+        if tmp_meta.uid() != dest_uid || tmp_meta.gid() != dest_gid {
+            std::os::unix::fs::chown(tmp.path(), Some(dest_uid), Some(dest_gid)).with_context(
+                || {
+                    format!(
+                        "Failed to preserve existing uid={dest_uid} gid={dest_gid} on {}",
+                        path.display()
+                    )
+                },
+            )?;
+        }
+    }
+    tmp.persist(path).map_err(|e| {
+        anyhow::anyhow!(
+            "Failed to rename temp file to {}: {}",
+            path.display(),
+            e.error
+        )
+    })?;
+    Ok(())
 }
 
 /// Ensures the secrets directory exists and has secure permissions.