From 97f74c3ab0c56900d942e06383d9ca1e407d8d37 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 18 Sep 2026 09:10:29 -0400 Subject: [PATCH] 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 --- lib/apps/fabro-server/src/server.rs | 19 +++++- lib/apps/fabro-server/src/server/tests.rs | 31 +++++++++ lib/apps/fabro-server/src/spawn_env.rs | 73 +++++++++++++++++++++ lib/apps/fabro-server/src/worker_runtime.rs | 6 ++ lib/foundation/fabro-static/src/env_vars.rs | 29 ++++++++ lib/foundation/fabro-test/src/lib.rs | 13 +++- 6 files changed, 167 insertions(+), 4 deletions(-) diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index a4e02eae2..39f6dec53 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -3775,6 +3775,7 @@ fn worker_launch_spec( run_dir: &std::path::Path, agent_fabro_tools_enabled: bool, github_app_private_key: Option, + daytona_api_key: Option, ) -> anyhow::Result { 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, 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, run_id: RunId) { &run_dir_for_build, agent_fabro_tools_enabled, github_app_private_key, + daytona_api_key, ) }) .await diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 75333bfe1..724422b1b 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -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)) } diff --git a/lib/apps/fabro-server/src/spawn_env.rs b/lib/apps/fabro-server/src/spawn_env.rs index 351939d7c..8510c5a94 100644 --- a/lib/apps/fabro-server/src/spawn_env.rs +++ b/lib/apps/fabro-server/src/spawn_env.rs @@ -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 diff --git a/lib/apps/fabro-server/src/worker_runtime.rs b/lib/apps/fabro-server/src/worker_runtime.rs index 7b4fa39b6..21271494b 100644 --- a/lib/apps/fabro-server/src/worker_runtime.rs +++ b/lib/apps/fabro-server/src/worker_runtime.rs @@ -48,6 +48,9 @@ pub(crate) struct WorkerLaunchSpec { pub(crate) fabro_log: Option, pub(crate) active_config_path: PathBuf, pub(crate) github_app_private_key: Option, + /// 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, /// 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()); diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index a14c8ce7e..ff3f29fbb 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -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, diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index 44a665d13..0ddafda47 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -177,7 +177,10 @@ pub fn isolated_env(home_dir: &Path) -> HashMap { 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); }