From a8ee894429a71541a96ac55c29bd5918b6249453 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 1 Jun 2026 21:37:48 +0200 Subject: [PATCH 1/3] feat(#1013): validate tool-group names in set_groups against ToolGroup::ALL --- hive-c0re/src/tool_groups.rs | 39 +++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/hive-c0re/src/tool_groups.rs b/hive-c0re/src/tool_groups.rs index c9341709..049f2e7b 100644 --- a/hive-c0re/src/tool_groups.rs +++ b/hive-c0re/src/tool_groups.rs @@ -26,6 +26,8 @@ use std::collections::BTreeMap; use std::path::PathBuf; +use anyhow::Context as _; + const TOOL_GROUPS_FILE: &str = "tool-groups.json"; #[must_use] @@ -55,7 +57,8 @@ pub fn groups_for(name: &str) -> Vec { /// Persist the full tool-groups map. Sorted JSON output keeps diffs /// minimal. Best-effort — returns `io::Error` so callers decide -/// whether to abort or log. +/// whether to abort or log. Prefer `set_groups` over calling this +/// directly — `set_groups` validates group names before writing. pub fn write(map: &BTreeMap>) -> std::io::Result<()> { let path = tool_groups_path(); if let Some(parent) = path.parent() { @@ -66,16 +69,46 @@ pub fn write(map: &BTreeMap>) -> std::io::Result<()> { std::fs::write(&path, format!("{text}\n")) } +/// Validate a slice of group name strings against `ToolGroup::ALL`. +/// Returns `Ok(())` when all names are known, or `Err` listing the +/// unrecognised names so callers can surface a useful error message. +pub fn validate_groups(groups: &[String]) -> anyhow::Result<()> { + let valid: std::collections::BTreeSet<&str> = + hive_sh4re::ToolGroup::ALL.iter().map(|g| g.as_str()).collect(); + let unknown: Vec<&str> = groups + .iter() + .map(String::as_str) + .filter(|s| !valid.contains(s)) + .collect(); + if unknown.is_empty() { + Ok(()) + } else { + anyhow::bail!( + "unknown tool group(s): {}; valid names are: {}", + unknown.join(", "), + hive_sh4re::ToolGroup::ALL + .iter() + .map(|g| g.as_str()) + .collect::>() + .join(", ") + ) + } +} + /// Set the tool groups for one agent and persist the map. An empty /// `groups` vec removes the entry (agent reverts to role default). -pub fn set_groups(name: &str, groups: &[String]) -> std::io::Result<()> { +/// Returns an error if any name is not in `ToolGroup::ALL`. +pub fn set_groups(name: &str, groups: &[String]) -> anyhow::Result<()> { + if !groups.is_empty() { + validate_groups(groups)?; + } let mut current = read(); if groups.is_empty() { current.remove(name); } else { current.insert(name.to_owned(), groups.to_vec()); } - write(¤t) + write(¤t).with_context(|| format!("write tool-groups for {name}")) } /// Drop the entry for an agent that is being destroyed. Idempotent. From 8c836039d3e92cb44dbeebf247abbf5e9c04613d Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 1 Jun 2026 21:47:29 +0200 Subject: [PATCH 2/3] fix(#1013): make validate_groups private (only called by set_groups) --- hive-c0re/src/tool_groups.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hive-c0re/src/tool_groups.rs b/hive-c0re/src/tool_groups.rs index 049f2e7b..77fdcd56 100644 --- a/hive-c0re/src/tool_groups.rs +++ b/hive-c0re/src/tool_groups.rs @@ -72,7 +72,7 @@ pub fn write(map: &BTreeMap>) -> std::io::Result<()> { /// Validate a slice of group name strings against `ToolGroup::ALL`. /// Returns `Ok(())` when all names are known, or `Err` listing the /// unrecognised names so callers can surface a useful error message. -pub fn validate_groups(groups: &[String]) -> anyhow::Result<()> { +fn validate_groups(groups: &[String]) -> anyhow::Result<()> { let valid: std::collections::BTreeSet<&str> = hive_sh4re::ToolGroup::ALL.iter().map(|g| g.as_str()).collect(); let unknown: Vec<&str> = groups From 93ecbcb879fef2a778ea1f3bd046fb49c91b8e09 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 1 Jun 2026 22:01:24 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(#1013):=20make=20write=20private=20?= =?UTF-8?q?=E2=80=94=20only=20set=5Fgroups=20and=20remove=5Fagent=20are=20?= =?UTF-8?q?the=20write=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hive-c0re/src/tool_groups.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hive-c0re/src/tool_groups.rs b/hive-c0re/src/tool_groups.rs index 77fdcd56..1d894e59 100644 --- a/hive-c0re/src/tool_groups.rs +++ b/hive-c0re/src/tool_groups.rs @@ -56,10 +56,9 @@ pub fn groups_for(name: &str) -> Vec { } /// Persist the full tool-groups map. Sorted JSON output keeps diffs -/// minimal. Best-effort — returns `io::Error` so callers decide -/// whether to abort or log. Prefer `set_groups` over calling this -/// directly — `set_groups` validates group names before writing. -pub fn write(map: &BTreeMap>) -> std::io::Result<()> { +/// minimal. Use `set_groups` (or `remove_agent`) from outside this +/// module — they go through the validated write path. +fn write(map: &BTreeMap>) -> std::io::Result<()> { let path = tool_groups_path(); if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?;