mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-01 02:04:24 +00:00
Forward every configured sandbox plugin to the Petri worker
The worker's environment carried only the host, Docker and Daytona plugin variables from the server's own environment, so a run on a third-party provider kind never learned where its plugin was, although the server kept `[server.sandbox.providers.<kind>]` plugin settings for its own attach. The launch spec now derives `PETRI_SANDBOX_<KIND>_PLUGIN` and `PETRI_SANDBOX_<KIND>_SHA256` from every enabled kind's plugin settings, and `PETRI_SANDBOX_PLUGIN_DEV=1` when any of them sets `dev`, set after the allowlist so the settings win over an ambient variable of the same name and the allowlist stays the fallback. The server's default plugin binary name is `sandbox-driver-<kind>`, the name Petri looks up, since one executable serves both sides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
956feda0ca
commit
809b3891b5
11 changed files with 246 additions and 19 deletions
|
|
@ -195,10 +195,17 @@ be lowercase ASCII letters, digits, and interior hyphens. The plugin starts with
|
|||
environment: only `env` and the ambient variables listed in `inherit_env` reach it. Bundled
|
||||
providers reject these plugin keys.
|
||||
|
||||
The same executable serves both sides of a run. The server launches it to reach a run's
|
||||
sandbox after the fact (the sandbox tab, files, terminal, Ask Fabro), and Petri launches it in
|
||||
the run's worker to create the sandbox. The server hands the worker `path` and `sha256` as
|
||||
`PETRI_SANDBOX_<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.
|
||||
|
||||
```toml title="settings.toml"
|
||||
[server.sandbox.providers.e2b]
|
||||
enabled = true
|
||||
path = "/opt/fabro/plugins/fabro-sandbox-e2b" # default: `fabro-sandbox-<kind>` on PATH
|
||||
path = "/opt/fabro/plugins/sandbox-driver-e2b" # default: `sandbox-driver-<kind>` on PATH
|
||||
sha256 = "0123…cdef" # pin the executable; `dev = true` skips it
|
||||
args = []
|
||||
inherit_env = ["PATH"]
|
||||
|
|
@ -210,7 +217,7 @@ E2B_API_URL = "https://api.e2b.example"
|
|||
| Key | Description | Default |
|
||||
|---|---|---|
|
||||
| `enabled` | Whether runs may select this provider | `true` |
|
||||
| `path` | Plugin executable path | `fabro-sandbox-<kind>` on `PATH` |
|
||||
| `path` | Plugin executable path | `sandbox-driver-<kind>` on `PATH` |
|
||||
| `sha256` | Pinned SHA-256 of the executable, hex | none |
|
||||
| `dev` | Allow launching without a checksum | `false` |
|
||||
| `args` | Arguments passed to the executable | `[]` |
|
||||
|
|
|
|||
|
|
@ -15447,7 +15447,7 @@ components:
|
|||
properties:
|
||||
path:
|
||||
type: string
|
||||
description: Executable path. Absent means `fabro-sandbox-<kind>` on `PATH`.
|
||||
description: Executable path. Absent means `sandbox-driver-<kind>` on `PATH`.
|
||||
sha256:
|
||||
type: string
|
||||
description: Pinned SHA-256 of the executable, hex.
|
||||
|
|
|
|||
|
|
@ -54,9 +54,10 @@ use tokio::time;
|
|||
pub(crate) const PETRI_RUN_LABEL: &str = "petri.run";
|
||||
|
||||
/// Binary naming prefix for a plugin provider's executable: a plugin for
|
||||
/// kind `e2b` is `fabro-sandbox-e2b` on `PATH` unless the settings name a
|
||||
/// path.
|
||||
const PLUGIN_BINARY_PREFIX: &str = "fabro-sandbox";
|
||||
/// kind `e2b` is `sandbox-driver-e2b` on `PATH` unless the settings name a
|
||||
/// path. The same executable serves Petri's run in the worker, which looks
|
||||
/// it up under the same name.
|
||||
const PLUGIN_BINARY_PREFIX: &str = "sandbox-driver";
|
||||
|
||||
/// `User-Agent` Fabro presents to remote sandbox control planes.
|
||||
const USER_AGENT: &str = concat!("fabro-server/", env!("CARGO_PKG_VERSION"));
|
||||
|
|
@ -839,7 +840,7 @@ mod tests {
|
|||
.insert(kind(name), ServerSandboxProviderSettings {
|
||||
enabled: true,
|
||||
plugin: Some(SandboxPluginSettings {
|
||||
path: Some(format!("/nonexistent/fabro-sandbox-{name}")),
|
||||
path: Some(format!("/nonexistent/sandbox-driver-{name}")),
|
||||
dev: true,
|
||||
..SandboxPluginSettings::default()
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -150,7 +150,7 @@ use crate::sandbox_access::{
|
|||
SandboxInventory,
|
||||
};
|
||||
use crate::server_secrets::ServerSecrets;
|
||||
use crate::spawn_env::apply_render_graph_env;
|
||||
use crate::spawn_env::{self, apply_render_graph_env};
|
||||
use crate::worker_control::{
|
||||
LocalWorkerControlBus, WORKER_CONTROL_ACK_WAIT, WorkerControlAcks, WorkerControlBus,
|
||||
WorkerControlBusError,
|
||||
|
|
@ -3673,6 +3673,9 @@ fn worker_launch_spec(
|
|||
github_app_private_key,
|
||||
daytona_api_key,
|
||||
fabro_home: fabro_config::Home::from_env().root().to_path_buf(),
|
||||
sandbox_plugin_env: spawn_env::sandbox_plugin_env(
|
||||
&state.server_settings().server.sandbox.providers,
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -121,7 +121,7 @@ mod tests {
|
|||
.insert(kind.clone(), ServerSandboxProviderSettings {
|
||||
enabled: true,
|
||||
plugin: Some(SandboxPluginSettings {
|
||||
path: Some(format!("/nonexistent/fabro-sandbox-{name}")),
|
||||
path: Some(format!("/nonexistent/sandbox-driver-{name}")),
|
||||
dev: true,
|
||||
..SandboxPluginSettings::default()
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -2319,6 +2319,49 @@ fn worker_command_forwards_daytona_api_key_from_vault() {
|
|||
);
|
||||
}
|
||||
|
||||
/// 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() {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
use std::ffi::OsString;
|
||||
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::server::ServerSandboxProvidersSettings;
|
||||
use tokio::process::Command;
|
||||
|
||||
const WORKER_ENV_ALLOWLIST: &[&str] = &[
|
||||
|
|
@ -53,6 +54,8 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[
|
|||
// Petri's sandbox-driver plugins are resolved in the worker, where a
|
||||
// Petri run executes: the plugin path, checksum and dev-mode overrides
|
||||
// cross with `PATH`, so the worker finds the plugins the server would.
|
||||
// A plugin the server's settings configure is set on top of these by
|
||||
// `sandbox_plugin_env`, for every configured kind.
|
||||
EnvVars::PETRI_SANDBOX_HOST_PLUGIN,
|
||||
EnvVars::PETRI_SANDBOX_HOST_SHA256,
|
||||
EnvVars::PETRI_SANDBOX_DOCKER_PLUGIN,
|
||||
|
|
@ -87,8 +90,55 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[
|
|||
|
||||
const RENDER_GRAPH_ENV_ALLOWLIST: &[&str] = &[EnvVars::PATH, EnvVars::HOME, EnvVars::TMPDIR];
|
||||
|
||||
pub(crate) fn apply_worker_env(cmd: &mut Command) {
|
||||
apply_allowlist(cmd, WORKER_ENV_ALLOWLIST, &process_env_var_os);
|
||||
/// The worker's environment: the allowlisted ambient variables, then the
|
||||
/// plugin variables the server's settings derive, which win over an
|
||||
/// ambient variable of the same name.
|
||||
pub(crate) fn apply_worker_env(cmd: &mut Command, sandbox_plugins: &[(String, String)]) {
|
||||
apply_worker_env_with(cmd, sandbox_plugins, &process_env_var_os);
|
||||
}
|
||||
|
||||
fn apply_worker_env_with(
|
||||
cmd: &mut Command,
|
||||
sandbox_plugins: &[(String, String)],
|
||||
lookup: &dyn Fn(&str) -> Option<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
|
||||
/// 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
|
||||
}
|
||||
|
||||
pub(crate) fn apply_render_graph_env(cmd: &mut Command) {
|
||||
|
|
@ -118,7 +168,15 @@ mod tests {
|
|||
use std::ffi::OsString;
|
||||
use std::path::Path;
|
||||
|
||||
use super::{RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist};
|
||||
use fabro_types::SandboxProviderKind;
|
||||
use fabro_types::settings::server::{
|
||||
SandboxPluginSettings, ServerSandboxProviderSettings, ServerSandboxProvidersSettings,
|
||||
};
|
||||
|
||||
use super::{
|
||||
RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist, apply_worker_env_with,
|
||||
sandbox_plugin_env,
|
||||
};
|
||||
|
||||
fn env_command() -> tokio::process::Command {
|
||||
assert!(Path::new("/usr/bin/env").exists());
|
||||
|
|
@ -322,6 +380,117 @@ mod tests {
|
|||
assert!(!actual.contains_key("MY_API_KEY"));
|
||||
}
|
||||
|
||||
fn provider(
|
||||
kind: &str,
|
||||
enabled: bool,
|
||||
plugin: SandboxPluginSettings,
|
||||
) -> (SandboxProviderKind, ServerSandboxProviderSettings) {
|
||||
(
|
||||
SandboxProviderKind::try_new(kind).expect("a valid kind"),
|
||||
ServerSandboxProviderSettings {
|
||||
enabled,
|
||||
plugin: Some(plugin),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// A configured plugin reaches the worker under the names Petri reads,
|
||||
/// a configured path wins over the ambient variable of the same name,
|
||||
/// a kind the settings leave to `PATH` keeps the ambient one, and a
|
||||
/// disabled kind's plugin never crosses.
|
||||
#[tokio::test]
|
||||
async fn configured_plugins_reach_the_worker_and_win_over_ambient_variables() {
|
||||
let mut providers = ServerSandboxProvidersSettings::default();
|
||||
providers.entries.extend([
|
||||
provider("e2b", true, SandboxPluginSettings {
|
||||
path: Some("/opt/fabro/plugins/sandbox-driver-e2b".to_string()),
|
||||
sha256: Some("0123abcd".to_string()),
|
||||
dev: true,
|
||||
..SandboxPluginSettings::default()
|
||||
}),
|
||||
provider("docker", true, SandboxPluginSettings {
|
||||
path: Some("/opt/fabro/plugins/sandbox-driver-docker".to_string()),
|
||||
..SandboxPluginSettings::default()
|
||||
}),
|
||||
provider("daytona", true, SandboxPluginSettings::default()),
|
||||
provider("fly-io", false, SandboxPluginSettings {
|
||||
path: Some("/opt/fabro/plugins/sandbox-driver-fly-io".to_string()),
|
||||
..SandboxPluginSettings::default()
|
||||
}),
|
||||
]);
|
||||
let env = HashMap::from([
|
||||
("PATH".to_string(), "/bin".to_string()),
|
||||
(
|
||||
"PETRI_SANDBOX_HOST_PLUGIN".to_string(),
|
||||
"/ambient/sandbox-driver-host".to_string(),
|
||||
),
|
||||
(
|
||||
"PETRI_SANDBOX_DOCKER_PLUGIN".to_string(),
|
||||
"/ambient/sandbox-driver-docker".to_string(),
|
||||
),
|
||||
(
|
||||
"PETRI_SANDBOX_DAYTONA_PLUGIN".to_string(),
|
||||
"/ambient/sandbox-driver-daytona".to_string(),
|
||||
),
|
||||
]);
|
||||
let mut cmd = env_command();
|
||||
apply_worker_env_with(&mut cmd, &sandbox_plugin_env(&providers), &|name| {
|
||||
env.get(name).map(OsString::from)
|
||||
});
|
||||
|
||||
let actual = env_output(cmd).await;
|
||||
|
||||
assert_eq!(
|
||||
actual.get("PETRI_SANDBOX_E2B_PLUGIN").map(String::as_str),
|
||||
Some("/opt/fabro/plugins/sandbox-driver-e2b")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("PETRI_SANDBOX_E2B_SHA256").map(String::as_str),
|
||||
Some("0123abcd")
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("PETRI_SANDBOX_PLUGIN_DEV").map(String::as_str),
|
||||
Some("1"),
|
||||
"one plugin in dev mode puts the worker's lookup in dev mode"
|
||||
);
|
||||
assert_eq!(
|
||||
actual
|
||||
.get("PETRI_SANDBOX_DOCKER_PLUGIN")
|
||||
.map(String::as_str),
|
||||
Some("/opt/fabro/plugins/sandbox-driver-docker"),
|
||||
"the settings win over the ambient variable"
|
||||
);
|
||||
assert_eq!(
|
||||
actual.get("PETRI_SANDBOX_HOST_PLUGIN").map(String::as_str),
|
||||
Some("/ambient/sandbox-driver-host"),
|
||||
"a kind without settings keeps the allowlisted ambient variable"
|
||||
);
|
||||
assert_eq!(
|
||||
actual
|
||||
.get("PETRI_SANDBOX_DAYTONA_PLUGIN")
|
||||
.map(String::as_str),
|
||||
Some("/ambient/sandbox-driver-daytona"),
|
||||
"settings without a path leave the ambient variable in place"
|
||||
);
|
||||
assert!(
|
||||
!actual.contains_key("PETRI_SANDBOX_FLY_IO_PLUGIN"),
|
||||
"a disabled kind's plugin does not cross"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_configured_plugin_derives_no_variables() {
|
||||
assert!(sandbox_plugin_env(&ServerSandboxProvidersSettings::default()).is_empty());
|
||||
let mut providers = ServerSandboxProvidersSettings::default();
|
||||
providers
|
||||
.entries
|
||||
.extend([provider("docker", true, SandboxPluginSettings::default())]);
|
||||
assert!(
|
||||
sandbox_plugin_env(&providers).is_empty(),
|
||||
"settings with neither a path nor a pin nor dev mode add nothing"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn render_graph_allowlist_is_fail_closed() {
|
||||
let env = HashMap::from([
|
||||
|
|
|
|||
|
|
@ -54,6 +54,10 @@ pub(crate) struct WorkerLaunchSpec {
|
|||
/// The Fabro home the server resolved, so a Petri run's skills step
|
||||
/// reads the same home whatever the worker's environment says.
|
||||
pub(crate) fabro_home: PathBuf,
|
||||
/// The sandbox-driver plugin variables the server's provider settings
|
||||
/// derive (`spawn_env::sandbox_plugin_env`), so Petri in the worker
|
||||
/// launches the plugin the settings name for every configured kind.
|
||||
pub(crate) sandbox_plugin_env: Vec<(String, String)>,
|
||||
}
|
||||
|
||||
pub(crate) struct StartedWorker {
|
||||
|
|
@ -101,7 +105,7 @@ impl LocalWorkerRuntime {
|
|||
.stdout(worker_stdout)
|
||||
.stderr(Stdio::piped());
|
||||
|
||||
apply_worker_env(&mut cmd);
|
||||
apply_worker_env(&mut cmd, &spec.sandbox_plugin_env);
|
||||
if let Some(level) = spec.fabro_log.as_deref() {
|
||||
cmd.env(EnvVars::FABRO_LOG, level);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -235,7 +235,7 @@ _version = 1
|
|||
methods = ["dev-token"]
|
||||
|
||||
[server.sandbox.providers.e2b]
|
||||
path = "/opt/fabro/plugins/fabro-sandbox-e2b"
|
||||
path = "/opt/fabro/plugins/sandbox-driver-e2b"
|
||||
sha256 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
|
||||
args = ["--region", "us"]
|
||||
inherit_env = ["PATH"]
|
||||
|
|
@ -255,7 +255,7 @@ E2B_API_URL = "https://api.e2b.example"
|
|||
.expect("plugin kinds carry launch settings");
|
||||
assert_eq!(
|
||||
plugin.path.as_deref(),
|
||||
Some("/opt/fabro/plugins/fabro-sandbox-e2b")
|
||||
Some("/opt/fabro/plugins/sandbox-driver-e2b")
|
||||
);
|
||||
assert_eq!(plugin.args, vec!["--region", "us"]);
|
||||
assert_eq!(plugin.inherit_env, vec!["PATH"]);
|
||||
|
|
@ -279,7 +279,7 @@ _version = 1
|
|||
methods = ["dev-token"]
|
||||
|
||||
[server.sandbox.providers.docker]
|
||||
path = "/usr/local/bin/fabro-sandbox-docker"
|
||||
path = "/usr/local/bin/sandbox-driver-docker"
|
||||
"#,
|
||||
)
|
||||
.expect_err("bundled providers take no plugin settings");
|
||||
|
|
|
|||
|
|
@ -187,8 +187,8 @@ impl Default for ServerSandboxProviderSettings {
|
|||
/// named in `inherit_env` reach it.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct SandboxPluginSettings {
|
||||
/// Executable path. When absent the server searches `PATH` for
|
||||
/// `fabro-sandbox-<kind>`.
|
||||
/// Executable path. When absent the server, and Petri in the run's
|
||||
/// worker, search `PATH` for `sandbox-driver-<kind>`.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub path: Option<String>,
|
||||
/// Pinned SHA-256 of the executable, hex.
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@
|
|||
*/
|
||||
export interface SandboxPluginSettings {
|
||||
/**
|
||||
* Executable path. Absent means `fabro-sandbox-<kind>` on `PATH`.
|
||||
* Executable path. Absent means `sandbox-driver-<kind>` on `PATH`.
|
||||
*/
|
||||
'path'?: string;
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue