scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-03 02:24:33 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix: address review of the in-process sandbox providers

Browse source 
- 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 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-09-25 12:31:37 -04:00
parent 8a09e8fa08
commit a1a98c69d0
18 changed files with 205 additions and 432 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
clippy.toml
View file

@ -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 = [

9
docs/public/administration/server-configuration.mdx
View file

@ -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_<KIND>_PLUGIN` and `PETRI_SANDBOX_<KIND>_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]

11
lib/apps/fabro-cli/src/commands/run/petri_worker.rs
View file

@ -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,

20
lib/apps/fabro-cli/tests/it/scenario/petri.rs
View file

@ -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::<Vec<_>>();
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"),

2
lib/apps/fabro-cli/tests/it/scenario/petri_fork.rs
View file

@ -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

26
lib/apps/fabro-cli/tests/it/workflow/docker.rs
View file

@ -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));

72
lib/apps/fabro-server/src/server.rs
View file

@ -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<SandboxProviderConfig, ApiError> {
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<SandboxProviderConfig, SecretStoreError> {
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<String>,
daytona_api_key: Option<String>,
) -> anyhow::Result<WorkerLaunchSpec> {
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<AppState>, 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<AppState>, run_id: RunId) {
&run_dir_for_build,
agent_fabro_tools_enabled,
github_app_private_key,
daytona_api_key,
)
})
.await

2
lib/apps/fabro-server/src/server/handler/lineage.rs
View file

@ -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"),

70
lib/apps/fabro-server/src/server/tests.rs
View file

@ -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.<kind>]` 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))
}

187
lib/apps/fabro-server/src/spawn_env.rs
View file

@ -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<OsString>,
) {
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.<kind>]` settings: for every enabled
/// third-party kind that carries plugin settings, `PETRI_SANDBOX_<KIND>_PLUGIN`
/// from its `path` and `PETRI_SANDBOX_<KIND>_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-<kind>` 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([

12
lib/apps/fabro-server/src/worker_runtime.rs
View file

@ -48,16 +48,9 @@ pub(crate) struct WorkerLaunchSpec {
pub(crate) fabro_log: Option<String>,
pub(crate) active_config_path: PathBuf,
pub(crate) github_app_private_key: Option<String>,
/// The vault's Daytona API key, for a run on a Daytona environment:
/// Petri's Daytona plugin reads it from the worker's process.
pub(crate) daytona_api_key: Option<String>,
/// The Fabro home the server resolved, so a Petri run's skills step
/// reads the same home whatever the worker's environment says.
pub(crate) fabro_home: PathBuf,
/// 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());

12
lib/components/fabro-petri/src/fork.rs
View file

@ -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<Forked, ForkError> {
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 {

130
lib/components/fabro-petri/src/providers.rs
View file

@ -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<String>) -> 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<String>) -> 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<fabro_http::HttpClient>) -> 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<String>,
pub docker_host_address: Option<String>,
pub daytona: Option<DaytonaCredentials>,
docker_host: Option<String>,
docker_host_address: Option<String>,
daytona: Option<DaytonaCredentials>,
}
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<DaytonaCredentials>,
lookup: impl Fn(&str) -> Option<String>,
) -> 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<Arc<dyn SandboxProvider>> {
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"));
}
}

5
lib/components/fabro-petri/src/prune.rs
View file

@ -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<PruneReport, PruneError> {
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)

12
lib/components/fabro-petri/src/runtime.rs
View file

@ -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()));
}

9
lib/components/fabro-petri/tests/projection.rs
View file

@ -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 {

12
lib/components/fabro-petri/tests/runs.rs
View file

@ -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"));

44
lib/foundation/fabro-test/src/lib.rs
View file

@ -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