diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index bb3ba7f6b..48315c0e6 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -195,10 +195,17 @@ be lowercase ASCII letters, digits, and interior hyphens. The plugin starts with environment: only `env` and the ambient variables listed in `inherit_env` reach it. Bundled providers reject these plugin keys. +The same executable serves both sides of a run. The server launches it to reach a run's +sandbox after the fact (the sandbox tab, files, terminal, Ask Fabro), and Petri launches it in +the run's worker to create the sandbox. The server hands the worker `path` and `sha256` as +`PETRI_SANDBOX__PLUGIN` and `PETRI_SANDBOX__SHA256` (the kind uppercased, hyphens +as underscores), and `PETRI_SANDBOX_PLUGIN_DEV=1` when any configured plugin sets `dev`, so a +plugin configured here needs no second configuration for the worker. + ```toml title="settings.toml" [server.sandbox.providers.e2b] enabled = true -path = "/opt/fabro/plugins/fabro-sandbox-e2b" # default: `fabro-sandbox-` on PATH +path = "/opt/fabro/plugins/sandbox-driver-e2b" # default: `sandbox-driver-` on PATH sha256 = "0123…cdef" # pin the executable; `dev = true` skips it args = [] inherit_env = ["PATH"] @@ -210,7 +217,7 @@ E2B_API_URL = "https://api.e2b.example" | Key | Description | Default | |---|---|---| | `enabled` | Whether runs may select this provider | `true` | -| `path` | Plugin executable path | `fabro-sandbox-` on `PATH` | +| `path` | Plugin executable path | `sandbox-driver-` on `PATH` | | `sha256` | Pinned SHA-256 of the executable, hex | none | | `dev` | Allow launching without a checksum | `false` | | `args` | Arguments passed to the executable | `[]` | diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 7c5cfc2b4..35bc36210 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -15447,7 +15447,7 @@ components: properties: path: type: string - description: Executable path. Absent means `fabro-sandbox-` on `PATH`. + description: Executable path. Absent means `sandbox-driver-` on `PATH`. sha256: type: string description: Pinned SHA-256 of the executable, hex. diff --git a/lib/apps/fabro-server/src/sandbox_access.rs b/lib/apps/fabro-server/src/sandbox_access.rs index 7df6f7d6c..2e770428f 100644 --- a/lib/apps/fabro-server/src/sandbox_access.rs +++ b/lib/apps/fabro-server/src/sandbox_access.rs @@ -54,9 +54,10 @@ use tokio::time; pub(crate) const PETRI_RUN_LABEL: &str = "petri.run"; /// Binary naming prefix for a plugin provider's executable: a plugin for -/// kind `e2b` is `fabro-sandbox-e2b` on `PATH` unless the settings name a -/// path. -const PLUGIN_BINARY_PREFIX: &str = "fabro-sandbox"; +/// kind `e2b` is `sandbox-driver-e2b` on `PATH` unless the settings name a +/// path. The same executable serves Petri's run in the worker, which looks +/// it up under the same name. +const PLUGIN_BINARY_PREFIX: &str = "sandbox-driver"; /// `User-Agent` Fabro presents to remote sandbox control planes. const USER_AGENT: &str = concat!("fabro-server/", env!("CARGO_PKG_VERSION")); @@ -839,7 +840,7 @@ mod tests { .insert(kind(name), ServerSandboxProviderSettings { enabled: true, plugin: Some(SandboxPluginSettings { - path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + path: Some(format!("/nonexistent/sandbox-driver-{name}")), dev: true, ..SandboxPluginSettings::default() }), diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 2b9b17159..9e42dc1f1 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -150,7 +150,7 @@ use crate::sandbox_access::{ SandboxInventory, }; use crate::server_secrets::ServerSecrets; -use crate::spawn_env::apply_render_graph_env; +use crate::spawn_env::{self, apply_render_graph_env}; use crate::worker_control::{ LocalWorkerControlBus, WORKER_CONTROL_ACK_WAIT, WorkerControlAcks, WorkerControlBus, WorkerControlBusError, @@ -3673,6 +3673,9 @@ fn worker_launch_spec( github_app_private_key, daytona_api_key, fabro_home: fabro_config::Home::from_env().root().to_path_buf(), + sandbox_plugin_env: spawn_env::sandbox_plugin_env( + &state.server_settings().server.sandbox.providers, + ), }) } diff --git a/lib/apps/fabro-server/src/server/handler/sandboxes.rs b/lib/apps/fabro-server/src/server/handler/sandboxes.rs index 83199d6f6..1e81706ce 100644 --- a/lib/apps/fabro-server/src/server/handler/sandboxes.rs +++ b/lib/apps/fabro-server/src/server/handler/sandboxes.rs @@ -121,7 +121,7 @@ mod tests { .insert(kind.clone(), ServerSandboxProviderSettings { enabled: true, plugin: Some(SandboxPluginSettings { - path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + path: Some(format!("/nonexistent/sandbox-driver-{name}")), dev: true, ..SandboxPluginSettings::default() }), diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 29182e8da..127e6667e 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -2319,6 +2319,49 @@ fn worker_command_forwards_daytona_api_key_from_vault() { ); } +/// A plugin configured under `[server.sandbox.providers.]` reaches +/// the worker under the names Petri reads, so a run on that kind finds its +/// plugin without a second configuration. +#[cfg(unix)] +#[test] +fn worker_command_forwards_configured_sandbox_plugins() { + let storage_dir = tempfile::tempdir().unwrap(); + let state = worker_command_test_state_with_extra_config( + storage_dir.path(), + &["dev-token"], + Some(TEST_DEV_TOKEN), + r#" +[server.sandbox.providers.e2b] +path = "/opt/fabro/plugins/sandbox-driver-e2b" +sha256 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +dev = true +"#, + ); + let cmd = worker_command( + state.as_ref(), + RunId::new(), + RunExecutionMode::Start, + storage_dir.path(), + false, + ) + .unwrap(); + + assert_eq!( + command_env_value(&cmd, "PETRI_SANDBOX_E2B_PLUGIN"), + EnvOverride::Set("/opt/fabro/plugins/sandbox-driver-e2b".to_string()) + ); + assert_eq!( + command_env_value(&cmd, "PETRI_SANDBOX_E2B_SHA256"), + EnvOverride::Set( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string() + ) + ); + assert_eq!( + command_env_value(&cmd, EnvVars::PETRI_SANDBOX_PLUGIN_DEV), + EnvOverride::Set("1".to_string()) + ); +} + #[cfg(unix)] #[test] fn worker_command_omits_github_app_private_key_when_unset() { diff --git a/lib/apps/fabro-server/src/spawn_env.rs b/lib/apps/fabro-server/src/spawn_env.rs index 5c7891e20..1ed46a372 100644 --- a/lib/apps/fabro-server/src/spawn_env.rs +++ b/lib/apps/fabro-server/src/spawn_env.rs @@ -1,6 +1,7 @@ use std::ffi::OsString; use fabro_static::EnvVars; +use fabro_types::settings::server::ServerSandboxProvidersSettings; use tokio::process::Command; const WORKER_ENV_ALLOWLIST: &[&str] = &[ @@ -53,6 +54,8 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ // Petri's sandbox-driver plugins are resolved in the worker, where a // Petri run executes: the plugin path, checksum and dev-mode overrides // cross with `PATH`, so the worker finds the plugins the server would. + // A plugin the server's settings configure is set on top of these by + // `sandbox_plugin_env`, for every configured kind. EnvVars::PETRI_SANDBOX_HOST_PLUGIN, EnvVars::PETRI_SANDBOX_HOST_SHA256, EnvVars::PETRI_SANDBOX_DOCKER_PLUGIN, @@ -87,8 +90,55 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ const RENDER_GRAPH_ENV_ALLOWLIST: &[&str] = &[EnvVars::PATH, EnvVars::HOME, EnvVars::TMPDIR]; -pub(crate) fn apply_worker_env(cmd: &mut Command) { - apply_allowlist(cmd, WORKER_ENV_ALLOWLIST, &process_env_var_os); +/// The worker's environment: the allowlisted ambient variables, then the +/// plugin variables the server's settings derive, which win over an +/// ambient variable of the same name. +pub(crate) fn apply_worker_env(cmd: &mut Command, sandbox_plugins: &[(String, String)]) { + apply_worker_env_with(cmd, sandbox_plugins, &process_env_var_os); +} + +fn apply_worker_env_with( + cmd: &mut Command, + sandbox_plugins: &[(String, String)], + lookup: &dyn Fn(&str) -> Option, +) { + apply_allowlist(cmd, WORKER_ENV_ALLOWLIST, lookup); + for (name, value) in sandbox_plugins { + cmd.env(name, value); + } +} + +/// The plugin variables Petri reads in the worker, derived from the +/// server's `[server.sandbox.providers.]` settings: for every enabled +/// kind that carries plugin settings, `PETRI_SANDBOX__PLUGIN` from +/// its `path` and `PETRI_SANDBOX__SHA256` from its `sha256`, and +/// `PETRI_SANDBOX_PLUGIN_DEV=1` when any of them sets `dev`. The kind is +/// uppercased with hyphens as underscores, as Petri names the variable. A +/// kind whose settings name no path is left to Petri's own lookup +/// (`sandbox-driver-` beside the executable, then on `PATH`), the +/// same lookup the server's attach uses. +pub(crate) fn sandbox_plugin_env( + providers: &ServerSandboxProvidersSettings, +) -> Vec<(String, String)> { + let mut env = Vec::new(); + let mut dev = false; + for (kind, plugin) in providers.enabled_plugins() { + let upper = kind.as_str().to_ascii_uppercase().replace('-', "_"); + if let Some(path) = &plugin.path { + env.push((format!("PETRI_SANDBOX_{upper}_PLUGIN"), path.clone())); + } + if let Some(sha256) = &plugin.sha256 { + env.push((format!("PETRI_SANDBOX_{upper}_SHA256"), sha256.clone())); + } + dev |= plugin.dev; + } + if dev { + env.push(( + EnvVars::PETRI_SANDBOX_PLUGIN_DEV.to_string(), + "1".to_string(), + )); + } + env } pub(crate) fn apply_render_graph_env(cmd: &mut Command) { @@ -118,7 +168,15 @@ mod tests { use std::ffi::OsString; use std::path::Path; - use super::{RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist}; + use fabro_types::SandboxProviderKind; + use fabro_types::settings::server::{ + SandboxPluginSettings, ServerSandboxProviderSettings, ServerSandboxProvidersSettings, + }; + + use super::{ + RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist, apply_worker_env_with, + sandbox_plugin_env, + }; fn env_command() -> tokio::process::Command { assert!(Path::new("/usr/bin/env").exists()); @@ -322,6 +380,117 @@ mod tests { assert!(!actual.contains_key("MY_API_KEY")); } + fn provider( + kind: &str, + enabled: bool, + plugin: SandboxPluginSettings, + ) -> (SandboxProviderKind, ServerSandboxProviderSettings) { + ( + SandboxProviderKind::try_new(kind).expect("a valid kind"), + ServerSandboxProviderSettings { + enabled, + plugin: Some(plugin), + }, + ) + } + + /// A configured plugin reaches the worker under the names Petri reads, + /// a configured path wins over the ambient variable of the same name, + /// a kind the settings leave to `PATH` keeps the ambient one, and a + /// disabled kind's plugin never crosses. + #[tokio::test] + async fn configured_plugins_reach_the_worker_and_win_over_ambient_variables() { + let mut providers = ServerSandboxProvidersSettings::default(); + providers.entries.extend([ + provider("e2b", true, SandboxPluginSettings { + path: Some("/opt/fabro/plugins/sandbox-driver-e2b".to_string()), + sha256: Some("0123abcd".to_string()), + dev: true, + ..SandboxPluginSettings::default() + }), + provider("docker", true, SandboxPluginSettings { + path: Some("/opt/fabro/plugins/sandbox-driver-docker".to_string()), + ..SandboxPluginSettings::default() + }), + provider("daytona", true, SandboxPluginSettings::default()), + provider("fly-io", false, SandboxPluginSettings { + path: Some("/opt/fabro/plugins/sandbox-driver-fly-io".to_string()), + ..SandboxPluginSettings::default() + }), + ]); + let env = HashMap::from([ + ("PATH".to_string(), "/bin".to_string()), + ( + "PETRI_SANDBOX_HOST_PLUGIN".to_string(), + "/ambient/sandbox-driver-host".to_string(), + ), + ( + "PETRI_SANDBOX_DOCKER_PLUGIN".to_string(), + "/ambient/sandbox-driver-docker".to_string(), + ), + ( + "PETRI_SANDBOX_DAYTONA_PLUGIN".to_string(), + "/ambient/sandbox-driver-daytona".to_string(), + ), + ]); + let mut cmd = env_command(); + apply_worker_env_with(&mut cmd, &sandbox_plugin_env(&providers), &|name| { + env.get(name).map(OsString::from) + }); + + let actual = env_output(cmd).await; + + assert_eq!( + actual.get("PETRI_SANDBOX_E2B_PLUGIN").map(String::as_str), + Some("/opt/fabro/plugins/sandbox-driver-e2b") + ); + assert_eq!( + actual.get("PETRI_SANDBOX_E2B_SHA256").map(String::as_str), + Some("0123abcd") + ); + assert_eq!( + actual.get("PETRI_SANDBOX_PLUGIN_DEV").map(String::as_str), + Some("1"), + "one plugin in dev mode puts the worker's lookup in dev mode" + ); + assert_eq!( + actual + .get("PETRI_SANDBOX_DOCKER_PLUGIN") + .map(String::as_str), + Some("/opt/fabro/plugins/sandbox-driver-docker"), + "the settings win over the ambient variable" + ); + assert_eq!( + actual.get("PETRI_SANDBOX_HOST_PLUGIN").map(String::as_str), + Some("/ambient/sandbox-driver-host"), + "a kind without settings keeps the allowlisted ambient variable" + ); + assert_eq!( + actual + .get("PETRI_SANDBOX_DAYTONA_PLUGIN") + .map(String::as_str), + Some("/ambient/sandbox-driver-daytona"), + "settings without a path leave the ambient variable in place" + ); + assert!( + !actual.contains_key("PETRI_SANDBOX_FLY_IO_PLUGIN"), + "a disabled kind's plugin does not cross" + ); + } + + #[test] + fn no_configured_plugin_derives_no_variables() { + assert!(sandbox_plugin_env(&ServerSandboxProvidersSettings::default()).is_empty()); + let mut providers = ServerSandboxProvidersSettings::default(); + providers + .entries + .extend([provider("docker", true, SandboxPluginSettings::default())]); + assert!( + sandbox_plugin_env(&providers).is_empty(), + "settings with neither a path nor a pin nor dev mode add nothing" + ); + } + #[tokio::test] async fn render_graph_allowlist_is_fail_closed() { let env = HashMap::from([ diff --git a/lib/apps/fabro-server/src/worker_runtime.rs b/lib/apps/fabro-server/src/worker_runtime.rs index 21271494b..2f76c2409 100644 --- a/lib/apps/fabro-server/src/worker_runtime.rs +++ b/lib/apps/fabro-server/src/worker_runtime.rs @@ -54,6 +54,10 @@ pub(crate) struct WorkerLaunchSpec { /// 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, + /// The sandbox-driver plugin variables the server's provider settings + /// derive (`spawn_env::sandbox_plugin_env`), so Petri in the worker + /// launches the plugin the settings name for every configured kind. + pub(crate) sandbox_plugin_env: Vec<(String, String)>, } pub(crate) struct StartedWorker { @@ -101,7 +105,7 @@ impl LocalWorkerRuntime { .stdout(worker_stdout) .stderr(Stdio::piped()); - apply_worker_env(&mut cmd); + apply_worker_env(&mut cmd, &spec.sandbox_plugin_env); if let Some(level) = spec.fabro_log.as_deref() { cmd.env(EnvVars::FABRO_LOG, level); } diff --git a/lib/foundation/fabro-config/src/tests/resolve_server.rs b/lib/foundation/fabro-config/src/tests/resolve_server.rs index a2fbb2807..33995be1f 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_server.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_server.rs @@ -235,7 +235,7 @@ _version = 1 methods = ["dev-token"] [server.sandbox.providers.e2b] -path = "/opt/fabro/plugins/fabro-sandbox-e2b" +path = "/opt/fabro/plugins/sandbox-driver-e2b" sha256 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" args = ["--region", "us"] inherit_env = ["PATH"] @@ -255,7 +255,7 @@ E2B_API_URL = "https://api.e2b.example" .expect("plugin kinds carry launch settings"); assert_eq!( plugin.path.as_deref(), - Some("/opt/fabro/plugins/fabro-sandbox-e2b") + Some("/opt/fabro/plugins/sandbox-driver-e2b") ); assert_eq!(plugin.args, vec!["--region", "us"]); assert_eq!(plugin.inherit_env, vec!["PATH"]); @@ -279,7 +279,7 @@ _version = 1 methods = ["dev-token"] [server.sandbox.providers.docker] -path = "/usr/local/bin/fabro-sandbox-docker" +path = "/usr/local/bin/sandbox-driver-docker" "#, ) .expect_err("bundled providers take no plugin settings"); diff --git a/lib/foundation/fabro-types/src/settings/server.rs b/lib/foundation/fabro-types/src/settings/server.rs index b704554c8..4f4279ded 100644 --- a/lib/foundation/fabro-types/src/settings/server.rs +++ b/lib/foundation/fabro-types/src/settings/server.rs @@ -187,8 +187,8 @@ impl Default for ServerSandboxProviderSettings { /// named in `inherit_env` reach it. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct SandboxPluginSettings { - /// Executable path. When absent the server searches `PATH` for - /// `fabro-sandbox-`. + /// Executable path. When absent the server, and Petri in the run's + /// worker, search `PATH` for `sandbox-driver-`. #[serde(default, skip_serializing_if = "Option::is_none")] pub path: Option, /// Pinned SHA-256 of the executable, hex. diff --git a/lib/packages/fabro-api-client/src/models/sandbox-plugin-settings.ts b/lib/packages/fabro-api-client/src/models/sandbox-plugin-settings.ts index 454c94cd0..f98d1a275 100644 --- a/lib/packages/fabro-api-client/src/models/sandbox-plugin-settings.ts +++ b/lib/packages/fabro-api-client/src/models/sandbox-plugin-settings.ts @@ -19,7 +19,7 @@ */ export interface SandboxPluginSettings { /** - * Executable path. Absent means `fabro-sandbox-` on `PATH`. + * Executable path. Absent means `sandbox-driver-` on `PATH`. */ 'path'?: string; /**