From 0cc645b2d1223ff94b7c1b807a22df7267509f5e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 19 Sep 2026 13:47:02 -0400 Subject: [PATCH] Reach run sandboxes from the server through the sandbox driver `fabro-server/src/sandbox_access.rs` is the server's own path to a run's sandbox: it connects the record's provider (the driver's Host, Docker and Daytona providers in process, a plugin executable for any other kind), keys ownership on the `petri.run` label Petri stamps on every sandbox it creates, attaches by the recorded id, and for a host record designates the recorded directory again when the id lives only in the worker's registry. The Docker client resolves its endpoint from the same variables Petri forwards to its plugin, so both meet on one daemon. The doctor's Docker check and the Daytona credential probe move here with `DaytonaCredentials`, and the `/sandboxes` inventory is rebuilt over the driver's `list`, narrowed to sandboxes that carry Petri's run label. Preflight asks the provider for its health instead of creating and deleting a throwaway sandbox in Fabro's own shape, which no run uses; the git retry policy behind the repository probe moves into run_manifest. The callers still on fabro-sandbox's reconnect read their access through a `legacy_provider_access` shim until they move. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 5 + lib/apps/fabro-server/Cargo.toml | 5 + lib/apps/fabro-server/src/diagnostics.rs | 12 +- lib/apps/fabro-server/src/install.rs | 9 +- lib/apps/fabro-server/src/lib.rs | 1 + lib/apps/fabro-server/src/run_files.rs | 2 +- lib/apps/fabro-server/src/run_manifest.rs | 472 +++++-- lib/apps/fabro-server/src/sandbox_access.rs | 1139 +++++++++++++++++ lib/apps/fabro-server/src/server.rs | 70 +- .../src/server/handler/sandbox.rs | 2 +- .../src/server/handler/sandboxes.rs | 99 +- .../src/server/handler/sessions.rs | 2 +- lib/apps/fabro-server/src/test_support.rs | 5 +- 13 files changed, 1609 insertions(+), 214 deletions(-) create mode 100644 lib/apps/fabro-server/src/sandbox_access.rs diff --git a/Cargo.lock b/Cargo.lock index f4a81c3f6..720d0b311 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2709,6 +2709,11 @@ dependencies = [ "rand 0.9.4", "reqwest 0.12.28", "sandbox-driver", + "sandbox-driver-daytona", + "sandbox-driver-docker", + "sandbox-driver-host", + "sandbox-driver-protocol", + "sandbox-driver-testing", "serde", "serde_json", "serde_yaml", diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index 2bc09d2e7..a7a5bc336 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -36,6 +36,10 @@ fabro-workflow-version = { path = "../../components/fabro-workflow-version" } fabro-sandbox = { path = "../../components/fabro-sandbox" } fabro-pebble-sandbox = { path = "../../components/fabro-pebble-sandbox" } sandbox-driver.workspace = true +sandbox-driver-host.workspace = true +sandbox-driver-docker.workspace = true +sandbox-driver-daytona.workspace = true +sandbox-driver-protocol.workspace = true fabro-github = { path = "../../components/fabro-github" } pebble-agent.workspace = true pebble-coding-agent.workspace = true @@ -124,6 +128,7 @@ tokio-util.workspace = true tokio-tungstenite.workspace = true fabro-macros = { path = "../../foundation/fabro-macros" } fabro-sandbox = { path = "../../components/fabro-sandbox", features = ["test-support"] } +sandbox-driver-testing.workspace = true fabro-store = { path = "../../components/fabro-store", features = ["test-support"] } fabro-test = { workspace = true } fabro-types = { path = "../../foundation/fabro-types", features = ["test-support"] } diff --git a/lib/apps/fabro-server/src/diagnostics.rs b/lib/apps/fabro-server/src/diagnostics.rs index c94c83769..d9f5111a2 100644 --- a/lib/apps/fabro-server/src/diagnostics.rs +++ b/lib/apps/fabro-server/src/diagnostics.rs @@ -9,7 +9,6 @@ use fabro_llm::Client; use fabro_llm::lithos_catalog::{Catalog, CatalogProvider}; use fabro_llm::probe::{self, ModelTestStatus}; use fabro_redact::redact_string; -use fabro_sandbox::daytona; use fabro_static::EnvVars; use fabro_types::SandboxProviderKind; use fabro_types::settings::ServerAuthMethod; @@ -24,6 +23,7 @@ use serde::Serialize; use tokio::time::error::Elapsed; use tokio::time::timeout; +use crate::sandbox_access::{self, DaytonaCredentialProbeTimeout, DaytonaKeyCheck}; use crate::server::AppState; const EXTERNAL_SERVICE_PROBE_TIMEOUT: Duration = Duration::from_secs(15); @@ -586,9 +586,9 @@ async fn check_docker_sandbox(state: &AppState) -> CheckResult { .providers .is_enabled(&SandboxProviderKind::DOCKER), || async { - fabro_sandbox::check_docker_daemon() + sandbox_access::check_docker_daemon() .await - .map_err(|err| err.display_with_causes()) + .map_err(|err| format!("{err:#}")) }, DOCKER_PROBE_TIMEOUT, ) @@ -671,7 +671,7 @@ async fn check_cloud_sandbox(state: &AppState) -> CheckResult { cloud_sandbox_probe_check(probe) } -fn cloud_sandbox_probe_check(probe: anyhow::Result) -> CheckResult { +fn cloud_sandbox_probe_check(probe: anyhow::Result) -> CheckResult { match probe { Ok(check) if check.ok() => CheckResult { name: "Cloud Sandbox".to_string(), @@ -695,7 +695,7 @@ fn cloud_sandbox_probe_check(probe: anyhow::Result) -> )), }, Err(err) => { - if let Some(timeout) = err.downcast_ref::() { + if let Some(timeout) = err.downcast_ref::() { return CheckResult { name: "Cloud Sandbox".to_string(), status: CheckStatus::Error, @@ -1240,7 +1240,7 @@ enabled = false #[test] fn check_cloud_sandbox_reports_timeout() { let result = cloud_sandbox_probe_check(Err(anyhow::Error::new( - daytona::DaytonaCredentialProbeTimeout::new(Duration::from_millis(1)), + DaytonaCredentialProbeTimeout::new(Duration::from_millis(1)), ))); assert_eq!(result.name, "Cloud Sandbox"); diff --git a/lib/apps/fabro-server/src/install.rs b/lib/apps/fabro-server/src/install.rs index 62cd6b006..3d3ee4547 100644 --- a/lib/apps/fabro-server/src/install.rs +++ b/lib/apps/fabro-server/src/install.rs @@ -26,8 +26,6 @@ use fabro_install::{ }; use fabro_llm::lithos_catalog::{Catalog, CatalogProvider}; use fabro_llm::probe::{self, ApiKeyProbeError, ModelTestStatus}; -use fabro_sandbox::daytona; -use fabro_sandbox::driver::DaytonaCredentials; use fabro_static::EnvVars; use fabro_store::ArtifactStore; use fabro_types::settings::server::ObjectStoreSettings; @@ -49,6 +47,9 @@ use tracing::{error, info, warn}; use zeroize::Zeroizing; use crate::error::ApiError; +use crate::sandbox_access::{ + DAYTONA_CREDENTIAL_PROBE_TIMEOUT, DaytonaCredentials, DaytonaKeyCheck, check_daytona_api_key, +}; use crate::serve::{self, DEFAULT_TCP_PORT}; use crate::server_secrets::{ServerSecrets, process_env_snapshot}; use crate::{security_headers, server, static_files}; @@ -1004,14 +1005,14 @@ async fn post_install_sandbox_test( async fn check_install_daytona_api_key( state: &InstallAppState, api_key: String, -) -> anyhow::Result { +) -> anyhow::Result { let credentials = DaytonaCredentials::new(api_key) .with_api_url(state.upstreams.daytona_api_base_url.clone()) .with_organization_id(state.upstreams.daytona_organization_id.clone()) .with_http_client(Some( fabro_http::http_client().context("failed to build HTTP client")?, )); - daytona::check_daytona_api_key(&credentials, daytona::DAYTONA_CREDENTIAL_PROBE_TIMEOUT).await + check_daytona_api_key(&credentials, DAYTONA_CREDENTIAL_PROBE_TIMEOUT).await } async fn put_install_sandbox( diff --git a/lib/apps/fabro-server/src/lib.rs b/lib/apps/fabro-server/src/lib.rs index ee3c1e02c..3584fd7e2 100644 --- a/lib/apps/fabro-server/src/lib.rs +++ b/lib/apps/fabro-server/src/lib.rs @@ -44,6 +44,7 @@ mod run_selector; mod run_title_generation; #[cfg(test)] mod run_tool_create; +mod sandbox_access; pub mod security_headers; pub mod serve; pub mod server; diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index e5c1254d1..80e40397c 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -1177,7 +1177,7 @@ async fn reconnect_run_sandbox( .cloned() .ok_or_else(|| ApiError::new(StatusCode::NOT_FOUND, "Run sandbox was not created."))?; let access = state - .provider_access() + .legacy_provider_access() .await .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; let sandbox = reconnect_for_run(&record, &access, Some(*run_id), None) diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 887d659e5..1e31c426a 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use std::future::Future; use std::path::{Path, PathBuf}; -use std::sync::Arc; -use std::time::Duration; +use std::sync::{Arc, Mutex, PoisonError}; +use std::time::{Duration, SystemTime}; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_api::types; @@ -19,9 +19,6 @@ use fabro_petri::check::Launch; use fabro_petri::run_graph; use fabro_petri::runtime::RuntimeSpec; use fabro_proc::ProcessError; -use fabro_sandbox::{ - CloneRequest, ProviderAccess, RunSandbox, SandboxSpec, sandbox_spec_for_environment, -}; use fabro_static::EnvVars; use fabro_types::diagnostic::{Diagnostic, Severity}; use fabro_types::settings::cli::OutputVerbosity; @@ -36,12 +33,17 @@ use fabro_workflow::Error as WorkflowError; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; use futures_util::stream::{self, StreamExt}; use lithos_llm::catalog::ProviderId; +use sandbox_driver::{ + GitBackoff, GitCredentials, GitFailure, GitFailureKind, GitRetryPolicy, HealthStatus, + ProviderHealth, +}; use tokio::process::Command; use tokio::task; #[cfg(test)] use tokio::time; use tokio_util::sync::CancellationToken; +use crate::sandbox_access::{self, ProviderAccess}; use crate::server::{AppState, petri_runs}; use crate::{petri_check, run_compiler}; @@ -830,50 +832,11 @@ async fn run_ls_remote(mut command: Command) -> std::result::Result<(), String> }) } -fn preflight_sandbox_spec( - sandbox_provider: &SandboxProviderKind, - prepared: &PreparedManifest, - resolved_run: &RunNamespace, - access: &ProviderAccess, -) -> std::result::Result { - let clone_origin_url = prepared - .git - .as_ref() - .map(|git| fabro_github::normalize_repo_origin_url(&git.origin_url)); - let clone_branch = prepared.git.as_ref().map(|git| git.branch.clone()); - - if sandbox_provider.bundled() == Some(BundledProvider::Local) { - let working_directory = resolved_run - .environment - .local_working_directory(Some(&prepared.source_directory)) - .map_err(|err| { - fabro_sandbox::Error::context( - "Failed to resolve local environment working directory", - err, - ) - })?; - return Ok(SandboxSpec::local(working_directory, access.clone())); - } - // No vault is available on this path, so a `{{ secrets.* }}` value keeps - // its source form. Preflight never clones. - let spec = sandbox_spec_for_environment( - &resolved_run.environment, - resolved_run.environment.unresolved_env(), - )?; - let clone = CloneRequest { - origin_url: clone_origin_url, - branch: clone_branch, - ..CloneRequest::none() - }; - Ok(SandboxSpec { - kind: sandbox_provider.clone(), - access: access.clone(), - spec, - clone, - run_id: None, - }) -} - +/// The sandbox check of preflight: the run's provider is reachable and its +/// credential accepted, as the provider's own health check reports. No +/// sandbox is created: Petri creates the run's, in the run's own shape, +/// when the run starts. A `local` environment must also resolve the +/// directory the run would work in. async fn run_sandbox_check( checks: &mut Vec, sandbox_provider: &SandboxProviderKind, @@ -881,86 +844,78 @@ async fn run_sandbox_check( resolved_run: &RunNamespace, access: &ProviderAccess, ) -> bool { - let spec = match preflight_sandbox_spec(sandbox_provider, prepared, resolved_run, access) { - Ok(spec) => spec, - Err(err) => { + if sandbox_provider.bundled() == Some(BundledProvider::Local) { + if let Err(err) = resolved_run + .environment + .local_working_directory(Some(&prepared.source_directory)) + { checks.push(CheckResult { name: "Sandbox".into(), status: CheckStatus::Error, summary: "failed".into(), details: vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))], - remediation: Some(err.to_string()), + remediation: Some(format!( + "Failed to resolve local environment working directory: {err}" + )), }); return false; } - }; - let sandbox_result: Result, String> = spec.build(None).await.map_err(|err| { - if *sandbox_provider == SandboxProviderKind::DAYTONA { - format!("Daytona sandbox creation failed: {err}") - } else { - err.to_string() - } - }); + } + let health = sandbox_access::provider_health(sandbox_provider, access) + .await + .map_err(|err| format!("{err:#}")); + let check = sandbox_health_check(sandbox_provider, health); + let passed = check.status == CheckStatus::Pass; + checks.push(check); + passed +} - match sandbox_result { - Ok(sandbox) => match sandbox.initialize().await { - Ok(()) => { - let mut details = vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))]; - if sandbox_provider.clones_workspace() - && prepared.git.is_none() - && !clone_disabled_for_provider(sandbox_provider, resolved_run) - { - details.push(CheckDetail { - text: "No clone source present; sandbox workspace will be empty".into(), - warn: true, - }); - } - if let Err(err) = sandbox.delete().await { - checks.push(CheckResult { - name: "Sandbox".into(), - status: CheckStatus::Error, - summary: "cleanup failed".into(), - details, - remediation: Some(format!("Sandbox cleanup failed: {err}")), - }); - return false; - } - checks.push(CheckResult { - name: "Sandbox".into(), - status: CheckStatus::Pass, - summary: sandbox_provider.to_string(), - details, - remediation: None, - }); - true - } - Err(err) => { - let cleanup_error = sandbox.delete().await.err(); - checks.push(CheckResult { - name: "Sandbox".into(), - status: CheckStatus::Error, - summary: "failed".into(), - details: vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))], - remediation: Some(cleanup_error.map_or_else( - || format!("Sandbox init failed: {err}"), - |cleanup| { - format!("Sandbox init failed: {err}; cleanup also failed: {cleanup}") - }, - )), - }); - false - } +/// The preflight check for a provider's health report: a connection +/// failure and an unreachable or rejected backend fail with the reason; a +/// healthy backend, or one whose provider has no health check, passes. +fn sandbox_health_check( + sandbox_provider: &SandboxProviderKind, + health: std::result::Result, +) -> CheckResult { + let details = vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))]; + let failure = |remediation: String| CheckResult { + name: "Sandbox".into(), + status: CheckStatus::Error, + summary: "failed".into(), + details: details.clone(), + remediation: Some(remediation), + }; + let health = match health { + Ok(health) => health, + Err(err) => return failure(err), + }; + match health.status { + HealthStatus::Ok | HealthStatus::Unknown => CheckResult { + name: "Sandbox".into(), + status: CheckStatus::Pass, + summary: sandbox_provider.to_string(), + details, + remediation: None, }, - Err(err) => { - checks.push(CheckResult { - name: "Sandbox".into(), - status: CheckStatus::Error, - summary: "failed".into(), - details: vec![CheckDetail::new(format!("Provider: {sandbox_provider}"))], - remediation: Some(err), - }); - false - } + HealthStatus::Unreachable => failure(format!( + "{sandbox_provider} backend is unreachable: {}", + health + .message + .unwrap_or_else(|| "the backend did not answer".to_string()) + )), + HealthStatus::Unauthorized if !health.missing_permissions.is_empty() => failure(format!( + "{sandbox_provider} credential is missing required permissions: {}", + health.missing_permissions.join(", ") + )), + HealthStatus::Unauthorized => failure(format!( + "{sandbox_provider} rejected the credential: {}", + health + .message + .unwrap_or_else(|| "the credential was rejected".to_string()) + )), + _ => failure(format!( + "{sandbox_provider} reported an unknown health state" + )), } } @@ -1268,8 +1223,8 @@ where F: FnMut() -> Fut, Fut: Future>, { - fabro_sandbox::retry_git_messages( - &fabro_sandbox::repository_probe_policy(), + retry_git_messages( + &repository_probe_policy(), Some(&snapshot), "repository probe", run, @@ -1277,6 +1232,105 @@ where .await } +// ── Fabro's retry budget for git operations against GitHub ───────────────── +// +// The driver owns the retry loop and the decision +// (`sandbox_driver::retry_git`): a remote that cannot be reached is retried, +// a rejected credential is retried only while the token is fresh enough to +// still be replicating to GitHub's git endpoints, a static credential fails +// fast, and a command whose outcome is unknown is never replayed. Fabro +// keeps what is policy: how many attempts the host-side repository probe +// gets, how it paces them, and when the credential it runs with was minted. +// +// Retries reuse the same token on purpose. Replication of a given token +// only makes progress, so each attempt strictly improves the odds, while +// re-minting would restart the replication clock. + +/// The username GitHub expects with an installation token or PAT. +const GITHUB_TOKEN_USERNAME: &str = "x-access-token"; + +/// Backoff between attempts: 3s, then 9s. +/// +/// GitHub's guidance for token replication is to wait a few seconds and +/// retry with the same token. Sub-second delays land inside the same +/// replication window and spend an attempt for nothing. +fn replication_backoff() -> GitBackoff { + GitBackoff::new(Duration::from_secs(3), 3.0, Duration::from_secs(10)) +} + +/// Host-side repository probes get 3 attempts at replication pacing, with +/// no deadline of their own. +fn repository_probe_policy() -> GitRetryPolicy { + GitRetryPolicy::new(3, replication_backoff()) +} + +/// Credentials carrying only the token's mint time, which is all the +/// driver's decision reads for git that ran outside a sandbox. The token +/// itself never leaves its snapshot. +fn credential_age(snapshot: Option<&TokenSnapshot>) -> Option { + let snapshot = snapshot?; + let credentials = GitCredentials::new(GITHUB_TOKEN_USERNAME, ""); + Some(match snapshot.minted_at() { + Some(minted_at) => credentials.minted_at(SystemTime::from(minted_at)), + None => credentials, + }) +} + +/// The driver's failure for a rendered git message, so git that ran +/// outside a sandbox (the host-side repository probe) is classified the +/// same way as git the driver ran. +fn classified_git_failure(operation: &str, message: &str) -> sandbox_driver::Error { + sandbox_driver::Error::Git(GitFailure::classified( + operation, + GitFailureKind::from_message(message), + None, + )) +} + +/// Runs a host-side git operation that reports failures as rendered +/// messages under `policy`, retrying while the driver's decision says the +/// message is transient for the token behind `snapshot`. The final failure +/// comes back as the operation's own message. +async fn retry_git_messages( + policy: &GitRetryPolicy, + snapshot: Option<&TokenSnapshot>, + operation: &str, + mut run: F, +) -> std::result::Result<(), String> +where + F: FnMut() -> Fut, + Fut: Future>, +{ + let credentials = credential_age(snapshot); + // The operation's own message is kept beside the classified failure the + // driver decides on, so the caller reads the message it knows. + let last_message = Mutex::new(None); + let result = sandbox_driver::retry_git( + policy, + credentials.as_ref(), + operation, + |_attempt, _timeout| { + let attempt = run(); + let last_message = &last_message; + async move { + attempt.await.map_err(|message| { + let error = classified_git_failure(operation, &message); + *last_message.lock().unwrap_or_else(PoisonError::into_inner) = Some(message); + error + }) + } + }, + ) + .await; + match result { + Ok(_) => Ok(()), + Err(failure) => Err(last_message + .into_inner() + .unwrap_or_else(PoisonError::into_inner) + .unwrap_or_else(|| failure.error.to_string())), + } +} + async fn run_probe_ls_remote(url: &str, token: &ResolvedToken) -> std::result::Result<(), String> { let mut command = Command::new("git"); fabro_github::apply_probe_git_env(&mut command, token.token.expose()); @@ -1933,29 +1987,177 @@ provider = "local" ); } + fn health(status: HealthStatus, message: Option<&str>) -> ProviderHealth { + let mut health = ProviderHealth::new(status); + health.message = message.map(str::to_string); + health + } + #[test] - fn preflight_sandbox_spec_disables_docker_clone_but_preserves_clone_metadata() { - let (prepared, resolved) = prepared_and_resolved_for_sandbox( + fn a_healthy_or_uncheckable_provider_passes_the_sandbox_check() { + for status in [HealthStatus::Ok, HealthStatus::Unknown] { + let check = + sandbox_health_check(&SandboxProviderKind::DOCKER, Ok(health(status, None))); + assert_eq!(check.status, CheckStatus::Pass, "{status:?}"); + assert_eq!(check.summary, "docker"); + assert_eq!(check.details[0].text, "Provider: docker"); + assert!(check.remediation.is_none()); + } + } + + #[test] + fn an_unreachable_backend_fails_the_sandbox_check_with_its_reason() { + let check = sandbox_health_check( &SandboxProviderKind::DOCKER, - true, - Some(git_context("https://github.com/acme/widgets", "main")), + Ok(health( + HealthStatus::Unreachable, + Some("connection refused on /var/run/docker.sock"), + )), + ); + assert_eq!(check.status, CheckStatus::Error); + assert_eq!(check.summary, "failed"); + assert_eq!( + check.remediation.as_deref(), + Some("docker backend is unreachable: connection refused on /var/run/docker.sock") + ); + } + + #[test] + fn a_rejected_credential_fails_the_sandbox_check_naming_the_missing_scopes() { + let mut unauthorized = health(HealthStatus::Unauthorized, Some("forbidden")); + unauthorized.missing_permissions = vec!["write:sandboxes".to_string()]; + let check = sandbox_health_check(&SandboxProviderKind::DAYTONA, Ok(unauthorized)); + assert_eq!(check.status, CheckStatus::Error); + assert_eq!( + check.remediation.as_deref(), + Some("daytona credential is missing required permissions: write:sandboxes") ); - let spec = preflight_sandbox_spec( - &SandboxProviderKind::DOCKER, + let check = sandbox_health_check( + &SandboxProviderKind::DAYTONA, + Ok(health(HealthStatus::Unauthorized, Some("bad key"))), + ); + assert_eq!( + check.remediation.as_deref(), + Some("daytona rejected the credential: bad key") + ); + } + + #[test] + fn a_provider_that_cannot_connect_fails_the_sandbox_check_with_the_connect_error() { + let check = sandbox_health_check( + &SandboxProviderKind::DAYTONA, + Err("Daytona sandboxes require DAYTONA_API_KEY in the vault".to_string()), + ); + assert_eq!(check.status, CheckStatus::Error); + assert_eq!( + check.remediation.as_deref(), + Some("Daytona sandboxes require DAYTONA_API_KEY in the vault") + ); + } + + #[tokio::test] + async fn the_local_sandbox_check_passes_through_the_host_providers_health() { + let (prepared, resolved) = + prepared_and_resolved_for_sandbox(&SandboxProviderKind::LOCAL, false, None); + let mut checks = Vec::new(); + let passed = run_sandbox_check( + &mut checks, + &SandboxProviderKind::LOCAL, &prepared, &resolved, &ProviderAccess::default(), - ); + ) + .await; + assert!(passed, "{checks:?}"); + assert_eq!(checks[0].name, "Sandbox"); + assert_eq!(checks[0].status, CheckStatus::Pass); + } - let spec = spec.expect("Docker preflight sandbox spec"); - assert_eq!(spec.kind, SandboxProviderKind::DOCKER); - assert!(spec.clone.skip); - assert_eq!( - spec.clone.origin_url.as_deref(), - Some("https://github.com/acme/widgets") + #[tokio::test] + async fn the_daytona_sandbox_check_fails_without_a_vault_key() { + let (prepared, resolved) = + prepared_and_resolved_for_sandbox(&SandboxProviderKind::DAYTONA, false, None); + let mut checks = Vec::new(); + let passed = run_sandbox_check( + &mut checks, + &SandboxProviderKind::DAYTONA, + &prepared, + &resolved, + &ProviderAccess::default(), + ) + .await; + assert!(!passed); + assert_eq!(checks[0].status, CheckStatus::Error); + assert!( + checks[0] + .remediation + .as_deref() + .unwrap_or_default() + .contains("DAYTONA_API_KEY"), + "{checks:?}" ); - assert_eq!(spec.clone.branch.as_deref(), Some("main")); + } + + fn snapshot(age: Duration) -> TokenSnapshot { + let now = chrono::Utc::now(); + TokenSnapshot { + generation: 1, + provenance: fabro_github::token_source::TokenProvenance::Minted { + minted_at: now - chrono::Duration::from_std(age).unwrap(), + expires_at: now + chrono::Duration::hours(1), + }, + } + } + + fn static_snapshot() -> TokenSnapshot { + TokenSnapshot { + generation: 0, + provenance: fabro_github::token_source::TokenProvenance::Static, + } + } + + #[test] + fn probe_backoff_paces_at_replication_intervals() { + let backoff = repository_probe_policy().backoff; + assert_eq!(backoff.delay_after(1), Duration::from_secs(3)); + assert_eq!(backoff.delay_after(2), Duration::from_secs(9)); + } + + #[tokio::test(start_paused = true)] + async fn host_side_retries_keep_the_operations_own_message() { + let calls = Mutex::new(0_u32); + let result = retry_git_messages( + &repository_probe_policy(), + Some(&snapshot(Duration::from_secs(1))), + "repository probe", + || { + let attempt = { + let mut calls = calls.lock().unwrap(); + *calls += 1; + *calls + }; + async move { + if attempt < 3 { + Err(format!("remote: Repository not found. (attempt {attempt})")) + } else { + Ok(()) + } + } + }, + ) + .await; + assert_eq!(result, Ok(())); + assert_eq!(*calls.lock().unwrap(), 3); + + let permanent = retry_git_messages( + &repository_probe_policy(), + Some(&static_snapshot()), + "repository probe", + || async { Err("remote: Repository not found.".to_owned()) }, + ) + .await; + assert_eq!(permanent, Err("remote: Repository not found.".to_owned())); } #[test] diff --git a/lib/apps/fabro-server/src/sandbox_access.rs b/lib/apps/fabro-server/src/sandbox_access.rs new file mode 100644 index 000000000..52798ad91 --- /dev/null +++ b/lib/apps/fabro-server/src/sandbox_access.rs @@ -0,0 +1,1139 @@ +//! The server's direct access to run sandboxes through the sandbox driver. +//! +//! Petri creates every run sandbox and records it: the provider, the +//! provider's id and the working directory travel on `scope.acquired` into +//! the run's [`RunSandboxInstance`], and every Docker or Daytona sandbox +//! carries the `petri.run` label with the run id. The server reaches a +//! run's sandbox for the sandbox tab, Run Files, the terminal, SSH, preview +//! URLs, VNC, `fabro cp` and Ask Fabro by connecting the record's provider +//! itself and attaching to the record's id, without going through Petri. +//! +//! Ownership is keyed on `petri.run`: a persisted id is acted on only when +//! the sandbox behind it still carries the run's label, so an id that has +//! come to name someone else's sandbox on a shared daemon is refused. A host +//! sandbox is a directory: it carries no labels, and its id is derived from +//! its path, so a reconnect from this process designates the directory +//! again whatever registry the creating process kept. +//! +//! Credentials arrive explicitly. Nothing here reads the process +//! environment for a secret: the Daytona key comes from the vault through +//! [`DaytonaCredentials`]. The Docker client resolves its endpoint from the +//! same variables Petri forwards to its Docker plugin (`DOCKER_HOST` and +//! its TLS companions), so the server and the run's containers meet on one +//! daemon. + +use std::collections::BTreeMap; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Context as _; +use fabro_static::EnvVars; +use fabro_types::settings::server::{ + SandboxPluginSettings, ServerSandboxProviderSettings, ServerSandboxProvidersSettings, +}; +use fabro_types::{ + BundledProvider, RunId, RunSandboxInstance, SandboxInfo, SandboxListMeta, SandboxListResponse, + SandboxProviderKind, SandboxProviderLookupError, +}; +use futures_util::future::join_all; +use sandbox_driver::{ + Error as DriverError, HealthStatus, OwnedProvider, Ownership, ProviderHealth, ProviderKind, + Sandbox, SandboxFilter, SandboxId, SandboxProvider, SandboxSource, SandboxSpec, SandboxState, + WaitOptions, +}; +use sandbox_driver_daytona::{DaytonaConfig, DaytonaProvider}; +use sandbox_driver_docker::DockerProvider; +use sandbox_driver_host::HostProvider; +use sandbox_driver_protocol::{PluginConfig, PluginSupervisor}; +use tokio::sync::OnceCell; +use tokio::time; + +/// The label Petri stamps on every sandbox it creates for a run, carrying +/// the run id. Fabro's ownership of a run's sandbox is this label. +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"; + +/// `User-Agent` Fabro presents to remote sandbox control planes. +const USER_AGENT: &str = concat!("fabro-server/", env!("CARGO_PKG_VERSION")); + +/// Budget for the credential probe `fabro doctor` and the install flow run. +pub(crate) const DAYTONA_CREDENTIAL_PROBE_TIMEOUT: Duration = Duration::from_secs(20); + +/// Explicit Daytona credentials: the SDK's configuration with the API key +/// always present and a `Debug` that never prints it. The process +/// environment is never consulted. +#[derive(Clone)] +pub(crate) struct DaytonaCredentials(DaytonaConfig); + +impl DaytonaCredentials { + /// Credentials for `api_key` against Daytona's public control plane, + /// presenting Fabro's `User-Agent`. + #[must_use] + pub(crate) fn new(api_key: String) -> Self { + Self(DaytonaConfig { + api_key: Some(api_key), + user_agent: Some(USER_AGENT.to_string()), + ..DaytonaConfig::default() + }) + } + + /// Credentials for a vault API key, with the control-plane URL and + /// organization taken from `lookup` (server configuration). Nothing is + /// read implicitly. + pub(crate) 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_organization_id(lookup(EnvVars::DAYTONA_ORGANIZATION_ID)) + } + + /// The control-plane URL; Daytona's public API when `None`. + #[must_use] + pub(crate) fn with_api_url(mut self, api_url: Option) -> Self { + self.0.api_url = api_url; + self + } + + #[must_use] + pub(crate) fn with_organization_id(mut self, organization_id: Option) -> Self { + self.0.organization_id = organization_id; + self + } + + /// A shared HTTP client; tests pass a no-proxy client here. + #[must_use] + pub(crate) fn with_http_client(mut self, http_client: Option) -> Self { + self.0.http_client = http_client; + self + } + + /// The SDK configuration the driver's Daytona provider connects with. + #[must_use] + fn config(&self) -> &DaytonaConfig { + &self.0 + } +} + +impl std::fmt::Debug for DaytonaCredentials { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + 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() + } +} + +/// What the server needs to reach every provider a run record can name: +/// its provider settings (which kinds are enabled, which run as plugins) +/// and the Daytona credentials from the vault. +#[derive(Clone, Debug, Default)] +pub(crate) struct ProviderAccess { + pub(crate) providers: ServerSandboxProvidersSettings, + pub(crate) daytona: Option, +} + +impl ProviderAccess { + /// The settings entry for `kind`. A bundled kind without an entry is + /// enabled with defaults; any other kind must be configured. + fn settings_for(&self, kind: &SandboxProviderKind) -> Option { + match self.providers.get(kind) { + Some(settings) => Some(settings.clone()), + None if kind.bundled().is_some() => Some(ServerSandboxProviderSettings::default()), + None => None, + } + } +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum ConnectError { + #[error( + "sandbox provider `{kind}` is not configured; add [server.sandbox.providers.{kind}] to settings.toml" + )] + Unconfigured { kind: SandboxProviderKind }, + #[error("sandbox provider `{kind}` is disabled by server.sandbox.providers.{kind}.enabled")] + Disabled { kind: SandboxProviderKind }, + #[error( + "sandbox provider `{kind}` has no plugin settings; add server.sandbox.providers.{kind}" + )] + MissingPluginSettings { kind: SandboxProviderKind }, + #[error( + "Daytona sandboxes require DAYTONA_API_KEY in the vault; run `fabro secret set DAYTONA_API_KEY`" + )] + MissingDaytonaCredentials, + #[error("sandbox provider `{kind}` is not a valid sandbox-driver kind")] + InvalidKind { + kind: SandboxProviderKind, + #[source] + source: sandbox_driver::InvalidIdError, + }, + #[error("failed to connect sandbox provider `{kind}`")] + Driver { + kind: SandboxProviderKind, + #[source] + source: sandbox_driver::Error, + }, +} + +/// Connects the provider behind `kind`, unscoped: every sandbox on the +/// backend is visible to it. Callers that act on a persisted id narrow it +/// with [`run_provider`]. +/// +/// Bundled kinds link the driver's provider crates in process. `local` is +/// the driver's Host provider with a fresh registry: a run's directory is +/// reached by the id its path derives, whatever registry the worker kept. +/// `docker` connects to the daemon the process environment names, the +/// same variables Petri hands its Docker plugin, without requiring the +/// daemon to answer: `health` reports an unreachable daemon so preflight +/// and the doctor see the cause. `daytona` needs the vault key. Any other +/// kind launches the plugin executable its settings name and supervises +/// it. Disabled entries are refused here so no caller has to remember the +/// policy check. +pub(crate) async fn connect_provider( + kind: &SandboxProviderKind, + access: &ProviderAccess, +) -> Result, ConnectError> { + let settings = access + .settings_for(kind) + .ok_or_else(|| ConnectError::Unconfigured { kind: kind.clone() })?; + if !settings.enabled { + return Err(ConnectError::Disabled { kind: kind.clone() }); + } + let driver = |source| ConnectError::Driver { + kind: kind.clone(), + source, + }; + Ok(match kind.bundled() { + Some(BundledProvider::Local) => Arc::new(HostProvider::new()), + Some(BundledProvider::Docker) => { + Arc::new(DockerProvider::connect_unverified().map_err(driver)?) + } + Some(BundledProvider::Daytona) => { + let credentials = access + .daytona + .as_ref() + .ok_or(ConnectError::MissingDaytonaCredentials)?; + Arc::new( + DaytonaProvider::connect_explicit(credentials.config().clone()) + .await + .map_err(driver)?, + ) + } + None => { + let plugin = settings + .plugin + .as_ref() + .ok_or_else(|| ConnectError::MissingPluginSettings { kind: kind.clone() })?; + let driver_kind = ProviderKind::try_new(kind.as_str()).map_err(|source| { + ConnectError::InvalidKind { + kind: kind.clone(), + source, + } + })?; + // The supervisor is the provider: it launches the executable now, + // so a misconfigured plugin fails at connect time, and relaunches + // it after a crash for new work only. + Arc::new( + PluginSupervisor::launch(PLUGIN_BINARY_PREFIX, plugin_config(driver_kind, plugin)) + .await + .map_err(driver)?, + ) + } + }) +} + +fn plugin_config(kind: ProviderKind, settings: &SandboxPluginSettings) -> PluginConfig { + PluginConfig { + kind, + path: settings.path.as_deref().map(PathBuf::from), + sha256: settings.sha256.clone(), + dev: settings.dev, + args: settings.args.clone(), + env: settings + .env + .iter() + .map(|(key, value)| (key.clone(), value.clone())) + .collect::>(), + inherit_env: settings.inherit_env.clone(), + } +} + +/// Fabro's ownership of a run's sandboxes: the `petri.run` label Petri +/// stamps, with the run id. +#[must_use] +pub(crate) fn run_ownership(run_id: RunId) -> Ownership { + Ownership::label(PETRI_RUN_LABEL, run_id.to_string()) +} + +/// Whether `labels` are a Petri run sandbox's: Petri's run label is +/// present, whichever run it names. +fn is_petri_sandbox(labels: &BTreeMap) -> bool { + labels.contains_key(PETRI_RUN_LABEL) +} + +/// The provider for `kind`, narrowed to the sandboxes of `run_id`: an +/// attach to or a delete of an id whose sandbox does not carry the run's +/// `petri.run` label is refused. The `local` kind is returned unscoped: a +/// host directory carries no labels, and nothing else shares the host's +/// directories with Fabro. +pub(crate) async fn run_provider( + kind: &SandboxProviderKind, + access: &ProviderAccess, + run_id: RunId, +) -> Result, ConnectError> { + let provider = connect_provider(kind, access).await?; + if kind.bundled() == Some(BundledProvider::Local) { + return Ok(provider); + } + Ok(Arc::new(OwnedProvider::new( + provider, + run_ownership(run_id), + ))) +} + +/// Attaches to a run's sandbox from its record, through the record's +/// provider scoped to the run. The handle is whatever state the sandbox is +/// in; [`activate`] brings it to `Running`. +/// +/// A host sandbox is the directory it designates. An id the Host provider +/// minted for a long path lives only in the registry of the process that +/// created it (the run's worker), so a reconnect from this process +/// designates the directory again: the same workspace, whatever the id. +pub(crate) async fn attach_run_sandbox( + access: &ProviderAccess, + record: &RunSandboxInstance, + run_id: RunId, +) -> anyhow::Result> { + let kind = &record.provider; + let sandbox_id = &record.runtime.id; + let provider = run_provider(kind, access, run_id) + .await + .with_context(|| format!("Failed to connect to the {kind} provider"))?; + let id = + SandboxId::try_new(sandbox_id).with_context(|| format!("Invalid {kind} sandbox id"))?; + match provider.attach(&id, None).await { + Ok(handle) => Ok(handle), + Err(DriverError::NotFound { .. }) if kind.bundled() == Some(BundledProvider::Local) => { + let working_directory = &record.runtime.working_directory; + let spec = SandboxSpec::new(SandboxSource::HostDirectory) + .working_directory(working_directory.clone()); + provider.create(&spec, None).await.with_context(|| { + format!("Failed to reconnect {kind} sandbox '{sandbox_id}' at {working_directory}") + }) + } + Err(error) => Err(anyhow::Error::new(error) + .context(format!("Failed to reconnect {kind} sandbox '{sandbox_id}'"))), + } +} + +/// Brings a sandbox back into use, idempotently: a running sandbox is left +/// alone; a stopped or paused one is started and its Bash verified. +pub(crate) async fn activate(sandbox: &dyn Sandbox) -> sandbox_driver::Result<()> { + let status = sandbox.describe().await?; + if status.state == SandboxState::Running { + return Ok(()); + } + sandbox_driver::activate(sandbox, &WaitOptions::default()).await +} + +/// Attaches to a run's sandbox and brings it to `Running`, for every +/// access-time caller. +pub(crate) async fn attach_running_run_sandbox( + access: &ProviderAccess, + record: &RunSandboxInstance, + run_id: RunId, +) -> anyhow::Result> { + let sandbox = attach_run_sandbox(access, record, run_id).await?; + activate(sandbox.as_ref()) + .await + .with_context(|| format!("Failed to start {} sandbox", record.provider))?; + Ok(sandbox) +} + +/// Whether the backend behind `kind` is reachable and the configured +/// credential accepted, for preflight. A connection failure is the +/// error; an unhealthy provider is an `Ok` report that says why. +pub(crate) async fn provider_health( + kind: &SandboxProviderKind, + access: &ProviderAccess, +) -> anyhow::Result { + let provider = connect_provider(kind, access) + .await + .with_context(|| format!("Failed to connect to the {kind} provider"))?; + provider + .health() + .await + .with_context(|| format!("{kind} health check failed")) +} + +/// Whether the Docker daemon answers, for `fabro doctor`. +pub(crate) async fn check_docker_daemon() -> anyhow::Result<()> { + let health = provider_health(&SandboxProviderKind::DOCKER, &ProviderAccess::default()).await?; + match health.status { + HealthStatus::Ok | HealthStatus::Unknown => Ok(()), + HealthStatus::Unreachable | HealthStatus::Unauthorized => Err(anyhow::anyhow!( + "{}", + health + .message + .unwrap_or_else(|| "Failed to reach Docker daemon".to_string()) + )), + _ => Err(anyhow::anyhow!( + "Docker daemon reported an unknown health state" + )), + } +} + +/// Outcome of probing a Daytona credential through the provider's health +/// check. The provider owns the list of scopes it needs and the order it +/// reports them in; Fabro only renders them. +#[derive(Debug)] +pub(crate) struct DaytonaKeyCheck { + /// Scopes the key lacks, in Daytona's wire names. + pub(crate) missing: Vec, + /// Every scope the provider requires, for the remediation text. + pub(crate) required: Vec, +} + +impl DaytonaKeyCheck { + #[must_use] + pub(crate) fn ok(&self) -> bool { + self.missing.is_empty() + } + + #[must_use] + pub(crate) fn missing_display(&self) -> String { + self.missing.join(", ") + } + + #[must_use] + pub(crate) fn missing_message(&self) -> String { + format!( + "Daytona API key is missing required scopes: {}. Regenerate the key with all \ + snapshot and sandbox scopes.", + self.missing_display() + ) + } + + /// Every scope the provider requires, comma separated, for remediation. + #[must_use] + pub(crate) fn required_display(&self) -> String { + self.required.join(", ") + } +} + +#[derive(Debug, thiserror::Error)] +#[error("Daytona credential probe timed out after {timeout:?}")] +pub(crate) struct DaytonaCredentialProbeTimeout { + timeout: Duration, +} + +impl DaytonaCredentialProbeTimeout { + #[must_use] + pub(crate) const fn new(timeout: Duration) -> Self { + Self { timeout } + } + + #[must_use] + pub(crate) const fn timeout(&self) -> Duration { + self.timeout + } +} + +/// Whether `credentials` reach Daytona, are accepted, and carry the scopes +/// Fabro needs. Reachability and authentication failures are errors; a key +/// that authenticates but lacks scopes is an `Ok` check that is not `ok()`. +pub(crate) async fn check_daytona_api_key( + credentials: &DaytonaCredentials, + probe_timeout: Duration, +) -> anyhow::Result { + let access = ProviderAccess { + providers: ServerSandboxProvidersSettings::default(), + daytona: Some(credentials.clone()), + }; + let probe = async { + let health = provider_health(&SandboxProviderKind::DAYTONA, &access).await?; + match health.status { + HealthStatus::Ok | HealthStatus::Unknown => Ok(DaytonaKeyCheck { + missing: Vec::new(), + required: health.required_permissions, + }), + HealthStatus::Unauthorized if !health.missing_permissions.is_empty() => { + Ok(DaytonaKeyCheck { + missing: health.missing_permissions, + required: health.required_permissions, + }) + } + HealthStatus::Unauthorized => Err(anyhow::anyhow!( + "failed to authenticate with Daytona: {}", + health + .message + .unwrap_or_else(|| "the credential was rejected".to_string()) + )), + _ => Err(anyhow::anyhow!( + "failed to reach Daytona: {}", + health + .message + .unwrap_or_else(|| "the control plane did not answer".to_string()) + )), + } + }; + match time::timeout(probe_timeout, probe).await { + Ok(result) => result, + Err(_) => Err(anyhow::Error::new(DaytonaCredentialProbeTimeout::new( + probe_timeout, + ))), + } +} + +/// The sandboxes Petri created for Fabro's runs, by provider, for the +/// `/sandboxes` endpoints. +/// +/// Every entry is a provider connected on first use: the inventory is +/// assembled synchronously at startup, and a provider that is down surfaces +/// as a lookup error rather than a startup failure. A listing keeps only +/// the sandboxes that carry Petri's run label, and a lookup by id answers +/// only for one that does. The `local` kind has an entry too, so a caller +/// can ask whether the kind is ready, but its sandboxes are directories the +/// run record names and there is nothing to list. +#[derive(Clone, Default)] +pub(crate) struct SandboxInventory { + entries: Vec>, +} + +struct InventoryEntry { + kind: SandboxProviderKind, + connection: Connection, +} + +enum Connection { + /// Sandboxes on this host are directories the run record names; + /// there is nothing to list. + HostDirectories, + #[cfg(test)] + Connected(Arc), + /// Connected through [`connect_provider`] on first use. + Lazy { + access: ProviderAccess, + provider: OnceCell>, + }, +} + +impl SandboxInventory { + #[must_use] + pub(crate) fn empty() -> Self { + Self::default() + } + + /// A kind whose sandboxes are directories on this host: ready to run, + /// nothing to list. + #[must_use] + pub(crate) fn with_host_directories(self, kind: SandboxProviderKind) -> Self { + self.with_entry(kind, Connection::HostDirectories) + } + + /// A provider already connected, tagged with the kind Fabro persists + /// for it. + #[cfg(test)] + #[must_use] + pub(crate) fn with_connected( + self, + kind: SandboxProviderKind, + provider: Arc, + ) -> Self { + self.with_entry(kind, Connection::Connected(provider)) + } + + /// A provider connected through `access` on first use. + #[must_use] + pub(crate) fn with_lazy(self, kind: SandboxProviderKind, access: ProviderAccess) -> Self { + self.with_entry(kind, Connection::Lazy { + access, + provider: OnceCell::new(), + }) + } + + fn with_entry(mut self, kind: SandboxProviderKind, connection: Connection) -> Self { + self.entries + .push(Arc::new(InventoryEntry { kind, connection })); + self + } + + /// The provider kinds this inventory covers. + pub(crate) fn kinds(&self) -> impl Iterator { + self.entries.iter().map(|entry| &entry.kind) + } + + pub(crate) async fn list_managed(&self) -> SandboxListResponse { + let results = join_all( + self.entries + .iter() + .map(|entry| async move { (&entry.kind, entry.list().await) }), + ) + .await; + + let mut data = Vec::new(); + let mut provider_errors = Vec::new(); + for (kind, result) in results { + match result { + Ok(mut sandboxes) => data.append(&mut sandboxes), + Err(err) => provider_errors.push(provider_error(kind.clone(), &err)), + } + } + + SandboxListResponse { + data, + meta: SandboxListMeta { provider_errors }, + } + } + + pub(crate) async fn get_managed_by_native_id( + &self, + id: &str, + ) -> Result { + let results = join_all( + self.entries + .iter() + .map(|entry| async move { (&entry.kind, entry.get(id).await) }), + ) + .await; + + let mut matches = Vec::new(); + let mut provider_errors = Vec::new(); + for (kind, result) in results { + match result { + Ok(Some(sandbox)) => matches.push(sandbox), + Ok(None) => {} + Err(err) => provider_errors.push(provider_error(kind.clone(), &err)), + } + } + + match matches.len() { + 1 => Ok(matches.remove(0)), + 0 if provider_errors.is_empty() => { + Err(SandboxLookupError::NotFound { id: id.to_string() }) + } + 0 => Err(SandboxLookupError::ProviderUnavailable { + id: id.to_string(), + provider_errors, + }), + _ => Err(SandboxLookupError::Conflict { + id: id.to_string(), + providers: matches + .into_iter() + .map(|sandbox| sandbox.provider) + .collect(), + }), + } + } +} + +impl InventoryEntry { + /// The provider, connected on first use; `None` when the kind has + /// nothing to list. + async fn provider(&self) -> anyhow::Result>> { + match &self.connection { + Connection::HostDirectories => Ok(None), + #[cfg(test)] + Connection::Connected(provider) => Ok(Some(provider)), + Connection::Lazy { access, provider } => provider + .get_or_try_init(|| async { + connect_provider(&self.kind, access) + .await + .with_context(|| format!("Failed to connect to the {} provider", self.kind)) + }) + .await + .map(Some), + } + } + + async fn list(&self) -> anyhow::Result> { + let Some(provider) = self.provider().await? else { + return Ok(Vec::new()); + }; + // The driver filters on a label's value; Petri's run label is a + // different run id on every sandbox, so the listing is narrowed to + // the key here. + let statuses = provider + .list(&SandboxFilter::default()) + .await + .with_context(|| format!("Failed to list {} sandboxes", self.kind))?; + Ok(statuses + .into_iter() + .filter(|status| is_petri_sandbox(&status.labels)) + .map(|status| SandboxInfo { + provider: self.kind.clone(), + status, + }) + .collect()) + } + + async fn get(&self, id: &str) -> anyhow::Result> { + let Some(provider) = self.provider().await? else { + return Ok(None); + }; + // An id the driver cannot even name is not one of ours. + let Ok(sandbox_id) = SandboxId::try_new(id) else { + return Ok(None); + }; + let handle = match provider.attach(&sandbox_id, None).await { + Ok(handle) => handle, + Err(DriverError::NotFound { .. }) => return Ok(None), + Err(error) => { + return Err(anyhow::Error::new(error) + .context(format!("Failed to look up {} sandbox '{id}'", self.kind))); + } + }; + let status = handle + .describe() + .await + .with_context(|| format!("Failed to describe {} sandbox '{id}'", self.kind))?; + // Unknown to the provider, deleted, or not a run's: none is in the + // inventory. + if status.state == SandboxState::Deleted || !is_petri_sandbox(&status.labels) { + return Ok(None); + } + Ok(Some(SandboxInfo { + provider: self.kind.clone(), + status, + })) + } +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum SandboxLookupError { + #[error("sandbox '{id}' was not found by any configured provider")] + NotFound { id: String }, + #[error("sandbox '{id}' matched more than one configured provider")] + Conflict { + id: String, + providers: Vec, + }, + #[error("sandbox '{id}' could not be found definitively because one or more providers failed")] + ProviderUnavailable { + id: String, + provider_errors: Vec, + }, +} + +fn provider_error( + provider: SandboxProviderKind, + err: &anyhow::Error, +) -> SandboxProviderLookupError { + SandboxProviderLookupError { + provider, + message: err + .chain() + .map(ToString::to_string) + .collect::>() + .join(": "), + } +} + +#[cfg(test)] +pub(crate) mod test_support { + //! Scripted providers holding Petri-labelled sandboxes, for the + //! inventory and the attach path. + + use std::sync::Arc; + + use sandbox_driver::{SandboxProvider, SandboxState}; + use sandbox_driver_testing::{ScriptedProvider, ScriptedSandbox}; + + use super::PETRI_RUN_LABEL; + + /// A running scripted sandbox Petri created for `run_id`, so a scoped + /// attach accepts it and the inventory lists it. + #[must_use] + pub(crate) fn petri_scripted_sandbox(id: &str, run_id: &str) -> Arc { + Arc::new( + ScriptedSandbox::with_id_and_working_dir(id, "/workspace") + .state(SandboxState::Running) + .label(PETRI_RUN_LABEL, run_id), + ) + } + + /// A scripted provider of `kind` holding `sandboxes`. + #[must_use] + pub(crate) fn scripted_provider( + kind: &str, + sandboxes: Vec>, + ) -> Arc { + let provider = ScriptedProvider::new(kind); + for sandbox in sandboxes { + provider.register(sandbox); + } + Arc::new(provider) + } +} + +#[cfg(test)] +mod tests { + use fabro_types::RunSandboxRuntime; + use fabro_types::settings::server::SandboxPluginSettings; + use sandbox_driver::SandboxProvider as _; + use sandbox_driver_testing::ScriptedSandbox; + + use super::test_support::{petri_scripted_sandbox, scripted_provider}; + use super::*; + + fn kind(name: &str) -> SandboxProviderKind { + SandboxProviderKind::try_new(name).expect("valid kind") + } + + fn record(provider: SandboxProviderKind, id: &str) -> RunSandboxInstance { + RunSandboxInstance { + provider, + image: None, + snapshot: None, + runtime: RunSandboxRuntime { + id: id.to_string(), + working_directory: "/workspace".to_string(), + repo_cloned: None, + clone_origin_url: None, + clone_branch: None, + workspace_root: None, + repos_root: None, + primary_repo_path: None, + primary_repo_link: None, + }, + ready_duration_ms: None, + retained: None, + } + } + + /// A plugin kind whose executable does not exist, so every connection + /// fails. + fn unreachable_plugin_access(name: &str) -> ProviderAccess { + let mut providers = ServerSandboxProvidersSettings::default(); + providers + .entries + .insert(kind(name), ServerSandboxProviderSettings { + enabled: true, + plugin: Some(SandboxPluginSettings { + path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + dev: true, + ..SandboxPluginSettings::default() + }), + }); + ProviderAccess { + providers, + daytona: None, + } + } + + #[test] + fn ownership_is_keyed_on_petris_run_label() { + let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); + let ownership = run_ownership(run_id); + assert_eq!(ownership.labels().iter().collect::>(), vec![( + &"petri.run".to_string(), + &run_id.to_string() + )]); + let mut labels = BTreeMap::new(); + assert!(!ownership.owns(&labels)); + labels.insert("sh.fabro.managed".to_string(), "true".to_string()); + assert!( + !ownership.owns(&labels), + "Fabro's old labels prove nothing; Petri stamps petri.*" + ); + labels.insert("petri.run".to_string(), "another-run".to_string()); + assert!(!ownership.owns(&labels)); + labels.insert("petri.run".to_string(), run_id.to_string()); + assert!(ownership.owns(&labels)); + } + + #[tokio::test] + async fn a_scoped_provider_attaches_only_to_the_runs_sandbox() { + let run_id = RunId::new(); + let other_run = RunId::new(); + let provider = scripted_provider("docker", vec![ + petri_scripted_sandbox("mine", &run_id.to_string()), + petri_scripted_sandbox("theirs", &other_run.to_string()), + Arc::new( + ScriptedSandbox::with_id_and_working_dir("unlabelled", "/work") + .state(SandboxState::Running), + ), + ]); + let scoped = OwnedProvider::new(provider, run_ownership(run_id)); + + let mine = scoped + .attach(&SandboxId::try_new("mine").unwrap(), None) + .await + .expect("the run's own sandbox attaches"); + assert_eq!(mine.id().as_str(), "mine"); + for foreign in ["theirs", "unlabelled"] { + let error = scoped + .attach(&SandboxId::try_new(foreign).unwrap(), None) + .await + .err() + .expect("a sandbox without the run's label is refused"); + assert!( + matches!(error, DriverError::NotOwned { .. }), + "{foreign}: {error:?}" + ); + } + } + + #[tokio::test] + async fn daytona_requires_explicit_credentials() { + let error = connect_provider(&SandboxProviderKind::DAYTONA, &ProviderAccess::default()) + .await + .err() + .expect("daytona must not fall back to the environment"); + assert!(matches!(error, ConnectError::MissingDaytonaCredentials)); + } + + #[tokio::test] + async fn disabled_and_unconfigured_kinds_are_refused_before_any_connection() { + let mut providers = ServerSandboxProvidersSettings::default(); + providers + .entries + .insert(SandboxProviderKind::DOCKER, ServerSandboxProviderSettings { + enabled: false, + plugin: None, + }); + let access = ProviderAccess { + providers, + daytona: None, + }; + let error = connect_provider(&SandboxProviderKind::DOCKER, &access) + .await + .err() + .expect("a disabled provider must not connect"); + assert!( + matches!(error, ConnectError::Disabled { kind } if kind == SandboxProviderKind::DOCKER) + ); + + let error = connect_provider(&kind("e2b"), &ProviderAccess::default()) + .await + .err() + .expect("a plugin kind without settings cannot launch"); + assert!(matches!(error, ConnectError::Unconfigured { kind: k } if k == kind("e2b"))); + } + + #[tokio::test] + async fn a_host_record_reconnects_by_designating_its_directory() { + let directory = tempfile::tempdir().unwrap(); + let working_directory = directory + .path() + .canonicalize() + .unwrap() + .display() + .to_string(); + // An id no registry of this process knows, as a worker mints for a + // long path. + let mut record = record(SandboxProviderKind::LOCAL, "host-0123456789abcdef"); + record.runtime.working_directory = working_directory.clone(); + + let sandbox = attach_run_sandbox(&ProviderAccess::default(), &record, RunId::new()) + .await + .expect("the directory is designated again"); + assert_eq!(sandbox.working_directory(), working_directory); + activate(sandbox.as_ref()) + .await + .expect("a host sandbox runs"); + assert!(directory.path().is_dir()); + + // The path-derived id attaches directly. + let derived = HostProvider::directory_id(directory.path()) + .await + .expect("an id for the directory"); + let mut record = record; + record.runtime.id = derived.to_string(); + let sandbox = attach_run_sandbox(&ProviderAccess::default(), &record, RunId::new()) + .await + .expect("the derived id attaches"); + assert_eq!(sandbox.id(), &derived); + } + + #[test] + fn plugin_config_carries_every_launch_setting() { + let config = plugin_config( + ProviderKind::try_new("e2b").unwrap(), + &SandboxPluginSettings { + path: Some("/opt/e2b".to_string()), + sha256: Some("abc".to_string()), + dev: true, + args: vec!["--flag".to_string()], + env: BTreeMap::from([("A".to_string(), "1".to_string())]), + inherit_env: vec!["PATH".to_string()], + }, + ); + assert_eq!( + config.path.as_deref(), + Some(std::path::Path::new("/opt/e2b")) + ); + assert_eq!(config.sha256.as_deref(), Some("abc")); + assert!(config.dev); + assert_eq!(config.args, vec!["--flag"]); + assert_eq!(config.env.get("A").map(String::as_str), Some("1")); + assert_eq!(config.inherit_env, vec!["PATH"]); + } + + #[test] + fn daytona_credentials_debug_never_prints_the_key() { + let credentials = DaytonaCredentials::from_api_key("dtn_secret_key".to_string(), |name| { + (name == EnvVars::DAYTONA_ORGANIZATION_ID).then(|| "org-1".to_string()) + }); + let rendered = format!("{credentials:?}"); + assert!(!rendered.contains("dtn_secret_key"), "{rendered}"); + assert!(rendered.contains("org-1"), "{rendered}"); + assert_eq!( + credentials.config().api_key.as_deref(), + Some("dtn_secret_key") + ); + } + + #[test] + fn missing_scopes_render_as_the_provider_reports_them() { + let check = DaytonaKeyCheck { + missing: vec!["write:snapshots".to_string(), "write:sandboxes".to_string()], + required: vec![ + "write:snapshots".to_string(), + "delete:snapshots".to_string(), + "write:sandboxes".to_string(), + "delete:sandboxes".to_string(), + ], + }; + assert!(!check.ok()); + assert_eq!(check.missing_display(), "write:snapshots, write:sandboxes"); + assert_eq!( + check.missing_message(), + "Daytona API key is missing required scopes: write:snapshots, write:sandboxes. \ + Regenerate the key with all snapshot and sandbox scopes." + ); + assert_eq!( + check.required_display(), + "write:snapshots, delete:snapshots, write:sandboxes, delete:sandboxes" + ); + } + + #[tokio::test] + async fn credential_probe_reports_configured_timeout() { + // A non-routable address: the probe cannot finish within the budget. + let credentials = DaytonaCredentials::new("dtn_test".to_string()) + .with_api_url(Some("http://10.255.255.1:1/api".to_string())); + let err = check_daytona_api_key(&credentials, Duration::from_millis(1)) + .await + .expect_err("probe should time out"); + let timeout = err + .downcast_ref::() + .expect("timeout should preserve its type"); + assert_eq!(timeout.timeout(), Duration::from_millis(1)); + assert_eq!( + err.to_string(), + "Daytona credential probe timed out after 1ms" + ); + } + + #[tokio::test] + async fn the_inventory_lists_petris_sandboxes_across_providers() { + let foreign = Arc::new( + ScriptedSandbox::with_id_and_working_dir("someone-elses", "/work") + .state(SandboxState::Running), + ); + let inventory = SandboxInventory::empty() + .with_host_directories(SandboxProviderKind::LOCAL) + .with_connected( + SandboxProviderKind::DOCKER, + scripted_provider("docker", vec![ + petri_scripted_sandbox("docker-1", "run-1"), + foreign, + ]), + ) + .with_connected( + SandboxProviderKind::DAYTONA, + scripted_provider("daytona", vec![petri_scripted_sandbox( + "daytona-1", + "run-2", + )]), + ); + + let response = inventory.list_managed().await; + + let mut ids: Vec<_> = response.data.iter().map(|s| s.status.id.as_str()).collect(); + ids.sort_unstable(); + assert_eq!(ids, ["daytona-1", "docker-1"]); + assert!(response.meta.provider_errors.is_empty()); + let kinds: Vec<_> = inventory.kinds().cloned().collect(); + assert_eq!(kinds, [ + SandboxProviderKind::LOCAL, + SandboxProviderKind::DOCKER, + SandboxProviderKind::DAYTONA + ]); + + let found = inventory + .get_managed_by_native_id("daytona-1") + .await + .expect("one provider matches"); + assert_eq!(found.provider, SandboxProviderKind::DAYTONA); + let error = inventory + .get_managed_by_native_id("someone-elses") + .await + .expect_err("a sandbox without Petri's label is not in the inventory"); + assert!(matches!(error, SandboxLookupError::NotFound { .. })); + } + + #[tokio::test] + async fn the_inventory_reports_a_provider_that_cannot_connect_beside_the_others() { + let inventory = SandboxInventory::empty() + .with_connected( + SandboxProviderKind::DOCKER, + scripted_provider("docker", vec![petri_scripted_sandbox("docker-1", "run-1")]), + ) + .with_lazy(kind("e2b"), unreachable_plugin_access("e2b")); + + let response = inventory.list_managed().await; + assert_eq!(response.data.len(), 1); + assert_eq!(response.meta.provider_errors.len(), 1); + assert_eq!(response.meta.provider_errors[0].provider, kind("e2b")); + assert!( + response.meta.provider_errors[0] + .message + .contains("Failed to connect to the e2b provider"), + "{}", + response.meta.provider_errors[0].message + ); + + let error = inventory + .get_managed_by_native_id("maybe-missing") + .await + .expect_err("the failed provider may have held it"); + assert!(matches!( + error, + SandboxLookupError::ProviderUnavailable { .. } + )); + } + + #[tokio::test] + async fn the_inventory_reports_a_conflict_when_two_providers_match() { + let inventory = SandboxInventory::empty() + .with_connected( + SandboxProviderKind::DOCKER, + scripted_provider("docker", vec![petri_scripted_sandbox("same-id", "run-1")]), + ) + .with_connected( + SandboxProviderKind::DAYTONA, + scripted_provider("daytona", vec![petri_scripted_sandbox("same-id", "run-1")]), + ); + + let error = inventory + .get_managed_by_native_id("same-id") + .await + .expect_err("two providers match"); + + let SandboxLookupError::Conflict { providers, .. } = error else { + panic!("expected a conflict, got {error:?}"); + }; + assert_eq!(providers, [ + SandboxProviderKind::DOCKER, + SandboxProviderKind::DAYTONA + ]); + } +} diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 0686efab0..dffe828b9 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -65,9 +65,7 @@ use fabro_petri::controls::{RunControls, SteerError}; use fabro_petri::projector::Projector; use fabro_redact::redact_jsonl_line; use fabro_sandbox::details::sandbox_details; -use fabro_sandbox::driver::{DaytonaCredentials, ProviderAccess, ProviderConnectOptions}; use fabro_sandbox::reconnect::reconnect_for_run; -use fabro_sandbox::{SandboxInventory, daytona}; use fabro_slack::client::{PostedMessage as SlackPostedMessage, SlackClient}; use fabro_slack::config::{ SlackCredentialResolution, @@ -150,6 +148,10 @@ use crate::principal_middleware::{ }; use crate::request_id::{self, RequestId}; use crate::run_files::{FilesInFlight, new_files_in_flight}; +use crate::sandbox_access::{ + self, DAYTONA_CREDENTIAL_PROBE_TIMEOUT, DaytonaCredentials, DaytonaKeyCheck, ProviderAccess, + SandboxInventory, +}; use crate::server_secrets::ServerSecrets; use crate::spawn_env::apply_render_graph_env; use crate::worker_control::{ @@ -1546,11 +1548,30 @@ impl AppState { }) } + /// The same access in `fabro-sandbox`'s shape, for the callers still on + /// its reconnect path. + pub(crate) async fn legacy_provider_access( + &self, + ) -> Result { + Ok(fabro_sandbox::ProviderAccess { + providers: self.server_settings().server.sandbox.providers.clone(), + daytona: self + .vault_secret(EnvVars::DAYTONA_API_KEY) + .await? + .map(|api_key| { + fabro_sandbox::DaytonaCredentials::from_api_key(api_key, |name| { + self.config_env_lookup(name) + }) + .with_http_client(self.http_client().ok()) + }), + }) + } + pub(crate) async fn check_daytona_api_key( &self, api_key: String, - ) -> anyhow::Result { - self.check_daytona_api_key_with_timeout(api_key, daytona::DAYTONA_CREDENTIAL_PROBE_TIMEOUT) + ) -> anyhow::Result { + self.check_daytona_api_key_with_timeout(api_key, DAYTONA_CREDENTIAL_PROBE_TIMEOUT) .await } @@ -1558,8 +1579,9 @@ impl AppState { &self, api_key: String, probe_timeout: Duration, - ) -> anyhow::Result { - daytona::check_daytona_api_key(&self.daytona_credentials(api_key), probe_timeout).await + ) -> anyhow::Result { + sandbox_access::check_daytona_api_key(&self.daytona_credentials(api_key), probe_timeout) + .await } /// Borrow the persistent store so sibling modules can open run readers @@ -2386,35 +2408,23 @@ fn build_sandbox_inventory( http_client: Option, ) -> SandboxInventory { let provider_settings = &server_settings.server.sandbox.providers; + let access = ProviderAccess { + providers: provider_settings.clone(), + daytona: daytona_api_key.map(|api_key| { + DaytonaCredentials::from_api_key(api_key, |name| env_lookup(name)) + .with_http_client(http_client) + }), + }; let mut inventory = SandboxInventory::empty(); if provider_settings.is_enabled(&SandboxProviderKind::LOCAL) { inventory = inventory.with_host_directories(SandboxProviderKind::LOCAL); } - - if let Some(docker) = provider_settings.get(&SandboxProviderKind::DOCKER) { - if docker.enabled { - inventory = inventory.with_lazy( - SandboxProviderKind::DOCKER, - docker.clone(), - ProviderConnectOptions::default(), - ); - } + if provider_settings.is_enabled(&SandboxProviderKind::DOCKER) { + inventory = inventory.with_lazy(SandboxProviderKind::DOCKER, access.clone()); } - - if let Some(daytona) = provider_settings.get(&SandboxProviderKind::DAYTONA) { - if let Some(api_key) = daytona_api_key.filter(|_| daytona.enabled) { - let credentials = DaytonaCredentials::from_api_key(api_key, |name| env_lookup(name)) - .with_http_client(http_client); - inventory = inventory.with_lazy( - SandboxProviderKind::DAYTONA, - daytona.clone(), - ProviderConnectOptions { - host_registry_root: None, - daytona: Some(credentials), - }, - ); - } + if provider_settings.is_enabled(&SandboxProviderKind::DAYTONA) && access.daytona.is_some() { + inventory = inventory.with_lazy(SandboxProviderKind::DAYTONA, access); } inventory @@ -2859,7 +2869,7 @@ async fn delete_run_sandbox_resource( } let access = state - .provider_access() + .legacy_provider_access() .await .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; let sandbox = match reconnect_for_run(&record, &access, Some(id), None).await { diff --git a/lib/apps/fabro-server/src/server/handler/sandbox.rs b/lib/apps/fabro-server/src/server/handler/sandbox.rs index 431170f99..6c9b56dae 100644 --- a/lib/apps/fabro-server/src/server/handler/sandbox.rs +++ b/lib/apps/fabro-server/src/server/handler/sandbox.rs @@ -748,7 +748,7 @@ async fn reconnect_run_sandbox_instance( } async fn load_provider_access(state: &AppState) -> Result { - state.provider_access().await.map_err(|err| { + state.legacy_provider_access().await.map_err(|err| { tracing::error!(error = ?err, "Loading Daytona API key failed"); ApiError::new( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/lib/apps/fabro-server/src/server/handler/sandboxes.rs b/lib/apps/fabro-server/src/server/handler/sandboxes.rs index d8b92cde2..83199d6f6 100644 --- a/lib/apps/fabro-server/src/server/handler/sandboxes.rs +++ b/lib/apps/fabro-server/src/server/handler/sandboxes.rs @@ -4,12 +4,12 @@ use axum::extract::{Path, State}; use axum::http::StatusCode; use axum::routing::get; use axum::{Json, Router}; -use fabro_sandbox::SandboxLookupError; use fabro_types::{SandboxInfo, SandboxListResponse, SandboxProviderKind}; use super::super::AppState; use crate::error::ApiError; use crate::principal_middleware::RequiredRunManagementActor; +use crate::sandbox_access::SandboxLookupError; pub(super) fn routes() -> Router> { Router::new() @@ -77,16 +77,20 @@ fn provider_list(providers: &[SandboxProviderKind]) -> String { #[cfg(test)] mod tests { + use std::sync::Arc; + use axum::body::{Body, to_bytes}; use axum::http::{Request, StatusCode}; - use fabro_sandbox::SandboxInventory; - use fabro_sandbox::driver::{ConnectedProvider, ProviderConnectOptions}; - use fabro_sandbox::test_support::{managed_scripted_sandbox, scripted_inventory_provider}; use fabro_types::SandboxProviderKind; - use fabro_types::settings::server::{SandboxPluginSettings, ServerSandboxProviderSettings}; + use fabro_types::settings::server::{ + SandboxPluginSettings, ServerSandboxProviderSettings, ServerSandboxProvidersSettings, + }; + use sandbox_driver::SandboxProvider; use serde_json::{Value, json}; use tower::ServiceExt; + use crate::sandbox_access::test_support::{petri_scripted_sandbox, scripted_provider}; + use crate::sandbox_access::{ProviderAccess, SandboxInventory}; use crate::test_support::{TestAppStateBuilder, build_test_router}; fn app_with_inventory(inventory: SandboxInventory) -> axum::Router { @@ -96,30 +100,36 @@ mod tests { build_test_router(state) } - /// A connected provider of `kind` holding fabro-managed sandboxes `ids`. - fn provider(kind: SandboxProviderKind, ids: &[&str]) -> ConnectedProvider { - scripted_inventory_provider( - kind, - ids.iter().map(|id| managed_scripted_sandbox(id)).collect(), + /// A provider of `kind` holding the sandboxes Petri created for a run, + /// `ids`. + fn provider(kind: &SandboxProviderKind, ids: &[&str]) -> Arc { + scripted_provider( + kind.as_str(), + ids.iter() + .map(|id| petri_scripted_sandbox(id, "01HY0000000000000000000000")) + .collect(), ) } /// A plugin kind whose executable does not exist, so every lookup fails /// to connect. fn with_unreachable_plugin(inventory: SandboxInventory, name: &str) -> SandboxInventory { - let settings = ServerSandboxProviderSettings { - enabled: true, - plugin: Some(SandboxPluginSettings { - path: Some(format!("/nonexistent/fabro-sandbox-{name}")), - dev: true, - ..SandboxPluginSettings::default() - }), - }; - inventory.with_lazy( - SandboxProviderKind::try_new(name).expect("valid kind"), - settings, - ProviderConnectOptions::default(), - ) + let kind = SandboxProviderKind::try_new(name).expect("valid kind"); + let mut providers = ServerSandboxProvidersSettings::default(); + providers + .entries + .insert(kind.clone(), ServerSandboxProviderSettings { + enabled: true, + plugin: Some(SandboxPluginSettings { + path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + dev: true, + ..SandboxPluginSettings::default() + }), + }); + inventory.with_lazy(kind, ProviderAccess { + providers, + daytona: None, + }) } fn req_get(uri: &str) -> Request { @@ -139,10 +149,10 @@ mod tests { #[tokio::test] async fn list_returns_provider_backed_data_without_run_projection_state() { - let app = app_with_inventory( - SandboxInventory::empty() - .with_connected(provider(SandboxProviderKind::DOCKER, &["docker-native-id"])), - ); + let app = app_with_inventory(SandboxInventory::empty().with_connected( + SandboxProviderKind::DOCKER, + provider(&SandboxProviderKind::DOCKER, &["docker-native-id"]), + )); let response = app.oneshot(req_get("/api/v1/sandboxes")).await.unwrap(); @@ -158,8 +168,14 @@ mod tests { async fn retrieve_searches_all_configured_providers() { let app = app_with_inventory( SandboxInventory::empty() - .with_connected(provider(SandboxProviderKind::DOCKER, &[])) - .with_connected(provider(SandboxProviderKind::DAYTONA, &["native-id"])), + .with_connected( + SandboxProviderKind::DOCKER, + provider(&SandboxProviderKind::DOCKER, &[]), + ) + .with_connected( + SandboxProviderKind::DAYTONA, + provider(&SandboxProviderKind::DAYTONA, &["native-id"]), + ), ); let response = app @@ -177,8 +193,14 @@ mod tests { async fn no_matching_sandbox_returns_404() { let app = app_with_inventory( SandboxInventory::empty() - .with_connected(provider(SandboxProviderKind::DOCKER, &[])) - .with_connected(provider(SandboxProviderKind::DAYTONA, &[])), + .with_connected( + SandboxProviderKind::DOCKER, + provider(&SandboxProviderKind::DOCKER, &[]), + ) + .with_connected( + SandboxProviderKind::DAYTONA, + provider(&SandboxProviderKind::DAYTONA, &[]), + ), ); let response = app @@ -193,8 +215,14 @@ mod tests { async fn duplicate_native_ids_return_409() { let app = app_with_inventory( SandboxInventory::empty() - .with_connected(provider(SandboxProviderKind::DOCKER, &["same-id"])) - .with_connected(provider(SandboxProviderKind::DAYTONA, &["same-id"])), + .with_connected( + SandboxProviderKind::DOCKER, + provider(&SandboxProviderKind::DOCKER, &["same-id"]), + ) + .with_connected( + SandboxProviderKind::DAYTONA, + provider(&SandboxProviderKind::DAYTONA, &["same-id"]), + ), ); let response = app @@ -215,7 +243,10 @@ mod tests { #[tokio::test] async fn provider_lookup_uncertainty_returns_502() { let app = app_with_inventory(with_unreachable_plugin( - SandboxInventory::empty().with_connected(provider(SandboxProviderKind::DOCKER, &[])), + SandboxInventory::empty().with_connected( + SandboxProviderKind::DOCKER, + provider(&SandboxProviderKind::DOCKER, &[]), + ), "e2b", )); diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index 75cc62613..f5311f5a7 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -727,7 +727,7 @@ async fn build_agent( AskFabroBuildError::SandboxUnavailable(anyhow::anyhow!("run sandbox was not created")) })?; let access = state - .provider_access() + .legacy_provider_access() .await .map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err)))?; let sandbox = reconnect_for_run(sandbox_instance, &access, Some(run_id), None) diff --git a/lib/apps/fabro-server/src/test_support.rs b/lib/apps/fabro-server/src/test_support.rs index 75864733b..dc3350fc3 100644 --- a/lib/apps/fabro-server/src/test_support.rs +++ b/lib/apps/fabro-server/src/test_support.rs @@ -18,7 +18,6 @@ use fabro_config::user::default_storage_dir; use fabro_config::{LlmLayer, RunLayer, ServerSettingsBuilder, Storage, envfile}; use fabro_db::DbPool; use fabro_llm::lithos_catalog::Catalog; -use fabro_sandbox::SandboxInventory; use fabro_static::EnvVars; use fabro_store::{ArtifactStore, Database, test_support as store_test_support}; use fabro_types::settings::ServerAuthMethod; @@ -39,6 +38,7 @@ use crate::interp::process_env_var; use crate::jwt_auth::{AuthMode, ConfiguredAuth}; #[cfg(test)] use crate::principal_middleware::{AuthContextSlot, RequestAuthContext}; +use crate::sandbox_access::SandboxInventory; use crate::server::{ self, AppState, AppStateConfig, EnvLookup, ResolvedAppStateSettings, RouterOptions, build_app_state, @@ -157,7 +157,8 @@ impl TestAppStateBuilder { self } - pub fn sandbox_inventory(mut self, sandbox_inventory: SandboxInventory) -> Self { + #[cfg(test)] + pub(crate) fn sandbox_inventory(mut self, sandbox_inventory: SandboxInventory) -> Self { self.sandbox_inventory = Some(sandbox_inventory); self }