hive-runtime, hive-agent: MCP permission only for kind other
A permission request counted as an MCP tool call whenever its title looked like `<server>_<tool>`, whatever its kind, and acp_permits allowed MCP calls before looking at the kind. So an `execute` request titled e.g. `hyperhive_x`, or a `fetch` without web_tools, was allowed. MCP tool calls come with kind `other` (opencode's toToolKind maps every tool it doesn't name, MCP tools included, to "other"). The runtime now sets PermissionAsk::mcp_server only for kind `other`, and acp_permits allows an MCP server's tool only under the `other` arm. Refs #4391
This commit is contained in:
parent
868fc789d1
commit
bed72ce280
3 changed files with 49 additions and 10 deletions
|
|
@ -344,8 +344,9 @@ fn acp_permission_policy() -> PermissionPolicy {
|
||||||
/// Whether an ACP agent may run the tool call it asks about. Default-deny,
|
/// Whether an ACP agent may run the tool call it asks about. Default-deny,
|
||||||
/// allowing what a claude agent has:
|
/// allowing what a claude agent has:
|
||||||
///
|
///
|
||||||
/// - tools of the MCP servers the session was handed — which of those an
|
/// - `other`-kind calls to tools of the MCP servers the session was handed
|
||||||
/// agent gets is already decided by its tool groups (`mcp_config`);
|
/// — which of those an agent gets is already decided by its tool groups
|
||||||
|
/// (`mcp_config`);
|
||||||
/// - the file tools, mirroring the claude built-ins
|
/// - the file tools, mirroring the claude built-ins
|
||||||
/// (`hive_sh4re::permissions`): `read`, `edit`, `search`;
|
/// (`hive_sh4re::permissions`): `read`, `edit`, `search`;
|
||||||
/// - `fetch` with the `web_tools` group (`web`).
|
/// - `fetch` with the `web_tools` group (`web`).
|
||||||
|
|
@ -353,10 +354,8 @@ fn acp_permission_policy() -> PermissionPolicy {
|
||||||
/// Everything else is refused, including a built-in shell (`execute`) —
|
/// Everything else is refused, including a built-in shell (`execute`) —
|
||||||
/// shell goes through `mcp__bash__run` — and any kind this list doesn't name.
|
/// shell goes through `mcp__bash__run` — and any kind this list doesn't name.
|
||||||
fn acp_permits(ask: &PermissionAsk<'_>, web: bool) -> bool {
|
fn acp_permits(ask: &PermissionAsk<'_>, web: bool) -> bool {
|
||||||
if ask.mcp_server.is_some() {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
match ask.kind {
|
match ask.kind {
|
||||||
|
"other" => ask.mcp_server.is_some(),
|
||||||
"read" | "edit" | "search" => true,
|
"read" | "edit" | "search" => true,
|
||||||
"fetch" => web,
|
"fetch" => web,
|
||||||
_ => false,
|
_ => false,
|
||||||
|
|
@ -891,6 +890,20 @@ mod tests {
|
||||||
assert!(acp_permits(&mcp, false));
|
assert!(acp_permits(&mcp, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_mcp_looking_title_does_not_lift_execute_or_fetch() {
|
||||||
|
let execute = PermissionAsk {
|
||||||
|
kind: "execute",
|
||||||
|
mcp_server: Some("hyperhive"),
|
||||||
|
};
|
||||||
|
assert!(!acp_permits(&execute, true));
|
||||||
|
let fetch = PermissionAsk {
|
||||||
|
kind: "fetch",
|
||||||
|
mcp_server: Some("hyperhive"),
|
||||||
|
};
|
||||||
|
assert!(!acp_permits(&fetch, false));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn acp_fetch_follows_the_web_tools_group() {
|
fn acp_fetch_follows_the_web_tools_group() {
|
||||||
assert!(acp_permits(&ask("fetch"), true));
|
assert!(acp_permits(&ask("fetch"), true));
|
||||||
|
|
|
||||||
|
|
@ -34,9 +34,10 @@ pub struct PermissionAsk<'a> {
|
||||||
/// The tool call's ACP `kind` (`read`, `edit`, `execute`, `fetch`, …;
|
/// The tool call's ACP `kind` (`read`, `edit`, `execute`, `fetch`, …;
|
||||||
/// `other` when the agent gives none).
|
/// `other` when the agent gives none).
|
||||||
pub kind: &'a str,
|
pub kind: &'a str,
|
||||||
/// The MCP server, of those handed to the session, whose tool this is:
|
/// For a `kind` of `other`: the MCP server, of those handed to the
|
||||||
/// the tool call's title names it as `<server>_<tool>`,
|
/// session, whose tool this is — the tool call's title names it as
|
||||||
/// `<server>__<tool>` or `mcp__<server>__<tool>`.
|
/// `<server>_<tool>`, `<server>__<tool>` or `mcp__<server>__<tool>`.
|
||||||
|
/// `None` for every other kind.
|
||||||
pub mcp_server: Option<&'a str>,
|
pub mcp_server: Option<&'a str>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -239,9 +239,14 @@ pub(super) fn permission_outcome(
|
||||||
) -> Value {
|
) -> Value {
|
||||||
let tool_call = params.get("toolCall");
|
let tool_call = params.get("toolCall");
|
||||||
let field = |k: &str| tool_call.and_then(|t| t.get(k)).and_then(Value::as_str);
|
let field = |k: &str| tool_call.and_then(|t| t.get(k)).and_then(Value::as_str);
|
||||||
|
let kind = field("kind").unwrap_or("other");
|
||||||
let ask = PermissionAsk {
|
let ask = PermissionAsk {
|
||||||
kind: field("kind").unwrap_or("other"),
|
kind,
|
||||||
mcp_server: field("title").and_then(|title| mcp_server_of(title, servers)),
|
// MCP tool calls are kind `other`; a title alone must not lift another
|
||||||
|
// kind (`execute`, `fetch`) into one.
|
||||||
|
mcp_server: (kind == "other")
|
||||||
|
.then(|| field("title").and_then(|title| mcp_server_of(title, servers)))
|
||||||
|
.flatten(),
|
||||||
};
|
};
|
||||||
let wanted = if permit(&ask) {
|
let wanted = if permit(&ask) {
|
||||||
["allow_once", "allow_always"]
|
["allow_once", "allow_always"]
|
||||||
|
|
@ -334,6 +339,26 @@ mod tests {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A title shaped like `<server>_<tool>` marks an MCP call only on kind
|
||||||
|
/// `other`, which is what an agent gives an MCP tool call.
|
||||||
|
#[test]
|
||||||
|
fn an_mcp_looking_title_on_execute_or_fetch_is_not_an_mcp_call() {
|
||||||
|
let policy: PermissionPolicy = Arc::new(|ask: &PermissionAsk<'_>| ask.mcp_server.is_some());
|
||||||
|
for kind in ["execute", "fetch"] {
|
||||||
|
assert_eq!(
|
||||||
|
permission_outcome(&request(kind, "hyperhive_send"), &policy, &servers())["outcome"]
|
||||||
|
["optionId"],
|
||||||
|
"reject",
|
||||||
|
"{kind}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
assert_eq!(
|
||||||
|
permission_outcome(&request("other", "hyperhive_send"), &policy, &servers())["outcome"]
|
||||||
|
["optionId"],
|
||||||
|
"once"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn no_matching_option_cancels() {
|
fn no_matching_option_cancels() {
|
||||||
let req = json!({ "toolCall": { "kind": "execute" },
|
let req = json!({ "toolCall": { "kind": "execute" },
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue