From a1a98c69d0d47046e26f6f715de2759c2862aa1d Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 25 Sep 2026 12:31:37 -0400 Subject: [PATCH] fix: address review of the in-process sandbox providers - Keep plugin-era Daytona lease fingerprints: read only DAYTONA_API_URL and DAYTONA_ORGANIZATION_ID (no URL alias, no placement target), and stop forwarding DAYTONA_SERVER_URL and DAYTONA_TARGET to the worker. - Take the Docker fingerprint and network from this process's DOCKER_HOST, the endpoint the Docker client actually connects to; make the provider configuration's fields private. - Return an error instead of panicking when Petri supplies no Host registry. - Run deletion reads the Daytona key only for a Daytona run, and a forced or restarted delete goes on when the secret store fails, as it does for every other prune failure. - Stop putting DAYTONA_API_KEY in the worker's environment; the worker reads it from the vault. Give the worker's Daytona client the shared HTTP client. - Fork, rewind and retry no longer read the vault: a fork acquires no sandbox. - Remove the dead worker plugin forwarding and document that runs execute only on the built-in providers. - Build every Petri runtime through providers::standard_runtime or bare_runtime, with a Clippy lint against Runtime::standard/bare. - Share the Docker require-or-skip policy in fabro-test, tighten the Host scope assertion. Co-Authored-By: Claude Opus 5.5 --- clippy.toml | 2 + .../administration/server-configuration.mdx | 9 +- .../src/commands/run/petri_worker.rs | 11 +- lib/apps/fabro-cli/tests/it/scenario/petri.rs | 20 +- .../fabro-cli/tests/it/scenario/petri_fork.rs | 2 +- .../fabro-cli/tests/it/workflow/docker.rs | 26 +-- lib/apps/fabro-server/src/server.rs | 72 ++++--- .../src/server/handler/lineage.rs | 2 - lib/apps/fabro-server/src/server/tests.rs | 70 ------- lib/apps/fabro-server/src/spawn_env.rs | 187 ++---------------- lib/apps/fabro-server/src/worker_runtime.rs | 12 +- lib/components/fabro-petri/src/fork.rs | 12 +- lib/components/fabro-petri/src/providers.rs | 130 ++++++------ lib/components/fabro-petri/src/prune.rs | 5 +- lib/components/fabro-petri/src/runtime.rs | 12 +- .../fabro-petri/tests/projection.rs | 9 +- lib/components/fabro-petri/tests/runs.rs | 12 +- lib/foundation/fabro-test/src/lib.rs | 44 +++-- 18 files changed, 205 insertions(+), 432 deletions(-) diff --git a/clippy.toml b/clippy.toml index 066d42b88..0a9a8af84 100644 --- a/clippy.toml +++ b/clippy.toml @@ -31,6 +31,8 @@ disallowed-methods = [ { path = "reqwest::blocking::Client::new", reason = "Use fabro_http::blocking_http_client() or fabro_http::blocking_test_http_client()", allow-invalid = true }, { path = "reqwest::blocking::Client::builder", reason = "Use fabro_http::BlockingHttpClientBuilder::new()", allow-invalid = true }, { path = "reqwest::get", reason = "Build a fabro_http client and send the request explicitly", allow-invalid = true }, + { path = "runtime::Runtime::standard", reason = "Use fabro_petri::providers::standard_runtime, which installs the built-in in-process sandbox providers; without them Petri launches provider plugins, which a release build cannot verify", allow-invalid = true }, + { path = "runtime::Runtime::bare", reason = "Use fabro_petri::providers::bare_runtime, which installs the built-in in-process sandbox providers; without them Petri launches provider plugins, which a release build cannot verify", allow-invalid = true }, { path = "fabro_types::settings::interp::InterpString::as_source", reason = "Returns the unresolved template source, which leaks {{ ... }} tokens as literal text downstream. Resolve via resolve()/resolve_with() or substitute via substitute_with() instead; document intentional raw-source access (serialization, error messages, deliberate source preservation) with #[expect(clippy::disallowed_methods, reason = \"...\")]", allow-invalid = true }, ] disallowed-types = [ diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 9e34237ea..12e7b03ce 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -200,12 +200,9 @@ 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. +The server launches the plugin to reach a sandbox after the fact (the sandbox tab, files, +terminal, Ask Fabro). Runs execute only on the built-in providers for now, so a run's worker +never launches a plugin and receives none of these settings. ```toml title="settings.toml" [server.sandbox.providers.e2b] diff --git a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs index 0b35ef58d..6534ac3a7 100644 --- a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs +++ b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs @@ -614,11 +614,12 @@ async fn runtime_spec( None } }; - let daytona = vault - .read() - .await - .get(EnvVars::DAYTONA_API_KEY) - .map(|key| DaytonaCredentials::from_api_key(key.to_owned(), crate::process_env_var)); + let daytona = vault.read().await.get(EnvVars::DAYTONA_API_KEY).map(|key| { + // The same shared client the server attaches, so the worker's + // Daytona calls take the server's proxy and CA policy. + DaytonaCredentials::from_api_key(key.to_owned(), crate::process_env_var) + .with_http_client(fabro_http::http_client().ok()) + }); Ok(RuntimeSpec { sandbox: SandboxProviderConfig::from_lookup(daytona, crate::process_env_var), settings_toml: None, diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri.rs b/lib/apps/fabro-cli/tests/it/scenario/petri.rs index 5324bc76a..44e709fe2 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri.rs @@ -1730,8 +1730,12 @@ async fn a_delete_right_after_the_run_reads_ended_is_accepted() { server.shutdown(); } -/// The release build must acquire and prune a real Host scope without any -/// plugin executable or checksum. This also runs in CI's release profile. +/// A Host run acquires and prunes a real scope with no plugin executable +/// anywhere the worker or server would look. The server is also handed +/// legacy Host plugin settings, which its prune must ignore; the worker +/// never receives them (its environment allowlist drops them). The release +/// workflow runs the suite in a release build, where no plugin checksum is +/// pinned, so this is the check that a release can run a sandbox at all. #[tokio::test(flavor = "multi_thread")] async fn built_in_host_runs_and_prunes_without_plugins() { let context = test_context!(); @@ -1764,12 +1768,14 @@ async fn built_in_host_runs_and_prunes_without_plugins() { wait_for_success(&server, &run_id).await; let run_dir = server.petri_run_dir(&run_id); let scopes = run_dir.join("scopes"); - let scope = std::fs::read_dir(&scopes) + let entries = std::fs::read_dir(&scopes) .expect("the real Host scope exists") - .next() - .expect("one scope") - .expect("the scope reads") - .path(); + .map(|entry| entry.expect("the scope entry reads").path()) + .collect::>(); + let [scope] = entries.as_slice() else { + panic!("expected exactly one Host scope, found {entries:?}"); + }; + let scope = scope.clone(); assert_eq!( std::fs::read_to_string(scope.join("work/built-in.txt")) .expect("the worker wrote its file"), diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri_fork.rs b/lib/apps/fabro-cli/tests/it/scenario/petri_fork.rs index 518214b75..b1b8ab053 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri_fork.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri_fork.rs @@ -1,5 +1,5 @@ //! Fork, rewind, retry and the timeline over Petri runs, through a real -//! server and its worker subprocess (the integration plan's F5.1). +//! server and its worker subprocess. //! //! The harness is `petri.rs`'s: a foreground server on disk storage, a run //! created and started with `fabro run --detach`, executed by the worker diff --git a/lib/apps/fabro-cli/tests/it/workflow/docker.rs b/lib/apps/fabro-cli/tests/it/workflow/docker.rs index f6028f9ca..a8f72ef43 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/docker.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/docker.rs @@ -7,17 +7,16 @@ #![expect( clippy::disallowed_methods, - reason = "test setup reads the process environment for its opt-in gate and probes Docker synchronously" + reason = "a failed setup reads the isolated server's log synchronously" )] #![expect( clippy::print_stderr, - reason = "a skipped scenario says why on the test's stderr" + reason = "a failed setup prints the server log tail on the test's stderr" )] use std::path::Path; -use std::process::{Command, Stdio}; -use fabro_test::{REQUIRE_SANDBOX_BACKENDS, TestContext, expect_reqwest_status}; +use fabro_test::{TestContext, expect_reqwest_status}; use serde_json::json; use crate::cmd::support::server_endpoint; @@ -30,13 +29,7 @@ pub(crate) const ENVIRONMENT: &str = "docker"; /// [`DOCKER_IMAGE`]. Returns the environment id, or `None` when the /// prerequisites are missing and the test should skip. pub(crate) fn configure(context: &mut TestContext) -> Option<&'static str> { - let required = std::env::var_os(REQUIRE_SANDBOX_BACKENDS).is_some(); - if !docker_image_available() { - assert!( - !required, - "{REQUIRE_SANDBOX_BACKENDS} is set but no Docker daemon with {DOCKER_IMAGE} is available" - ); - eprintln!("skipping: no Docker daemon with {DOCKER_IMAGE}"); + if !fabro_test::docker_image_available(DOCKER_IMAGE) { return None; } @@ -56,15 +49,6 @@ methods = ["dev-token"] Some(ENVIRONMENT) } -fn docker_image_available() -> bool { - Command::new("docker") - .args(["image", "inspect", DOCKER_IMAGE]) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .is_ok_and(|status| status.success()) -} - fn toml_path(path: &Path) -> String { path.display().to_string().replace('\\', "/") } @@ -106,7 +90,7 @@ fn create_environment(storage_dir: &Path) { } /// Run a scenario; when it fails, print the isolated server's log first, since -/// the worker's stderr (and so a plugin's launch failure) lands only there +/// the worker's stderr (and so a sandbox provider failure) lands only there /// and the server root is removed when the context drops. pub(crate) fn run_with_server_log(context: &TestContext, scenario: impl FnOnce()) { let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(scenario)); diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index c1aea9c4a..2a6046844 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -151,7 +151,7 @@ use crate::sandbox_access::{ SandboxInventory, }; use crate::server_secrets::ServerSecrets; -use crate::spawn_env::{self, apply_render_graph_env}; +use crate::spawn_env::apply_render_graph_env; use crate::worker_control::{ LocalWorkerControlBus, WORKER_CONTROL_ACK_WAIT, WorkerControlAcks, WorkerControlBus, WorkerControlBusError, @@ -1558,22 +1558,18 @@ impl AppState { ) } - /// [`Self::sandbox_provider_config`] with the Daytona key read from the - /// vault, for server-side fork and prune; a secret store failure is a - /// 500. + /// [`Self::sandbox_provider_config`] for a server-side prune of + /// `provider`'s sandboxes, with the Daytona key read from the vault only + /// when `provider` is Daytona. pub(crate) async fn load_sandbox_provider_config( &self, - ) -> Result { - let daytona_api_key = self - .vault_secret(EnvVars::DAYTONA_API_KEY) - .await - .map_err(|err| { - error!(error = ?err, "Loading sandbox credentials failed"); - ApiError::new( - StatusCode::INTERNAL_SERVER_ERROR, - "secret store operation failed", - ) - })?; + provider: &SandboxProviderKind, + ) -> Result { + let daytona_api_key = if *provider == SandboxProviderKind::DAYTONA { + self.vault_secret(EnvVars::DAYTONA_API_KEY).await? + } else { + None + }; Ok(self.sandbox_provider_config(daytona_api_key)) } @@ -2907,7 +2903,27 @@ async fn delete_run_sandbox_resource( .run_scratch(&id) .root() .join("petri"); - let sandbox = state.load_sandbox_provider_config().await?; + let sandbox = match state.load_sandbox_provider_config(&record.provider).await { + Ok(sandbox) => sandbox, + // A forced or restarted delete goes on without the sandboxes, as it + // does for any other prune failure below. + Err(err) if force || delete_started => { + tracing::warn!( + run_id = %id, + provider = %record.provider, + error = ?err, + "Skipping the sandbox prune after loading sandbox credentials failed during run deletion" + ); + return Ok(SandboxDeleteOutcome::Cleaned); + } + Err(err) => { + error!(error = ?err, "Loading sandbox credentials failed"); + return Err(ApiError::new( + StatusCode::INTERNAL_SERVER_ERROR, + "secret store operation failed", + )); + } + }; let report = prune::prune(PruneRequest { sandbox, run_id: id.to_string(), @@ -3811,7 +3827,6 @@ 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 = @@ -3850,11 +3865,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(), - sandbox_plugin_env: spawn_env::sandbox_plugin_env( - &state.server_settings().server.sandbox.providers, - ), }) } @@ -4171,21 +4182,9 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { return; } - // 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 { + // The worker reads the Daytona key from the vault itself; only the + // GitHub App key crosses on its command. + let github_app_private_key = match state.vault_secret(EnvVars::GITHUB_APP_PRIVATE_KEY).await { Ok(value) => value, Err(err) => { fail_run_before_execution( @@ -4209,7 +4208,6 @@ 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/handler/lineage.rs b/lib/apps/fabro-server/src/server/handler/lineage.rs index bd4ad38ac..05fde2318 100644 --- a/lib/apps/fabro-server/src/server/handler/lineage.rs +++ b/lib/apps/fabro-server/src/server/handler/lineage.rs @@ -281,7 +281,6 @@ async fn fork_at( .await .map_err(|err| fork_error(&err))?; - let sandbox = state.load_sandbox_provider_config().await?; let new_run_id = RunId::new(); let storage = Storage::new(state.server_storage_dir()); let source_run_dir = storage.run_scratch(&id).root().to_path_buf(); @@ -300,7 +299,6 @@ async fn fork_at( .map_err(workflow_operation_error)?; let seeded = petri_fork::fork(ForkRequest { - sandbox, source: id, fork: new_run_id, source_run_dir: source_run_dir.join("petri"), diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 94a5ab8db..30c0af478 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -2274,7 +2274,6 @@ 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); @@ -2289,74 +2288,6 @@ fn worker_command_forwards_github_app_private_key_from_vault() { ); } -/// 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()) - ); -} - -/// 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() { @@ -2559,7 +2490,6 @@ 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 cf7c2669d..333a48723 100644 --- a/lib/apps/fabro-server/src/spawn_env.rs +++ b/lib/apps/fabro-server/src/spawn_env.rs @@ -1,7 +1,6 @@ use std::ffi::OsString; use fabro_static::EnvVars; -use fabro_types::settings::server::ServerSandboxProvidersSettings; use tokio::process::Command; const WORKER_ENV_ALLOWLIST: &[&str] = &[ @@ -51,9 +50,8 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ EnvVars::AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, EnvVars::AWS_CONTAINER_CREDENTIALS_FULL_URI, EnvVars::AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, - // Preserve generic plugin configuration. Built-in providers run in process - // and never receive executable paths or checksum overrides. - EnvVars::PETRI_SANDBOX_PLUGIN_DEV, + // Petri's sandbox settings the worker's in-process providers read. No + // plugin settings cross: the worker never launches a provider plugin. EnvVars::PETRI_SANDBOX_DOCKER_HOST_ADDRESS, EnvVars::PETRI_SANDBOX_ACTION_HOST_IMAGE, // The Docker daemon selection: the worker's Docker provider reads these @@ -69,8 +67,6 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ // Daytona's non-secret selection. The worker reads the API key from // the vault and supplies it explicitly to the in-process provider. EnvVars::DAYTONA_API_URL, - EnvVars::DAYTONA_SERVER_URL, - EnvVars::DAYTONA_TARGET, 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. @@ -82,55 +78,9 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ const RENDER_GRAPH_ENV_ALLOWLIST: &[&str] = &[EnvVars::PATH, EnvVars::HOME, EnvVars::TMPDIR]; -/// 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 -/// third-party 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 +/// The worker's environment: the allowlisted ambient variables only. +pub(crate) fn apply_worker_env(cmd: &mut Command) { + apply_allowlist(cmd, WORKER_ENV_ALLOWLIST, &process_env_var_os); } pub(crate) fn apply_render_graph_env(cmd: &mut Command) { @@ -160,15 +110,7 @@ mod tests { use std::ffi::OsString; use std::path::Path; - 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, - }; + use super::{RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist}; fn env_command() -> tokio::process::Command { assert!(Path::new("/usr/bin/env").exists()); @@ -292,16 +234,10 @@ mod tests { Some("xterm-256color") ); assert_eq!(actual.get("NO_COLOR").map(String::as_str), Some("1")); - // Built-in plugin paths and pins never reach the worker. - for kind in ["HOST", "DOCKER", "DAYTONA"] { - for suffix in ["PLUGIN", "SHA256"] { - assert!(!actual.contains_key(&format!("PETRI_SANDBOX_{kind}_{suffix}"))); - } - } - assert_eq!( - actual.get("PETRI_SANDBOX_PLUGIN_DEV").map(String::as_str), - Some("1") - ); + // No plugin setting reaches the worker: it never launches a + // provider plugin. + assert!(!actual.contains_key("PETRI_SANDBOX_HOST_PLUGIN")); + assert!(!actual.contains_key("PETRI_SANDBOX_PLUGIN_DEV")); // The Docker daemon selection crosses whole, so the worker's Docker // provider drives the daemon the server uses. assert_eq!( @@ -338,11 +274,11 @@ mod tests { actual.get("DAYTONA_ORGANIZATION_ID").map(String::as_str), Some("org-1") ); - assert_eq!( - actual.get("DAYTONA_SERVER_URL").map(String::as_str), - Some("https://daytona-alias.internal/api") - ); - assert_eq!(actual.get("DAYTONA_TARGET").map(String::as_str), Some("us")); + // The URL alias and placement target never reached the Daytona + // plugin, so they stay out and plugin-era leases keep their + // fingerprint. + assert!(!actual.contains_key("DAYTONA_SERVER_URL")); + assert!(!actual.contains_key("DAYTONA_TARGET")); 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")); @@ -382,99 +318,6 @@ 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, - /// while built-in paths and pins and disabled third-party plugins never - /// cross. - #[tokio::test] - async fn third_party_plugins_reach_the_worker_and_built_ins_never_do() { - 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 mut env = HashMap::from([("PATH".to_string(), "/bin".to_string())]); - for kind in ["HOST", "DOCKER", "DAYTONA"] { - let lower = kind.to_ascii_lowercase(); - env.insert( - format!("PETRI_SANDBOX_{kind}_PLUGIN"), - format!("/ambient/sandbox-driver-{lower}"), - ); - env.insert( - format!("PETRI_SANDBOX_{kind}_SHA256"), - "invalid-pin".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" - ); - for kind in ["HOST", "DOCKER", "DAYTONA"] { - for suffix in ["PLUGIN", "SHA256"] { - assert!(!actual.contains_key(&format!("PETRI_SANDBOX_{kind}_{suffix}"))); - } - } - 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 2f76c2409..7b4fa39b6 100644 --- a/lib/apps/fabro-server/src/worker_runtime.rs +++ b/lib/apps/fabro-server/src/worker_runtime.rs @@ -48,16 +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, - /// 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 { @@ -105,7 +98,7 @@ impl LocalWorkerRuntime { .stdout(worker_stdout) .stderr(Stdio::piped()); - apply_worker_env(&mut cmd, &spec.sandbox_plugin_env); + apply_worker_env(&mut cmd); if let Some(level) = spec.fabro_log.as_deref() { cmd.env(EnvVars::FABRO_LOG, level); } @@ -116,9 +109,6 @@ 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/components/fabro-petri/src/fork.rs b/lib/components/fabro-petri/src/fork.rs index 664b62b80..44f551aa8 100644 --- a/lib/components/fabro-petri/src/fork.rs +++ b/lib/components/fabro-petri/src/fork.rs @@ -1,6 +1,5 @@ //! Forking a Fabro run at a checkpoint: the seam over Petri's -//! `host::fork_from` that rewind, fork and retry are built on (the -//! integration plan's F5.1). +//! `host::fork_from` that rewind, fork and retry are built on. //! //! Fabro's checkpoint record ties a Petri position `(execution, firing)` to //! a Git commit. A fork seeds a new run from the source's records up to such @@ -48,8 +47,8 @@ use petri_execution::{ Access, CoordinatorEvent, ExecutionId, InvocationId, RunKey, RunStore, StoreError as CoordinatorStoreError, }; +use petri_runtime::RunOptions; use petri_runtime::ir::FiringId; -use petri_runtime::{RunOptions, Runtime}; use petri_store::StoreError; use tokio::fs; use tokio::process::Command; @@ -63,8 +62,6 @@ use crate::providers::{self, SandboxProviderConfig}; /// One fork to seed. pub struct ForkRequest { - /// The provider configuration used by this server-side operation. - pub sandbox: SandboxProviderConfig, /// The run whose records are copied. pub source: RunId, /// The new run's id: its Petri run key and its own run scratch. @@ -180,8 +177,9 @@ pub async fn fork(request: ForkRequest) -> Result { let mut options = RunOptions::new(&request.fork_run_dir); options.run_key = Some(fork_key.clone()); - let runtime = Runtime::standard() - .in_process_providers(providers::built_in_providers(&request.sandbox)) + // A fork only copies records and acquires no sandbox, so it needs no + // provider configuration. + let runtime = providers::standard_runtime(&SandboxProviderConfig::default()) .options(options) .store(Arc::clone(&request.store)); let forked = host::fork_from(&runtime, &*source_logs, request.position, ForkOptions { diff --git a/lib/components/fabro-petri/src/providers.rs b/lib/components/fabro-petri/src/providers.rs index 662c94bb9..8a68eb3b4 100644 --- a/lib/components/fabro-petri/src/providers.rs +++ b/lib/components/fabro-petri/src/providers.rs @@ -4,13 +4,18 @@ //! the vault. Factories connect lazily per run; a Host-only run needs neither //! Docker nor Daytona. Host registry ownership stays with Petri: server //! attach uses an observer instead of these factories. +//! +//! Every Petri runtime Fabro builds starts from [`standard_runtime`] or +//! [`bare_runtime`], so none falls back to launching a provider plugin, +//! which a release build cannot verify. +use std::path::Path; use std::sync::Arc; use async_trait::async_trait; use fabro_static::EnvVars; use petri_runtime::{ - InProcessProviders, ProviderContext, ProviderFactory, ProviderNetwork, fingerprint, + InProcessProviders, ProviderContext, ProviderFactory, ProviderNetwork, Runtime, fingerprint, }; use sandbox_driver::{AuthError, Error, ProviderKind, SandboxProvider}; use sandbox_driver_daytona::{DaytonaConfig, DaytonaProvider}; @@ -40,13 +45,14 @@ impl DaytonaCredentials { /// Credentials for a vault API key, with the control-plane URL and /// organization taken from `lookup` (server configuration). Nothing is /// read implicitly. + /// + /// These are the two settings the Daytona plugin read in the worker, so + /// a lease it recorded keeps its fingerprint: no URL alias and no + /// placement target, neither of which reached the plugin. pub fn from_api_key(api_key: String, lookup: impl Fn(&str) -> Option) -> Self { Self::new(api_key) - .with_api_url( - lookup(EnvVars::DAYTONA_API_URL).or_else(|| lookup(EnvVars::DAYTONA_SERVER_URL)), - ) + .with_api_url(lookup(EnvVars::DAYTONA_API_URL)) .with_organization_id(lookup(EnvVars::DAYTONA_ORGANIZATION_ID)) - .with_target(lookup(EnvVars::DAYTONA_TARGET)) } /// The control-plane URL; Daytona's public API when `None`. @@ -62,13 +68,6 @@ impl DaytonaCredentials { self } - /// The configured Daytona placement target, kept unset when omitted. - #[must_use] - pub fn with_target(mut self, target: Option) -> Self { - self.0.target = target; - self - } - /// A shared HTTP client; tests pass a no-proxy client here. #[must_use] pub fn with_http_client(mut self, http_client: Option) -> Self { @@ -88,7 +87,6 @@ impl std::fmt::Debug for DaytonaCredentials { f.debug_struct("DaytonaCredentials") .field("api_url", &self.0.api_url) .field("organization_id", &self.0.organization_id) - .field("target", &self.0.target) .finish_non_exhaustive() } } @@ -98,20 +96,30 @@ impl std::fmt::Debug for DaytonaCredentials { /// configuration never connects to a backend or requires a credential. #[derive(Clone, Debug, Default)] pub struct SandboxProviderConfig { - pub docker_host: Option, - pub docker_host_address: Option, - pub daytona: Option, + docker_host: Option, + docker_host_address: Option, + daytona: Option, } impl SandboxProviderConfig { - /// Snapshot the Docker network/fingerprint selection from the same - /// environment the Docker client uses. Daytona credentials are explicit. + /// The configuration for `daytona`'s credentials, with how a remote + /// Docker daemon's containers reach this machine from `lookup`. + /// + /// The Docker endpoint is read from this process's `DOCKER_HOST`, never + /// from `lookup`: the Docker client connects to the daemon that + /// variable names, so the lease fingerprint and network name the daemon + /// the sandboxes are actually created on. pub fn from_lookup( daytona: Option, lookup: impl Fn(&str) -> Option, ) -> Self { + #[expect( + clippy::disallowed_methods, + reason = "the Docker client reads DOCKER_HOST from this process; the fingerprint must name the same daemon" + )] + let docker_host = std::env::var(EnvVars::DOCKER_HOST).ok(); Self { - docker_host: lookup(EnvVars::DOCKER_HOST), + docker_host, docker_host_address: lookup(EnvVars::PETRI_SANDBOX_DOCKER_HOST_ADDRESS) .filter(|value| !value.trim().is_empty()), daytona, @@ -119,6 +127,28 @@ impl SandboxProviderConfig { } } +/// Petri's standard runtime with Fabro's built-in providers installed. +#[must_use] +pub fn standard_runtime(config: &SandboxProviderConfig) -> Runtime { + #[expect( + clippy::disallowed_methods, + reason = "the one place a standard runtime is built, with the built-in providers installed" + )] + let runtime = Runtime::standard(); + runtime.in_process_providers(built_in_providers(config)) +} + +/// Petri's bare runtime with Fabro's built-in providers installed. +#[must_use] +pub fn bare_runtime(config: &SandboxProviderConfig) -> Runtime { + #[expect( + clippy::disallowed_methods, + reason = "the one place a bare runtime is built, with the built-in providers installed" + )] + let runtime = Runtime::bare(); + runtime.in_process_providers(built_in_providers(config)) +} + /// The Docker connection used by both the server and Petri. The driver reads /// the caller process's Docker endpoint/TLS environment; health is checked /// by the caller so diagnostics can report an unavailable daemon. @@ -142,7 +172,7 @@ pub async fn connect_daytona( /// One lazy factory per built-in kind. Missing Daytona credentials fail /// only when a Daytona scope is acquired, never for admission or a Host run. -pub fn built_in_providers(config: &SandboxProviderConfig) -> InProcessProviders { +fn built_in_providers(config: &SandboxProviderConfig) -> InProcessProviders { InProcessProviders::new() .with(Arc::new(HostFactory)) .with(Arc::new(DockerFactory { @@ -160,12 +190,10 @@ impl ProviderFactory for HostFactory { "host" } + /// An empty registry path when Petri supplies none, as the plugin + /// recorded it. fn fingerprint_seed(&self, context: &ProviderContext) -> String { - fingerprint::host( - context - .host_registry() - .expect("Petri supplies the Host registry"), - ) + fingerprint::host(context.host_registry().unwrap_or(Path::new(""))) } fn network(&self) -> ProviderNetwork { @@ -176,9 +204,12 @@ impl ProviderFactory for HostFactory { &self, context: &ProviderContext, ) -> sandbox_driver::Result> { - let registry = context - .host_registry() - .expect("Petri supplies the Host registry"); + let registry = context.host_registry().ok_or_else(|| { + Error::invalid_spec( + "host_registry", + "Petri supplied no Host registry for this run", + ) + })?; Ok(Arc::new(HostProvider::with_registry(registry).await?)) } } @@ -224,7 +255,7 @@ impl DaytonaFactory { fingerprint::daytona( config.and_then(|config| config.api_url.as_deref()), config.and_then(|config| config.organization_id.as_deref()), - config.and_then(|config| config.target.as_deref()), + None, ) } } @@ -239,13 +270,6 @@ impl ProviderFactory for DaytonaFactory { self.seed() } - fn region(&self) -> Option<&str> { - self.0 - .as_ref() - .and_then(|credentials| credentials.config().target.as_deref()) - .filter(|region| !region.is_empty()) - } - fn network(&self) -> ProviderNetwork { ProviderNetwork::none() } @@ -277,21 +301,16 @@ mod tests { "docker:unix:///var/run/docker.sock", ), ] { - let config = SandboxProviderConfig::from_lookup(None, |name| { - (name == EnvVars::DOCKER_HOST) - .then(|| host.map(str::to_owned)) - .flatten() - }); let factory = DockerFactory { - host: config.docker_host, - host_address: config.docker_host_address, + host: host.map(str::to_owned), + host_address: None, }; assert_eq!(factory.seed(), expected); } } #[test] - fn daytona_fingerprint_keeps_unset_values_and_the_configured_target() { + fn daytona_fingerprint_keeps_the_plugin_seed() { let unset = DaytonaCredentials::from_api_key("test-key".to_string(), |_| None); assert_eq!(DaytonaFactory(Some(unset)).seed(), "daytona:::"); let configured = @@ -303,34 +322,33 @@ mod tests { _ => None, }); let factory = DaytonaFactory(Some(configured)); - assert_eq!(factory.seed(), "daytona:https://daytona.example:org-1:us"); - assert_eq!(factory.region(), Some("us")); - let blank = - DaytonaCredentials::new("test-key".to_string()).with_target(Some(String::new())); - assert_eq!(DaytonaFactory(Some(blank)).region(), None); + assert_eq!(factory.seed(), "daytona:https://daytona.example:org-1:"); + assert_eq!(factory.region(), None); } #[test] - fn daytona_url_alias_and_http_client_survive_the_shared_configuration() { + fn daytona_configuration_ignores_the_url_alias_and_keeps_the_http_client() { let credentials = DaytonaCredentials::from_api_key("test-key".to_string(), |name| { (name == EnvVars::DAYTONA_SERVER_URL).then(|| "https://alias.example".to_string()) }) .with_http_client(Some(fabro_test::test_http_client())); - assert_eq!( - credentials.config().api_url.as_deref(), - Some("https://alias.example") - ); + assert_eq!(credentials.config().api_url, None); assert!(credentials.config().http_client.is_some()); } #[test] - fn provider_configuration_debug_never_prints_the_key() { + fn provider_configuration_keeps_the_key_but_never_prints_it() { let key = "dtn_test_sensitive_value"; let config = SandboxProviderConfig::from_lookup( - Some(DaytonaCredentials::from_api_key(key.to_string(), |_| None)), + Some(DaytonaCredentials::from_api_key(key.to_string(), |name| { + (name == EnvVars::DAYTONA_ORGANIZATION_ID).then(|| "org-1".to_string()) + })), |_| None, ); + let daytona = config.daytona.as_ref().expect("the credentials are kept"); + assert_eq!(daytona.config().api_key.as_deref(), Some(key)); let rendered = format!("{config:?}"); assert!(!rendered.contains(key)); + assert!(rendered.contains("org-1")); } } diff --git a/lib/components/fabro-petri/src/prune.rs b/lib/components/fabro-petri/src/prune.rs index ee2f2b5e2..8b9ccae48 100644 --- a/lib/components/fabro-petri/src/prune.rs +++ b/lib/components/fabro-petri/src/prune.rs @@ -26,7 +26,7 @@ use fabro_types::SandboxProviderKind; pub use petri_execution::prune::PruneReport; use petri_execution::prune::{self as petri_prune}; use petri_execution::{RunKey, RunStore}; -use petri_runtime::{RunOptions, Runtime}; +use petri_runtime::RunOptions; use crate::engine; use crate::providers::{self, SandboxProviderConfig}; @@ -72,8 +72,7 @@ pub async fn prune(request: PruneRequest) -> Result { options.run_key = Some(RunKey::new(request.run_id.as_str())); options.retention = engine::RETENTION; options.sandbox.backend = backend; - let runtime = Runtime::bare() - .in_process_providers(providers::built_in_providers(&request.sandbox)) + let runtime = providers::bare_runtime(&request.sandbox) .store(request.store) .options(options); petri_prune::prune(&runtime) diff --git a/lib/components/fabro-petri/src/runtime.rs b/lib/components/fabro-petri/src/runtime.rs index 937109471..a19603f9b 100644 --- a/lib/components/fabro-petri/src/runtime.rs +++ b/lib/components/fabro-petri/src/runtime.rs @@ -70,13 +70,11 @@ impl RuntimeSpec { /// registry: only execution swaps in the stubs. #[must_use] pub fn runtime(&self, for_execution: bool) -> Runtime { - let mut runtime = Runtime::standard() - .in_process_providers(providers::built_in_providers(&self.sandbox)) - .frontend( - Fabro::new() - .with_settings_toml(self.settings_toml.clone()) - .with_mcp_catalog_toml(self.mcp_catalog_toml.clone()), - ); + let mut runtime = providers::standard_runtime(&self.sandbox).frontend( + Fabro::new() + .with_settings_toml(self.settings_toml.clone()) + .with_mcp_catalog_toml(self.mcp_catalog_toml.clone()), + ); if let Some(client) = &self.model_client { runtime = runtime.capability(PebbleClient(client.clone())); } diff --git a/lib/components/fabro-petri/tests/projection.rs b/lib/components/fabro-petri/tests/projection.rs index 7597e2def..8e9a9f9b6 100644 --- a/lib/components/fabro-petri/tests/projection.rs +++ b/lib/components/fabro-petri/tests/projection.rs @@ -36,10 +36,10 @@ use fabro_types::{ }; use petri_execution::host::{self, HostRun}; use petri_frontend_fabro::Fabro; +use petri_runtime::RunOptions; use petri_runtime::executor::Retention; use petri_runtime::frontend::CompileInputs; use petri_runtime::ir::RunStatus as PetriRunStatus; -use petri_runtime::{RunOptions, Runtime}; use petri_store::{RunKey, RunStore}; use tokio::fs; use tokio::time::sleep; @@ -206,11 +206,8 @@ async fn run_workflow( workflow: &Path, stubs: bool, ) { - let runtime = Runtime::standard() - .in_process_providers(providers::built_in_providers( - &SandboxProviderConfig::default(), - )) - .frontend(Fabro::new()); + let runtime = + providers::standard_runtime(&SandboxProviderConfig::default()).frontend(Fabro::new()); let runtime = if stubs { petri_attractor_steps::register_stubs(runtime) } else { diff --git a/lib/components/fabro-petri/tests/runs.rs b/lib/components/fabro-petri/tests/runs.rs index 5c56aa83b..fe2e164e1 100644 --- a/lib/components/fabro-petri/tests/runs.rs +++ b/lib/components/fabro-petri/tests/runs.rs @@ -115,11 +115,7 @@ async fn the_hello_bundle_runs_in_memory_on_the_stub_registry() { .await; let store = Arc::new(MemoryRunStore::new()); let rt = petri_attractor_steps::register_stubs( - Runtime::standard() - .in_process_providers(providers::built_in_providers( - &SandboxProviderConfig::default(), - )) - .frontend(Fabro::new()), + providers::standard_runtime(&SandboxProviderConfig::default()).frontend(Fabro::new()), ) .store(store.clone()) .options(run_options(&root.path().join("run"), "hello")); @@ -144,11 +140,7 @@ async fn a_command_workflow_runs_on_the_host_sandbox() { .await; let store = Arc::new(MemoryRunStore::new()); let rt = petri_attractor_steps::register( - Runtime::standard() - .in_process_providers(providers::built_in_providers( - &SandboxProviderConfig::default(), - )) - .frontend(Fabro::new()), + providers::standard_runtime(&SandboxProviderConfig::default()).frontend(Fabro::new()), ) .store(store.clone()) .options(run_options(&root.path().join("run"), "command")); diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index e192452c5..f0514e84f 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -162,25 +162,47 @@ pub const REQUIRE_SANDBOX_BACKENDS: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; /// A reachable Docker daemon. When [`REQUIRE_SANDBOX_BACKENDS`] is set, a /// missing daemon fails the test instead of skipping it. #[must_use] +pub fn docker_available() -> bool { + sandbox_backend_available( + docker_succeeds(&["version", "--format", "{{.Server.Version}}"]), + "no Docker daemon answers", + ) +} + +/// A Docker daemon that already holds `image`, under the same +/// fail-or-skip policy as [`docker_available`]. +#[must_use] +pub fn docker_image_available(image: &str) -> bool { + sandbox_backend_available( + docker_succeeds(&["image", "inspect", image]), + &format!("no Docker daemon with {image}"), + ) +} + +fn docker_succeeds(args: &[&str]) -> bool { + std::process::Command::new("docker") + .args(args) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|status| status.success()) +} + +/// `available`, or a skip notice for `missing` (a failure when +/// [`REQUIRE_SANDBOX_BACKENDS`] is set). #[allow( clippy::print_stderr, reason = "Skip notices go to stderr so stdout stays assertable." )] -pub fn docker_available() -> bool { - let daemon = std::process::Command::new("docker") - .args(["version", "--format", "{{.Server.Version}}"]) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .is_ok_and(|status| status.success()); - if !daemon { +fn sandbox_backend_available(available: bool, missing: &str) -> bool { + if !available { assert!( std::env::var_os(REQUIRE_SANDBOX_BACKENDS).is_none(), - "{REQUIRE_SANDBOX_BACKENDS} is set, but no Docker daemon answers" + "{REQUIRE_SANDBOX_BACKENDS} is set, but {missing}" ); - eprintln!("skipping: no Docker daemon answers"); + eprintln!("skipping: {missing}"); } - daemon + available } /// Apply baseline environment isolation to a `Command` that spawns the