swarm-controller: refuse an inference model name that starts with "-"
`check_model` accepted a value like `--dangerously-skip-permissions`, and the stored model ends up in the agent's `claude` argv. A PUT whose `model`, or any `subagent_models` entry (same validator), starts with `-` is now a 400 with the same problem+json shape as the other model checks. A dash inside a name (`claude-opus-4-8`) is still accepted. Raised by argus on #5084. Refs #5034
This commit is contained in:
parent
19a045a401
commit
47bd83054f
3 changed files with 32 additions and 18 deletions
|
|
@ -83,9 +83,9 @@ which swarm-controller alone writes:
|
||||||
Every field is optional, and `{}` reads the same as no key.
|
Every field is optional, and `{}` reads the same as no key.
|
||||||
|
|
||||||
| method | path | body |
|
| method | path | body |
|
||||||
| -------- | ------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ |
|
| -------- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||||
| `GET` | `/api/agents/<agent>/inference` | —; answers `{"configured": <the stored value, {} when none>, "effective": <what the agent reported, or null>}` |
|
| `GET` | `/api/agents/<agent>/inference` | —; answers `{"configured": <the stored value, {} when none>, "effective": <what the agent reported, or null>}` |
|
||||||
| `PUT` | `/api/agents/<agent>/inference` | the whole value, replacing the stored one; the controller refuses unknown fields, an unknown effort, or a model name with whitespace |
|
| `PUT` | `/api/agents/<agent>/inference` | the whole value, replacing the stored one; the controller refuses unknown fields, an unknown effort, or a model name with whitespace or a leading `-` |
|
||||||
| `DELETE` | `/api/agents/<agent>/inference` | —; answers 204 |
|
| `DELETE` | `/api/agents/<agent>/inference` | —; answers 204 |
|
||||||
|
|
||||||
An agent's own queue credential grants it
|
An agent's own queue credential grants it
|
||||||
|
|
|
||||||
|
|
@ -59,7 +59,8 @@ impl From<AgentInference> for InferenceSettings {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A model name as the agent passes it to claude: one non-empty word.
|
/// A model name as the agent passes it to claude: one non-empty word that
|
||||||
|
/// cannot be read as a command-line flag.
|
||||||
fn check_model(name: &str, what: &str) -> Result<(), String> {
|
fn check_model(name: &str, what: &str) -> Result<(), String> {
|
||||||
if name.is_empty() {
|
if name.is_empty() {
|
||||||
return Err(format!("the {what} is empty"));
|
return Err(format!("the {what} is empty"));
|
||||||
|
|
@ -67,6 +68,9 @@ fn check_model(name: &str, what: &str) -> Result<(), String> {
|
||||||
if name.chars().any(|c| c.is_whitespace() || c.is_control()) {
|
if name.chars().any(|c| c.is_whitespace() || c.is_control()) {
|
||||||
return Err(format!("the {what} {name:?} contains whitespace"));
|
return Err(format!("the {what} {name:?} contains whitespace"));
|
||||||
}
|
}
|
||||||
|
if name.starts_with('-') {
|
||||||
|
return Err(format!("the {what} {name:?} starts with \"-\""));
|
||||||
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -259,7 +263,8 @@ pub(crate) async fn get_agent_inference(
|
||||||
(status = 200, description = "the settings as stored; they replace the previous \
|
(status = 200, description = "the settings as stored; they replace the previous \
|
||||||
ones whole", body = InferenceSettings),
|
ones whole", body = InferenceSettings),
|
||||||
(status = 400, description = "not an identifier, not declared on any hive, or a \
|
(status = 400, description = "not an identifier, not declared on any hive, or a \
|
||||||
model name is empty, has whitespace, or is listed twice (problem+json)",
|
model name is empty, has whitespace, starts with `-`, or is listed twice \
|
||||||
|
(problem+json)",
|
||||||
body = String),
|
body = String),
|
||||||
(status = 422, description = "an unknown field, or an effort that is not one of \
|
(status = 422, description = "an unknown field, or an effort that is not one of \
|
||||||
the five levels"),
|
the five levels"),
|
||||||
|
|
@ -465,8 +470,18 @@ mod tests {
|
||||||
r#"{"subagent_models":[]}"#,
|
r#"{"subagent_models":[]}"#,
|
||||||
r#"{"subagent_models":["haiku",""]}"#,
|
r#"{"subagent_models":["haiku",""]}"#,
|
||||||
r#"{"subagent_models":["haiku","haiku"]}"#,
|
r#"{"subagent_models":["haiku","haiku"]}"#,
|
||||||
|
r#"{"model":"--dangerously-skip-permissions"}"#,
|
||||||
|
r#"{"model":"-x"}"#,
|
||||||
|
r#"{"subagent_models":["haiku","--print"]}"#,
|
||||||
] {
|
] {
|
||||||
assert!(settings(json).unwrap().into_stored().is_err(), "{json}");
|
assert!(settings(json).unwrap().into_stored().is_err(), "{json}");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Control for the leading-dash refusal: full model ids have dashes inside.
|
||||||
|
#[test]
|
||||||
|
fn a_dash_inside_a_model_name_is_accepted() {
|
||||||
|
let json = r#"{"model":"claude-opus-4-8","subagent_models":["claude-haiku-4-5"]}"#;
|
||||||
|
assert!(settings(json).unwrap().into_stored().is_ok());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -5544,18 +5544,17 @@ mod tests {
|
||||||
"DELETE {name}"
|
"DELETE {name}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
for bad in [r#"{"model":"opus 4"}"#, r#"{"model":"--print"}"#] {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
status(
|
status(
|
||||||
put_agent_inference(
|
put_agent_inference(State(state.clone()), Path("iris".to_owned()), body(bad))
|
||||||
State(state),
|
|
||||||
Path("iris".to_owned()),
|
|
||||||
body(r#"{"model":"opus 4"}"#)
|
|
||||||
)
|
|
||||||
.await
|
.await
|
||||||
),
|
),
|
||||||
Some(StatusCode::BAD_REQUEST)
|
Some(StatusCode::BAD_REQUEST),
|
||||||
|
"{bad}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn listing_subagents_needs_a_queue_and_an_identifier() {
|
async fn listing_subagents_needs_a_queue_and_an_identifier() {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue