hive-c0re: fail on an unparseable permission file, write it atomically

tool_groups::read and capabilities::read returned an empty map when
their file existed but didn't parse. Every set_*/remove_agent is a
read-modify-write, and write() rewrote the file in place, so a crash or
ENOSPC mid-write left a truncated file, and the next write (e.g. the
manager-spawn seed of ruth's tool groups) replaced it with a map holding
only one agent. The scheduling and approval gates then denied every
other agent, recoverable only from meta git history.

- Both registries now read through agent_config::read_map: a missing
  file is still the empty map, any other read failure or a parse
  failure is an io::Error. set_groups / set_caps / remove_agent fail
  without writing.
- Writes go through agent_config::write_map: temp file in the same
  directory, fsync, rename, fsync the directory. hive-c0re had no
  shared atomic-write helper (the existing tmp+rename sites are inline
  and don't fsync).
- Callers of read / groups_for / has_cap now handle the error:
  * dashboard GET /api/tool-groups, /api/capabilities,
    /api/permissions/stale return 500 instead of an empty table;
  * the SSE permission snapshots are skipped with a warn;
  * render_flake returns Result, so sync_agents fails instead of
    rendering every agent without its tool groups / capabilities;
  * set_nspawn_flags propagates has_cap's error;
  * the socket tool-group gates deny with the read error as message;
  * seed_manager_tool_groups logs and does not seed.
- capabilities::write had no callers left once set_caps writes through
  write_map, and is removed.

Closes #4719

hive-c0re/src/agent_config/capabilities.rs

@@ -22,7 +22,7 @@
 
 use hive_sh4re::permissions::Capability;
 use std::collections::BTreeMap;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 const CAPABILITIES_FILE: &str = "capabilities.json";
 
@@ -61,50 +61,35 @@ fn prune_unknown(map: &mut BTreeMap<String, Vec<String>>) -> bool {
     dropped_any
 }
 
-/// Read the per-agent capability map. Returns an empty map when the
-/// file is absent or unparsable — callers treat a missing entry as
-/// "no extra capabilities". Any entry that isn't a recognised
-/// [`Capability`] is dropped (with a `warn!`) from what's returned —
-/// a stale or typo'd name is never honoured — but `read` itself never
-/// writes; the on-disk file only gets repaired the next time something
-/// calls `write`/`set_caps` anyway.
-#[must_use]
-pub fn read() -> BTreeMap<String, Vec<String>> {
-    let path = capabilities_path();
-    let Ok(raw) = std::fs::read_to_string(&path) else {
-        return BTreeMap::new();
-    };
-    let mut map: BTreeMap<String, Vec<String>> = serde_json::from_str(&raw).unwrap_or_default();
+/// Read the per-agent capability map. An absent file is the empty map —
+/// callers treat a missing entry as "no extra capabilities". A file
+/// that exists but can't be read or parsed is an error. Any entry that
+/// isn't a recognised [`Capability`] is dropped (with a `warn!`) from
+/// what's returned — a stale or typo'd name is never honoured — but
+/// `read` itself never writes; the on-disk file only gets repaired the
+/// next time something calls `set_caps`/`remove_agent` anyway.
+pub fn read() -> std::io::Result<BTreeMap<String, Vec<String>>> {
+    read_from(&capabilities_path())
+}
+
+fn read_from(path: &Path) -> std::io::Result<BTreeMap<String, Vec<String>>> {
+    let mut map = super::read_map(path)?;
     prune_unknown(&mut map);
-    map
+    Ok(map)
 }
 
 /// Look up the configured capabilities for one agent. Returns an empty
-/// vec when the agent has no entry.
-#[must_use]
-pub fn caps_for(name: &str) -> Vec<String> {
-    read().get(name).cloned().unwrap_or_default()
+/// vec when the agent has no entry. Errors as [`read`] does.
+pub fn caps_for(name: &str) -> std::io::Result<Vec<String>> {
+    Ok(read()?.get(name).cloned().unwrap_or_default())
 }
 
-/// Check whether an agent holds a specific capability.
-#[must_use]
-pub fn has_cap(name: &str, cap: hive_sh4re::permissions::Capability) -> bool {
-    caps_for(name)
+/// Check whether an agent holds a specific capability. Errors as
+/// [`read`] does.
+pub fn has_cap(name: &str, cap: hive_sh4re::permissions::Capability) -> std::io::Result<bool> {
+    Ok(caps_for(name)?
         .iter()
-        .any(|s| s.eq_ignore_ascii_case(<&str>::from(cap)))
-}
-
-/// Persist the full capability map. Sorted JSON output keeps diffs
-/// minimal. Best-effort — returns `io::Error` so callers decide
-/// whether to abort or log.
-pub fn write(map: &BTreeMap<String, Vec<String>>) -> std::io::Result<()> {
-    let path = capabilities_path();
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)?;
-    }
-    let text = serde_json::to_string_pretty(map)
-        .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
-    std::fs::write(&path, format!("{text}\n"))
+        .any(|s| s.eq_ignore_ascii_case(<&str>::from(cap))))
 }
 
 /// Filter `caps` down to recognised [`Capability`] names (warning per
@@ -137,20 +122,30 @@ fn apply_known_caps(current: &mut BTreeMap<String, Vec<String>>, name: &str, cap
 /// rather than written — an unknown grant should never look like it
 /// took effect. An empty `caps` vec, or one that becomes empty after
 /// dropping unknown names, removes the entry (agent has no
-/// capabilities).
+/// capabilities). Fails without writing if the existing file can't be
+/// read or parsed.
 pub fn set_caps(name: &str, caps: &[String]) -> std::io::Result<()> {
-    let mut current = read();
+    set_caps_at(&capabilities_path(), name, caps)
+}
+
+fn set_caps_at(path: &Path, name: &str, caps: &[String]) -> std::io::Result<()> {
+    let mut current = read_from(path)?;
     apply_known_caps(&mut current, name, caps);
-    write(&current)
+    super::write_map(path, &current)
 }
 
 /// Remove an agent from the capability map entirely. Called by
 /// `meta::sync_agents` when an agent is deprovisioned so stale entries
-/// don't accumulate. No-op if the agent has no entry.
+/// don't accumulate. No-op if the agent has no entry. Fails without
+/// writing if the existing file can't be read or parsed.
 pub fn remove_agent(name: &str) -> std::io::Result<()> {
-    let mut current = read();
+    remove_agent_at(&capabilities_path(), name)
+}
+
+fn remove_agent_at(path: &Path, name: &str) -> std::io::Result<()> {
+    let mut current = read_from(path)?;
     if current.remove(name).is_some() {
-        write(&current)?;
+        super::write_map(path, &current)?;
     }
     Ok(())
 }
@@ -159,11 +154,51 @@ pub fn remove_agent(name: &str) -> std::io::Result<()> {
 mod tests {
     use super::*;
 
-    // `read`/`write`/`set_caps` shell out to `crate::paths::meta_root`,
+    // `read`/`set_caps`/`remove_agent` shell out to `crate::paths::meta_root`,
     // which is hardcoded to `/var/lib/hyperhive` (no test override) — so
     // these tests pin the pure decision logic (`prune_unknown`,
-    // `apply_known_caps`) directly against in-memory maps rather than
-    // round-tripping through the real capabilities file.
+    // `apply_known_caps`) against in-memory maps, and the file handling
+    // through the path-taking `*_at` / `read_from` variants.
+
+    const TRUNCATED: &str = "{\n  \"atlas\": [\"manage_root_ag";
+
+    fn corrupt_file() -> (tempfile::TempDir, std::path::PathBuf) {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join(CAPABILITIES_FILE);
+        std::fs::write(&path, TRUNCATED).expect("seed");
+        (dir, path)
+    }
+
+    #[test]
+    fn read_of_a_corrupt_file_is_an_error() {
+        let (_dir, path) = corrupt_file();
+        read_from(&path).expect_err("truncated JSON must not read as a map");
+    }
+
+    #[test]
+    fn set_caps_leaves_a_corrupt_file_untouched() {
+        let (_dir, path) = corrupt_file();
+        set_caps_at(&path, "ruth", &["manage_root_agent".to_owned()])
+            .expect_err("a corrupt file must not be overwritten");
+        assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes());
+    }
+
+    #[test]
+    fn remove_agent_leaves_a_corrupt_file_untouched() {
+        let (_dir, path) = corrupt_file();
+        remove_agent_at(&path, "atlas").expect_err("a corrupt file must not be overwritten");
+        assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes());
+    }
+
+    #[test]
+    fn set_caps_round_trips_through_a_missing_file() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join(CAPABILITIES_FILE);
+        assert!(read_from(&path).expect("missing file is empty").is_empty());
+        set_caps_at(&path, "ruth", &["manage_root_agent".to_owned()]).expect("set");
+        let map = read_from(&path).expect("read");
+        assert_eq!(map["ruth"], vec!["manage_root_agent".to_owned()]);
+    }
 
     #[test]
     fn prune_unknown_keeps_known_name() {

hive-c0re/src/agent_config/mod.rs

@@ -13,3 +13,91 @@ pub mod limits;
 pub mod resource_limits;
 pub mod tool_groups;
 pub mod topology;
+
+use std::collections::BTreeMap;
+use std::io::{self, Write as _};
+use std::path::Path;
+
+/// Read a per-agent `name → [string]` JSON map. A missing file is the
+/// empty map; any other read failure, or content that doesn't parse, is
+/// an error, so a read-modify-write can't write back a map that has lost
+/// every other agent's entry.
+fn read_map(path: &Path) -> io::Result<BTreeMap<String, Vec<String>>> {
+    let raw = match std::fs::read_to_string(path) {
+        Ok(raw) => raw,
+        Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(BTreeMap::new()),
+        Err(e) => {
+            return Err(io::Error::new(
+                e.kind(),
+                format!("read {}: {e}", path.display()),
+            ));
+        }
+    };
+    serde_json::from_str(&raw).map_err(|e| {
+        io::Error::new(
+            io::ErrorKind::InvalidData,
+            format!("parse {}: {e}", path.display()),
+        )
+    })
+}
+
+/// Serialise `map` as pretty JSON and replace `path` with it atomically:
+/// a temp file in the same directory (`rename(2)` is only atomic within a
+/// filesystem), fsynced, renamed over `path`, then the directory fsynced
+/// so the rename itself survives a crash. A reader sees the old file or
+/// the new one, never a truncated one.
+fn write_map(path: &Path, map: &BTreeMap<String, Vec<String>>) -> io::Result<()> {
+    let text = serde_json::to_string_pretty(map)
+        .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
+    let (Some(dir), Some(name)) = (path.parent(), path.file_name()) else {
+        return Err(io::Error::new(
+            io::ErrorKind::InvalidInput,
+            format!("{} has no parent directory or file name", path.display()),
+        ));
+    };
+    std::fs::create_dir_all(dir)?;
+    let tmp = dir.join(format!(".{}.tmp", name.to_string_lossy()));
+    let mut file = std::fs::File::create(&tmp)?;
+    file.write_all(format!("{text}\n").as_bytes())?;
+    file.sync_all()?;
+    drop(file);
+    std::fs::rename(&tmp, path)?;
+    std::fs::File::open(dir)?.sync_all()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn write_map_round_trips_and_leaves_no_temp_file() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("map.json");
+        let map = BTreeMap::from([("alice".to_owned(), vec!["messaging".to_owned()])]);
+        write_map(&path, &map).expect("first write");
+        let replaced = BTreeMap::from([("bob".to_owned(), vec!["inbox".to_owned()])]);
+        write_map(&path, &replaced).expect("replacing write");
+        assert_eq!(read_map(&path).expect("read"), replaced);
+        let entries: Vec<_> = std::fs::read_dir(dir.path())
+            .expect("read_dir")
+            .map(|e| e.expect("entry").file_name())
+            .collect();
+        assert_eq!(entries, vec![std::ffi::OsString::from("map.json")]);
+    }
+
+    #[test]
+    fn read_map_of_a_missing_file_is_empty() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let map = read_map(&dir.path().join("absent.json")).expect("missing file is not an error");
+        assert!(map.is_empty());
+    }
+
+    #[test]
+    fn read_map_of_a_truncated_file_is_an_error() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("map.json");
+        std::fs::write(&path, "{\n  \"alice\": [\"messag").expect("seed");
+        let err = read_map(&path).expect_err("truncated JSON must not read as a map");
+        assert_eq!(err.kind(), io::ErrorKind::InvalidData);
+    }
+}

hive-c0re/src/agent_config/tool_groups.rs

@@ -22,7 +22,7 @@
 //! that the operator uses to grant/revoke tool groups per agent.
 
 use std::collections::BTreeMap;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::Context as _;
 
@@ -33,37 +33,18 @@ pub fn tool_groups_path() -> PathBuf {
     crate::paths::meta_root().join(TOOL_GROUPS_FILE)
 }
 
-/// Read the per-agent tool-group map. Returns an empty map when the
-/// file is absent or unparsable — callers treat a missing entry as
-/// "use role default".
-#[must_use]
-pub fn read() -> BTreeMap<String, Vec<String>> {
-    let path = tool_groups_path();
-    let Ok(raw) = std::fs::read_to_string(&path) else {
-        return BTreeMap::new();
-    };
-    serde_json::from_str(&raw).unwrap_or_default()
+/// Read the per-agent tool-group map. An absent file is the empty map —
+/// callers treat a missing entry as "use role default". A file that
+/// exists but can't be read or parsed is an error.
+pub fn read() -> std::io::Result<BTreeMap<String, Vec<String>>> {
+    super::read_map(&tool_groups_path())
 }
 
 /// Look up the configured tool groups for one agent. Returns an empty
 /// vec when the agent has no entry — callers should treat this as
-/// "use the harness role default."
-#[must_use]
-pub fn groups_for(name: &str) -> Vec<String> {
-    read().get(name).cloned().unwrap_or_default()
-}
-
-/// Persist the full tool-groups map. Sorted JSON output keeps diffs
-/// minimal. Use `set_groups` (or `remove_agent`) from outside this
-/// module — they go through the validated write path.
-fn write(map: &BTreeMap<String, Vec<String>>) -> std::io::Result<()> {
-    let path = tool_groups_path();
-    if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)?;
-    }
-    let text = serde_json::to_string_pretty(map)
-        .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
-    std::fs::write(&path, format!("{text}\n"))
+/// "use the harness role default." Errors as [`read`] does.
+pub fn groups_for(name: &str) -> std::io::Result<Vec<String>> {
+    Ok(read()?.get(name).cloned().unwrap_or_default())
 }
 
 /// Validate a slice of group name strings against `ToolGroup::ALL`.
@@ -96,25 +77,76 @@ pub fn validate_groups(groups: &[String]) -> anyhow::Result<()> {
 
 /// Set the tool groups for one agent and persist the map. An empty
 /// `groups` vec removes the entry (agent reverts to role default).
-/// Returns an error if any name is not in `ToolGroup::ALL`.
+/// Returns an error if any name is not in `ToolGroup::ALL`, or if the
+/// existing file can't be read or parsed — the file is then left
+/// untouched.
 pub fn set_groups(name: &str, groups: &[String]) -> anyhow::Result<()> {
+    set_groups_at(&tool_groups_path(), name, groups)
+}
+
+fn set_groups_at(path: &Path, name: &str, groups: &[String]) -> anyhow::Result<()> {
     if !groups.is_empty() {
         validate_groups(groups)?;
     }
-    let mut current = read();
+    let mut current = super::read_map(path)?;
     if groups.is_empty() {
         current.remove(name);
     } else {
         current.insert(name.to_owned(), groups.to_vec());
     }
-    write(&current).with_context(|| format!("write tool-groups for {name}"))
+    super::write_map(path, &current).with_context(|| format!("write tool-groups for {name}"))
 }
 
 /// Drop the entry for an agent that is being destroyed. Idempotent.
+/// Fails without writing if the existing file can't be read or parsed.
 pub fn remove_agent(name: &str) -> std::io::Result<()> {
-    let mut current = read();
+    remove_agent_at(&tool_groups_path(), name)
+}
+
+fn remove_agent_at(path: &Path, name: &str) -> std::io::Result<()> {
+    let mut current = super::read_map(path)?;
     if current.remove(name).is_some() {
-        write(&current)?;
+        super::write_map(path, &current)?;
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const TRUNCATED: &str = "{\n  \"alice\": [\"messaging\", \"meta\"],\n  \"bob\": [\"mess";
+
+    fn corrupt_file() -> (tempfile::TempDir, PathBuf) {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join(TOOL_GROUPS_FILE);
+        std::fs::write(&path, TRUNCATED).expect("seed");
+        (dir, path)
+    }
+
+    #[test]
+    fn set_groups_leaves_a_corrupt_file_untouched() {
+        let (_dir, path) = corrupt_file();
+        set_groups_at(&path, "ruth", &["messaging".to_owned()])
+            .expect_err("a corrupt file must not be overwritten");
+        assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes());
+    }
+
+    #[test]
+    fn remove_agent_leaves_a_corrupt_file_untouched() {
+        let (_dir, path) = corrupt_file();
+        remove_agent_at(&path, "alice").expect_err("a corrupt file must not be overwritten");
+        assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes());
+    }
+
+    #[test]
+    fn set_groups_keeps_other_agents_entries() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join(TOOL_GROUPS_FILE);
+        set_groups_at(&path, "alice", &["inbox".to_owned()]).expect("set on missing file");
+        set_groups_at(&path, "ruth", &["messaging".to_owned()]).expect("set");
+        let map = crate::agent_config::read_map(&path).expect("read");
+        assert_eq!(map["alice"], vec!["inbox".to_owned()]);
+        assert_eq!(map["ruth"], vec!["messaging".to_owned()]);
+    }
+}