subagent: gate the daemon's model against availableModels
The subagent daemon put `model` straight onto claude's argv with no validation, so an agent could spawn nested sessions on any model the operator had deliberately kept off its harness. Forward the existing `hyperhive.availableModels` onto the daemon unit as HIVE_AVAILABLE_MODELS (same rail `HIVE_TOOL_GROUPS` uses) and check `start`/`continue` against it before building the config. Default open: an absent var restricts nothing, so an agent deployed before this keeps working. An omitted `model` is always allowed — it lets claude pick its own default rather than naming one. Refs #4436
This commit is contained in:
parent
f4b7a1e357
commit
46a6317f13
2 changed files with 113 additions and 3 deletions
|
|
@ -814,6 +814,52 @@ fn validate_name(name: &str) -> anyhow::Result<()> {
|
||||||
.map_err(|e| anyhow::anyhow!("invalid subagent name {name:?}: {e}"))
|
.map_err(|e| anyhow::anyhow!("invalid subagent name {name:?}: {e}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Env var carrying the nix-configured `hyperhive.availableModels` as a
|
||||||
|
/// comma-separated list, forwarded onto this daemon's unit by
|
||||||
|
/// `nix/agent-modules/mcp.nix` the same way `HIVE_TOOL_GROUPS` is.
|
||||||
|
const AVAILABLE_MODELS_ENV: &str = "HIVE_AVAILABLE_MODELS";
|
||||||
|
|
||||||
|
/// The models this daemon may spawn a subagent on, from
|
||||||
|
/// [`AVAILABLE_MODELS_ENV`]. `None` when the var is absent.
|
||||||
|
fn available_models() -> Option<String> {
|
||||||
|
std::env::var(AVAILABLE_MODELS_ENV).ok()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Refuse a `model` the operator hasn't made available to this agent.
|
||||||
|
///
|
||||||
|
/// Default-open on purpose: this is a safety rail, not a security boundary,
|
||||||
|
/// so an absent `allowed` (or one naming nothing) restricts nothing and an
|
||||||
|
/// agent deployed before this gate existed keeps working unchanged. A `None`
|
||||||
|
/// model isn't a choice of model either — it lets claude fall back to its own
|
||||||
|
/// default — so there is nothing to check.
|
||||||
|
///
|
||||||
|
/// # Errors
|
||||||
|
///
|
||||||
|
/// A named model absent from `allowed`; the message names it and every
|
||||||
|
/// permitted one, since it is all the caller sees.
|
||||||
|
fn check_model(model: Option<&str>, allowed: Option<&str>) -> anyhow::Result<()> {
|
||||||
|
let (Some(model), Some(allowed)) = (model, allowed) else {
|
||||||
|
return Ok(());
|
||||||
|
};
|
||||||
|
// Same split/trim/skip-empty rules as `ToolGroup::parse_list`.
|
||||||
|
let allowed: Vec<&str> = allowed
|
||||||
|
.split(',')
|
||||||
|
.map(str::trim)
|
||||||
|
.filter(|token| !token.is_empty())
|
||||||
|
.collect();
|
||||||
|
if allowed.is_empty()
|
||||||
|
|| allowed
|
||||||
|
.iter()
|
||||||
|
.any(|entry| entry.eq_ignore_ascii_case(model.trim()))
|
||||||
|
{
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
anyhow::bail!(
|
||||||
|
"model `{model}` is not available to this agent — {AVAILABLE_MODELS_ENV} permits: {}",
|
||||||
|
allowed.join(", ")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// Extend the ambient `OTEL_RESOURCE_ATTRIBUTES` with a `subagent=<name>`
|
/// Extend the ambient `OTEL_RESOURCE_ATTRIBUTES` with a `subagent=<name>`
|
||||||
/// attribute, so every token/cost/tool-call data point this subagent's own
|
/// attribute, so every token/cost/tool-call data point this subagent's own
|
||||||
/// claude process emits carries it alongside the parent's `agent=<name>`
|
/// claude process emits carries it alongside the parent's `agent=<name>`
|
||||||
|
|
@ -949,7 +995,8 @@ pub struct StartRequest {
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// A name already running, an invalid name, an archive failure, or the
|
/// A name already running, an invalid name, a `model` the operator hasn't
|
||||||
|
/// made available (see [`check_model`]), an archive failure, or the
|
||||||
/// underlying `Claude::spawn` failing (binary missing, etc.) — the last
|
/// underlying `Claude::spawn` failing (binary missing, etc.) — the last
|
||||||
/// case is the only one that can happen *after* commit-to-run, and it's
|
/// case is the only one that can happen *after* commit-to-run, and it's
|
||||||
/// exactly why nothing is registered in `running` until spawn actually
|
/// exactly why nothing is registered in `running` until spawn actually
|
||||||
|
|
@ -957,6 +1004,7 @@ pub struct StartRequest {
|
||||||
pub fn start(state: &Arc<State>, req: StartRequest) -> anyhow::Result<String> {
|
pub fn start(state: &Arc<State>, req: StartRequest) -> anyhow::Result<String> {
|
||||||
let name = req.name.as_str();
|
let name = req.name.as_str();
|
||||||
validate_name(name)?;
|
validate_name(name)?;
|
||||||
|
check_model(req.model.as_deref(), available_models().as_deref())?;
|
||||||
if !state.reserve(name) {
|
if !state.reserve(name) {
|
||||||
anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`");
|
anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`");
|
||||||
}
|
}
|
||||||
|
|
@ -1064,8 +1112,9 @@ fn start_reserved(
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// An invalid name, one already running, `Claude::spawn` failing, or the
|
/// An invalid name, one already running, a `model` the operator hasn't made
|
||||||
/// resumed turn failing within `RESUME_GRACE` — in practice a missed resume.
|
/// available (see [`check_model`]), `Claude::spawn` failing, or the resumed
|
||||||
|
/// turn failing within `RESUME_GRACE` — in practice a missed resume.
|
||||||
pub async fn continue_(
|
pub async fn continue_(
|
||||||
state: &Arc<State>,
|
state: &Arc<State>,
|
||||||
name: &str,
|
name: &str,
|
||||||
|
|
@ -1075,6 +1124,7 @@ pub async fn continue_(
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
validate_name(name)?;
|
validate_name(name)?;
|
||||||
|
check_model(model.as_deref(), available_models().as_deref())?;
|
||||||
if !state.reserve(name) {
|
if !state.reserve(name) {
|
||||||
anyhow::bail!(
|
anyhow::bail!(
|
||||||
"subagent `{name}` is already running — use `interrupt` first if you meant to redirect it"
|
"subagent `{name}` is already running — use `interrupt` first if you meant to redirect it"
|
||||||
|
|
@ -3325,4 +3375,50 @@ mod tests {
|
||||||
"the safety property is unchanged: no ambient MCP discovery either way"
|
"the safety property is unchanged: no ambient MCP discovery either way"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_unset_model_whitelist_allows_any_model() {
|
||||||
|
assert!(check_model(Some("opus"), None).is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_unset_model_whitelist_allows_an_omitted_model() {
|
||||||
|
assert!(check_model(None, None).is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_listed_model_is_allowed() {
|
||||||
|
assert!(check_model(Some("sonnet"), Some("haiku,sonnet,opus")).is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_unlisted_model_is_rejected_naming_what_is_permitted() {
|
||||||
|
let err = check_model(Some("opus"), Some("haiku,sonnet"))
|
||||||
|
.expect_err("a model outside the list must be refused")
|
||||||
|
.to_string();
|
||||||
|
assert!(
|
||||||
|
err.contains("opus") && err.contains("haiku, sonnet"),
|
||||||
|
"the rejection is all the caller sees, so it must name the model \
|
||||||
|
and the permitted ones: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_set_whitelist_still_allows_an_omitted_model() {
|
||||||
|
assert!(
|
||||||
|
check_model(None, Some("haiku")).is_ok(),
|
||||||
|
"omitting `model` lets claude pick its own default — it is not a \
|
||||||
|
choice of model, so there is nothing to gate"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn whitespace_and_empty_entries_are_ignored() {
|
||||||
|
assert!(check_model(Some("sonnet"), Some(" haiku , sonnet ,, ")).is_ok());
|
||||||
|
assert!(check_model(Some("opus"), Some(" haiku , sonnet ,, ")).is_err());
|
||||||
|
assert!(
|
||||||
|
check_model(Some("opus"), Some(" ,, ")).is_ok(),
|
||||||
|
"a var naming nothing restricts nothing — default open"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -388,6 +388,20 @@ in
|
||||||
# `null` when the agent has no groups declared, which systemd drops
|
# `null` when the agent has no groups declared, which systemd drops
|
||||||
# — the same "absent" the harness itself would see.
|
# — the same "absent" the harness itself would see.
|
||||||
HIVE_TOOL_GROUPS = config.systemd.services.hive-agent.environment.HIVE_TOOL_GROUPS or null;
|
HIVE_TOOL_GROUPS = config.systemd.services.hive-agent.environment.HIVE_TOOL_GROUPS or null;
|
||||||
|
# Same `hyperhive.availableModels` the harness's own assertions gate
|
||||||
|
# the primary session's model against, so a subagent can't be spawned
|
||||||
|
# on a model the operator didn't make available to this agent. The
|
||||||
|
# option renders into the *global* environment for the web UI's
|
||||||
|
# quick-picker, which a systemd unit doesn't inherit, so set it here
|
||||||
|
# from the option directly. `null` for an empty list, which systemd
|
||||||
|
# drops: the daemon reads an absent var as "no restriction" (this is
|
||||||
|
# a safety rail, not a security boundary), matching the harness
|
||||||
|
# assertion that an empty list waives too.
|
||||||
|
HIVE_AVAILABLE_MODELS =
|
||||||
|
if config.hyperhive.availableModels == [ ] then
|
||||||
|
null
|
||||||
|
else
|
||||||
|
lib.concatStringsSep "," config.hyperhive.availableModels;
|
||||||
# HYPERHIVE_HARNESS_DIR / HYPERHIVE_STATE_DIR: see
|
# HYPERHIVE_HARNESS_DIR / HYPERHIVE_STATE_DIR: see
|
||||||
# `hive-bash-daemon`'s own comment above — same global injection,
|
# `hive-bash-daemon`'s own comment above — same global injection,
|
||||||
# same reasoning.
|
# same reasoning.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue