subagent: grant claude's Bash when the agent holds the execution group
A subagent inherited the parent's built-in tool list, which correctly has no `Bash` -- the agent reaches a shell through the `bash` MCP server, not the built-in. Subagents get no such server, so the intersection was empty and they could not run a command at all: no commits, no pushes, no gates. Add `subagent_builtin_tools_for`/`_arg`, which reuse the shared resolver and append `Bash` only when `Execution` -- the group that gates the `bash` MCP server -- is present. Only the subagent spawn path calls them, so the harness's own `--tools`/`--allowedTools` are unchanged. The capability transfers; the mechanism does not. Refs #4422
This commit is contained in:
parent
9cd30a58ba
commit
5ce0a357b4
4 changed files with 122 additions and 32 deletions
|
|
@ -227,19 +227,31 @@ particular `--tools` does **not** filter `mcp__*` tools.
|
||||||
|
|
||||||
### Built-in tools (`--tools`)
|
### Built-in tools (`--tools`)
|
||||||
|
|
||||||
**A subagent gets exactly the built-ins its parent agent has** — the same
|
**A subagent gets exactly the built-ins its parent agent has, plus one
|
||||||
list, resolved by the same function
|
deliberate exception** — the base resolution
|
||||||
(`hive_sh4re::permissions::builtin_tools_arg`) from the same
|
(`hive_sh4re::permissions::subagent_builtin_tools_arg`, wrapping the
|
||||||
`HIVE_TOOL_GROUPS`: `Edit`, `Glob`, `Grep`, `Read`, `Skill`,
|
harness's own `builtin_tools_for`) from the same `HIVE_TOOL_GROUPS`:
|
||||||
`Write`, plus `WebFetch`/`WebSearch` for an agent granted the `web_tools`
|
`Edit`, `Glob`, `Grep`, `Read`, `Skill`, `Write`, plus `WebFetch`/
|
||||||
tool group and not otherwise. See
|
`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)
|
[the harness's own allowlist](../turn-loop/mcp.md#tool-allowlist-hive_sh4repermissionsallowed_builtin_tools)
|
||||||
for what that list contains and why.
|
for what that list contains and why.
|
||||||
|
|
||||||
|
The exception: **a subagent gets Claude's built-in `Bash` iff its parent
|
||||||
|
holds the `execution` tool group** — the group that already grants the
|
||||||
|
parent shell execution, via the out-of-process `bash` MCP server
|
||||||
|
(`mcp__bash__run`/`status`/`kill`). A subagent has no MCP server of its
|
||||||
|
own by default (see [MCP servers](#mcp-servers---strict-mcp-config)
|
||||||
|
below), so there is no `mcp__bash__*` for it to inherit that capability
|
||||||
|
through; it gets Claude's own tool instead. The rule is the capability
|
||||||
|
transfers, not the mechanism — the parent itself never gains built-in
|
||||||
|
`Bash` no matter which groups it holds, only a subagent spawned by an
|
||||||
|
`execution`-holding parent does.
|
||||||
|
|
||||||
Inheriting rather than listing is the point: a hardcoded subagent list
|
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
|
would hand web egress (or `Bash`) to the subagent of an agent that isn't
|
||||||
egress, and would diverge from the parent's on the first tool anyone adds
|
allowed it, and would diverge from the parent's on the first tool anyone
|
||||||
to either.
|
adds to either.
|
||||||
|
|
||||||
Everything else in claude's built-in set is absent, in particular the
|
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
|
tools that let a session act outside the run it was started for: peer and
|
||||||
|
|
|
||||||
|
|
@ -23,9 +23,11 @@ pub const DEFAULT_MCP_HTTP_PORT: u16 = 8790;
|
||||||
/// in `hive_sh4re::permissions`, next to `ToolGroup` itself. Re-exported here
|
/// in `hive_sh4re::permissions`, next to `ToolGroup` itself. Re-exported here
|
||||||
/// because this module is where the rest of the claude launch config is
|
/// because this module is where the rest of the claude launch config is
|
||||||
/// assembled, and because the subagent daemon (`hive-subagent-mcp`) spawns
|
/// assembled, and because the subagent daemon (`hive-subagent-mcp`) spawns
|
||||||
/// its own `claude` from the same resolution: `hive-agent` is a binary-only
|
/// its own `claude` from the same base resolution (plus its own `Bash`
|
||||||
/// crate with no lib target, so a shared home was the only way for both
|
/// exception — see `hive_sh4re::permissions::subagent_builtin_tools_arg`):
|
||||||
/// spawners to read one list rather than two that drift.
|
/// `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};
|
pub use hive_sh4re::permissions::{builtin_tools_arg, effective_tool_groups};
|
||||||
|
|
||||||
/// `HIVE_CAPABILITIES` env var injected by `meta::render_flake` when the
|
/// `HIVE_CAPABILITIES` env var injected by `meta::render_flake` when the
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,11 @@
|
||||||
//! groups rather than in any one consumer. Both processes that spawn a
|
//! groups rather than in any one consumer. Both processes that spawn a
|
||||||
//! `claude` — the harness itself and the subagent daemon — call the same
|
//! `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
|
//! function, so a subagent can never be handed a built-in its parent does
|
||||||
//! not have.
|
//! not have — with one deliberate exception: [`subagent_builtin_tools_arg`]
|
||||||
|
//! additionally grants a **subagent** Claude's own `Bash` when the parent
|
||||||
|
//! holds [`ToolGroup::Execution`]. The parent never gets built-in `Bash`
|
||||||
|
//! itself (it has the out-of-process `bash` MCP server instead); the
|
||||||
|
//! capability transfers to a subagent, not the mechanism.
|
||||||
|
|
||||||
use std::str::FromStr;
|
use std::str::FromStr;
|
||||||
|
|
||||||
|
|
@ -87,6 +91,40 @@ pub fn builtin_tools_arg() -> String {
|
||||||
builtin_tools_for(&effective_tool_groups()).join(",")
|
builtin_tools_for(&effective_tool_groups()).join(",")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// [`builtin_tools_for`] plus Claude's built-in `Bash`, granted iff `groups`
|
||||||
|
/// contains [`ToolGroup::Execution`] — the **subagent**-only `--tools`
|
||||||
|
/// resolution.
|
||||||
|
///
|
||||||
|
/// `Execution` already grants shell execution to the parent agent, via the
|
||||||
|
/// out-of-process `bash` MCP server (`mcp__bash__run`/`status`/`kill` —
|
||||||
|
/// see the variant's own doc comment). A subagent has no MCP server at all
|
||||||
|
/// by default (`docs/tools/subagent.md`), so there is no `mcp__bash__*` for
|
||||||
|
/// it to inherit that capability through; it gets Claude's own `Bash`
|
||||||
|
/// instead. The capability transfers from the parent, the mechanism it
|
||||||
|
/// arrives by does not have to match.
|
||||||
|
///
|
||||||
|
/// 🩸 Deliberately **not** folded into [`builtin_tools_for`]/[`ToolGroup::builtin_tools`]:
|
||||||
|
/// those are shared with the harness's own `--tools` (`hive-agent`'s
|
||||||
|
/// `mcp_config::allowed_tools_arg`), and the main agent must never gain
|
||||||
|
/// built-in `Bash` no matter which groups it holds — only a *subagent*
|
||||||
|
/// spawned by an `Execution`-holding parent does.
|
||||||
|
#[must_use]
|
||||||
|
pub fn subagent_builtin_tools_for(groups: &[ToolGroup]) -> Vec<&'static str> {
|
||||||
|
let mut tools = builtin_tools_for(groups);
|
||||||
|
if groups.contains(&ToolGroup::Execution) {
|
||||||
|
tools.push("Bash");
|
||||||
|
}
|
||||||
|
tools
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The value for a **subagent's** `--tools` flag: [`subagent_builtin_tools_for`]
|
||||||
|
/// resolved against [`effective_tool_groups`]. See that function for why this
|
||||||
|
/// differs from [`builtin_tools_arg`].
|
||||||
|
#[must_use]
|
||||||
|
pub fn subagent_builtin_tools_arg() -> String {
|
||||||
|
subagent_builtin_tools_for(&effective_tool_groups()).join(",")
|
||||||
|
}
|
||||||
|
|
||||||
/// Named group of MCP tools an agent may be granted. The harness reads
|
/// Named group of MCP tools an agent may be granted. The harness reads
|
||||||
/// `HIVE_TOOL_GROUPS` from the environment (a comma-separated list of
|
/// `HIVE_TOOL_GROUPS` from the environment (a comma-separated list of
|
||||||
/// `snake_case` group names written by the meta renderer from per-agent
|
/// `snake_case` group names written by the meta renderer from per-agent
|
||||||
|
|
@ -464,4 +502,41 @@ mod tests {
|
||||||
assert!(!tool.contains("__"), "{tool} looks like an MCP tool name");
|
assert!(!tool.contains("__"), "{tool} looks like an MCP tool name");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A subagent whose parent holds `Execution` gets Claude's own `Bash` —
|
||||||
|
/// the parent already has shell execution (via the out-of-process
|
||||||
|
/// `bash` MCP server), and a subagent has no MCP server of its own to
|
||||||
|
/// inherit that through, so it gets Claude's built-in tool instead.
|
||||||
|
#[test]
|
||||||
|
fn subagent_gets_bash_when_parent_has_execution() {
|
||||||
|
let tools = subagent_builtin_tools_for(&[ToolGroup::Execution]);
|
||||||
|
assert!(tools.contains(&"Bash"));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 🎯 The negative case that matters: without `Execution`, a subagent
|
||||||
|
/// must not get `Bash` either. Without this, a later refactor that
|
||||||
|
/// grants it unconditionally would go unnoticed.
|
||||||
|
#[test]
|
||||||
|
fn subagent_gets_no_bash_without_execution() {
|
||||||
|
for groups in [&[][..], &[ToolGroup::Messaging, ToolGroup::WebTools][..]] {
|
||||||
|
let tools = subagent_builtin_tools_for(groups);
|
||||||
|
assert!(!tools.contains(&"Bash"), "{groups:?} must not grant Bash");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The parent (harness) resolver is untouched: `builtin_tools_for` /
|
||||||
|
/// `builtin_tools_arg` — what `hive-agent`'s own `--tools` and
|
||||||
|
/// `--allowedTools` are built from — never produce `Bash`, with or
|
||||||
|
/// without `Execution`. Only the subagent-specific resolver above adds
|
||||||
|
/// it; the operator's ruling was explicit that the main agent does not.
|
||||||
|
#[test]
|
||||||
|
fn parent_resolver_never_gains_bash() {
|
||||||
|
for groups in [&[][..], &[ToolGroup::Execution][..], ToolGroup::ALL] {
|
||||||
|
let tools = builtin_tools_for(groups);
|
||||||
|
assert!(
|
||||||
|
!tools.contains(&"Bash"),
|
||||||
|
"{groups:?} must not grant Bash to the parent"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -836,16 +836,15 @@ fn subagent_otel_attrs(name: &str) -> String {
|
||||||
/// same "cheaper than you" cost-consciousness the `base:claude-subagents`
|
/// same "cheaper than you" cost-consciousness the `base:claude-subagents`
|
||||||
/// skill already asks of `model`. `prompt_file`, when given, becomes
|
/// skill already asks of `model`. `prompt_file`, when given, becomes
|
||||||
/// `--append-system-prompt-file` — the subagent's task instructions.
|
/// `--append-system-prompt-file` — the subagent's task instructions.
|
||||||
/// `dir`, when given, becomes `Config::cwd` (e.g. a worktree
|
/// `dir`, when given, becomes `Config::cwd` (e.g. a worktree the caller
|
||||||
/// the caller already prepared); `None` inherits this daemon's own working
|
/// already prepared); `None` inherits this daemon's own working directory.
|
||||||
/// directory, same as before this field existed. Always
|
/// Always `--dangerously-skip-permissions --strict-mcp-config --tools` — two
|
||||||
/// `--dangerously-skip-permissions --strict-mcp-config --tools` — two gates
|
/// gates covering one half each, neither substituting for the other:
|
||||||
/// covering one half each, neither substituting for the other:
|
|
||||||
/// `strict_mcp_config` over **MCP** tools (no ambient discovery, so exactly
|
/// `strict_mcp_config` over **MCP** tools (no ambient discovery, so exactly
|
||||||
/// what [`crate::mcp_config::build`] renders), `--tools` over **built-in**
|
/// what [`crate::mcp_config::build`] renders), `--tools` over **built-in**
|
||||||
/// ones, via [`hive_sh4re::permissions::builtin_tools_arg`] — the parent
|
/// ones, via [`hive_sh4re::permissions::subagent_builtin_tools_arg`], which
|
||||||
/// agent's own resolved set, never wider. `--tools` does not filter
|
/// documents its one `Bash` exception. `--tools` does not filter `mcp__*`,
|
||||||
/// `mcp__*`, so the signal tools are unnamed in it (`docs/tools/subagent.md`).
|
/// so the signal tools are unnamed in it (`docs/tools/subagent.md`).
|
||||||
///
|
///
|
||||||
/// `signal_url` is what makes `goal_reached`/`need_help` callable at all: a
|
/// `signal_url` is what makes `goal_reached`/`need_help` callable at all: a
|
||||||
/// subagent reaches them over the same streamable-http listener its parent
|
/// subagent reaches them over the same streamable-http listener its parent
|
||||||
|
|
@ -865,7 +864,7 @@ fn build_config(
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
signal_url: Option<&str>,
|
signal_url: Option<&str>,
|
||||||
) -> Config {
|
) -> Config {
|
||||||
let tools = hive_sh4re::permissions::builtin_tools_arg();
|
let tools = hive_sh4re::permissions::subagent_builtin_tools_arg();
|
||||||
// Refuse to spawn on an empty `--tools` rather than reason about what it
|
// 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
|
// would do: what an empty value means is disputed and depends on the
|
||||||
// installed claude release (see `builtin_tools_arg`). Nothing here wants
|
// installed claude release (see `builtin_tools_arg`). Nothing here wants
|
||||||
|
|
@ -2017,13 +2016,15 @@ mod tests {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A subagent's built-ins are a subset of its parent's resolved set —
|
/// A subagent's built-ins are a subset of `subagent_builtin_tools_for` —
|
||||||
/// the property that makes this safe to ship, since a subagent that can
|
/// the parent's own resolved set, plus `Bash` iff the parent holds
|
||||||
/// reach a tool its parent cannot is a privilege escalation dressed as a
|
/// `Execution` (see that function's doc comment). This is the property
|
||||||
/// convenience. Both sides are read from
|
/// that makes shipping the daemon safe: a subagent reaching a tool
|
||||||
/// `hive_sh4re::permissions`, deliberately: it is the parent harness's
|
/// outside that resolution is a privilege escalation dressed as a
|
||||||
/// own resolver, so this compares against what the parent actually gets
|
/// convenience. Read from `hive_sh4re::permissions`, deliberately: it is
|
||||||
/// rather than against a restatement of it that could drift.
|
/// the parent harness's own resolver, so this compares against what the
|
||||||
|
/// parent actually licenses rather than against a restatement of it that
|
||||||
|
/// could drift.
|
||||||
///
|
///
|
||||||
/// ⚠️ **Scope: this catches a code divergence, not an environment one.**
|
/// ⚠️ **Scope: this catches a code divergence, not an environment one.**
|
||||||
/// Both sides resolve in one process off one `HIVE_TOOL_GROUPS`, so it
|
/// Both sides resolve in one process off one `HIVE_TOOL_GROUPS`, so it
|
||||||
|
|
@ -2032,14 +2033,14 @@ mod tests {
|
||||||
/// Nothing in a unit test reaches that; it is a deployment property.
|
/// Nothing in a unit test reaches that; it is a deployment property.
|
||||||
#[test]
|
#[test]
|
||||||
fn a_subagent_gets_no_builtin_its_parent_lacks() {
|
fn a_subagent_gets_no_builtin_its_parent_lacks() {
|
||||||
let parent = hive_sh4re::permissions::builtin_tools_for(
|
let allowed = hive_sh4re::permissions::subagent_builtin_tools_for(
|
||||||
&hive_sh4re::permissions::effective_tool_groups(),
|
&hive_sh4re::permissions::effective_tool_groups(),
|
||||||
);
|
);
|
||||||
let config = build_config("n", None, None, None, None, None);
|
let config = build_config("n", None, None, None, None, None);
|
||||||
for tool in spawned_tools(&config).split(',') {
|
for tool in spawned_tools(&config).split(',') {
|
||||||
assert!(
|
assert!(
|
||||||
parent.contains(&tool),
|
allowed.contains(&tool),
|
||||||
"subagent got {tool}, which the parent's resolved set does not have"
|
"subagent got {tool}, which the parent's resolved set does not license"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue