swarm-review-classifier: claude's environment is an allowlist, not the daemon's
hive-claude adds its variables on top of the parent's environment, so claude inherited the daemon's store identity paths, CREDENTIALS_DIRECTORY and queue coordinates. claude now starts as this same binary with SWARM_REVIEW_CLASSIFIER_EXEC_CLAUDE naming the real one; main execs it before the runtime starts, with the environment cleared down to the API key, PATH, HOME, CLAUDE_CONFIG_DIR and claude's telemetry settings. The exec keeps the pid, so hive-claude's cancel and idle timeout still reach it. An integration test runs the built binary as the trampoline and checks that a BAO_* variable does not reach the program it execs.
This commit is contained in:
parent
8e33de6fb7
commit
af1495731e
4 changed files with 176 additions and 21 deletions
85
swarm-review-classifier/src/child_env.rs
Normal file
85
swarm-review-classifier/src/child_env.rs
Normal file
|
|
@ -0,0 +1,85 @@
|
||||||
|
//! The `claude` child's environment: an allowlist, not the daemon's.
|
||||||
|
//!
|
||||||
|
//! `hive-claude` sets its variables on top of the parent's environment, and
|
||||||
|
//! the daemon's holds the store identity paths and the queue coordinates. So
|
||||||
|
//! `claude` is spawned as this same binary with [`EXEC_CLAUDE`] naming the
|
||||||
|
//! real one: [`exec_if_requested`] runs first in `main`, clears the
|
||||||
|
//! environment down to [`ALLOWED`] and execs `claude` with the same
|
||||||
|
//! arguments, in the same process.
|
||||||
|
|
||||||
|
use std::ffi::OsString;
|
||||||
|
use std::os::unix::process::CommandExt as _;
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
|
/// Set on the child only: the `claude` binary to exec.
|
||||||
|
pub const EXEC_CLAUDE: &str = "SWARM_REVIEW_CLASSIFIER_EXEC_CLAUDE";
|
||||||
|
|
||||||
|
/// Every variable `claude` keeps: the key, what it needs to run, and its
|
||||||
|
/// telemetry settings.
|
||||||
|
const ALLOWED: &[&str] = &[
|
||||||
|
"ANTHROPIC_API_KEY",
|
||||||
|
"PATH",
|
||||||
|
"HOME",
|
||||||
|
"CLAUDE_CONFIG_DIR",
|
||||||
|
"CLAUDE_CODE_ENABLE_TELEMETRY",
|
||||||
|
"OTEL_METRICS_EXPORTER",
|
||||||
|
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||||
|
"OTEL_EXPORTER_OTLP_PROTOCOL",
|
||||||
|
"OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE",
|
||||||
|
"OTEL_RESOURCE_ATTRIBUTES",
|
||||||
|
];
|
||||||
|
|
||||||
|
/// The variables of `vars` on [`ALLOWED`].
|
||||||
|
pub fn allowed(vars: impl IntoIterator<Item = (OsString, OsString)>) -> Vec<(OsString, OsString)> {
|
||||||
|
vars.into_iter()
|
||||||
|
.filter(|(name, _)| ALLOWED.iter().any(|a| name == a))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// When this process is the trampoline, exec `claude` and never return.
|
||||||
|
pub fn exec_if_requested() {
|
||||||
|
let Some(claude) = std::env::var_os(EXEC_CLAUDE) else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let error = Command::new(&claude)
|
||||||
|
.args(std::env::args_os().skip(1))
|
||||||
|
.env_clear()
|
||||||
|
.envs(allowed(std::env::vars_os()))
|
||||||
|
.exec();
|
||||||
|
eprintln!("exec {}: {error}", claude.to_string_lossy());
|
||||||
|
std::process::exit(127);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn only_the_allowlist_reaches_claude() {
|
||||||
|
let daemon = [
|
||||||
|
("ANTHROPIC_API_KEY", "k"),
|
||||||
|
("HOME", "/tmp/run"),
|
||||||
|
("OTEL_RESOURCE_ATTRIBUTES", "repo=o/r,pr=7"),
|
||||||
|
(
|
||||||
|
"BAO_CLIENT_KEY",
|
||||||
|
"/run/credentials/x/review-classifier-bao-key",
|
||||||
|
),
|
||||||
|
("BAO_ADDR", "https://bao.example:8200"),
|
||||||
|
("CREDENTIALS_DIRECTORY", "/run/credentials/x"),
|
||||||
|
(
|
||||||
|
"SWARM_REVIEW_CLASSIFIER_NATS_URL",
|
||||||
|
"tls://nats.example:4222",
|
||||||
|
),
|
||||||
|
(EXEC_CLAUDE, "/bin/claude"),
|
||||||
|
]
|
||||||
|
.map(|(n, v)| (OsString::from(n), OsString::from(v)));
|
||||||
|
let kept: Vec<String> = allowed(daemon)
|
||||||
|
.into_iter()
|
||||||
|
.map(|(n, _)| n.to_string_lossy().into_owned())
|
||||||
|
.collect();
|
||||||
|
assert_eq!(
|
||||||
|
kept,
|
||||||
|
["ANTHROPIC_API_KEY", "HOME", "OTEL_RESOURCE_ATTRIBUTES"]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -20,6 +20,7 @@ use swarm_queue_client::{ClientSecret, QueueConfig};
|
||||||
|
|
||||||
mod aggregate;
|
mod aggregate;
|
||||||
mod answer;
|
mod answer;
|
||||||
|
mod child_env;
|
||||||
mod prompt;
|
mod prompt;
|
||||||
mod run;
|
mod run;
|
||||||
|
|
||||||
|
|
@ -34,8 +35,17 @@ const PREFIX: &str = "SWARM_REVIEW_CLASSIFIER";
|
||||||
/// longer outage fails the start, and the unit's `Restart=` tries again.
|
/// longer outage fails the start, and the unit's `Restart=` tries again.
|
||||||
const RETRY_DELAYS_S: [u64; 6] = [1, 2, 4, 8, 16, 30];
|
const RETRY_DELAYS_S: [u64; 6] = [1, 2, 4, 8, 16, 30];
|
||||||
|
|
||||||
#[tokio::main]
|
fn main() -> Result<()> {
|
||||||
async fn main() -> Result<()> {
|
// Before anything else: as the trampoline, this process becomes `claude`.
|
||||||
|
child_env::exec_if_requested();
|
||||||
|
tokio::runtime::Builder::new_multi_thread()
|
||||||
|
.enable_all()
|
||||||
|
.build()
|
||||||
|
.context("starting the async runtime")?
|
||||||
|
.block_on(serve())
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn serve() -> Result<()> {
|
||||||
hive_log::init();
|
hive_log::init();
|
||||||
swarm_queue_client::install_crypto_provider();
|
swarm_queue_client::install_crypto_provider();
|
||||||
let var = |name: &str| {
|
let var = |name: &str| {
|
||||||
|
|
@ -45,6 +55,8 @@ async fn main() -> Result<()> {
|
||||||
let client_id = var("OIDC_CLIENT_ID")?;
|
let client_id = var("OIDC_CLIENT_ID")?;
|
||||||
let claude = run::Claude {
|
let claude = run::Claude {
|
||||||
program: PathBuf::from(var("CLAUDE")?),
|
program: PathBuf::from(var("CLAUDE")?),
|
||||||
|
trampoline: std::env::current_exe()
|
||||||
|
.context("finding this binary for the claude trampoline")?,
|
||||||
};
|
};
|
||||||
let secret = client_secret_with_retry(&client_id).await?;
|
let secret = client_secret_with_retry(&client_id).await?;
|
||||||
let queue = swarm_queue_client::connect(QueueConfig {
|
let queue = swarm_queue_client::connect(QueueConfig {
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,8 @@
|
||||||
//! Each classification is one `claude --print` with no session, no tools and
|
//! Each classification is one `claude --print` with no session, no tools and
|
||||||
//! no MCP server, in a directory made for it and removed after it, so nothing
|
//! no MCP server, in a directory made for it and removed after it, so nothing
|
||||||
//! carries from one run to the next. The API key is read from the store for
|
//! carries from one run to the next. The API key is read from the store for
|
||||||
//! every run and reaches only the child's environment.
|
//! every run and reaches only the child's environment, which keeps nothing of
|
||||||
|
//! the daemon's beyond [`crate::child_env`]'s allowlist.
|
||||||
|
|
||||||
use std::future::Future;
|
use std::future::Future;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
|
|
@ -113,9 +114,13 @@ impl KeySource for Store {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The `claude` binary the classifications run on.
|
/// The `claude` binary the classifications run on, started through the
|
||||||
|
/// trampoline that clears its environment.
|
||||||
pub struct Claude {
|
pub struct Claude {
|
||||||
pub program: PathBuf,
|
pub program: PathBuf,
|
||||||
|
/// This binary, which execs [`Self::program`] when started as the
|
||||||
|
/// trampoline.
|
||||||
|
pub trampoline: PathBuf,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Model for Claude {
|
impl Model for Claude {
|
||||||
|
|
@ -142,23 +147,7 @@ impl Claude {
|
||||||
.await
|
.await
|
||||||
.and(tokio::fs::write(&system, prompt::SYSTEM).await)
|
.and(tokio::fs::write(&system, prompt::SYSTEM).await)
|
||||||
.map_err(|e| format!("preparing the run's directory: {e}"))?;
|
.map_err(|e| format!("preparing the run's directory: {e}"))?;
|
||||||
let config = Config {
|
let config = self.config(dir, system, api_key, attributes);
|
||||||
model: Some(MODEL.to_owned()),
|
|
||||||
cwd: Some(dir.to_owned()),
|
|
||||||
system_prompt_file: Some(system),
|
|
||||||
strict_mcp_config: true,
|
|
||||||
// `--tools ""`: no built-in tool at all.
|
|
||||||
tools: Some(String::new()),
|
|
||||||
program: Some(self.program.clone()),
|
|
||||||
idle_timeout: Some(IDLE_TIMEOUT),
|
|
||||||
claude_config_dir: Some(dir.join("config")),
|
|
||||||
env: vec![
|
|
||||||
("ANTHROPIC_API_KEY".to_owned(), api_key.to_owned()),
|
|
||||||
("HOME".to_owned(), dir.display().to_string()),
|
|
||||||
("OTEL_RESOURCE_ATTRIBUTES".to_owned(), attributes.to_owned()),
|
|
||||||
],
|
|
||||||
..Config::default()
|
|
||||||
};
|
|
||||||
let running =
|
let running =
|
||||||
hive_claude::Claude::spawn(&config, &Attach::OneOff).map_err(|e| e.to_string())?;
|
hive_claude::Claude::spawn(&config, &Attach::OneOff).map_err(|e| e.to_string())?;
|
||||||
let cancel = running.cancel_handle();
|
let cancel = running.cancel_handle();
|
||||||
|
|
@ -178,6 +167,32 @@ impl Claude {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// One classification's invocation: the trampoline as the program, with
|
||||||
|
/// the real `claude` and the run's own variables in its environment.
|
||||||
|
fn config(&self, dir: &Path, system: PathBuf, api_key: &str, attributes: &str) -> Config {
|
||||||
|
Config {
|
||||||
|
model: Some(MODEL.to_owned()),
|
||||||
|
cwd: Some(dir.to_owned()),
|
||||||
|
system_prompt_file: Some(system),
|
||||||
|
strict_mcp_config: true,
|
||||||
|
// `--tools ""`: no built-in tool at all.
|
||||||
|
tools: Some(String::new()),
|
||||||
|
program: Some(self.trampoline.clone()),
|
||||||
|
idle_timeout: Some(IDLE_TIMEOUT),
|
||||||
|
claude_config_dir: Some(dir.join("config")),
|
||||||
|
env: vec![
|
||||||
|
(
|
||||||
|
crate::child_env::EXEC_CLAUDE.to_owned(),
|
||||||
|
self.program.display().to_string(),
|
||||||
|
),
|
||||||
|
("ANTHROPIC_API_KEY".to_owned(), api_key.to_owned()),
|
||||||
|
("HOME".to_owned(), dir.display().to_string()),
|
||||||
|
("OTEL_RESOURCE_ATTRIBUTES".to_owned(), attributes.to_owned()),
|
||||||
|
],
|
||||||
|
..Config::default()
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A directory of its own for one classification.
|
/// A directory of its own for one classification.
|
||||||
|
|
@ -329,6 +344,27 @@ mod tests {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn claude_is_started_through_the_trampoline() {
|
||||||
|
let claude = Claude {
|
||||||
|
program: PathBuf::from("/nix/store/x-claude-code/bin/claude"),
|
||||||
|
trampoline: PathBuf::from(
|
||||||
|
"/nix/store/y-swarm-review-classifier/bin/swarm-review-classifier",
|
||||||
|
),
|
||||||
|
};
|
||||||
|
let config = claude.config(
|
||||||
|
Path::new("/tmp/run"),
|
||||||
|
PathBuf::from("/tmp/run/system.md"),
|
||||||
|
"k",
|
||||||
|
"a=b",
|
||||||
|
);
|
||||||
|
assert_eq!(config.program, Some(claude.trampoline.clone()));
|
||||||
|
assert!(config.env.contains(&(
|
||||||
|
crate::child_env::EXEC_CLAUDE.to_owned(),
|
||||||
|
"/nix/store/x-claude-code/bin/claude".to_owned()
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_repo_and_pr_extend_the_containers_own_attributes() {
|
fn the_repo_and_pr_extend_the_containers_own_attributes() {
|
||||||
let r = run(&[]);
|
let r = run(&[]);
|
||||||
|
|
|
||||||
22
swarm-review-classifier/tests/child_env.rs
Normal file
22
swarm-review-classifier/tests/child_env.rs
Normal file
|
|
@ -0,0 +1,22 @@
|
||||||
|
//! The trampoline through the real binary: a variable of the daemon's
|
||||||
|
//! environment does not reach the program it execs, the API key does.
|
||||||
|
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_daemons_store_identity_does_not_reach_claude() {
|
||||||
|
let out = Command::new(env!("CARGO_BIN_EXE_swarm-review-classifier"))
|
||||||
|
// Stands in for `claude`: prints what it was handed.
|
||||||
|
.env("SWARM_REVIEW_CLASSIFIER_EXEC_CLAUDE", "/bin/sh")
|
||||||
|
.env("BAO_CLIENT_KEY", "/run/credentials/x/review-classifier-bao-key")
|
||||||
|
.env("SWARM_REVIEW_CLASSIFIER_NATS_URL", "tls://nats.example:4222")
|
||||||
|
.env("ANTHROPIC_API_KEY", "k")
|
||||||
|
.args([
|
||||||
|
"-c",
|
||||||
|
r#"printf '%s|%s|%s' "${BAO_CLIENT_KEY-unset}" "${SWARM_REVIEW_CLASSIFIER_NATS_URL-unset}" "${ANTHROPIC_API_KEY-unset}""#,
|
||||||
|
])
|
||||||
|
.output()
|
||||||
|
.expect("the binary runs");
|
||||||
|
assert!(out.status.success(), "{out:?}");
|
||||||
|
assert_eq!(String::from_utf8_lossy(&out.stdout), "unset|unset|k");
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue