mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-01 02:04:24 +00:00
Forward the Docker daemon selection and Daytona credentials to Petri workers
A Petri run's worker launches the sandbox-driver plugins itself, and the Docker plugin forwards `DOCKER_HOST`, `DOCKER_TLS_VERIFY`, `DOCKER_CERT_PATH`, `DOCKER_API_VERSION`, `DOCKER_CONFIG` and `DOCKER_CONTEXT` from the process that launches it. They now cross the worker's environment allowlist, so the worker's sandboxes go to the daemon the server uses. The concern that kept them out, the legacy worker's own Docker client picking up a daemon it was not meant to, is moot: the legacy executor is being deleted. The same variables pass through the test harness's isolation, so a developer's daemon selection reaches the servers tests start. Daytona's non-secret selectors, `DAYTONA_API_URL` and `DAYTONA_ORGANIZATION_ID`, cross the allowlist too. The API key stays the vault's: a Daytona run's worker command carries it the way the GitHub app key travels, and the Daytona plugin reads it from the worker's process. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
2a000cc7d3
commit
97f74c3ab0
6 changed files with 167 additions and 4 deletions
|
|
@ -3775,6 +3775,7 @@ fn worker_launch_spec(
|
|||
run_dir: &std::path::Path,
|
||||
agent_fabro_tools_enabled: bool,
|
||||
github_app_private_key: Option<String>,
|
||||
daytona_api_key: Option<String>,
|
||||
) -> anyhow::Result<WorkerLaunchSpec> {
|
||||
let current_exe = std::env::current_exe().context("reading current executable path")?;
|
||||
let executable =
|
||||
|
|
@ -3813,6 +3814,7 @@ fn worker_launch_spec(
|
|||
fabro_log,
|
||||
active_config_path: state.active_config_path().to_path_buf(),
|
||||
github_app_private_key,
|
||||
daytona_api_key,
|
||||
fabro_home: fabro_config::Home::from_env().root().to_path_buf(),
|
||||
})
|
||||
}
|
||||
|
|
@ -4458,7 +4460,21 @@ async fn execute_run_subprocess(state: Arc<AppState>, run_id: RunId) {
|
|||
return;
|
||||
}
|
||||
|
||||
let github_app_private_key = match state.vault_secret(EnvVars::GITHUB_APP_PRIVATE_KEY).await {
|
||||
// A Daytona run's worker hands the vault's key to Petri's Daytona
|
||||
// plugin through its own environment; any other run's worker never
|
||||
// sees it.
|
||||
let wants_daytona =
|
||||
run_state.spec.settings.run.environment.provider == SandboxProviderKind::DAYTONA;
|
||||
let secrets = async {
|
||||
let github_app_private_key = state.vault_secret(EnvVars::GITHUB_APP_PRIVATE_KEY).await?;
|
||||
let daytona_api_key = if wants_daytona {
|
||||
state.vault_secret(EnvVars::DAYTONA_API_KEY).await?
|
||||
} else {
|
||||
None
|
||||
};
|
||||
Ok::<_, SecretStoreError>((github_app_private_key, daytona_api_key))
|
||||
};
|
||||
let (github_app_private_key, daytona_api_key) = match secrets.await {
|
||||
Ok(value) => value,
|
||||
Err(err) => {
|
||||
fail_run_before_execution(
|
||||
|
|
@ -4482,6 +4498,7 @@ async fn execute_run_subprocess(state: Arc<AppState>, run_id: RunId) {
|
|||
&run_dir_for_build,
|
||||
agent_fabro_tools_enabled,
|
||||
github_app_private_key,
|
||||
daytona_api_key,
|
||||
)
|
||||
})
|
||||
.await
|
||||
|
|
|
|||
|
|
@ -2402,6 +2402,7 @@ fn worker_command_forwards_github_app_private_key_from_vault() {
|
|||
storage_dir.path(),
|
||||
false,
|
||||
Some("test-private-key".to_string()),
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
let cmd = LocalWorkerRuntime::command_for_spec(&spec);
|
||||
|
|
@ -2410,6 +2411,35 @@ fn worker_command_forwards_github_app_private_key_from_vault() {
|
|||
command_env_value(&cmd, EnvVars::GITHUB_APP_PRIVATE_KEY),
|
||||
EnvOverride::Set("test-private-key".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
command_env_value(&cmd, EnvVars::DAYTONA_API_KEY),
|
||||
EnvOverride::Unchanged
|
||||
);
|
||||
}
|
||||
|
||||
/// A Daytona run's worker carries the vault's key for Petri's Daytona
|
||||
/// plugin.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn worker_command_forwards_daytona_api_key_from_vault() {
|
||||
let storage_dir = tempfile::tempdir().unwrap();
|
||||
let state = worker_command_test_state(storage_dir.path(), &["dev-token"], Some(TEST_DEV_TOKEN));
|
||||
let spec = worker_launch_spec(
|
||||
state.as_ref(),
|
||||
RunId::new(),
|
||||
RunExecutionMode::Start,
|
||||
storage_dir.path(),
|
||||
false,
|
||||
None,
|
||||
Some("dtn_test-key".to_string()),
|
||||
)
|
||||
.unwrap();
|
||||
let cmd = LocalWorkerRuntime::command_for_spec(&spec);
|
||||
|
||||
assert_eq!(
|
||||
command_env_value(&cmd, EnvVars::DAYTONA_API_KEY),
|
||||
EnvOverride::Set("dtn_test-key".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
|
|
@ -2661,6 +2691,7 @@ fn worker_command(
|
|||
run_dir,
|
||||
agent_fabro_tools_enabled,
|
||||
None,
|
||||
None,
|
||||
)?;
|
||||
Ok(LocalWorkerRuntime::command_for_spec(&spec))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -62,6 +62,21 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[
|
|||
EnvVars::PETRI_SANDBOX_PLUGIN_DEV,
|
||||
EnvVars::PETRI_SANDBOX_DOCKER_HOST_ADDRESS,
|
||||
EnvVars::PETRI_SANDBOX_ACTION_HOST_IMAGE,
|
||||
// The Docker daemon selection: the worker's Docker plugin reads these
|
||||
// from its own process, so the worker's sandboxes go to the daemon the
|
||||
// server uses (a remote or TLS daemon, a named context), not the
|
||||
// default socket.
|
||||
EnvVars::DOCKER_HOST,
|
||||
EnvVars::DOCKER_TLS_VERIFY,
|
||||
EnvVars::DOCKER_CERT_PATH,
|
||||
EnvVars::DOCKER_API_VERSION,
|
||||
EnvVars::DOCKER_CONFIG,
|
||||
EnvVars::DOCKER_CONTEXT,
|
||||
// Daytona's control-plane selection, the non-secret half: the plugin
|
||||
// reads them from the worker. The API key comes from the vault, set on
|
||||
// the command by the launch (`WorkerLaunchSpec::daytona_api_key`).
|
||||
EnvVars::DAYTONA_API_URL,
|
||||
EnvVars::DAYTONA_ORGANIZATION_ID,
|
||||
// A test's checkpoint gates: the worker's hooks hold at a named point
|
||||
// until the test releases them, so a crash can be placed there.
|
||||
EnvVars::FABRO_TEST_CHECKPOINT_GATES,
|
||||
|
|
@ -164,6 +179,27 @@ mod tests {
|
|||
"/opt/petri/sandbox-driver-host".to_string(),
|
||||
),
|
||||
("PETRI_SANDBOX_PLUGIN_DEV".to_string(), "1".to_string()),
|
||||
(
|
||||
"DOCKER_HOST".to_string(),
|
||||
"tcp://build-daemon.internal:2376".to_string(),
|
||||
),
|
||||
("DOCKER_TLS_VERIFY".to_string(), "1".to_string()),
|
||||
(
|
||||
"DOCKER_CERT_PATH".to_string(),
|
||||
"/etc/docker/certs".to_string(),
|
||||
),
|
||||
("DOCKER_API_VERSION".to_string(), "1.47".to_string()),
|
||||
(
|
||||
"DOCKER_CONFIG".to_string(),
|
||||
"/etc/docker/client".to_string(),
|
||||
),
|
||||
("DOCKER_CONTEXT".to_string(), "build".to_string()),
|
||||
(
|
||||
"DAYTONA_API_URL".to_string(),
|
||||
"https://daytona.internal/api".to_string(),
|
||||
),
|
||||
("DAYTONA_ORGANIZATION_ID".to_string(), "org-1".to_string()),
|
||||
("DAYTONA_API_KEY".to_string(), "leak".to_string()),
|
||||
]);
|
||||
let mut cmd = env_command();
|
||||
apply_allowlist(&mut cmd, WORKER_ENV_ALLOWLIST, &|name| {
|
||||
|
|
@ -208,6 +244,43 @@ mod tests {
|
|||
actual.get("PETRI_SANDBOX_PLUGIN_DEV").map(String::as_str),
|
||||
Some("1")
|
||||
);
|
||||
// The Docker daemon selection crosses whole, so the worker's Docker
|
||||
// plugin drives the daemon the server uses.
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_HOST").map(String::as_str),
|
||||
Some("tcp://build-daemon.internal:2376")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_TLS_VERIFY").map(String::as_str),
|
||||
Some("1")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_CERT_PATH").map(String::as_str),
|
||||
Some("/etc/docker/certs")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_API_VERSION").map(String::as_str),
|
||||
Some("1.47")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_CONFIG").map(String::as_str),
|
||||
Some("/etc/docker/client")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DOCKER_CONTEXT").map(String::as_str),
|
||||
Some("build")
|
||||
);
|
||||
// Daytona's non-secret selectors cross; its key is the vault's,
|
||||
// never the server's environment.
|
||||
assert_eq!(
|
||||
actual.get("DAYTONA_API_URL").map(String::as_str),
|
||||
Some("https://daytona.internal/api")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("DAYTONA_ORGANIZATION_ID").map(String::as_str),
|
||||
Some("org-1")
|
||||
);
|
||||
assert!(!actual.contains_key("DAYTONA_API_KEY"));
|
||||
assert_eq!(actual.get("CLICOLOR").map(String::as_str), Some("0"));
|
||||
assert_eq!(actual.get("CLICOLOR_FORCE").map(String::as_str), Some("1"));
|
||||
// Bedrock SigV4 chain inputs cross into the worker so it can re-resolve
|
||||
|
|
|
|||
|
|
@ -48,6 +48,9 @@ pub(crate) struct WorkerLaunchSpec {
|
|||
pub(crate) fabro_log: Option<String>,
|
||||
pub(crate) active_config_path: PathBuf,
|
||||
pub(crate) github_app_private_key: Option<String>,
|
||||
/// The vault's Daytona API key, for a run on a Daytona environment:
|
||||
/// Petri's Daytona plugin reads it from the worker's process.
|
||||
pub(crate) daytona_api_key: Option<String>,
|
||||
/// The Fabro home the server resolved, so a Petri run's skills step
|
||||
/// reads the same home whatever the worker's environment says.
|
||||
pub(crate) fabro_home: PathBuf,
|
||||
|
|
@ -109,6 +112,9 @@ impl LocalWorkerRuntime {
|
|||
if let Some(pem) = spec.github_app_private_key.as_deref() {
|
||||
cmd.env(EnvVars::GITHUB_APP_PRIVATE_KEY, pem);
|
||||
}
|
||||
if let Some(key) = spec.daytona_api_key.as_deref() {
|
||||
cmd.env(EnvVars::DAYTONA_API_KEY, key);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fabro_proc::pre_exec_setpgid(cmd.as_std_mut());
|
||||
|
|
|
|||
|
|
@ -74,6 +74,29 @@ impl EnvVars {
|
|||
Self::PETRI_SANDBOX_ACTION_HOST_IMAGE,
|
||||
];
|
||||
|
||||
// The Docker daemon selection the Docker CLI and its client libraries
|
||||
// read: which daemon, over which transport, with which TLS material,
|
||||
// client configuration and context. Petri's Docker plugin forwards them
|
||||
// from the process that launches it, so a run's worker must carry the
|
||||
// server's.
|
||||
pub const DOCKER_HOST: &'static str = "DOCKER_HOST";
|
||||
pub const DOCKER_TLS_VERIFY: &'static str = "DOCKER_TLS_VERIFY";
|
||||
pub const DOCKER_CERT_PATH: &'static str = "DOCKER_CERT_PATH";
|
||||
pub const DOCKER_API_VERSION: &'static str = "DOCKER_API_VERSION";
|
||||
pub const DOCKER_CONFIG: &'static str = "DOCKER_CONFIG";
|
||||
pub const DOCKER_CONTEXT: &'static str = "DOCKER_CONTEXT";
|
||||
|
||||
/// Every Docker daemon selection variable, in one list for the process
|
||||
/// boundaries that forward them.
|
||||
pub const DOCKER_VARS: &'static [&'static str] = &[
|
||||
Self::DOCKER_HOST,
|
||||
Self::DOCKER_TLS_VERIFY,
|
||||
Self::DOCKER_CERT_PATH,
|
||||
Self::DOCKER_API_VERSION,
|
||||
Self::DOCKER_CONFIG,
|
||||
Self::DOCKER_CONTEXT,
|
||||
];
|
||||
|
||||
// LLM providers and tool integrations
|
||||
pub const ANTHROPIC_API_KEY: &'static str = "ANTHROPIC_API_KEY";
|
||||
pub const AWS_BEARER_TOKEN_BEDROCK: &'static str = "AWS_BEARER_TOKEN_BEDROCK";
|
||||
|
|
@ -239,6 +262,12 @@ mod tests {
|
|||
EnvVars::PETRI_SANDBOX_PLUGIN_DEV,
|
||||
EnvVars::PETRI_SANDBOX_DOCKER_HOST_ADDRESS,
|
||||
EnvVars::PETRI_SANDBOX_ACTION_HOST_IMAGE,
|
||||
EnvVars::DOCKER_HOST,
|
||||
EnvVars::DOCKER_TLS_VERIFY,
|
||||
EnvVars::DOCKER_CERT_PATH,
|
||||
EnvVars::DOCKER_API_VERSION,
|
||||
EnvVars::DOCKER_CONFIG,
|
||||
EnvVars::DOCKER_CONTEXT,
|
||||
EnvVars::ANTHROPIC_API_KEY,
|
||||
EnvVars::ANTHROPIC_BASE_URL,
|
||||
EnvVars::AWS_BEARER_TOKEN_BEDROCK,
|
||||
|
|
|
|||
|
|
@ -177,7 +177,10 @@ pub fn isolated_env(home_dir: &Path) -> HashMap<String, String> {
|
|||
if let Some(path) = std::env::var_os(EnvVars::PATH).and_then(|value| value.into_string().ok()) {
|
||||
env.insert(EnvVars::PATH.to_string(), path);
|
||||
}
|
||||
for name in EnvVars::PETRI_SANDBOX_PLUGIN_VARS {
|
||||
for name in EnvVars::PETRI_SANDBOX_PLUGIN_VARS
|
||||
.iter()
|
||||
.chain(EnvVars::DOCKER_VARS)
|
||||
{
|
||||
if let Some(value) = std::env::var_os(name).and_then(|value| value.into_string().ok()) {
|
||||
env.insert((*name).to_string(), value);
|
||||
}
|
||||
|
|
@ -223,8 +226,12 @@ fn apply_test_isolation_with_lookup(
|
|||
}
|
||||
// Petri resolves its sandbox-driver plugins from these, in the server a
|
||||
// test starts and in the workers that server launches; a developer's
|
||||
// plugin override reaches them like `PATH` does.
|
||||
for name in EnvVars::PETRI_SANDBOX_PLUGIN_VARS {
|
||||
// plugin override reaches them like `PATH` does, and so does the Docker
|
||||
// daemon selection the Docker plugin needs.
|
||||
for name in EnvVars::PETRI_SANDBOX_PLUGIN_VARS
|
||||
.iter()
|
||||
.chain(EnvVars::DOCKER_VARS)
|
||||
{
|
||||
if let Some(value) = lookup(name) {
|
||||
cmd.env(name, value);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue