diff --git a/Cargo.lock b/Cargo.lock index d2922c3e1..7a1060669 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7008,7 +7008,7 @@ dependencies = [ [[package]] name = "sandbox-driver" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "async-trait", "globset", @@ -7024,7 +7024,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "anyhow", "async-trait", @@ -7049,7 +7049,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "sandbox-driver-docker-config", "serde", @@ -7059,7 +7059,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "anyhow", "async-trait", @@ -7080,7 +7080,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "serde", "serde_json", @@ -7089,7 +7089,7 @@ dependencies = [ [[package]] name = "sandbox-driver-host" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "anyhow", "async-trait", @@ -7107,7 +7107,7 @@ dependencies = [ [[package]] name = "sandbox-driver-protocol" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=b9d07cf3ef1861173f8134498e2115b041f00b7d#b9d07cf3ef1861173f8134498e2115b041f00b7d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" dependencies = [ "async-trait", "base64", diff --git a/Cargo.toml b/Cargo.toml index 430e334c3..ccb2cc771 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -97,14 +97,15 @@ tokio-tungstenite = { version = "0.26", features = ["rustls-tls-webpki-roots"] } futures-util = "0.3" # sandbox-driver: the sandbox provider layer. Bundled Host, Docker, and # Daytona providers link in-process; third-party providers run as stdio -# plugins through sandbox-driver-protocol. Pinned by rev like the Daytona SDK. -sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } -sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "b9d07cf3ef1861173f8134498e2115b041f00b7d" } +# plugins through sandbox-driver-protocol. Pinned by rev; currently the head of +# sandbox-driver PR #9 (trust the configured plugin kind), to move to main on merge. +sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] } fork = "0.2" exec = "0.3" diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 8adb4362c..9c0f53508 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -201,12 +201,8 @@ enabled = true Any other key names a [sandbox-driver](https://github.com/lithoscomputer/sandbox-driver) plugin: an executable that speaks the sandbox-driver JSON-RPC protocol on stdin and stdout. The kind must be lowercase ASCII letters, digits, and interior hyphens. The plugin starts with a scrubbed -environment: only `env` and the ambient variables listed in `inherit_env` reach it. - -A bundled provider accepts the same plugin keys. Setting any of them runs that provider out of -process through the driver's executable for the kind (`fabro-sandbox-docker` for `docker`), which -isolates the server from provider crashes at the cost of a process per connection. Without them -the bundled provider links in-process. +environment: only `env` and the ambient variables listed in `inherit_env` reach it. Bundled +providers reject these plugin keys. ```toml title="settings.toml" [server.sandbox.providers.e2b] diff --git a/lib/apps/fabro-cli/tests/it/workflow/mod.rs b/lib/apps/fabro-cli/tests/it/workflow/mod.rs index 7ea94125a..ba180288c 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/mod.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/mod.rs @@ -174,8 +174,8 @@ fn run_events(run_dir: &Path) -> Vec { /// - `daytona`: the bundled Daytona provider, live credentials required. /// - `host-plugin`: the driver's Host executable over stdio under the /// non-bundled `host` kind, a clone-based managed workspace. -/// - `docker-plugin`: the bundled `docker` kind served out of process by the -/// driver's Docker executable. +/// - `docker-plugin`: the driver's Docker executable over stdio under the +/// non-bundled `docker-plugin` kind. /// /// The plugin variants need the executables `cargo` builds for /// `fabro-sandbox`; without them (or without a Docker daemon) they skip, diff --git a/lib/apps/fabro-cli/tests/it/workflow/plugin.rs b/lib/apps/fabro-cli/tests/it/workflow/plugin.rs index 26d26c484..907b35c9e 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/plugin.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/plugin.rs @@ -2,7 +2,9 @@ //! workflow scenarios. //! //! The executables come from the `fabro-sandbox` package's `[[bin]]` targets, -//! which `cargo` places beside the `fabro` binary under test. A scenario +//! which `cargo` places beside the `fabro` binary under test. Each runs under +//! a kind of the scenario's choosing (`host`, `docker-plugin`): the configured +//! kind names the plugin, whatever the executable declares. A scenario //! configured here runs against its own server so the plugin settings and the //! environment it creates never leak into the shared session server. @@ -32,8 +34,8 @@ const DOCKER_IMAGE: &str = "buildpack-deps:noble"; pub(crate) enum Plugin { /// The driver's Host executable under the non-bundled `host` kind. Host, - /// The driver's Docker executable serving the bundled `docker` kind out - /// of process. + /// The driver's Docker executable under the non-bundled `docker-plugin` + /// kind: the same containers, reached over stdio. Docker, } @@ -41,7 +43,7 @@ impl Plugin { fn kind(self) -> &'static str { match self { Self::Host => "host", - Self::Docker => "docker", + Self::Docker => "docker-plugin", } } @@ -117,7 +119,7 @@ root = "{storage}" [server.auth] methods = ["dev-token"] -[server.sandbox.providers.docker] +[server.sandbox.providers.docker-plugin] path = "{path}" dev = true inherit_env = ["PATH", "HOME", "DOCKER_HOST", "DOCKER_CERT_PATH", "DOCKER_TLS_VERIFY"] diff --git a/lib/components/fabro-sandbox/src/driver.rs b/lib/components/fabro-sandbox/src/driver.rs index d6b54df04..12fb872d8 100644 --- a/lib/components/fabro-sandbox/src/driver.rs +++ b/lib/components/fabro-sandbox/src/driver.rs @@ -154,12 +154,12 @@ pub enum ConnectError { /// Connects the provider behind `kind`. /// -/// Bundled kinds return the in-process driver provider unless their entry -/// names a plugin executable, in which case the same kind is served out of -/// process. Any other kind launches the plugin named by `settings.plugin`. -/// A plugin returns a supervised handle that relaunches it after a crash -/// for new work only. Disabled entries are refused here so no caller has to -/// remember the policy check. +/// Bundled kinds return the in-process driver provider. Any other kind +/// launches the plugin named by `settings.plugin` and returns a supervised +/// handle that relaunches it after a crash for new work only. The +/// configured kind is fabro's name for whatever the executable serves; the +/// kind the plugin declares is not compared against it. Disabled entries +/// are refused here so no caller has to remember the policy check. pub async fn connect_provider( kind: &SandboxProviderKind, settings: &ServerSandboxProviderSettings, @@ -168,12 +168,6 @@ pub async fn connect_provider( if !settings.enabled { return Err(ConnectError::Disabled { kind: kind.clone() }); } - if let Some(plugin) = &settings.plugin { - return Ok(ConnectedProvider { - kind: kind.clone(), - provider: Arc::new(PluginBackedProvider::launch(kind, plugin).await?), - }); - } let driver = |source| ConnectError::Driver { kind: kind.clone(), source, @@ -208,7 +202,13 @@ pub async fn connect_provider( .map_err(driver)?, ) } - None => return Err(ConnectError::MissingPluginSettings { kind: kind.clone() }), + None => { + let plugin = settings + .plugin + .as_ref() + .ok_or_else(|| ConnectError::MissingPluginSettings { kind: kind.clone() })?; + Arc::new(PluginBackedProvider::launch(kind, plugin).await?) + } }; Ok(ConnectedProvider { kind: kind.clone(), diff --git a/lib/components/fabro-sandbox/src/reconnect.rs b/lib/components/fabro-sandbox/src/reconnect.rs index ce7d2162e..2eb655ac1 100644 --- a/lib/components/fabro-sandbox/src/reconnect.rs +++ b/lib/components/fabro-sandbox/src/reconnect.rs @@ -46,14 +46,7 @@ pub async fn reconnect_driver_for_run( event_callback: Option, ) -> Result { let runtime = &record.runtime; - let settings = access.settings_for(&record.provider); - // A bundled kind whose entry names a plugin executable is served out of - // process, exactly as a third-party kind is. - let plugin_served = settings - .as_ref() - .is_some_and(|settings| settings.plugin.is_some()); - let bundled = record.provider.bundled().filter(|_| !plugin_served); - match bundled { + match record.provider.bundled() { // A local sandbox is its working directory: rebuilding the handle // over that directory is the reconnect. The per-process Host // registry holds no state worth attaching to. @@ -107,7 +100,7 @@ pub async fn reconnect_driver_for_run( Ok(sandbox) } None => { - let settings = settings.with_context(|| { + let settings = access.settings_for(&record.provider).with_context(|| { format!( "sandbox provider `{}` is not configured; add [server.sandbox.providers.{}] to settings.toml", record.provider, record.provider diff --git a/lib/components/fabro-sandbox/tests/plugin_provider.rs b/lib/components/fabro-sandbox/tests/plugin_provider.rs index 1a5224489..ddabc2e02 100644 --- a/lib/components/fabro-sandbox/tests/plugin_provider.rs +++ b/lib/components/fabro-sandbox/tests/plugin_provider.rs @@ -90,21 +90,21 @@ async fn host_plugin_under_a_non_bundled_kind_creates_and_reattaches_by_persiste ); } +/// The configured kind is fabro's name for the executable it points at; the +/// plugin's own declared kind is information, not a gate. #[tokio::test] -async fn a_plugin_that_declares_another_kind_is_rejected() { +async fn the_configured_kind_names_the_plugin_whatever_it_declares() { let registry = tempfile::tempdir().expect("registry tempdir"); - let kind = SandboxProviderKind::try_new("e2b").expect("valid kind"); - let error = connect_provider( + let kind = SandboxProviderKind::try_new("host-alias").expect("valid kind"); + let connected = connect_provider( &kind, &host_plugin_settings(registry.path()), &ProviderConnectOptions::default(), ) .await - .err() - .expect("the host executable declares `host`, not `e2b`"); - let rendered = format!( - "{error}: {:?}", - std::error::Error::source(&error).map(ToString::to_string) - ); - assert!(rendered.contains("e2b"), "{rendered}"); + .expect("an aliased plugin launches"); + assert_eq!(connected.kind, kind); + // Fabro's handle on the plugin carries the configured name, so records, + // events, and errors all speak of the kind the operator wrote down. + assert_eq!(connected.provider.kind().as_str(), "host-alias"); } diff --git a/lib/foundation/fabro-config/src/resolve/server.rs b/lib/foundation/fabro-config/src/resolve/server.rs index 6e097a0fc..a4321c1a4 100644 --- a/lib/foundation/fabro-config/src/resolve/server.rs +++ b/lib/foundation/fabro-config/src/resolve/server.rs @@ -72,18 +72,20 @@ fn resolve_sandbox( layer: Option<&ServerSandboxLayer>, errors: &mut Vec, ) -> ServerSandboxSettings { - let _ = errors; let configured = layer .and_then(|sandbox| sandbox.providers.as_ref()) .map(|providers| &providers.entries); let mut entries = BTreeMap::new(); - // Bundled providers always have a policy entry; missing means enabled - // and served in-process. + // Bundled providers always have a policy entry; missing means enabled. for kind in SandboxProviderKind::bundled_kinds() { let layer = configured.and_then(|entries| entries.get(&kind)); + let path = format!("server.sandbox.providers.{kind}"); + if let Some(layer) = layer { + reject_plugin_fields_for_bundled(layer, &path, errors); + } entries.insert(kind, ServerSandboxProviderSettings { enabled: layer.and_then(|provider| provider.enabled).unwrap_or(true), - plugin: layer.and_then(plugin_settings), + plugin: None, }); } for (kind, layer) in configured.into_iter().flatten() { @@ -92,7 +94,14 @@ fn resolve_sandbox( } entries.insert(kind.clone(), ServerSandboxProviderSettings { enabled: layer.enabled.unwrap_or(true), - plugin: Some(plugin_settings(layer).unwrap_or_default()), + plugin: Some(SandboxPluginSettings { + path: layer.path.clone(), + sha256: layer.sha256.clone(), + dev: layer.dev.unwrap_or(false), + args: layer.args.clone().unwrap_or_default(), + env: layer.env.clone().unwrap_or_default(), + inherit_env: layer.inherit_env.clone().unwrap_or_default(), + }), }); } ServerSandboxSettings { @@ -100,33 +109,37 @@ fn resolve_sandbox( } } -/// Plugin launch settings when the entry names any. A bundled kind with -/// none runs in-process; a bundled kind with any runs out of process -/// through the driver's executable for that kind. -fn plugin_settings(layer: &ServerSandboxProviderLayer) -> Option { +fn reject_plugin_fields_for_bundled( + layer: &ServerSandboxProviderLayer, + path: &str, + errors: &mut Vec, +) { let ServerSandboxProviderLayer { enabled: _, - path, + path: plugin_path, sha256, dev, args, env, inherit_env, } = layer; - let any_set = path.is_some() - || sha256.is_some() - || dev.is_some() - || args.is_some() - || env.is_some() - || inherit_env.is_some(); - any_set.then(|| SandboxPluginSettings { - path: path.clone(), - sha256: sha256.clone(), - dev: dev.unwrap_or(false), - args: args.clone().unwrap_or_default(), - env: env.clone().unwrap_or_default(), - inherit_env: inherit_env.clone().unwrap_or_default(), - }) + let set = [ + ("path", plugin_path.is_some()), + ("sha256", sha256.is_some()), + ("dev", dev.is_some()), + ("args", args.is_some()), + ("env", env.is_some()), + ("inherit_env", inherit_env.is_some()), + ]; + for (field, is_set) in set { + if is_set { + errors.push(ResolveError::Invalid { + path: format!("{path}.{field}"), + reason: "bundled sandbox providers run in-process and take no plugin settings" + .to_string(), + }); + } + } } fn resolve_storage(layer: Option<&ServerStorageLayer>) -> ServerStorageSettings { diff --git a/lib/foundation/fabro-config/src/tests/resolve_server.rs b/lib/foundation/fabro-config/src/tests/resolve_server.rs index c066825a4..0fc982b98 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_server.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_server.rs @@ -285,8 +285,8 @@ E2B_API_URL = "https://api.e2b.example" } #[test] -fn server_sandbox_plugin_settings_on_a_bundled_provider_serve_it_out_of_process() { - let settings = ServerSettingsBuilder::from_toml( +fn server_sandbox_rejects_plugin_settings_on_bundled_providers() { + let err = ServerSettingsBuilder::from_toml( r#" _version = 1 @@ -295,33 +295,15 @@ methods = ["dev-token"] [server.sandbox.providers.docker] path = "/usr/local/bin/fabro-sandbox-docker" -dev = true -inherit_env = ["PATH", "DOCKER_HOST"] "#, ) - .expect("bundled providers accept plugin settings"); + .expect_err("bundled providers take no plugin settings"); - let providers = &settings.server.sandbox.providers; - let docker = providers - .get(&SandboxProviderKind::DOCKER) - .expect("docker entry"); - assert!(docker.enabled); - let plugin = docker.plugin.as_ref().expect("docker runs as a plugin"); - assert_eq!( - plugin.path.as_deref(), - Some("/usr/local/bin/fabro-sandbox-docker") - ); - assert!(plugin.dev); - assert_eq!(plugin.inherit_env, vec!["PATH", "DOCKER_HOST"]); assert!( - providers - .get(&SandboxProviderKind::LOCAL) - .expect("local entry") - .plugin - .is_none(), - "an entry without plugin keys stays in-process" + err.to_string() + .contains("server.sandbox.providers.docker.path"), + "unexpected error: {err}" ); - assert_eq!(providers.enabled_plugins().count(), 1); } #[test]