capabilities: read() filters but no longer persists the prune

Operator ruling: repair belongs on the write path, not the read path.
read() still drops unrecognised names from what it returns (with a
warn!) so a stale name is never honoured, but it no longer writes the
pruned form back — the file heals the next time set_caps()/write() run,
which prune anyway since they're fed a map read() already filtered.

Refs #4474

hive-c0re/src/agent_config/capabilities.rs

@@ -21,6 +21,7 @@
 //! Write path: `set_caps` is called from the dashboard action handler
 //! that the operator uses to grant/revoke capabilities per agent.
 
+use hive_sh4re::permissions::Capability;
 use std::collections::BTreeMap;
 use std::path::PathBuf;
 
@@ -31,16 +32,52 @@ pub fn capabilities_path() -> PathBuf {
     crate::paths::meta_root().join(CAPABILITIES_FILE)
 }
 
+/// True if `name` matches a recognised [`Capability`], case-insensitive
+/// (same comparison [`has_cap`] uses).
+#[must_use]
+fn is_known(name: &str) -> bool {
+    Capability::ALL
+        .iter()
+        .any(|cap| name.eq_ignore_ascii_case(<&str>::from(*cap)))
+}
+
+/// Drop any capability string in `map` that isn't a recognised
+/// [`Capability`], warning per dropped string so a typo'd or stale
+/// grant doesn't look like it worked. An agent left with no
+/// capabilities is removed entirely, matching [`set_caps`]'s
+/// empty-vec behaviour. Returns whether anything was dropped.
+fn prune_unknown(map: &mut BTreeMap<String, Vec<String>>) -> bool {
+    let mut dropped_any = false;
+    map.retain(|agent, caps| {
+        caps.retain(|cap| {
+            let known = is_known(cap);
+            if !known {
+                dropped_any = true;
+                tracing::warn!(agent, capability = %cap, "dropping unrecognised capability");
+            }
+            known
+        });
+        !caps.is_empty()
+    });
+    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".
+/// "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();
     };
-    serde_json::from_str(&raw).unwrap_or_default()
+    let mut map: BTreeMap<String, Vec<String>> = serde_json::from_str(&raw).unwrap_or_default();
+    prune_unknown(&mut map);
+    map
 }
 
 /// Look up the configured capabilities for one agent. Returns an empty
@@ -71,15 +108,40 @@ pub fn write(map: &BTreeMap<String, Vec<String>>) -> std::io::Result<()> {
     std::fs::write(&path, format!("{text}\n"))
 }
 
-/// Set the capabilities for one agent and persist the map. An empty
+/// Filter `caps` down to recognised [`Capability`] names (warning per
-/// `caps` vec removes the entry (agent has no capabilities).
+/// drop) and apply the result to `current` for `name`: an empty
-pub fn set_caps(name: &str, caps: &[String]) -> std::io::Result<()> {
+/// result — whether `caps` started empty or every name in it was
-    let mut current = read();
+/// unknown — removes the entry, matching `read`'s empty-entry
-    if caps.is_empty() {
+/// convention. Split out from [`set_caps`] so the decision logic is
+/// testable without touching the real capabilities file.
+fn apply_known_caps(current: &mut BTreeMap<String, Vec<String>>, name: &str, caps: &[String]) {
+    let known: Vec<String> = caps
+        .iter()
+        .filter(|cap| {
+            let ok = is_known(cap);
+            if !ok {
+                tracing::warn!(agent = name, capability = %cap, "dropping unrecognised capability");
+            }
+            ok
+        })
+        .cloned()
+        .collect();
+    if known.is_empty() {
         current.remove(name);
     } else {
-        current.insert(name.to_owned(), caps.to_vec());
+        current.insert(name.to_owned(), known);
     }
+}
+
+/// Set the capabilities for one agent and persist the map. Any name
+/// that isn't a recognised [`Capability`] is dropped (with a `warn!`)
+/// 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).
+pub fn set_caps(name: &str, caps: &[String]) -> std::io::Result<()> {
+    let mut current = read();
+    apply_known_caps(&mut current, name, caps);
     write(&current)
 }
 
@@ -93,3 +155,72 @@ pub fn remove_agent(name: &str) -> std::io::Result<()> {
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // `read`/`write`/`set_caps` 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.
+
+    #[test]
+    fn prune_unknown_keeps_known_name() {
+        let mut map = BTreeMap::from([("atlas".to_owned(), vec!["read_host_journal".to_owned()])]);
+        assert!(!prune_unknown(&mut map));
+        assert_eq!(map["atlas"], vec!["read_host_journal".to_owned()]);
+    }
+
+    #[test]
+    fn prune_unknown_drops_unrecognised_name() {
+        let mut map = BTreeMap::from([("atlas".to_owned(), vec!["fly_to_the_moon".to_owned()])]);
+        assert!(prune_unknown(&mut map));
+        assert!(!map.contains_key("atlas"));
+    }
+
+    #[test]
+    fn prune_unknown_keeps_known_and_drops_unknown_in_same_entry() {
+        let mut map = BTreeMap::from([(
+            "atlas".to_owned(),
+            vec!["read_host_journal".to_owned(), "fly_to_the_moon".to_owned()],
+        )]);
+        assert!(prune_unknown(&mut map));
+        assert_eq!(map["atlas"], vec!["read_host_journal".to_owned()]);
+    }
+
+    #[test]
+    fn manage_root_agent_is_still_a_known_name() {
+        let mut map = BTreeMap::from([("atlas".to_owned(), vec!["manage_root_agent".to_owned()])]);
+        assert!(!prune_unknown(&mut map));
+        assert_eq!(map["atlas"], vec!["manage_root_agent".to_owned()]);
+    }
+
+    #[test]
+    fn apply_known_caps_removes_entry_on_empty_input() {
+        let mut current =
+            BTreeMap::from([("atlas".to_owned(), vec!["read_host_journal".to_owned()])]);
+        apply_known_caps(&mut current, "atlas", &[]);
+        assert!(!current.contains_key("atlas"));
+    }
+
+    #[test]
+    fn apply_known_caps_removes_entry_when_only_unknown_names_given() {
+        let mut current =
+            BTreeMap::from([("atlas".to_owned(), vec!["read_host_journal".to_owned()])]);
+        apply_known_caps(&mut current, "atlas", &["fly_to_the_moon".to_owned()]);
+        assert!(!current.contains_key("atlas"));
+    }
+
+    #[test]
+    fn apply_known_caps_keeps_only_the_known_name() {
+        let mut current = BTreeMap::new();
+        apply_known_caps(
+            &mut current,
+            "atlas",
+            &["read_host_journal".to_owned(), "fly_to_the_moon".to_owned()],
+        );
+        assert_eq!(current["atlas"], vec!["read_host_journal".to_owned()]);
+    }
+}