diff --git a/docs/process/conventions.md b/docs/process/conventions.md index 36551b01..912475e7 100644 --- a/docs/process/conventions.md +++ b/docs/process/conventions.md @@ -311,6 +311,15 @@ the default (`Permission::DEFAULT`): `messaging`, `meta`, `inbox`, none. swarm-controller logs every refused `PUT` or `DELETE` on that route at `warn` as `agent permissions: request refused`, with the fields `op`, `agent`, `status` and `detail`. +**Internet flag** — the record also carries `unrestricted_internet`, a +boolean that isn't a `Permission`. An absent value means on, so every agent +that existed before the field keeps the open access it had. `POST /api/agents` +always writes the record with an explicit value, `false` unless the request +names it. A `PUT` that leaves the field out and a `DELETE` (`swarmctl agent +permissions clear`) keep the stored value; a `DELETE` on an agent that has one +rewrites the record to just that value. Read it with +`swarm_queue_client::agent_permissions::unrestricted`. Nothing acts on it yet. + **How a change applies** — - The agent's harness reads its own key every 10 seconds and builds every turn's `--tools`, `--allowedTools` and MCP config from the last value, so a diff --git a/docs/tools/swarmctl-cli.md b/docs/tools/swarmctl-cli.md index e032ad98..1f948028 100644 --- a/docs/tools/swarmctl-cli.md +++ b/docs/tools/swarmctl-cli.md @@ -221,7 +221,7 @@ The agent applies its tool permissions at its next turn; the hive running it res ###### **Subcommands:** * `get` — Show the permissions the swarm grants an agent, or that it sets none -* `set` — Replace the permissions the swarm grants an agent +* `set` — Replace the permissions the swarm grants an agent; the internet flag stays unless given * `clear` — Clear the agent's permissions, so the default applies again @@ -246,7 +246,7 @@ Show the permissions the swarm grants an agent, or that it sets none ## `swarmctl agent permissions set` -Replace the permissions the swarm grants an agent +Replace the permissions the swarm grants an agent; the internet flag stays unless given **Usage:** `swarmctl agent permissions set [OPTIONS] --permissions ` @@ -257,6 +257,10 @@ Replace the permissions the swarm grants an agent ###### **Options:** * `--permissions ` — The permissions to grant, comma-separated: any of `messaging`, `meta`, `inbox`, `execution`, `web_tools`, `manage_root_agent`. `--permissions ''` grants none +* `--unrestricted-internet ` — Whether the agent's traffic may leave the host unfiltered. Left out, the stored value stays + + Possible values: `true`, `false` + * `--controller-socket ` — swarm-controller's unix socket. Supplied by the nix module that installs this binary, from the same `socketPath` option the daemon binds; falls back to `SWARM_CONTROLLER_SOCKET`. diff --git a/swarm-controller-api/src/agents.rs b/swarm-controller-api/src/agents.rs index 14449e08..3d2d7eb5 100644 --- a/swarm-controller-api/src/agents.rs +++ b/swarm-controller-api/src/agents.rs @@ -81,6 +81,10 @@ pub struct CreateAgentRequest { #[serde(default, skip_serializing_if = "Option::is_none")] #[cfg_attr(feature = "schema", schema(value_type = Option>))] pub permissions: Option>, + /// Whether the agent's traffic leaves the host unfiltered. Left out, the + /// agent is created with `false`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unrestricted_internet: Option, /// The archetype to build the agent from, one `GET /api/archetypes` /// lists. The agent then gets no `agent-configs/` repo: its hive /// builds it from the archetype's module. Absent is a custom agent. @@ -116,6 +120,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }; diff --git a/swarm-controller-api/src/permissions.rs b/swarm-controller-api/src/permissions.rs index a4ce7fcc..94d2ee9e 100644 --- a/swarm-controller-api/src/permissions.rs +++ b/swarm-controller-api/src/permissions.rs @@ -5,7 +5,8 @@ use hive_types::Permission; use serde::{Deserialize, Serialize}; /// One agent's permissions. A field left out is not set by the swarm, and the -/// compiled-in default applies; an empty list grants nothing. +/// compiled-in default applies; an empty list grants nothing. A +/// `PUT` that leaves out `unrestricted_internet` keeps the stored value. #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(utoipa::ToSchema))] #[serde(deny_unknown_fields)] @@ -18,6 +19,11 @@ pub struct Permissions { schema(value_type = Option>, example = json!(["messaging", "meta", "inbox", "execution"])) )] pub permissions: Option>, + /// Whether the agent's traffic leaves the host unfiltered. Absent in a + /// `GET` means the swarm never set it, which counts as on. Absent in a + /// `PUT` keeps the stored value. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unrestricted_internet: Option, } #[cfg(test)] @@ -32,12 +38,21 @@ mod tests { ( Permissions { permissions: Some(vec![]), + unrestricted_internet: None, }, r#"{"permissions":[]}"#, ), + ( + Permissions { + permissions: None, + unrestricted_internet: Some(false), + }, + r#"{"unrestricted_internet":false}"#, + ), ( Permissions { permissions: Some(vec![Permission::Messaging, Permission::ManageRootAgent]), + unrestricted_internet: None, }, r#"{"permissions":["messaging","manage_root_agent"]}"#, ), diff --git a/swarm-controller/src/agent_permissions.rs b/swarm-controller/src/agent_permissions.rs index cd996bdf..94b99110 100644 --- a/swarm-controller/src/agent_permissions.rs +++ b/swarm-controller/src/agent_permissions.rs @@ -36,14 +36,13 @@ use crate::wanted::WantedWriter; /// `wanted::MAX_ATTEMPTS`. const MAX_ATTEMPTS: usize = 5; -/// The permissions to store, or why they are refused. -fn into_stored(req: Permissions) -> Result { - if let Some(perms) = &req.permissions { - unique(perms)?; - } - Ok(AgentPermissions { +/// The permissions to store for `req`, whose list is already checked. The list +/// replaces the stored one; `unrestricted_internet` left out keeps the one in `current`. +fn into_stored(req: Permissions, current: &AgentPermissions) -> AgentPermissions { + AgentPermissions { permissions: req.permissions, - }) + unrestricted_internet: req.unrestricted_internet.or(current.unrestricted_internet), + } } /// Refuse a list that names a permission twice. @@ -62,6 +61,7 @@ pub(crate) fn unique(perms: &[Permission]) -> Result<(), String> { fn view(v: AgentPermissions) -> Permissions { Permissions { permissions: v.permissions, + unrestricted_internet: v.unrestricted_internet, } } @@ -127,7 +127,8 @@ pub(crate) async fn get_agent_permissions( /// Set the permissions the swarm grants one agent. /// -/// The value replaces the stored one whole. The hive running the agent reads +/// The permission list replaces the stored one; `unrestricted_internet` left +/// out keeps the stored flag. The hive running the agent reads /// it within 30 seconds and restarts the agent, after its current turn, when /// `manage_root_agent` changed; the agent applies the rest at its next turn. #[utoipa::path( @@ -156,8 +157,9 @@ pub(crate) async fn put_agent_permissions( let Json(req) = req.map_err(|rejection| refuse(error_problem(rejection.status(), &rejection.body_text())))?; let agent = agent_name(&name).map_err(|r| refuse(refused(r)))?; - let value = - into_stored(req).map_err(|why| refuse(error_problem(StatusCode::BAD_REQUEST, &why)))?; + if let Some(perms) = &req.permissions { + unique(perms).map_err(|why| refuse(error_problem(StatusCode::BAD_REQUEST, &why)))?; + } let client = queue(&state).map_err(|r| refuse(refused(r)))?; if !live(&state).await.map_err(refuse)?.contains(&agent) { return Err(refuse(error_problem( @@ -170,6 +172,7 @@ pub(crate) async fn put_agent_permissions( let (current, revision, undecodable) = read_or_default::(&store, &agent) .await .map_err(|e| queue_problem(&e))?; + let value = into_stored(req.clone(), ¤t); if (current == value && !undecodable) || write(&store, &agent, &value, revision) .await @@ -189,7 +192,8 @@ pub(crate) async fn put_agent_permissions( path = "/api/agents/{name}/permissions", params(("name" = String, Path, description = "agent name")), responses( - (status = 204, description = "cleared; the hive's default applies again"), + (status = 204, description = "cleared; the hive's default applies again, and a stored \ + `unrestricted_internet` is kept"), (status = 400, description = "not an identifier (problem+json)", body = String), (status = 503, description = "no swarm queue is configured on this host, or it is \ not connected (problem+json)", body = String), @@ -206,17 +210,37 @@ pub(crate) async fn delete_agent_permissions( let agent = agent_name(&name).map_err(|r| refuse(refused(r)))?; let client = queue(&state).map_err(|r| refuse(refused(r)))?; let store = open_or_create(&client).await?; - store - .delete(&agent) - .await - .map_err(|e| queue_problem(&anyhow::Error::new(e)))?; - tracing::info!(%agent, "cleared agent permissions"); - Ok(StatusCode::NO_CONTENT) + for _ in 0..MAX_ATTEMPTS { + let (current, revision, _) = read_or_default::(&store, &agent) + .await + .map_err(|e| queue_problem(&e))?; + // Only the flag outlives a clear: dropping it would turn an agent + // created with `false` on. + let kept = AgentPermissions { + unrestricted_internet: current.unrestricted_internet, + ..AgentPermissions::default() + }; + if kept == AgentPermissions::default() { + store + .delete(&agent) + .await + .map_err(|e| queue_problem(&anyhow::Error::new(e)))?; + } else if !write(&store, &agent, &kept, revision) + .await + .map_err(|e| queue_problem(&e))? + { + continue; + } + tracing::info!(%agent, "cleared agent permissions"); + return Ok(StatusCode::NO_CONTENT); + } + Err(queue_problem(&anyhow::anyhow!( + "gave up clearing the permissions of {agent} after {MAX_ATTEMPTS} conflicting writes" + ))) } /// Delete a destroyed agent's key. A name is free again once its agent is -/// destroyed, and a create left on the default stores nothing, so a key left -/// here would hand the next agent of that name this one's grant. +/// destroyed, so it starts with no record. /// /// Like the credential revoke it runs beside, a failure is logged and does /// not stop the destroy; declaring the agent destroyed again re-runs it. @@ -236,9 +260,9 @@ pub(crate) async fn clear_on_destroy(state: &AppState, hive: &str, agent: &str) hive, agent, error = %format!("{e:#}"), - "clearing this agent's permissions FAILED; it was destroyed anyway, so a new \ - agent of the same name created on the default gets its grant. Declaring the \ - agent destroyed again re-runs this." + "clearing this agent's permissions FAILED; it was destroyed anyway, so a read \ + of that name finds the old record until a create writes a new one. \ + Declaring the agent destroyed again re-runs this." ), } } @@ -267,13 +291,21 @@ pub(crate) async fn get_permission_catalogue() -> Json { }) } -/// The value `POST /api/agents` stores for `perms`, or why it is refused. +/// The value `POST /api/agents` stores, or why it is refused. `perms` left +/// out keeps the default set; `unrestricted_internet` left out is `false`, so +/// every new agent has an explicit value and only older records read as on. /// The agent is not declared on its hive yet, so the "declared by a hive" /// check of [`put_agent_permissions`] does not apply. -pub(crate) fn at_create(perms: Vec) -> Result { - unique(&perms)?; +pub(crate) fn at_create( + perms: Option>, + unrestricted_internet: Option, +) -> Result { + if let Some(perms) = &perms { + unique(perms)?; + } Ok(AgentPermissions { - permissions: Some(perms), + permissions: perms, + unrestricted_internet: Some(unrestricted_internet.unwrap_or(false)), }) } @@ -484,6 +516,7 @@ async fn seed_into( perms.push(Permission::ManageRootAgent); let value = AgentPermissions { permissions: Some(perms), + ..AgentPermissions::default() }; let written = write(store, SEEDED_AGENT, &value, revision).await?; if written { @@ -511,6 +544,7 @@ async fn import_into( } let value = AgentPermissions { permissions: Some(caps), + ..AgentPermissions::default() }; if write(store, &agent, &value, None).await? { written.push(agent); @@ -551,6 +585,7 @@ mod tests { fn granted() -> Permissions { Permissions { permissions: Some(vec![Permission::ManageRootAgent]), + unrestricted_internet: None, } } @@ -594,6 +629,7 @@ mod tests { let none = Permissions { permissions: Some(Vec::new()), + unrestricted_internet: None, }; let _ = put(&state, "atlas", none.clone()).await.expect("set"); assert_eq!( @@ -609,6 +645,70 @@ mod tests { assert_eq!(get(&state, "atlas").await, Permissions::default()); } + /// An agent stored with `unrestricted_internet: false` keeps it through a + /// `PUT` that leaves it out and through `DELETE`; a `PUT` that names it + /// changes it. Without the flag, `DELETE` still removes the key. + #[tokio::test] + async fn put_and_delete_keep_the_internet_flag_unless_named() { + let nats = TestNats::start(); + let client = nats.client().await; + let state = crate::agent_limits::tests::declared(&client).await; + let off = r#"{"unrestricted_internet":false}"#; + crate::agent_permissions::store( + &client, + "atlas", + &super::at_create(None, None).expect("valid"), + ) + .await + .expect("created"); + assert_eq!(published(&client, "atlas").await.as_deref(), Some(off)); + + let stored = put(&state, "atlas", granted()).await.expect("set").0; + assert_eq!(stored.unrestricted_internet, Some(false)); + assert_eq!( + published(&client, "atlas").await.as_deref(), + Some(r#"{"permissions":["manage_root_agent"],"unrestricted_internet":false}"#) + ); + + assert_eq!( + delete_agent_permissions(State(state.clone()), Path("atlas".to_owned())).await, + Ok(StatusCode::NO_CONTENT) + ); + assert_eq!(published(&client, "atlas").await.as_deref(), Some(off)); + assert_eq!( + get(&state, "atlas").await.unrestricted_internet, + Some(false) + ); + + let on = Permissions { + unrestricted_internet: Some(true), + ..granted() + }; + let stored = put(&state, "atlas", on).await.expect("set").0; + assert_eq!(stored.unrestricted_internet, Some(true)); + assert_eq!( + delete_agent_permissions(State(state.clone()), Path("atlas".to_owned())).await, + Ok(StatusCode::NO_CONTENT) + ); + assert_eq!( + published(&client, "atlas").await.as_deref(), + Some(r#"{"unrestricted_internet":true}"#) + ); + + // Control: an agent that never had the flag has nothing left after a + // clear, and a `PUT` does not invent one. + crate::agent_permissions::store(&client, "atlas", &super::AgentPermissions::default()) + .await + .expect("an older record"); + let _ = put(&state, "atlas", granted()).await.expect("set"); + assert_eq!(get(&state, "atlas").await.unrestricted_internet, None); + assert_eq!( + delete_agent_permissions(State(state.clone()), Path("atlas".to_owned())).await, + Ok(StatusCode::NO_CONTENT) + ); + assert_eq!(published(&client, "atlas").await, None); + } + /// A name no hive declares, or a name that is not an identifier, is a 400 /// and writes nothing. #[tokio::test] @@ -903,7 +1003,7 @@ mod tests { let nats = TestNats::start(); let client = nats.client().await; let state = crate::agent_limits::tests::declared(&client).await; - let create = |name: &str, permissions| { + let create = |name: &str, permissions, unrestricted_internet| { crate::routes::agents::create_agent( State(state.clone()), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -912,6 +1012,7 @@ mod tests { limits: None, inference: None, permissions, + unrestricted_internet, archetype: None, agents_md: None, }), @@ -921,6 +1022,7 @@ mod tests { let problem = create( "iris", Some(vec![Permission::Messaging, Permission::Messaging]), + None, ) .await .expect_err("a duplicate is refused"); @@ -935,21 +1037,31 @@ mod tests { .count(); assert_eq!(queued, 0, "nothing is queued for a refused create"); - let _ = create("iris", Some(vec![Permission::Messaging])) + let _ = create("iris", Some(vec![Permission::Messaging]), None) .await .expect("created"); assert_eq!( published(&client, "iris").await.as_deref(), - Some(r#"{"permissions":["messaging"]}"#) + Some(r#"{"permissions":["messaging"],"unrestricted_internet":false}"#) ); - // Control: no `permissions` writes no key, so the default applies. - let _ = create("damocles", None).await.expect("created"); - assert_eq!(published(&client, "damocles").await, None); + // No `permissions` still writes the record: the explicit `false`. + let _ = create("damocles", None, None).await.expect("created"); + assert_eq!( + published(&client, "damocles").await.as_deref(), + Some(r#"{"unrestricted_internet":false}"#) + ); + + // The request can name the flag. + let _ = create("hermes", None, Some(true)).await.expect("created"); + assert_eq!( + published(&client, "hermes").await.as_deref(), + Some(r#"{"unrestricted_internet":true}"#) + ); } /// Destroying an agent deletes its key, so a new agent of the same name - /// created on the default gets the default, not the old grant. + /// created on the default gets only its own record, not the old grant. #[tokio::test] async fn a_recreated_agent_does_not_inherit_the_destroyed_ones_grant() { let nats = TestNats::start(); @@ -974,6 +1086,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -981,8 +1094,10 @@ mod tests { .await .expect("recreated"); - assert_eq!(published(&client, "atlas").await, None); - assert_eq!(get(&state, "atlas").await, Permissions::default()); + assert_eq!( + published(&client, "atlas").await.as_deref(), + Some(r#"{"unrestricted_internet":false}"#) + ); } #[tokio::test] diff --git a/swarm-controller/src/agents_md_initial.rs b/swarm-controller/src/agents_md_initial.rs index a0c70df8..54b584c0 100644 --- a/swarm-controller/src/agents_md_initial.rs +++ b/swarm-controller/src/agents_md_initial.rs @@ -148,6 +148,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md, } diff --git a/swarm-controller/src/jobq.rs b/swarm-controller/src/jobq.rs index 08f73662..47b103ea 100644 --- a/swarm-controller/src/jobq.rs +++ b/swarm-controller/src/jobq.rs @@ -1787,7 +1787,7 @@ pub(crate) fn queue_agent_renewals( #[cfg(test)] mod tests { use super::*; - use crate::routes::tests::state_with_roster; + use crate::routes::tests::state_with_roster_queued; pub(crate) fn changed(agent: &str, rev: &str, pr: u64) -> crate::config_pr::ChangedConfigPr { crate::config_pr::ChangedConfigPr { @@ -2219,7 +2219,7 @@ mod tests { async fn the_deploy_waits_for_the_pause_to_be_declared() { use hive_jobq_wire::WireNode as _; - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let _queued = crate::routes::agents::create_agent( axum::extract::State(state), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -2228,6 +2228,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -3031,7 +3032,7 @@ mod tests { async fn the_deploy_waits_for_the_mint_and_is_not_cancelled_by_it() { use hive_jobq_wire::WireNode as _; - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let _queued = crate::routes::agents::create_agent( axum::extract::State(state), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -3040,6 +3041,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -3083,7 +3085,7 @@ mod tests { async fn the_forge_token_mint_follows_the_user_and_does_not_block_the_deploy() { use hive_jobq_wire::WireNode as _; - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let _queued = crate::routes::agents::create_agent( axum::extract::State(state), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -3092,6 +3094,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -3143,7 +3146,7 @@ mod tests { async fn the_docs_grant_follows_the_user_and_is_not_a_deploy_parent() { use hive_jobq_wire::WireNode as _; - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let _queued = crate::routes::agents::create_agent( axum::extract::State(state), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -3152,6 +3155,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -3350,7 +3354,7 @@ mod tests { async fn the_matrix_account_mint_is_a_root_and_does_not_block_the_deploy() { use hive_jobq_wire::WireNode as _; - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let _queued = crate::routes::agents::create_agent( axum::extract::State(state), axum::Json(swarm_controller_api::agents::CreateAgentRequest { @@ -3359,6 +3363,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), diff --git a/swarm-controller/src/routes/agents.rs b/swarm-controller/src/routes/agents.rs index 2dfea217..7f9836fd 100644 --- a/swarm-controller/src/routes/agents.rs +++ b/swarm-controller/src/routes/agents.rs @@ -646,21 +646,16 @@ pub(crate) async fn create_agent( // reads these by agent name when it first runs the agent. A failure from // here on puts back what was stored, so a refused creation leaves no // settings behind for the name. - let stored = match &settings_queue { - Some(client) => { - StoredSettings::store( - client, - &agent, - settings.limits.as_ref(), - settings.inference.as_ref(), - settings.permissions.as_ref(), - settings.archetype.as_ref(), - settings.agents_md.as_deref(), - ) - .await? - } - None => StoredSettings::default(), - }; + let stored = StoredSettings::store( + &settings_queue, + &agent, + settings.limits.as_ref(), + settings.inference.as_ref(), + Some(&settings.permissions), + settings.archetype.as_ref(), + settings.agents_md.as_deref(), + ) + .await?; let queued = { let mut sched = state .jobq @@ -673,9 +668,7 @@ pub(crate) async fn create_agent( let ids = match queued { Ok(ids) => ids, Err(e) => { - if let Some(client) = &settings_queue { - stored.roll_back(client, &agent).await; - } + stored.roll_back(&settings_queue, &agent).await; return Err(error_problem( axum::http::StatusCode::INTERNAL_SERVER_ERROR, &e.to_string(), @@ -695,7 +688,7 @@ pub(crate) async fn create_agent( struct CreateSettings { limits: Option, inference: Option, - permissions: Option, + permissions: swarm_wire::agent_permissions::AgentPermissions, archetype: Option, agents_md: Option, } @@ -709,6 +702,7 @@ impl CreateSettings { limits, inference, permissions, + unrestricted_internet, archetype, agents_md, .. @@ -725,35 +719,24 @@ impl CreateSettings { Ok(Self { limits: limits.as_ref().map(agent_limits::into_stored).transpose()?, inference: inference.map(inference::into_stored).transpose()?, - permissions: permissions.map(agent_permissions::at_create).transpose()?, + permissions: agent_permissions::at_create(permissions, unrestricted_internet)?, archetype, agents_md, }) } - /// Whether any setting is given. - fn any(&self) -> bool { - self.limits.is_some() - || self.inference.is_some() - || self.permissions.is_some() - || self.archetype.is_some() - || self.agents_md.is_some() - } - - /// The queue to store the settings through, `None` when none is given. - /// An archetype is refused here unless the stored list holds it. + /// The queue to store the settings through: the permissions are always + /// stored, so it is always needed. An archetype is refused here unless the + /// stored list holds it. async fn queue( &self, state: &AppState, - ) -> Result, problem_details::ProblemDetails> { - if !self.any() { - return Ok(None); - } + ) -> Result { let client = schedules::queue(state).map_err(schedules::refused)?; if let Some(archetype) = &self.archetype { agent_archetype::check(&client, archetype).await?; } - Ok(Some(client)) + Ok(client) } } @@ -1159,7 +1142,7 @@ pub(crate) async fn get_agent_icon( #[cfg(test)] mod tests { use super::*; - use crate::routes::tests::{SharedSched, state_with_roster}; + use crate::routes::tests::{SharedSched, state_with_roster, state_with_roster_queued}; /// A swarm with no identity bridge cannot answer the roster, and must /// say so rather than answer `[]`. @@ -1244,6 +1227,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -1273,7 +1257,7 @@ mod tests { /// could equally mean the endpoint refuses everything. #[tokio::test] async fn a_hive_in_the_roster_is_accepted_and_queues_the_graph() { - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; // The response is bound rather than asserted on: node ids start at // zero, so every property I reached for first ("> 0") was a claim @@ -1287,6 +1271,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -1335,7 +1320,7 @@ mod tests { (r#"{"model":"-opus"}"#, "inference"), ]; for (value, field) in bad { - let (state, sched) = state_with_roster(); + let (_nats, state, sched) = state_with_roster_queued().await; let body = format!(r#"{{"name":"atlas","hive":"pr1ma","{field}":{value}}}"#); let req: CreateAgentRequest = serde_json::from_str(&body).expect("body parses"); let err = super::create_agent(axum::extract::State(state), axum::Json(req)) @@ -1492,8 +1477,11 @@ mod tests { .await .expect("a bucket that refuses any value"); let state = crate::agent_limits::tests::state(&client); - let earlier = crate::agent_permissions::at_create(vec![hive_types::Permission::Messaging]) - .expect("valid"); + let earlier = crate::agent_permissions::at_create( + Some(vec![hive_types::Permission::Messaging]), + None, + ) + .expect("valid"); crate::agent_permissions::store(&client, "atlas", &earlier) .await .expect("an earlier grant"); @@ -1526,7 +1514,7 @@ mod tests { |value: Option| value.map(|v| String::from_utf8_lossy(&v).into_owned()); assert_eq!( held(bucket.get("atlas").await.expect("read")).as_deref(), - Some(r#"{"permissions":["messaging"]}"#), + Some(r#"{"permissions":["messaging"],"unrestricted_internet":false}"#), "the earlier grant comes back" ); assert_eq!( @@ -1564,8 +1552,12 @@ mod tests { } /// `state_with_roster` with a second hive, `sec0nd`. - fn state_with_two_hives() -> (crate::routes::AppState, SharedSched) { - let (state, sched) = state_with_roster(); + async fn state_with_two_hives() -> ( + crate::test_nats::TestNats, + crate::routes::AppState, + SharedSched, + ) { + let (nats, state, sched) = state_with_roster_queued().await; let mut hives = (*state.hives).clone(); hives.push(HiveEntry { name: "sec0nd".to_owned(), @@ -1576,7 +1568,7 @@ mod tests { hives: std::sync::Arc::new(hives), ..state }; - (state, sched) + (nats, state, sched) } pub(crate) async fn create( @@ -1592,6 +1584,7 @@ mod tests { limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: None, }), @@ -1614,7 +1607,7 @@ mod tests { /// wanted-state read alone would miss. #[tokio::test] async fn a_name_placed_on_another_hive_is_refused_before_anything_is_queued() { - let (state, sched) = state_with_two_hives(); + let (_nats, state, sched) = state_with_two_hives().await; create(&state, "atlas", "pr1ma") .await .expect("the first creation of a name must be accepted"); @@ -1643,7 +1636,7 @@ mod tests { /// that agent being re-created, and queues again. #[tokio::test] async fn re_creating_an_agent_on_its_own_hive_is_accepted() { - let (state, sched) = state_with_two_hives(); + let (_nats, state, sched) = state_with_two_hives().await; create(&state, "atlas", "pr1ma") .await .expect("the first creation of a name must be accepted"); @@ -1659,7 +1652,7 @@ mod tests { /// "placed nowhere", and queues nothing. #[tokio::test] async fn an_unreadable_wanted_state_refuses_creation() { - let (state, sched) = state_with_two_hives(); + let (_nats, state, sched) = state_with_two_hives().await; let state = crate::routes::AppState { wanted: Some(std::sync::Arc::new(wanted::WantedWriter::new( disconnected_client().await, @@ -2098,7 +2091,7 @@ mod tests { agent: &str, wanted_state: swarm_wire::wanted::AgentState, ) -> problem_details::ProblemDetails { - let (state, _sched) = state_with_two_hives(); + let (_nats, state, _sched) = state_with_two_hives().await; let state = crate::routes::AppState { wanted: Some(std::sync::Arc::new(wanted::WantedWriter::new( disconnected_client().await, diff --git a/swarm-controller/src/routes/mod.rs b/swarm-controller/src/routes/mod.rs index 47e55b74..003d677b 100644 --- a/swarm-controller/src/routes/mod.rs +++ b/swarm-controller/src/routes/mod.rs @@ -315,6 +315,21 @@ pub(crate) mod tests { (state, sched) } + /// [`state_with_roster`] with a connected queue, for creations: they + /// store the agent's permissions through it. The server lives as long as + /// the first element. + pub(crate) async fn state_with_roster_queued() + -> (crate::test_nats::TestNats, super::AppState, SharedSched) { + let nats = crate::test_nats::TestNats::start(); + let client = nats.client().await; + let (mut state, sched) = state_with_roster(); + state.status = Some(std::sync::Arc::new(crate::status::StatusReader::new( + client, + std::time::Duration::from_mins(1), + ))); + (nats, state, sched) + } + /// `wanted_error_status` as a pure function first: build the exact /// `swarm_queue_client::Error::NotConnected` `ensure_connected` raises, /// wrap it the same way `?` inside `WantedWriter::view`/`set` does, and diff --git a/swarm-queue-client/src/agent_permissions.rs b/swarm-queue-client/src/agent_permissions.rs index 51e60224..73aca48b 100644 --- a/swarm-queue-client/src/agent_permissions.rs +++ b/swarm-queue-client/src/agent_permissions.rs @@ -50,6 +50,15 @@ pub fn held(record: &AgentPermissions) -> Vec { .unwrap_or_else(|| hive_types::Permission::DEFAULT.to_vec()) } +/// Whether `record` lets the agent's traffic leave unfiltered: its flag when +/// set, **on** when absent. A record written before the flag existed, or a +/// missing key, is an agent that has always had open access. +#[cfg(feature = "agent-permissions")] +#[must_use] +pub fn unrestricted(record: &AgentPermissions) -> bool { + record.unrestricted_internet.unwrap_or(true) +} + /// Read `agent`'s current permissions with one direct get: the one subject /// an agent's own grant covers, and one of those a hive's covers. /// @@ -129,6 +138,35 @@ mod tests { ); } + /// An absent flag reads on, and so does a missing or deleted key; only an + /// explicit `false` reads off. + #[test] + fn an_absent_internet_flag_reads_on() { + let record = |json: &str| -> AgentPermissions { serde_json::from_str(json).unwrap() }; + assert!(unrestricted(&record("{}"))); + assert!(unrestricted(&record(r#"{"permissions":[]}"#))); + assert!(unrestricted(&record(r#"{"unrestricted_internet":true}"#))); + assert!(!unrestricted(&record(r#"{"unrestricted_internet":false}"#))); + assert!(unrestricted(&decode(Some("DEL"), b"").unwrap())); + } + + /// A decoder built before the flag existed has no such field and ignores + /// unknown ones; it reads a record carrying the flag. + #[test] + fn a_decoder_without_the_flag_tolerates_it() { + #[derive(serde::Deserialize)] + struct Old { + permissions: Option>, + } + let old: Old = + serde_json::from_str(r#"{"permissions":["web_tools"],"unrestricted_internet":false}"#) + .unwrap(); + assert_eq!( + old.permissions, + Some(vec![hive_types::Permission::WebTools]) + ); + } + /// Fields a newer controller adds must not cost an older hive the ones it /// knows; a deleted key is unset. #[cfg(feature = "agent-permissions")] diff --git a/swarm-wire/src/agent_permissions.rs b/swarm-wire/src/agent_permissions.rs index acafb0d7..7ba46fde 100644 --- a/swarm-wire/src/agent_permissions.rs +++ b/swarm-wire/src/agent_permissions.rs @@ -71,6 +71,12 @@ pub const IMPORT_REPLY_DEADLINE: std::time::Duration = std::time::Duration::from pub struct AgentPermissions { #[serde(default, skip_serializing_if = "Option::is_none")] pub permissions: Option>, + /// Whether the agent's traffic leaves the host unfiltered. Absent is "on": + /// an agent whose key predates the field keeps the open access it had, and + /// the controller writes an explicit `false` when it creates an agent. Read + /// it through `swarm_queue_client::agent_permissions::unrestricted`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unrestricted_internet: Option, } /// What a hive sends once: the permissions its meta repo held, by agent. @@ -140,4 +146,19 @@ mod tests { serde_json::from_str(r#"{"permissions":["manage_root_agent"]}"#).unwrap(); assert_eq!(ruth.permissions, Some(vec![Permission::ManageRootAgent])); } + + /// The flag is left out when unset, so a record that never named it + /// encodes as before, and an explicit `false` is kept on the wire. + #[test] + fn the_internet_flag_is_left_out_unless_set() { + let off = AgentPermissions { + unrestricted_internet: Some(false), + ..AgentPermissions::default() + }; + let wire = r#"{"unrestricted_internet":false}"#; + assert_eq!(serde_json::to_string(&off).unwrap(), wire); + assert_eq!(serde_json::from_str::(wire).unwrap(), off); + let unset: AgentPermissions = serde_json::from_str("{}").unwrap(); + assert_eq!(unset.unrestricted_internet, None); + } } diff --git a/swarmctl/src/agent.rs b/swarmctl/src/agent.rs index d941e151..ee3337c0 100644 --- a/swarmctl/src/agent.rs +++ b/swarmctl/src/agent.rs @@ -128,6 +128,7 @@ async fn post_create( limits: None, inference: None, permissions: None, + unrestricted_internet: None, archetype: None, agents_md: agents_md.map(str::to_owned), }, diff --git a/swarmctl/src/main.rs b/swarmctl/src/main.rs index 5f12945d..1561c1a4 100644 --- a/swarmctl/src/main.rs +++ b/swarmctl/src/main.rs @@ -346,7 +346,8 @@ struct AgentsMdApplyArgs { enum PermissionsVerb { /// Show the permissions the swarm grants an agent, or that it sets none. Get(InferenceAgentArgs), - /// Replace the permissions the swarm grants an agent. + /// Replace the permissions the swarm grants an agent; the internet flag + /// stays unless given. Set(PermissionsSetArgs), /// Clear the agent's permissions, so the default applies again. Clear(InferenceAgentArgs), @@ -366,6 +367,10 @@ struct PermissionsSetArgs { required = true )] permissions: Vec, + /// Whether the agent's traffic may leave the host unfiltered. Left out, + /// the stored value stays. + #[arg(long, value_name = "BOOL")] + unrestricted_internet: Option, #[command(flatten)] socket: ControllerSocketArg, } @@ -765,9 +770,12 @@ fn inference(remote: &RemoteArgs, command: InferenceVerb) -> Result<()> { fn permissions(remote: &RemoteArgs, command: PermissionsVerb) -> Result<()> { match command { PermissionsVerb::Get(a) => permissions::get(&a.socket.resolve(remote)?, &a.agent), - PermissionsVerb::Set(a) => { - permissions::set(&a.socket.resolve(remote)?, &a.agent, &a.permissions) - } + PermissionsVerb::Set(a) => permissions::set( + &a.socket.resolve(remote)?, + &a.agent, + &a.permissions, + a.unrestricted_internet, + ), PermissionsVerb::Clear(a) => permissions::clear(&a.socket.resolve(remote)?, &a.agent), } } diff --git a/swarmctl/src/permissions.rs b/swarmctl/src/permissions.rs index ea39c37c..28686f0c 100644 --- a/swarmctl/src/permissions.rs +++ b/swarmctl/src/permissions.rs @@ -45,9 +45,14 @@ pub(crate) fn get(target: &Target, agent: &str) -> Result<()> { /// Run `swarmctl agent permissions set`: store the given permissions, which /// replace the agent's record whole. -pub(crate) fn set(target: &Target, agent: &str, permissions: &[String]) -> Result<()> { +pub(crate) fn set( + target: &Target, + agent: &str, + permissions: &[String], + unrestricted_internet: Option, +) -> Result<()> { let uri = member(agent)?; - let wanted = with_permissions(permissions)?; + let wanted = with_permissions(permissions, unrestricted_internet)?; let stored: Permissions = runtime()?.block_on(send( target, hyper::Method::PUT, @@ -61,7 +66,10 @@ pub(crate) fn set(target: &Target, agent: &str, permissions: &[String]) -> Resul /// The record granting `permissions`. An empty name, as `--permissions ''` /// gives, is dropped, so that spelling grants nothing. -fn with_permissions(permissions: &[String]) -> Result { +fn with_permissions( + permissions: &[String], + unrestricted_internet: Option, +) -> Result { let granted = permissions .iter() .filter(|name| !name.is_empty()) @@ -69,6 +77,7 @@ fn with_permissions(permissions: &[String]) -> Result { .collect::>>()?; Ok(Permissions { permissions: Some(granted), + unrestricted_internet, }) } @@ -112,7 +121,12 @@ fn render(p: &Permissions) -> String { .collect::>() .join(", "), }; - format!("permissions: {caps}\n") + let internet = match p.unrestricted_internet { + None => "(not set; unrestricted)", + Some(true) => "unrestricted", + Some(false) => "restricted", + }; + format!("permissions: {caps}\nunrestricted_internet: {internet}\n") } #[cfg(test)] @@ -122,18 +136,48 @@ mod tests { #[test] fn get_tells_unset_from_none() { let unset: Permissions = serde_json::from_str("{}").unwrap(); - assert_eq!(render(&unset), "permissions: (not set; the default)\n"); + assert_eq!( + render(&unset), + "permissions: (not set; the default)\nunrestricted_internet: (not set; unrestricted)\n" + ); let none: Permissions = serde_json::from_str(r#"{"permissions":[]}"#).unwrap(); - assert_eq!(render(&none), "permissions: (none)\n"); + assert_eq!( + render(&none), + "permissions: (none)\nunrestricted_internet: (not set; unrestricted)\n" + ); let ruth: Permissions = serde_json::from_str(r#"{"permissions":["manage_root_agent"]}"#).unwrap(); - assert_eq!(render(&ruth), "permissions: manage_root_agent\n"); + assert_eq!( + render(&ruth), + "permissions: manage_root_agent\nunrestricted_internet: (not set; unrestricted)\n" + ); + } + + /// An explicit flag is shown and sent; left out it is not sent, so a `PUT` + /// keeps the stored one. + #[test] + fn the_internet_flag_is_sent_only_when_given() { + let off = with_permissions(&[], Some(false)).unwrap(); + assert_eq!( + serde_json::to_string(&off).unwrap(), + r#"{"permissions":[],"unrestricted_internet":false}"# + ); + assert_eq!( + render(&off), + "permissions: (none)\nunrestricted_internet: restricted\n" + ); + let left_out = with_permissions(&[], None).unwrap(); + assert!( + !serde_json::to_string(&left_out) + .unwrap() + .contains("internet") + ); } /// `--permissions ''` stores an empty list, not one empty name. #[test] fn an_empty_list_flag_grants_nothing() { - let set = with_permissions(&[String::new()]).unwrap(); + let set = with_permissions(&[String::new()], None).unwrap(); assert_eq!( serde_json::to_string(&set).unwrap(), r#"{"permissions":[]}"# @@ -143,11 +187,11 @@ mod tests { /// A name that is not a permission fails locally and lists the valid ones. #[test] fn an_unknown_permission_is_refused() { - let err = with_permissions(&["messaging".to_owned(), "fly".to_owned()]).unwrap_err(); + let err = with_permissions(&["messaging".to_owned(), "fly".to_owned()], None).unwrap_err(); let text = err.to_string(); assert!(text.contains("invalid permission \"fly\""), "{text}"); assert!(text.contains("manage_root_agent"), "{text}"); - let ok = with_permissions(&["web_tools".to_owned()]).unwrap(); + let ok = with_permissions(&["web_tools".to_owned()], None).unwrap(); assert_eq!( serde_json::to_string(&ok).unwrap(), r#"{"permissions":["web_tools"]}"#