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 }