diff --git a/Cargo.lock b/Cargo.lock index 1e0f2b33..29c3f839 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1972,7 +1972,6 @@ dependencies = [ "serde", "serde_json", "strum", - "tracing", ] [[package]] @@ -1995,7 +1994,6 @@ dependencies = [ "clap", "hive-agent-sock", "hive-claude", - "hive-sh4re", "hive-sock-client", "hive-types", "libc", diff --git a/docs/tools/subagent.md b/docs/tools/subagent.md index 05aedc96..f797904c 100644 --- a/docs/tools/subagent.md +++ b/docs/tools/subagent.md @@ -218,55 +218,13 @@ agent that needs a stable or non-default port. Own systemd unit, defined alongside the other per-agent MCP daemons in `nix/agent-modules/mcp.nix`. -## The tool surface a subagent gets - -Two flags, each covering one half, and neither covering the other: -`--tools` governs claude's built-in tools, `--strict-mcp-config` governs -the MCP ones. Dropping either brings that half back in full; in -particular `--tools` does **not** filter `mcp__*` tools. - -### Built-in tools (`--tools`) - -**A subagent gets exactly the built-ins its parent agent has** — the same -list, resolved by the same function -(`hive_sh4re::permissions::builtin_tools_arg`) from the same -`HIVE_TOOL_GROUPS`: `Edit`, `Glob`, `Grep`, `Read`, `Skill`, -`Write`, plus `WebFetch`/`WebSearch` for an agent granted the `web_tools` -tool group and not otherwise. See -[the harness's own allowlist](../turn-loop/mcp.md#tool-allowlist-hive_sh4repermissionsallowed_builtin_tools) -for what that list contains and why. - -Inheriting rather than listing is the point: a hardcoded subagent list -would hand web egress to the subagent of an agent that isn't allowed web -egress, and would diverge from the parent's on the first tool anyone adds -to either. - -Everything else in claude's built-in set is absent, in particular the -tools that let a session act outside the run it was started for: peer and -operator messaging, nested agents (including the stop verb, which takes -an _agent_ id rather than a session), schedule and webhook creation, and -worktree switching. Before this flag was passed, a subagent reached all of -them — `--dangerously-skip-permissions` had removed the only thing that -would have asked, and `--allowedTools` would not have helped: it approves -prompts in advance rather than restricting anything. - -One rule worth knowing before editing any of this: **the daemon never -emits an empty `--tools` value**, and asserts rather than doing so. Not -because of what an empty value does — that's exactly the point. Our own -measurement and the installed `claude --help` disagree about whether -`--tools ""` means "no tools" or reads as the flag being absent, and the -answer belongs to whichever claude release is installed rather than to -this repo. Refusing the case is correct under either reading, so the code -never has to know which holds. Nothing here wants a subagent with no -built-in tools anyway. - -### MCP servers (`--strict-mcp-config`) +## MCP servers available to a subagent A subagent runs with `--strict-mcp-config` and, by default, exactly one -MCP server: the two-tool `subagent_control` route above — not the -parent's `mcp__bash__*` / `mcp__hyperhive__*` surface. Those two signal -tools are deliberately unnamed in `--tools`, which doesn't govern them; -they survive on this flag alone. Nothing implicit reaches it: the built-in +MCP server: the two-tool `subagent_control` route above. It otherwise +falls back to claude's own native tools (`Bash`, `WebFetch`, etc.), not +the parent's `mcp__bash__*` / `mcp__hyperhive__*` surface. Nothing +implicit reaches it: the built-in hyperhive surface (todos/messaging) isn't an `extraMcpServers` entry at all, and the automatically injected `bash`/`subagent` entries default to excluded too (a subagent can't spawn hive-bash tasks or its own nested subagents diff --git a/docs/turn-loop/mcp.md b/docs/turn-loop/mcp.md index c565a322..eb54fc57 100644 --- a/docs/turn-loop/mcp.md +++ b/docs/turn-loop/mcp.md @@ -203,12 +203,7 @@ 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 (`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). +## Tool allowlist (`mcp_config::ALLOWED_BUILTIN_TOOLS`) - Allowed built-ins: `Edit`, `Glob`, `Grep`, `Read`, `Skill`, `Write`. `Skill` is what makes an installed plugin's `SKILL.md` invokable — diff --git a/hive-agent/src/mcp_config.rs b/hive-agent/src/mcp_config.rs index 0818c7ba..e13a4585 100644 --- a/hive-agent/src/mcp_config.rs +++ b/hive-agent/src/mcp_config.rs @@ -18,15 +18,27 @@ pub const SERVER_NAME: &str = "hyperhive"; /// so `127.0.0.1:` is per-container-private (no cross-agent collision). pub const DEFAULT_MCP_HTTP_PORT: u16 = 8790; -/// 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}; +/// 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"; /// `HIVE_CAPABILITIES` env var injected by `meta::render_flake` when the /// operator grants capabilities to this agent. Comma-separated @@ -61,6 +73,40 @@ fn allowed_capability_tools() -> Vec { 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 { + 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::( + 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 @@ -154,12 +200,19 @@ pub fn allowed_mcp_tools(groups: &[hive_sh4re::permissions::ToolGroup]) -> Vec String { let groups = effective_tool_groups(); - // 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 = hive_sh4re::permissions::builtin_tools_for(&groups) - .into_iter() - .map(ToOwned::to_owned) + // Base built-ins always present. + let mut all: Vec = ALLOWED_BUILTIN_TOOLS + .iter() + .map(|s| (*s).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 @@ -171,6 +224,23 @@ 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 `. /// The built-in `hyperhive` surface is an HTTP entry pointing at the /// persistent `hive-mcp-http` daemon (see [`DEFAULT_MCP_HTTP_PORT`]); there diff --git a/hive-sh4re/Cargo.toml b/hive-sh4re/Cargo.toml index 2efaebd9..f7a8ae73 100644 --- a/hive-sh4re/Cargo.toml +++ b/hive-sh4re/Cargo.toml @@ -14,10 +14,6 @@ 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 diff --git a/hive-sh4re/src/permissions.rs b/hive-sh4re/src/permissions.rs index f47de653..b7f8d8eb 100644 --- a/hive-sh4re/src/permissions.rs +++ b/hive-sh4re/src/permissions.rs @@ -3,108 +3,16 @@ //! 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::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** — not because of what an empty -/// value does, but because we don't know. Our own measurement and the -/// installed `claude --help` disagree about whether `--tools ""` means "no -/// tools" or parses as the flag being unset, and the answer is a property of -/// whichever claude release is installed, not of this code. So an empty -/// value is refused rather than relied on: the rule holds under either -/// reading, and no caller has to know which one is true today. -/// [`ALLOWED_BUILTIN_TOOLS`] is non-empty, which keeps the case from -/// arising in the first place. -/// -/// ⚠️ 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, - strum::EnumString, -)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, strum::IntoStaticStr)] #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub enum ToolGroup { @@ -142,42 +50,6 @@ 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 { - 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()`. @@ -373,95 +245,3 @@ 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")); - } - - /// No resolution may produce an empty `--tools` value. What an empty one - /// means is disputed and release-dependent (see [`builtin_tools_arg`]), - /// so this pins the rule that makes the question moot rather than any - /// answer to it. - #[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"); - } - } -} diff --git a/hive-subagent-mcp/Cargo.toml b/hive-subagent-mcp/Cargo.toml index e1719919..f7223e86 100644 --- a/hive-subagent-mcp/Cargo.toml +++ b/hive-subagent-mcp/Cargo.toml @@ -13,10 +13,6 @@ axum.workspace = true clap.workspace = true hive-agent-sock.workspace = true hive-claude.workspace = true -# `permissions::builtin_tools_arg` — the same `--tools` resolution the parent -# harness spawns its own claude with, so a subagent's built-in surface is its -# parent's rather than a second list that drifts. See `session::build_config`. -hive-sh4re.workspace = true hive-sock-client.workspace = true hive-types.workspace = true libc.workspace = true diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index 4759c24e..48230c58 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -834,18 +834,18 @@ fn subagent_otel_attrs(name: &str) -> String { /// (`high` on most models) — a deliberate hive policy for subagent work /// specifically, not a reflection of Anthropic's own recommendation, on the /// same "cheaper than you" cost-consciousness the `base:claude-subagents` -/// skill already asks of `model`. `prompt_file`, when given, becomes -/// `--append-system-prompt-file` — the subagent's task instructions. -/// `dir`, when given, becomes `Config::cwd` (e.g. a worktree +/// skill already asks of `model`. `prompt_file`, when +/// given, becomes `--append-system-prompt-file` — the subagent's task +/// instructions. `dir`, when given, becomes `Config::cwd` (e.g. a worktree /// the caller already prepared); `None` inherits this daemon's own working /// directory, same as before this field existed. Always -/// `--dangerously-skip-permissions --strict-mcp-config --tools` — two gates -/// covering one half each, neither substituting for the other: -/// `strict_mcp_config` over **MCP** tools (no ambient discovery, so exactly -/// what [`crate::mcp_config::build`] renders), `--tools` over **built-in** -/// ones, via [`hive_sh4re::permissions::builtin_tools_arg`] — the parent -/// agent's own resolved set, never wider. `--tools` does not filter -/// `mcp__*`, so the signal tools are unnamed in it (`docs/tools/subagent.md`). +/// `--dangerously-skip-permissions --strict-mcp-config` — the safety +/// property is `strict_mcp_config: true` with no ambient MCP discovery, not +/// an unconditional absence of `--mcp-config`: a subagent gets exactly the +/// `hyperhive.extraMcpServers` entries an operator has explicitly opted in +/// via `availableToSubagents = true` (`crate::mcp_config::build`), plus this +/// daemon's own two-tool signal surface when `signal_url` is given — nothing +/// implicit and nothing more. /// /// `signal_url` is what makes `goal_reached`/`need_help` callable at all: a /// subagent reaches them over the same streamable-http listener its parent @@ -865,22 +865,7 @@ fn build_config( dir: Option<&str>, signal_url: Option<&str>, ) -> Config { - let tools = hive_sh4re::permissions::builtin_tools_arg(); - // Refuse to spawn on an empty `--tools` rather than reason about what it - // would do: what an empty value means is disputed and depends on the - // installed claude release (see `builtin_tools_arg`). Nothing here wants - // "a subagent with no built-ins" anyway, so an empty resolution is a bug - // either way, and failing on it is safe under every reading. - assert!( - !tools.is_empty(), - "resolved an empty --tools value — a subagent is never spawned on one, \ - since what an empty value grants is release-dependent" - ); - let mut extra_args = vec![ - "--dangerously-skip-permissions".to_owned(), - "--tools".to_owned(), - tools, - ]; + let mut extra_args = vec!["--dangerously-skip-permissions".to_owned()]; if let Some(path) = prompt_file { extra_args.push("--append-system-prompt-file".to_owned()); extra_args.push(path.to_owned()); @@ -1983,100 +1968,6 @@ mod tests { ); } - /// The `--tools` value as it reaches the spawned argv. - fn spawned_tools(config: &Config) -> &str { - let flag = config - .extra_args - .iter() - .position(|a| a == "--tools") - .expect("--tools is passed on every spawn"); - config - .extra_args - .get(flag + 1) - .expect("--tools carries a value") - } - - /// `--tools` reaches the argv on every caller shape, carrying a non-empty - /// value. Non-empty matters separately from "the flag is there": what an - /// empty value grants is disputed and release-dependent, so we never ship - /// one and never depend on the answer. Checked for each shape because a - /// flag that appears on only some spawn paths is no restriction at all. - /// `signal_url` stays `None` throughout — it only steers `mcp_config`, - /// which `--tools` does not govern, and passing it would make this test - /// need the container's injected `HYPERHIVE_HARNESS_DIR`. - #[test] - fn build_config_always_passes_a_non_empty_tools_list() { - for config in [ - build_config("n", None, None, None, None, None), - build_config("n", None, None, Some("/tmp/p.md"), None, None), - build_config("n", None, None, None, Some("/tmp/wt"), None), - ] { - let tools = spawned_tools(&config); - assert!(!tools.is_empty(), "--tools must never be empty"); - assert!(tools.split(',').all(|t| !t.trim().is_empty())); - } - } - - /// A subagent's built-ins are a subset of its parent's resolved set — - /// the property that makes this safe to ship, since a subagent that can - /// reach a tool its parent cannot is a privilege escalation dressed as a - /// convenience. Both sides are read from - /// `hive_sh4re::permissions`, deliberately: it is the parent harness's - /// own resolver, so this compares against what the parent actually gets - /// rather than against a restatement of it that could drift. - /// - /// ⚠️ **Scope: this catches a code divergence, not an environment one.** - /// Both sides resolve in one process off one `HIVE_TOOL_GROUPS`, so it - /// cannot see the two real units disagreeing about that var — which they - /// did until `nix/agent-modules/mcp.nix` forwarded it onto this daemon. - /// Nothing in a unit test reaches that; it is a deployment property. - #[test] - fn a_subagent_gets_no_builtin_its_parent_lacks() { - let parent = hive_sh4re::permissions::builtin_tools_for( - &hive_sh4re::permissions::effective_tool_groups(), - ); - let config = build_config("n", None, None, None, None, None); - for tool in spawned_tools(&config).split(',') { - assert!( - parent.contains(&tool), - "subagent got {tool}, which the parent's resolved set does not have" - ); - } - } - - /// Named individually rather than inferred from "the list is short": the - /// tools this daemon used to hand a subagent that reach outside the run - /// it was started for. They are absent because the parent has never had - /// them — this pins the consequence, not a second allow-list. - /// - /// ⚠️ **A canary, not a guarantee.** The names are hardcoded, so this - /// only fails for a tool someone thought to list here; the *guarantee* - /// is the subset property above, which holds for tools nobody has - /// imagined yet. Do not read a pass as "no escaping tool is reachable" — - /// read it as "none of these ten came back". - #[test] - fn no_spawned_tool_escapes_the_session() { - let config = build_config("n", None, None, None, None, None); - let tools: Vec<&str> = spawned_tools(&config).split(',').collect(); - for escaping in [ - "SendMessage", - "ListAgents", - "Task", - "TaskStop", - "TaskOutput", - "CronCreate", - "CronDelete", - "RemoteTrigger", - "EnterWorktree", - "ExitWorktree", - ] { - assert!( - !tools.contains(&escaping), - "{escaping} must not be in a subagent's --tools" - ); - } - } - #[test] fn resolve_dir_remembers_an_explicit_dir_and_falls_back_to_it_when_omitted() { let state = State::new(PathBuf::from("/dev/null"), signal_url()); diff --git a/nix/agent-modules/mcp.nix b/nix/agent-modules/mcp.nix index 3c96ab36..f65ba05f 100644 --- a/nix/agent-modules/mcp.nix +++ b/nix/agent-modules/mcp.nix @@ -378,16 +378,6 @@ in # HIVE_AGENT_SOCKET (agent-service.nix). HIVE_AGENT_SOCKET = "/run/hive-agent/${userName}/agent.sock"; RUST_LOG = "info"; - # This daemon resolves each subagent's `--tools` from the same - # tool groups the harness resolves its own session from, so that a - # subagent's built-in tools are its parent agent's and never wider. - # The meta renderer writes this var onto the harness unit alone, so - # forward it from there rather than re-deriving it: without it the - # daemon falls back to the default groups and an agent granted - # `web_tools` would spawn subagents that quietly lack web access. - # `null` when the agent has no groups declared, which systemd drops - # — the same "absent" the harness itself would see. - HIVE_TOOL_GROUPS = config.systemd.services.hive-agent.environment.HIVE_TOOL_GROUPS or null; # HYPERHIVE_HARNESS_DIR / HYPERHIVE_STATE_DIR: see # `hive-bash-daemon`'s own comment above — same global injection, # same reasoning.