From 693e6e738bdcc5b0e1edd3c75f0baab13237f284 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 14 Oct 2026 23:11:09 +0200 Subject: [PATCH] swarm-claude-login: refuse a step whose agent name is not an identifier The minter parses `Request.agent` with `Ident::parse`, as the controller does, before it makes a scratch directory, runs `claude auth login` or touches the store, and answers `Failed` otherwise. The name becomes a directory name and a store path in the one service that writes every agent's credentials, so it is checked at this service's own boundary. The doc states that the `operatorsOnly` authelia rule, not the controller, keeps the routes to operators, and that a stale version record waits for a relogin. The env-strip comment says it is defence in depth. Refs #5268 --- Cargo.lock | 1 + docs/swarm/claude-login.md | 16 +++++++ swarm-claude-login/Cargo.toml | 2 + swarm-claude-login/src/login.rs | 8 ++-- swarm-claude-login/src/main.rs | 80 +++++++++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2308ca36..d4df3417 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4859,6 +4859,7 @@ dependencies = [ "async-nats", "futures-util", "hive-log", + "hive-types", "serde_json", "swarm-queue-client", "swarm-secret-client", diff --git a/docs/swarm/claude-login.md b/docs/swarm/claude-login.md index 10fa3d42..18b06f92 100644 --- a/docs/swarm/claude-login.md +++ b/docs/swarm/claude-login.md @@ -60,6 +60,22 @@ The minter has no read on the leaf and nothing else under an agent. Every hive's policy reads `swarm/agents/*`, so every hive can read every agent's claude session, as it can read the GitHub token. +The `operatorsOnly` authelia rule on the swarm domain, not the controller, +keeps these routes to operators: the controller checks only that the gateway +named a user. Without that rule any account authelia admits could store its +own claude session as any agent's, so no agent account may exist before the +rule reaches the swarm. + +The minter refuses a step whose agent name isn't an identifier before it +makes a directory or runs `claude`. + +## Caveat + +The installed-version record lives under the harness dir and the credentials +file under `~/.claude`. If only the record survives a rebuild, the agent +ignores the stored version until the operator logs it in again, which writes a +newer one. + ## What stays The agent's own web UI login (`/login`) still works, unchanged. It goes away diff --git a/swarm-claude-login/Cargo.toml b/swarm-claude-login/Cargo.toml index d2e73949..68238712 100644 --- a/swarm-claude-login/Cargo.toml +++ b/swarm-claude-login/Cargo.toml @@ -16,6 +16,8 @@ futures-util.workspace = true hive-log.workspace = true serde_json.workspace = true # The login subject and the shapes the controller sends and reads back. +# The agent name a request carries is parsed as the controller parses it. +hive-types.workspace = true swarm-queue-client.workspace = true # The credentials and queue client secret paths, and the store login. swarm-secret-client.workspace = true diff --git a/swarm-claude-login/src/login.rs b/swarm-claude-login/src/login.rs index 92adca08..17246354 100644 --- a/swarm-claude-login/src/login.rs +++ b/swarm-claude-login/src/login.rs @@ -27,7 +27,9 @@ const CREDENTIALS_FILE: &str = ".credentials.json"; const TAIL_LINES: usize = 20; /// The variables naming this service's own store identity, kept out of the -/// login's environment. +/// login's environment. Defence in depth only, not isolation: `claude` runs +/// as this unit's uid, and `CREDENTIALS_DIRECTORY` still names the key that +/// writes every agent's leaf. const STRIPPED_ENV: [&str; 4] = [ "BAO_ADDR", "BAO_CLIENT_CERT", @@ -352,7 +354,7 @@ fn failed(why: &str, session: Option<&Session>) -> Reply { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use std::os::unix::fs::PermissionsExt as _; use super::*; @@ -390,7 +392,7 @@ echo "Login successful." /// Every stub, written once before any test spawns one: a file still /// open for writing in a child another test forked meanwhile cannot be /// executed (`ETXTBSY`). - fn stubs() -> &'static Path { + pub(crate) fn stubs() -> &'static Path { static DIR: std::sync::OnceLock = std::sync::OnceLock::new(); DIR.get_or_init(|| { let dir = std::env::temp_dir().join(format!( diff --git a/swarm-claude-login/src/main.rs b/swarm-claude-login/src/main.rs index 99af5729..114e15bf 100644 --- a/swarm-claude-login/src/main.rs +++ b/swarm-claude-login/src/main.rs @@ -124,6 +124,16 @@ async fn handle(payload: &[u8], logins: &Arc) -> Reply { }; } }; + // Before anything else: the name becomes a scratch directory's name and + // a store path, and this service holds write on every agent's leaf. + let (Request::Start { agent, .. } | Request::Code { agent, .. } | Request::Cancel { agent }) = + &request; + if let Err(reason) = hive_types::Ident::parse(agent) { + tracing::warn!(agent = ?agent, reason, "a login step for an invalid agent name; refused"); + return Reply::Failed { + error: format!("{agent:?} is not an agent name: {reason}"), + }; + } let (agent, reply) = match request { Request::Start { agent, operator } => { tracing::info!(%agent, %operator, "starting a claude login"); @@ -190,3 +200,73 @@ async fn client_secret(client_id: &str) -> Result { ), } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + use std::time::Duration; + + use swarm_queue_client::claude_login::{Reply, Request}; + + use super::{handle, login}; + + #[tokio::test] + async fn a_step_for_an_invalid_agent_name_is_refused_before_anything_is_made() { + let root = std::env::temp_dir().join(format!( + "swarm-claude-login-test-hostile-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&root); + let scratch = root.join("scratch"); + std::fs::create_dir_all(&scratch).expect("temp dir"); + let logins = Arc::new(login::Logins::new( + login::tests::stubs().join("claude"), + scratch.clone(), + Duration::from_secs(10), + )); + for agent in ["../escaped", "..", "", "a/b", "Atlas"] { + for request in [ + Request::Start { + agent: agent.to_owned(), + operator: "op".to_owned(), + }, + Request::Code { + agent: agent.to_owned(), + code: "c".to_owned(), + }, + Request::Cancel { + agent: agent.to_owned(), + }, + ] { + let payload = serde_json::to_vec(&request).expect("encodes"); + let reply = handle(&payload, &logins).await; + assert!( + matches!(reply, Reply::Failed { .. }), + "{agent:?}: {reply:?}" + ); + } + let made: Vec<_> = std::fs::read_dir(&root) + .expect("root") + .map(|e| e.expect("entry").file_name()) + .collect(); + assert_eq!(made, vec!["scratch"], "{agent:?} made a directory"); + assert_eq!( + std::fs::read_dir(&scratch).expect("scratch").count(), + 0, + "{agent:?}" + ); + } + // The control: a valid name starts. + let payload = serde_json::to_vec(&Request::Start { + agent: "atlas".to_owned(), + operator: "op".to_owned(), + }) + .expect("encodes"); + assert!(matches!( + handle(&payload, &logins).await, + Reply::Started { .. } + )); + drop(logins); + let _ = std::fs::remove_dir_all(&root); + } +}