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
This commit is contained in:
parent
d5de1e4943
commit
693e6e738b
5 changed files with 104 additions and 3 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -4859,6 +4859,7 @@ dependencies = [
|
||||||
"async-nats",
|
"async-nats",
|
||||||
"futures-util",
|
"futures-util",
|
||||||
"hive-log",
|
"hive-log",
|
||||||
|
"hive-types",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
"swarm-queue-client",
|
"swarm-queue-client",
|
||||||
"swarm-secret-client",
|
"swarm-secret-client",
|
||||||
|
|
|
||||||
|
|
@ -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
|
hive's policy reads `swarm/agents/*`, so every hive can read every agent's
|
||||||
claude session, as it can read the GitHub token.
|
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
|
## What stays
|
||||||
|
|
||||||
The agent's own web UI login (`/login`) still works, unchanged. It goes away
|
The agent's own web UI login (`/login`) still works, unchanged. It goes away
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,8 @@ futures-util.workspace = true
|
||||||
hive-log.workspace = true
|
hive-log.workspace = true
|
||||||
serde_json.workspace = true
|
serde_json.workspace = true
|
||||||
# The login subject and the shapes the controller sends and reads back.
|
# 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
|
swarm-queue-client.workspace = true
|
||||||
# The credentials and queue client secret paths, and the store login.
|
# The credentials and queue client secret paths, and the store login.
|
||||||
swarm-secret-client.workspace = true
|
swarm-secret-client.workspace = true
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,9 @@ const CREDENTIALS_FILE: &str = ".credentials.json";
|
||||||
const TAIL_LINES: usize = 20;
|
const TAIL_LINES: usize = 20;
|
||||||
|
|
||||||
/// The variables naming this service's own store identity, kept out of the
|
/// 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] = [
|
const STRIPPED_ENV: [&str; 4] = [
|
||||||
"BAO_ADDR",
|
"BAO_ADDR",
|
||||||
"BAO_CLIENT_CERT",
|
"BAO_CLIENT_CERT",
|
||||||
|
|
@ -352,7 +354,7 @@ fn failed(why: &str, session: Option<&Session>) -> Reply {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
pub(crate) mod tests {
|
||||||
use std::os::unix::fs::PermissionsExt as _;
|
use std::os::unix::fs::PermissionsExt as _;
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
@ -390,7 +392,7 @@ echo "Login successful."
|
||||||
/// Every stub, written once before any test spawns one: a file still
|
/// Every stub, written once before any test spawns one: a file still
|
||||||
/// open for writing in a child another test forked meanwhile cannot be
|
/// open for writing in a child another test forked meanwhile cannot be
|
||||||
/// executed (`ETXTBSY`).
|
/// executed (`ETXTBSY`).
|
||||||
fn stubs() -> &'static Path {
|
pub(crate) fn stubs() -> &'static Path {
|
||||||
static DIR: std::sync::OnceLock<PathBuf> = std::sync::OnceLock::new();
|
static DIR: std::sync::OnceLock<PathBuf> = std::sync::OnceLock::new();
|
||||||
DIR.get_or_init(|| {
|
DIR.get_or_init(|| {
|
||||||
let dir = std::env::temp_dir().join(format!(
|
let dir = std::env::temp_dir().join(format!(
|
||||||
|
|
|
||||||
|
|
@ -124,6 +124,16 @@ async fn handle(payload: &[u8], logins: &Arc<login::Logins>) -> 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 {
|
let (agent, reply) = match request {
|
||||||
Request::Start { agent, operator } => {
|
Request::Start { agent, operator } => {
|
||||||
tracing::info!(%agent, %operator, "starting a claude login");
|
tracing::info!(%agent, %operator, "starting a claude login");
|
||||||
|
|
@ -190,3 +200,73 @@ async fn client_secret(client_id: &str) -> Result<String> {
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue