permissions: give the built-in tool list one home, next to ToolGroup
The `--tools` list a harness session gets is not a constant: the base set plus whatever the agent's `HIVE_TOOL_GROUPS` add (today, `web_tools` → `WebFetch`/`WebSearch`). That resolution lived in `hive-agent`'s `mcp_config`, which is fine while the harness is the only thing that spawns a `claude` — and it is not: `hive-subagent-mcp` spawns one too. `hive-agent` is binary-only (no `src/lib.rs`, no lib target), so nothing can depend on it to reach `builtin_tools_arg`. The alternative to a shared home is a second list in the subagent daemon, which diverges on the first tool anyone adds to either — and diverging upward is a subagent holding a built-in its parent does not have. So move the base list, the `HIVE_TOOL_GROUPS` parse and the resolution into `hive_sh4re::permissions`, beside `ToolGroup` — whose `builtin_tools()` was already half of the answer. `hive-agent` re-exports them, so `mcp_config::builtin_tools_arg()` still reads the same at the call site, and `allowed_tools_arg` now derives its built-in half from the same function rather than repeating the merge loop. Behaviour is unchanged. The parse is `strum::EnumString` rather than a `serde_json::from_value` round-trip through a `Value::String`: same `snake_case` names (a test pins the two derives against each other), without `hive-sh4re` needing `serde_json` outside its dev-dependencies. It is now a pure function of its input, so the fallbacks are testable without mutating the environment — which under edition 2024 is `unsafe` and racy across a test binary's threads. Refs #4416
This commit is contained in:
parent
0890b1eaa0
commit
8b01dbeef1
5 changed files with 240 additions and 86 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1972,6 +1972,7 @@ dependencies = [
|
|||
"serde",
|
||||
"serde_json",
|
||||
"strum",
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
|
|
|||
|
|
@ -203,7 +203,12 @@ than deriving from SSE events.
|
|||
runs the body, logs the result. Pre-/post-log only — the inbox
|
||||
status hint lives in the wake prompt + UI header, not here.
|
||||
|
||||
## Tool allowlist (`mcp_config::ALLOWED_BUILTIN_TOOLS`)
|
||||
## Tool allowlist (`hive_sh4re::permissions::ALLOWED_BUILTIN_TOOLS`)
|
||||
|
||||
The built-in list and the `--tools` value it resolves to live in
|
||||
`hive-sh4re` alongside `ToolGroup`, not in the harness, because the
|
||||
subagent daemon spawns its own `claude` and must resolve the same set —
|
||||
see [`docs/tools/subagent.md`](../tools/subagent.md).
|
||||
|
||||
- Allowed built-ins: `Edit`, `Glob`, `Grep`, `Read`, `Skill`, `Write`.
|
||||
`Skill` is what makes an installed plugin's `SKILL.md` invokable —
|
||||
|
|
|
|||
|
|
@ -18,27 +18,15 @@ pub const SERVER_NAME: &str = "hyperhive";
|
|||
/// so `127.0.0.1:<port>` is per-container-private (no cross-agent collision).
|
||||
pub const DEFAULT_MCP_HTTP_PORT: u16 = 8790;
|
||||
|
||||
/// Built-in claude tools always present in every session. Anything not
|
||||
/// in this list (or added by `extra_builtin_tools`) literally doesn't
|
||||
/// exist in the session. Web egress (`WebFetch`/`WebSearch`) are
|
||||
/// tool-group-gated (`web_tools`) — off by default. Nested agents
|
||||
/// (`Task`/`Agent`) are intentionally omitted. `Bash` is disallowed — shell
|
||||
/// execution goes through `mcp__bash__run` (background tasks
|
||||
/// with structured output via `hive-bash-mcp`) instead of a raw interactive shell. `TodoWrite`
|
||||
/// is omitted because the todo list lives in claude's in-process session
|
||||
/// state and silently evaporates on /compact or session reset — agents
|
||||
/// should plan in /state notes instead. `Skill` is included so installed
|
||||
/// plugin skills (`base@hyperhive` and friends) are actually invokable —
|
||||
/// without it here, every `SKILL.md`'s `description` frontmatter is inert:
|
||||
/// the model never sees the tool that triggers a skill, no matter how well
|
||||
/// it matches the task at hand.
|
||||
pub const ALLOWED_BUILTIN_TOOLS: &[&str] = &["Edit", "Glob", "Grep", "Read", "Skill", "Write"];
|
||||
|
||||
/// Env var written by the meta renderer with a comma-separated list of
|
||||
/// `hive_sh4re::permissions::ToolGroup` `snake_case` names (e.g. `"messaging,inbox,meta"`).
|
||||
/// When present, the harness expands the groups into per-tool allow entries
|
||||
/// instead of using the hardcoded flavor default. See `docs/process/conventions.md::Tool groups`.
|
||||
const TOOL_GROUPS_ENV: &str = "HIVE_TOOL_GROUPS";
|
||||
/// The built-in tool surface — the base list, the group-gated additions, the
|
||||
/// `HIVE_TOOL_GROUPS` parse, and the `--tools` value they resolve to — lives
|
||||
/// in `hive_sh4re::permissions`, next to `ToolGroup` itself. Re-exported here
|
||||
/// because this module is where the rest of the claude launch config is
|
||||
/// assembled, and because the subagent daemon (`hive-subagent-mcp`) spawns
|
||||
/// its own `claude` from the same resolution: `hive-agent` is a binary-only
|
||||
/// crate with no lib target, so a shared home was the only way for both
|
||||
/// spawners to read one list rather than two that drift.
|
||||
pub use hive_sh4re::permissions::{builtin_tools_arg, effective_tool_groups};
|
||||
|
||||
/// `HIVE_CAPABILITIES` env var injected by `meta::render_flake` when the
|
||||
/// operator grants capabilities to this agent. Comma-separated
|
||||
|
|
@ -73,40 +61,6 @@ fn allowed_capability_tools() -> Vec<String> {
|
|||
tools
|
||||
}
|
||||
|
||||
/// Resolve the active tool groups for a harness session.
|
||||
///
|
||||
/// Reads `HIVE_TOOL_GROUPS` from the environment first. Each comma-separated
|
||||
/// token is matched (case-insensitive) against the `ToolGroup` serde names
|
||||
/// (`messaging`, `meta`, `inbox`, `lifecycle`, `approvals`, `scheduling`,
|
||||
/// `diagnostics`, `execution`). Unrecognised tokens are logged and skipped.
|
||||
/// Falls back to `AGENT_DEFAULT` when the env var is absent or empty.
|
||||
fn effective_tool_groups() -> Vec<hive_sh4re::permissions::ToolGroup> {
|
||||
let raw = match std::env::var(TOOL_GROUPS_ENV) {
|
||||
Ok(v) if !v.trim().is_empty() => v,
|
||||
_ => return hive_sh4re::permissions::ToolGroup::AGENT_DEFAULT.to_vec(),
|
||||
};
|
||||
let mut groups = Vec::new();
|
||||
for token in raw.split(',') {
|
||||
let t = token.trim().to_ascii_lowercase();
|
||||
// Parse via serde_json (the canonical deserialization path).
|
||||
if let Ok(g) = serde_json::from_value::<hive_sh4re::permissions::ToolGroup>(
|
||||
serde_json::Value::String(t.clone()),
|
||||
) {
|
||||
groups.push(g);
|
||||
} else {
|
||||
tracing::warn!(token = %t, "{TOOL_GROUPS_ENV}: unknown tool group, skipping");
|
||||
}
|
||||
}
|
||||
if groups.is_empty() {
|
||||
tracing::warn!(
|
||||
"{TOOL_GROUPS_ENV} set but contained no recognised groups; \
|
||||
falling back to AGENT_DEFAULT"
|
||||
);
|
||||
return hive_sh4re::permissions::ToolGroup::AGENT_DEFAULT.to_vec();
|
||||
}
|
||||
groups
|
||||
}
|
||||
|
||||
/// Tool group an extra (out-of-process) MCP server is gated behind, if any.
|
||||
///
|
||||
/// Most `hyperhive.extraMcpServers` entries are ungated — available whenever
|
||||
|
|
@ -200,19 +154,12 @@ pub fn allowed_mcp_tools(groups: &[hive_sh4re::permissions::ToolGroup]) -> Vec<S
|
|||
#[must_use]
|
||||
pub fn allowed_tools_arg() -> String {
|
||||
let groups = effective_tool_groups();
|
||||
// Base built-ins always present.
|
||||
let mut all: Vec<String> = ALLOWED_BUILTIN_TOOLS
|
||||
.iter()
|
||||
.map(|s| (*s).to_owned())
|
||||
// The same built-ins `--tools` makes exist this session, so a tool that
|
||||
// exists is never one claude has to prompt about.
|
||||
let mut all: Vec<String> = hive_sh4re::permissions::builtin_tools_for(&groups)
|
||||
.into_iter()
|
||||
.map(ToOwned::to_owned)
|
||||
.collect();
|
||||
// Extra built-ins gated by tool groups (e.g. WebFetch/WebSearch via web_tools).
|
||||
for group in &groups {
|
||||
for tool in group.builtin_tools() {
|
||||
if !all.iter().any(|t| t == *tool) {
|
||||
all.push((*tool).to_owned());
|
||||
}
|
||||
}
|
||||
}
|
||||
all.extend(allowed_mcp_tools(&groups));
|
||||
// Capability-gated MCP tools: added to --allowedTools when HIVE_CAPABILITIES
|
||||
// includes the corresponding capability. hive-c0re performs a second
|
||||
|
|
@ -224,23 +171,6 @@ pub fn allowed_tools_arg() -> String {
|
|||
all.join(",")
|
||||
}
|
||||
|
||||
/// Built-in tools list for `--tools` (which built-ins exist in this
|
||||
/// session). Base set plus any group-gated built-ins (e.g.
|
||||
/// `WebFetch`/`WebSearch` when the `web_tools` group is active).
|
||||
#[must_use]
|
||||
pub fn builtin_tools_arg() -> String {
|
||||
let groups = effective_tool_groups();
|
||||
let mut tools: Vec<&str> = ALLOWED_BUILTIN_TOOLS.to_vec();
|
||||
for group in &groups {
|
||||
for t in group.builtin_tools() {
|
||||
if !tools.contains(t) {
|
||||
tools.push(t);
|
||||
}
|
||||
}
|
||||
}
|
||||
tools.join(",")
|
||||
}
|
||||
|
||||
/// Render the MCP config blob claude reads from `--mcp-config <path>`.
|
||||
/// The built-in `hyperhive` surface is an HTTP entry pointing at the
|
||||
/// persistent `hive-mcp-http` daemon (see [`DEFAULT_MCP_HTTP_PORT`]); there
|
||||
|
|
|
|||
|
|
@ -14,6 +14,10 @@ hive-types.workspace = true
|
|||
schemars.workspace = true
|
||||
serde.workspace = true
|
||||
strum.workspace = true
|
||||
# Facade only, for the one warn in `permissions::ToolGroup::parse_list`: an
|
||||
# unknown tool-group name is skipped rather than fatal, so the log line is the
|
||||
# only trace it leaves.
|
||||
tracing.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
serde_json.workspace = true
|
||||
|
|
|
|||
|
|
@ -3,16 +3,104 @@
|
|||
//! per-agent config (`tool-groups.json` / `capabilities.json`), injected
|
||||
//! into the container as env vars, and read by the harness to decide
|
||||
//! which MCP tools claude actually sees.
|
||||
//!
|
||||
//! It also resolves the *built-in* claude tool list ([`builtin_tools_arg`]):
|
||||
//! that set is group-gated too (`web_tools`), so it belongs next to the
|
||||
//! groups rather than in any one consumer. Both processes that spawn a
|
||||
//! `claude` — the harness itself and the subagent daemon — call the same
|
||||
//! function, so a subagent can never be handed a built-in its parent does
|
||||
//! not have.
|
||||
|
||||
use std::str::FromStr;
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
/// Env var the meta renderer writes with a comma-separated list of
|
||||
/// [`ToolGroup`] `snake_case` names (e.g. `"messaging,inbox,meta"`). Absent
|
||||
/// or unparseable means [`ToolGroup::AGENT_DEFAULT`]. See
|
||||
/// `docs/process/conventions.md::Tool groups`.
|
||||
pub const TOOL_GROUPS_ENV: &str = "HIVE_TOOL_GROUPS";
|
||||
|
||||
/// Built-in claude tools present in every session regardless of tool
|
||||
/// groups. Anything not in this list (and not added by a group's
|
||||
/// [`ToolGroup::builtin_tools`]) literally doesn't exist in the session —
|
||||
/// this is the `--tools` list, which holds even under
|
||||
/// `--dangerously-skip-permissions`.
|
||||
///
|
||||
/// Web egress (`WebFetch`/`WebSearch`) is group-gated (`web_tools`) and so
|
||||
/// lives there, off by default. Nested agents (`Task`/`Agent`) are
|
||||
/// intentionally omitted. `Bash` is disallowed — shell execution goes
|
||||
/// through `mcp__bash__run` (background tasks with structured output via
|
||||
/// `hive-bash-mcp`) instead of a raw interactive shell. `TodoWrite` is
|
||||
/// omitted because the todo list lives in claude's in-process session state
|
||||
/// and silently evaporates on /compact or session reset — agents should
|
||||
/// plan in /state notes instead. `Skill` is included so installed plugin
|
||||
/// skills (`base@hyperhive` and friends) are actually invokable — without
|
||||
/// it here, every `SKILL.md`'s `description` frontmatter is inert: the
|
||||
/// model never sees the tool that triggers a skill, no matter how well it
|
||||
/// matches the task at hand.
|
||||
pub const ALLOWED_BUILTIN_TOOLS: &[&str] = &["Edit", "Glob", "Grep", "Read", "Skill", "Write"];
|
||||
|
||||
/// The tool groups in effect for this process, from [`TOOL_GROUPS_ENV`].
|
||||
///
|
||||
/// Absent, blank, or naming nothing recognisable all fall back to
|
||||
/// [`ToolGroup::AGENT_DEFAULT`] — the var is written per agent by the meta
|
||||
/// renderer, so "unset" is an ordinary state, not an error.
|
||||
#[must_use]
|
||||
pub fn effective_tool_groups() -> Vec<ToolGroup> {
|
||||
ToolGroup::parse_list(&std::env::var(TOOL_GROUPS_ENV).unwrap_or_default())
|
||||
}
|
||||
|
||||
/// The built-in claude tools `groups` resolve to: [`ALLOWED_BUILTIN_TOOLS`]
|
||||
/// plus each group's own built-ins, de-duplicated, base set first.
|
||||
#[must_use]
|
||||
pub fn builtin_tools_for(groups: &[ToolGroup]) -> Vec<&'static str> {
|
||||
let mut tools: Vec<&'static str> = ALLOWED_BUILTIN_TOOLS.to_vec();
|
||||
for group in groups {
|
||||
for tool in group.builtin_tools() {
|
||||
if !tools.contains(tool) {
|
||||
tools.push(tool);
|
||||
}
|
||||
}
|
||||
}
|
||||
tools
|
||||
}
|
||||
|
||||
/// The value for claude's `--tools` flag: which built-in tools exist in this
|
||||
/// session at all.
|
||||
///
|
||||
/// ⚠️ **Never emit this as an empty string.** An empty `--tools` value parses
|
||||
/// as *unset* and yields **more** tools than omitting the flag, so there is
|
||||
/// no way to spell "no built-in tools" — an empty list is a fail-loudly bug,
|
||||
/// not a lockdown. [`ALLOWED_BUILTIN_TOOLS`] is non-empty, which is what
|
||||
/// keeps that from arising.
|
||||
///
|
||||
/// ⚠️ This governs built-ins only. `mcp__*` tools are not filtered by
|
||||
/// `--tools` at all; they are governed by `--strict-mcp-config` plus
|
||||
/// whatever the caller renders into `--mcp-config`.
|
||||
#[must_use]
|
||||
pub fn builtin_tools_arg() -> String {
|
||||
builtin_tools_for(&effective_tool_groups()).join(",")
|
||||
}
|
||||
|
||||
/// Named group of MCP tools an agent may be granted. The harness reads
|
||||
/// `HIVE_TOOL_GROUPS` from the environment (a comma-separated list of
|
||||
/// `snake_case` group names written by the meta renderer from per-agent
|
||||
/// config) and expands it to the matching tool names for `--allowedTools`.
|
||||
/// When the env var is absent the harness falls back to `AGENT_DEFAULT`.
|
||||
/// See `docs/process/conventions.md::Tool groups`.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, strum::IntoStaticStr)]
|
||||
#[derive(
|
||||
Debug,
|
||||
Clone,
|
||||
Copy,
|
||||
PartialEq,
|
||||
Eq,
|
||||
Hash,
|
||||
Serialize,
|
||||
Deserialize,
|
||||
strum::IntoStaticStr,
|
||||
strum::EnumString,
|
||||
)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
pub enum ToolGroup {
|
||||
|
|
@ -50,6 +138,42 @@ pub enum ToolGroup {
|
|||
}
|
||||
|
||||
impl ToolGroup {
|
||||
/// Parse a comma-separated [`TOOL_GROUPS_ENV`] value. Tokens are trimmed
|
||||
/// and lowercased; an unrecognised one is warned about and skipped
|
||||
/// rather than failing the whole list, so one stale name in an agent's
|
||||
/// config can't take its whole tool surface away.
|
||||
///
|
||||
/// Returns [`AGENT_DEFAULT`](Self::AGENT_DEFAULT) when `raw` is blank or
|
||||
/// names nothing recognisable — the same fallback either way, because
|
||||
/// "the operator granted no groups" and "the var is missing" are not
|
||||
/// distinguishable here and neither should mean "no tools".
|
||||
#[must_use]
|
||||
pub fn parse_list(raw: &str) -> Vec<Self> {
|
||||
let mut groups = Vec::new();
|
||||
for token in raw.split(',') {
|
||||
let token = token.trim().to_ascii_lowercase();
|
||||
if token.is_empty() {
|
||||
continue;
|
||||
}
|
||||
match Self::from_str(&token) {
|
||||
Ok(group) => groups.push(group),
|
||||
Err(_) => {
|
||||
tracing::warn!(token = %token, "{TOOL_GROUPS_ENV}: unknown tool group, skipping");
|
||||
}
|
||||
}
|
||||
}
|
||||
if groups.is_empty() {
|
||||
if !raw.trim().is_empty() {
|
||||
tracing::warn!(
|
||||
"{TOOL_GROUPS_ENV} set but contained no recognised groups; \
|
||||
falling back to AGENT_DEFAULT"
|
||||
);
|
||||
}
|
||||
return Self::AGENT_DEFAULT.to_vec();
|
||||
}
|
||||
groups
|
||||
}
|
||||
|
||||
/// The MCP tool names (without the `mcp__hyperhive__` prefix) in this group.
|
||||
/// Returns `&[]` for `WebTools` — it enables Claude built-in tools,
|
||||
/// not MCP tools; see `builtin_tools()`.
|
||||
|
|
@ -245,3 +369,93 @@ impl Capability {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The env-var spellings the meta renderer actually writes must round-trip
|
||||
/// back to groups. `strum::EnumString` and serde's `rename_all` are two
|
||||
/// independent derives over the same names; this pins them together, since
|
||||
/// the renderer writes what serde would produce and the parse is strum's.
|
||||
#[test]
|
||||
fn parse_list_accepts_the_snake_case_names_the_renderer_writes() {
|
||||
assert_eq!(
|
||||
ToolGroup::parse_list("messaging, Meta ,web_tools"),
|
||||
vec![ToolGroup::Messaging, ToolGroup::Meta, ToolGroup::WebTools]
|
||||
);
|
||||
for group in ToolGroup::ALL {
|
||||
let name: &'static str = (*group).into();
|
||||
assert_eq!(ToolGroup::parse_list(name), vec![*group], "{name}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Blank, absent and all-garbage are the same state as far as the tool
|
||||
/// surface is concerned: the default groups, never an empty list.
|
||||
#[test]
|
||||
fn parse_list_falls_back_to_the_agent_default_rather_than_nothing() {
|
||||
for raw in ["", " ", ",,", "not_a_group,also_not"] {
|
||||
assert_eq!(
|
||||
ToolGroup::parse_list(raw),
|
||||
ToolGroup::AGENT_DEFAULT.to_vec(),
|
||||
"{raw:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Adding groups only ever adds built-ins — the base set is in every
|
||||
/// resolution, and no group can remove one. Both `--tools` callers (the
|
||||
/// harness and the subagent daemon) rely on this to reason about "the
|
||||
/// parent's set" without re-deriving it.
|
||||
#[test]
|
||||
fn builtin_tools_are_the_base_set_plus_whatever_the_groups_add() {
|
||||
assert_eq!(
|
||||
builtin_tools_for(&[]),
|
||||
ALLOWED_BUILTIN_TOOLS.to_vec(),
|
||||
"no groups resolves to exactly the base set"
|
||||
);
|
||||
assert_eq!(
|
||||
builtin_tools_for(ToolGroup::AGENT_DEFAULT),
|
||||
ALLOWED_BUILTIN_TOOLS.to_vec(),
|
||||
"the default groups add no built-ins"
|
||||
);
|
||||
for group in ToolGroup::ALL {
|
||||
let resolved = builtin_tools_for(&[*group]);
|
||||
for base in ALLOWED_BUILTIN_TOOLS {
|
||||
assert!(resolved.contains(base), "{group:?} dropped {base}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `web_tools` is the one group that widens the built-in set, and it is
|
||||
/// opt-in. A subagent inherits this resolution rather than a list of its
|
||||
/// own precisely so it cannot get web egress from a parent without the
|
||||
/// group.
|
||||
#[test]
|
||||
fn web_egress_is_present_only_with_the_web_tools_group() {
|
||||
let with = builtin_tools_for(&[ToolGroup::WebTools]);
|
||||
assert!(with.contains(&"WebFetch") && with.contains(&"WebSearch"));
|
||||
let without = builtin_tools_for(ToolGroup::MANAGER_DEFAULT);
|
||||
assert!(!without.contains(&"WebFetch") && !without.contains(&"WebSearch"));
|
||||
}
|
||||
|
||||
/// An empty `--tools` value parses as *unset* and grants more than
|
||||
/// omitting the flag, so no resolution may ever produce one.
|
||||
#[test]
|
||||
fn no_group_combination_resolves_to_an_empty_tools_arg() {
|
||||
assert!(!ALLOWED_BUILTIN_TOOLS.is_empty());
|
||||
assert!(!builtin_tools_for(&[]).is_empty());
|
||||
assert!(!builtin_tools_for(ToolGroup::ALL).is_empty());
|
||||
assert!(!builtin_tools_arg().is_empty());
|
||||
}
|
||||
|
||||
/// Built-ins are not MCP tools and the two lists must not be mixed: a
|
||||
/// `mcp__*` name in `--tools` would be silently inert, since `--tools`
|
||||
/// does not filter the MCP surface at all.
|
||||
#[test]
|
||||
fn the_builtin_list_names_no_mcp_tools() {
|
||||
for tool in builtin_tools_for(ToolGroup::ALL) {
|
||||
assert!(!tool.contains("__"), "{tool} looks like an MCP tool name");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue